D9020: sshpeer: avoid having a destructor that can block forks forever

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

D9020: sshpeer: avoid having a destructor that can block forks forever

marmoute (Pierre-Yves David)
valentin.gatienbaron created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  This is solution 2. that was described in the previous commit.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D9020

AFFECTED FILES
  mercurial/sshpeer.py

CHANGE DETAILS

diff --git a/mercurial/sshpeer.py b/mercurial/sshpeer.py
--- a/mercurial/sshpeer.py
+++ b/mercurial/sshpeer.py
@@ -7,6 +7,7 @@
 
 from __future__ import absolute_import
 
+import os
 import re
 import uuid
 
@@ -145,7 +146,7 @@
         return self._main.flush()
 
 
-def _cleanuppipes(ui, pipei, pipeo, pipee):
+def _cleanuppipes(ui, pipei, pipeo, pipee, origpid):
     """Clean up pipes used by an SSH connection."""
     if pipeo:
         pipeo.close()
@@ -153,12 +154,15 @@
         pipei.close()
 
     if pipee:
-        # Try to read from the err descriptor until EOF.
-        try:
-            for l in pipee:
-                ui.status(_(b'remote: '), l)
-        except (IOError, ValueError):
-            pass
+        if origpid == os.getpid():
+            # Try to read from the err descriptor until EOF. But if we
+            # "leak" an ssh peer in a fork, just close without
+            # reading.
+            try:
+                for l in pipee:
+                    ui.status(_(b'remote: '), l)
+            except (IOError, ValueError):
+                pass
 
         pipee.close()
 
@@ -411,6 +415,7 @@
         self._pipeo = stdin
         self._pipei = stdout
         self._pipee = stderr
+        self._origpid = os.getpid() # recorded to detect calls to fork()
         self._caps = caps
         self._autoreadstderr = autoreadstderr
 
@@ -454,7 +459,8 @@
         raise exception
 
     def _cleanup(self):
-        _cleanuppipes(self.ui, self._pipei, self._pipeo, self._pipee)
+        _cleanuppipes(self.ui, self._pipei, self._pipeo, self._pipee,
+                      self._origpid)
 
     __del__ = _cleanup
 
@@ -605,10 +611,11 @@
     servers and clients via non-standard means, which can be useful for
     testing.
     """
+    origgpid = os.getpid()
     try:
         protoname, caps = _performhandshake(ui, stdin, stdout, stderr)
     except Exception:
-        _cleanuppipes(ui, stdout, stdin, stderr)
+        _cleanuppipes(ui, stdout, stdin, stderr, origgpid)
         raise
 
     if protoname == wireprototypes.SSHV1:
@@ -634,7 +641,7 @@
             autoreadstderr=autoreadstderr,
         )
     else:
-        _cleanuppipes(ui, stdout, stdin, stderr)
+        _cleanuppipes(ui, stdout, stdin, stderr, origgpid)
         raise error.RepoError(
             _(b'unknown version of SSH protocol: %s') % protoname
         )



To: valentin.gatienbaron, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel