[Python-checkins] r60104 - python/trunk/Lib/subprocess.py

[gregory.p.smith]

Author: gregory.p.smith
Date: Sat Jan 19 21:57:59 2008
New Revision: 60104

Modified:
   python/trunk/Lib/subprocess.py
Log:
Fixes issue1336 - a race condition could occur when forking if the gc
kicked in during the critical section.  solution: disable gc during
that section.  Patch contributed by jpa and updated by me to cover the
race condition still existing what therve from twistedmatrix pointed
out (already seen and fixed in twisted's own subprocess code).


Modified: python/trunk/Lib/subprocess.py
==============================================================================
--- python/trunk/Lib/subprocess.py	(original)
+++ python/trunk/Lib/subprocess.py	Sat Jan 19 21:57:59 2008
@@ -356,6 +356,7 @@
 import os
 import types
 import traceback
+import gc
 
 # Exception classes used by this module.
 class CalledProcessError(Exception):
@@ -994,66 +995,78 @@
             errpipe_read, errpipe_write = os.pipe()
             self._set_cloexec_flag(errpipe_write)
 
-            self.pid = os.fork()
-            self._child_created = True
-            if self.pid == 0:
-                # Child
-                try:
-                    # Close parent's pipe ends
-                    if p2cwrite is not None:
-                        os.close(p2cwrite)
-                    if c2pread is not None:
-                        os.close(c2pread)
-                    if errread is not None:
-                        os.close(errread)
-                    os.close(errpipe_read)
-
-                    # Dup fds for child
-                    if p2cread is not None:
-                        os.dup2(p2cread, 0)
-                    if c2pwrite is not None:
-                        os.dup2(c2pwrite, 1)
-                    if errwrite is not None:
-                        os.dup2(errwrite, 2)
-
-                    # Close pipe fds.  Make sure we don't close the same
-                    # fd more than once, or standard fds.
-                    if p2cread is not None and p2cread not in (0,):
-                        os.close(p2cread)
-                    if c2pwrite is not None and c2pwrite not in (p2cread, 1):
-                        os.close(c2pwrite)
-                    if errwrite is not None and errwrite not in (p2cread, c2pwrite, 2):
-                        os.close(errwrite)
-
-                    # Close all other fds, if asked for
-                    if close_fds:
-                        self._close_fds(but=errpipe_write)
-
-                    if cwd is not None:
-                        os.chdir(cwd)
-
-                    if preexec_fn:
-                        apply(preexec_fn)
-
-                    if env is None:
-                        os.execvp(executable, args)
-                    else:
-                        os.execvpe(executable, args, env)
-
-                except:
-                    exc_type, exc_value, tb = sys.exc_info()
-                    # Save the traceback and attach it to the exception object
-                    exc_lines = traceback.format_exception(exc_type,
-                                                           exc_value,
-                                                           tb)
-                    exc_value.child_traceback = ''.join(exc_lines)
-                    os.write(errpipe_write, pickle.dumps(exc_value))
-
-                # This exitcode won't be reported to applications, so it
-                # really doesn't matter what we return.
-                os._exit(255)
+            gc_was_enabled = gc.isenabled()
+            # Disable gc to avoid bug where gc -> file_dealloc ->
+            # write to stderr -> hang.  http://bugs.python.org/issue1336
+            gc.disable()
+            try:
+                self.pid = os.fork()
+            except:
+                if gc_was_enabled:
+                    gc.enable()
+                raise
+            else:
+                self._child_created = True
+                if self.pid == 0:
+                    # Child
+                    try:
+                        # Close parent's pipe ends
+                        if p2cwrite is not None:
+                            os.close(p2cwrite)
+                        if c2pread is not None:
+                            os.close(c2pread)
+                        if errread is not None:
+                            os.close(errread)
+                        os.close(errpipe_read)
+
+                        # Dup fds for child
+                        if p2cread is not None:
+                            os.dup2(p2cread, 0)
+                        if c2pwrite is not None:
+                            os.dup2(c2pwrite, 1)
+                        if errwrite is not None:
+                            os.dup2(errwrite, 2)
+
+                        # Close pipe fds.  Make sure we don't close the same
+                        # fd more than once, or standard fds.
+                        if p2cread is not None and p2cread not in (0,):
+                            os.close(p2cread)
+                        if c2pwrite is not None and c2pwrite not in (p2cread, 1):
+                            os.close(c2pwrite)
+                        if errwrite is not None and errwrite not in (p2cread, c2pwrite, 2):
+                            os.close(errwrite)
+
+                        # Close all other fds, if asked for
+                        if close_fds:
+                            self._close_fds(but=errpipe_write)
+
+                        if cwd is not None:
+                            os.chdir(cwd)
+
+                        if preexec_fn:
+                            apply(preexec_fn)
+
+                        if env is None:
+                            os.execvp(executable, args)
+                        else:
+                            os.execvpe(executable, args, env)
+
+                    except:
+                        exc_type, exc_value, tb = sys.exc_info()
+                        # Save the traceback and attach it to the exception object
+                        exc_lines = traceback.format_exception(exc_type,
+                                                               exc_value,
+                                                               tb)
+                        exc_value.child_traceback = ''.join(exc_lines)
+                        os.write(errpipe_write, pickle.dumps(exc_value))
+
+                    # This exitcode won't be reported to applications, so it
+                    # really doesn't matter what we return.
+                    os._exit(255)
 
             # Parent
+            if gc_was_enabled:
+                gc.enable()
             os.close(errpipe_write)
             if p2cread is not None and p2cwrite is not None:
                 os.close(p2cread)