| Submitter | mercurial-bugs@mercurial-scm.org |
|---|---|
| Date | Aug. 1, 2017, 3:41 a.m. |
| Message ID | <bug-5646-285@https.bz.mercurial-scm.org/> |
| Download | mbox | patch |
| Permalink | /patch/22598/ |
| State | Not Applicable |
| Headers | show |
Comments
Patch
diff --git mercurial/bundle2.py mercurial/bundle2.py --- mercurial/bundle2.py +++ mercurial/bundle2.py @@ -542,7 +542,8 @@ class bundle20(object): self._params = [] self._parts = [] self.capabilities = dict(capabilities) - self._compengine = util.compengines.forbundletype('UN') + self._compengine = util.compengines.forbundletype('ZS') + self.addparam('Compression', 'ZS') self._compopts = None def setcompression(self, alg, compopts=None):
https://bz.mercurial-scm.org/show_bug.cgi?id=5646 Bug ID: 5646 Summary: zstd-compressed bundles don't work over the ssh protocol Product: Mercurial Version: unspecified Hardware: All OS: All Status: UNCONFIRMED Severity: bug Priority: normal Component: Mercurial Assignee: bugzilla@mercurial-scm.org Reporter: sid+hgbz@less-broken.com CC: gregory.szorc@gmail.com, mercurial-devel@mercurial-scm.org With this patch applied: an hg pull over ssh hangs with this stack trace: https://bpaste.net/show/6e8ad89b79e6 The 'size' at line 40 of the stack trace is 131075. This appears to be something inside zstd requesting 128KiB chunks. This causes the os.read(..., _chunksize) at line 46 to hang, because that method doesn't return unless _chunksize (= 4096 in this case) bytes are read off the wire. This isn't an issue with stock Mercurial because it always sends uncompressed bundles over the wire. Some IRC discussion: 14:49 <indygreg> sid0: iirc it buffers the wire up to 128kb then feeds that into the decompressor 14:49 <sid0> indygreg: what if there are less than 128kb available? 14:49 <indygreg> i'd have to look at the code. it either tries again or runs with what it's got 14:49 <sid0> indygreg: (also this behavior doesn't happen with uncompressed bundles) 14:50 <sid0> actually it hangs :) 14:51 <indygreg> hanging in util.chunkbuffer? 14:55 <sid0> indygreg: https://bpaste.net/show/66f17d9abf36 14:55 <sid0> indygreg: the "size" in the read call at line 40 is 131075 14:56 <indygreg> welcome to the hell that is the buffering code for bundle reading 14:57 <indygreg> why is os.read() being used? 14:57 <indygreg> i think that will hang until all requested data is available 14:58 <indygreg> that's the wrong function to be used where you can't guarantee other end will produce that much data 14:58 <sid0> indygreg: hey, this is the first time I've seen this code :) 14:58 <indygreg> same here 14:58 <indygreg> this code is pretty dangerous 14:58 <indygreg> i'm mildly surprised others aren't complaining about deadlocks 14:59 <sid0> indygreg: I did have to trigger this by patching the code to send zstd-compressed bundles over the wire 14:59 <sid0> indygreg: but yeah 14:59 <sid0> indygreg: so if not os.read, what should it be using instead? 15:00 <indygreg> well, it depends if O_NONBLOCK was set 15:01 <indygreg> as long as the file descriptor isn't in blocking mode, it should be OK 15:01 <indygreg> but since bufferedinputpipe() doesn't control the socket, ugh 15:01 <indygreg> err, file descriptor 15:01 <indygreg> also, i think python may retry automatically in some cases 15:01 <indygreg> i'd have to look at cpython source 15:02 <indygreg> this feels fishy to me