sshpeer: defer pipe buffering and stderr sidechannel binding
authorGregory Szorc <gregory.szorc@gmail.com>
Wed, 21 Feb 2018 14:02:23 -0800
changeset 36370 11ba1a96f946
parent 36369 066e6a9d52bb
child 36371 0c231df1ffdc
sshpeer: defer pipe buffering and stderr sidechannel binding The doublepipe and bufferedinputpipe types facilitate polling multiple pipes without blocking and for automatically forwarding output from the SSH server's stderr pipe to the ui as "remote: " output. This all happens automatically and callers don't need to worry about reading from multiple pipes. An upcoming change to version 2 of the SSH wire protocol will eliminate the use of stderr and move side-channel output into the "main" pipe. The SSH wire protocol will use a pair of unidirectional pipes - just like the HTTP protocol. In this future world, the doublepipe primitive isn't necessary because the stderr pipe won't be used. To prepare for eventually not using doublepipe, we delay the construction of this primitive from immediately after connection establishment to inside construction of the peer instance. The handshake occurs between these two events. So we had to teach the handshake code to read from stderr so any stderr output from the server is still attended to early in the connection lifetime. Differential Revision: https://phab.mercurial-scm.org/D2383
mercurial/sshpeer.py
tests/test-check-interfaces.py
--- a/mercurial/sshpeer.py	Wed Feb 21 13:08:55 2018 -0800
+++ b/mercurial/sshpeer.py	Wed Feb 21 14:02:23 2018 -0800
@@ -156,13 +156,13 @@
     # move to threading.
     stdin, stdout, stderr, proc = util.popen4(cmd, bufsize=0, env=sshenv)
 
-    stdout = doublepipe(ui, util.bufferedinputpipe(stdout), stderr)
-    stdin = doublepipe(ui, stdin, stderr)
-
     return proc, stdin, stdout, stderr
 
 def _performhandshake(ui, stdin, stdout, stderr):
     def badresponse():
+        # Flush any output on stderr.
+        _forwardoutput(ui, stderr)
+
         msg = _('no suitable response from remote hg')
         hint = ui.config('ui', 'ssherrorhint')
         raise error.RepoError(msg, hint=hint)
@@ -331,6 +331,9 @@
     if not caps:
         badresponse()
 
+    # Flush any output on stderr before proceeding.
+    _forwardoutput(ui, stderr)
+
     return protoname, caps
 
 class sshv1peer(wireproto.wirepeer):
@@ -347,6 +350,12 @@
         # self._subprocess is unused. Keeping a handle on the process
         # holds a reference and prevents it from being garbage collected.
         self._subprocess = proc
+
+        # And we hook up our "doublepipe" wrapper to allow querying
+        # stderr any time we perform I/O.
+        stdout = doublepipe(ui, util.bufferedinputpipe(stdout), stderr)
+        stdin = doublepipe(ui, stdin, stderr)
+
         self._pipeo = stdin
         self._pipei = stdout
         self._pipee = stderr
--- a/tests/test-check-interfaces.py	Wed Feb 21 13:08:55 2018 -0800
+++ b/tests/test-check-interfaces.py	Wed Feb 21 14:02:23 2018 -0800
@@ -59,16 +59,20 @@
     def badmethod(self):
         pass
 
+class dummypipe(object):
+    def close(self):
+        pass
+
 def main():
     ui = uimod.ui()
 
     checkobject(badpeer())
     checkobject(httppeer.httppeer(ui, 'http://localhost'))
     checkobject(localrepo.localpeer(dummyrepo()))
-    checkobject(sshpeer.sshv1peer(ui, 'ssh://localhost/foo', None, None, None,
-                                  None, None))
-    checkobject(sshpeer.sshv2peer(ui, 'ssh://localhost/foo', None, None, None,
-                                  None, None))
+    checkobject(sshpeer.sshv1peer(ui, 'ssh://localhost/foo', None, dummypipe(),
+                                  dummypipe(), None, None))
+    checkobject(sshpeer.sshv2peer(ui, 'ssh://localhost/foo', None, dummypipe(),
+                                  dummypipe(), None, None))
     checkobject(bundlerepo.bundlepeer(dummyrepo()))
     checkobject(statichttprepo.statichttppeer(dummyrepo()))
     checkobject(unionrepo.unionpeer(dummyrepo()))