Move the wrapping of the subprocess filehandles before executing the child. Fixes Python bug 8458

gevent/subprocess.py

@@ -318,6 +318,37 @@ class Popen(object):
          c2pread, c2pwrite,
          errread, errwrite) = self._get_handles(stdin, stdout, stderr)
 
+        # We wrap OS handles *before* launching the child, otherwise a
+        # quickly terminating child could make our fds unwrappable
+        # (see #8458).
+
+        if mswindows:
+            if p2cwrite is not None:
+                p2cwrite = msvcrt.open_osfhandle(p2cwrite.Detach(), 0)
+            if c2pread is not None:
+                c2pread = msvcrt.open_osfhandle(c2pread.Detach(), 0)
+            if errread is not None:
+                errread = msvcrt.open_osfhandle(errread.Detach(), 0)
+
+        if p2cwrite is not None:
+            if PY3 and universal_newlines:
+                self.stdin = FileObject(p2cwrite, 'w', bufsize)
+                self.stdin._tranlate = True
+                self.stdin.io = io.TextIOWrapper(self.stdin.io, write_through=True,
+                                                 line_buffering=(bufsize == 1))
+            else:
+                self.stdin = FileObject(p2cwrite, 'wb', bufsize)
+        if c2pread is not None:
+            if universal_newlines:
+                self.stdout = FileObject(c2pread, 'rU', bufsize)
+            else:
+                self.stdout = FileObject(c2pread, 'rb', bufsize)
+        if errread is not None:
+            if universal_newlines:
+                self.stderr = FileObject(errread, 'rU', bufsize)
+            else:
+                self.stderr = FileObject(errread, 'rb', bufsize)
+
         self._closed_child_pipe_fds = False
         try:
             self._execute_child(args, executable, preexec_fn, close_fds,
@@ -328,7 +359,9 @@ class Popen(object):
                                 errread, errwrite)
         except:
             # Cleanup if the child failed starting.
-            # (gevent: New in python3, but reported as gevent bug in #347)
+            # (gevent: New in python3, but reported as gevent bug in #347.
+            # Note that under Py2, any error raised below will replace the
+            # original error. This could be worked around using hub.reraise)
             for f in filter(None, (self.stdin, self.stdout, self.stderr)):
                 try:
                     f.close()
@@ -353,33 +386,6 @@ class Popen(object):
 
             raise
 
-        if mswindows:
-            if p2cwrite is not None:
-                p2cwrite = msvcrt.open_osfhandle(p2cwrite.Detach(), 0)
-            if c2pread is not None:
-                c2pread = msvcrt.open_osfhandle(c2pread.Detach(), 0)
-            if errread is not None:
-                errread = msvcrt.open_osfhandle(errread.Detach(), 0)
-
-        if p2cwrite is not None:
-            if PY3 and universal_newlines:
-                self.stdin = FileObject(p2cwrite, 'w', bufsize)
-                self.stdin._tranlate = True
-                self.stdin.io = io.TextIOWrapper(self.stdin.io, write_through=True,
-                                                 line_buffering=(bufsize == 1))
-            else:
-                self.stdin = FileObject(p2cwrite, 'wb', bufsize)
-        if c2pread is not None:
-            if universal_newlines:
-                self.stdout = FileObject(c2pread, 'rU', bufsize)
-            else:
-                self.stdout = FileObject(c2pread, 'rb', bufsize)
-        if errread is not None:
-            if universal_newlines:
-                self.stderr = FileObject(errread, 'rU', bufsize)
-            else:
-                self.stderr = FileObject(errread, 'rb', bufsize)
-
     def __repr__(self):
         return '<%s at 0x%x pid=%r returncode=%r>' % (self.__class__.__name__, id(self), self.pid, self.returncode)