[PATCH] sshpeer: store subprocess so it cleans up correctly
Durham Goode
durham at fb.com
Fri Mar 8 19:21:23 CST 2013
# HG changeset patch
# User Durham Goode <durham at fb.com>
# Date 1362790776 28800
# Fri Mar 08 16:59:36 2013 -0800
# Node ID f22f59d4265e10959313f801e7b0d642d154a81f
# Parent 2b1729b20820c0eeb0857bb224d009db698faeef
sshpeer: store subprocess so it cleans up correctly
When running 'hg pull --rebase', I was seeing this exception over 100% of the
time as the python process was closing down:
Exception TypeError: TypeError("'NoneType' object is not callable",) in
<bound method Popen.__del__ of <subprocess.Popen object at 0x937c10>> ignored
By storing the subprocess on the sshpeer, the subprocess seems to clean up
correctly, and I no longer see the exception. I have no idea why this actually
works, but I get a 0% repro if I store the subprocess in self.subprocess,
and a 100% repro if I store None in self.subprocess.
Possibly related to issue 2240.
diff --git a/mercurial/sshpeer.py b/mercurial/sshpeer.py
--- a/mercurial/sshpeer.py
+++ b/mercurial/sshpeer.py
@@ -70,7 +70,14 @@
(_serverquote(remotecmd), _serverquote(self.path))))
ui.note(_('running %s\n') % cmd)
cmd = util.quotecommand(cmd)
- self.pipeo, self.pipei, self.pipee = util.popen3(cmd)
+
+ # while self.subprocess isn't used, having it allows the subprocess to
+ # to clean up correctly later
+ self.subprocess = util.popen(cmd)
+ # in and out are reversed, because it's always been that way
+ self.pipeo = self.subprocess.stdin
+ self.pipei = self.subprocess.stdout
+ self.pipee = self.subprocess.stderr
# skip any noise generated by remote shell
self._callstream("hello")
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -117,6 +117,14 @@
import subprocess
closefds = os.name == 'posix'
+def popen(cmd, env=None, newlines=False):
+ return subprocess.Popen(cmd, shell=True, bufsize=-1,
+ close_fds=closefds,
+ stdin=subprocess.PIPE, stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE,
+ universal_newlines=newlines,
+ env=env)
+
def popen2(cmd, env=None, newlines=False):
# Setting bufsize to -1 lets the system decide the buffer size.
# The default for bufsize is 0, meaning unbuffered. This leads to
@@ -129,12 +137,7 @@
return p.stdin, p.stdout
def popen3(cmd, env=None, newlines=False):
- p = subprocess.Popen(cmd, shell=True, bufsize=-1,
- close_fds=closefds,
- stdin=subprocess.PIPE, stdout=subprocess.PIPE,
- stderr=subprocess.PIPE,
- universal_newlines=newlines,
- env=env)
+ p = popen(cmd, env, newlines)
return p.stdin, p.stdout, p.stderr
def version():
More information about the Mercurial-devel
mailing list