Patchwork [Bug,5646] New: zstd-compressed bundles don't work over the ssh protocol

mail settings
Date Aug. 1, 2017, 3:41 a.m.
Message ID <>
Download mbox | patch
Permalink /patch/22598/
State Not Applicable
Headers show

Comments - Aug. 1, 2017, 3:41 a.m.

            Bug ID: 5646
           Summary: zstd-compressed bundles don't work over the ssh
           Product: Mercurial
           Version: unspecified
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: bug
          Priority: normal
         Component: Mercurial

With this patch applied:

an hg pull over ssh hangs with this stack trace:

The 'size' at line 40 of the stack trace is 131075. This appears to be
something inside zstd requesting 128KiB chunks.

This causes the, _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
14:50 <sid0> actually it hangs :)
14:51 <indygreg> hanging in util.chunkbuffer?
14:55 <sid0> indygreg:
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
14:57 <indygreg> why is 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, 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


diff --git mercurial/ mercurial/
--- mercurial/
+++ mercurial/
@@ -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):