]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
Issue #12650: Fix a race condition where a subprocess.Popen could leak
authorCharles-François Natali <neologix@free.fr>
Thu, 18 Aug 2011 17:11:29 +0000 (19:11 +0200)
committerCharles-François Natali <neologix@free.fr>
Thu, 18 Aug 2011 17:11:29 +0000 (19:11 +0200)
resources (FD/zombie) when killed at the wrong time.

1  2 
Lib/subprocess.py
Lib/test/test_subprocess.py
Misc/NEWS

index ec417c20ea70293ba4ae92571cfe4a43800edc6d,017f58d7174597a33c607b0f809be2ed3416fb2c..db64588697c13ff6c0e1990bb81b368651389230
@@@ -1245,33 -1166,149 +1249,34 @@@ class Popen(object)
              errpipe_read, errpipe_write = _create_pipe()
              try:
                  try:
 -
 -                    if _posixsubprocess:
 -                        # We must avoid complex work that could involve
 -                        # malloc or free in the child process to avoid
 -                        # potential deadlocks, thus we do all this here.
 -                        # and pass it to fork_exec()
 -
 -                        if env is not None:
 -                            env_list = [os.fsencode(k) + b'=' + os.fsencode(v)
 -                                        for k, v in env.items()]
 -                        else:
 -                            env_list = None  # Use execv instead of execve.
 -                        executable = os.fsencode(executable)
 -                        if os.path.dirname(executable):
 -                            executable_list = (executable,)
 -                        else:
 -                            # This matches the behavior of os._execvpe().
 -                            executable_list = tuple(
 -                                os.path.join(os.fsencode(dir), executable)
 -                                for dir in os.get_exec_path(env))
 -                        fds_to_keep = set(pass_fds)
 -                        fds_to_keep.add(errpipe_write)
 -                        self.pid = _posixsubprocess.fork_exec(
 -                                args, executable_list,
 -                                close_fds, sorted(fds_to_keep), cwd, env_list,
 -                                p2cread, p2cwrite, c2pread, c2pwrite,
 -                                errread, errwrite,
 -                                errpipe_read, errpipe_write,
 -                                restore_signals, start_new_session, preexec_fn)
 -                        self._child_created = True
 +                    # We must avoid complex work that could involve
 +                    # malloc or free in the child process to avoid
 +                    # potential deadlocks, thus we do all this here.
 +                    # and pass it to fork_exec()
 +
 +                    if env is not None:
 +                        env_list = [os.fsencode(k) + b'=' + os.fsencode(v)
 +                                    for k, v in env.items()]
                      else:
 -                        # Pure Python implementation: It is not thread safe.
 -                        # This implementation may deadlock in the child if your
 -                        # parent process has any other threads running.
 -
 -                        gc_was_enabled = gc.isenabled()
 -                        # Disable gc to avoid bug where gc -> file_dealloc ->
 -                        # write to stderr -> hang.  See issue1336
 -                        gc.disable()
 -                        try:
 -                            self.pid = os.fork()
 -                        except:
 -                            if gc_was_enabled:
 -                                gc.enable()
 -                            raise
 -                        self._child_created = True
 -                        if self.pid == 0:
 -                            # Child
 -                            try:
 -                                # Close parent's pipe ends
 -                                if p2cwrite != -1:
 -                                    os.close(p2cwrite)
 -                                if c2pread != -1:
 -                                    os.close(c2pread)
 -                                if errread != -1:
 -                                    os.close(errread)
 -                                os.close(errpipe_read)
 -
 -                                # When duping fds, if there arises a situation
 -                                # where one of the fds is either 0, 1 or 2, it
 -                                # is possible that it is overwritten (#12607).
 -                                if c2pwrite == 0:
 -                                    c2pwrite = os.dup(c2pwrite)
 -                                if errwrite == 0 or errwrite == 1:
 -                                    errwrite = os.dup(errwrite)
 -
 -                                # Dup fds for child
 -                                def _dup2(a, b):
 -                                    # dup2() removes the CLOEXEC flag but
 -                                    # we must do it ourselves if dup2()
 -                                    # would be a no-op (issue #10806).
 -                                    if a == b:
 -                                        _set_cloexec(a, False)
 -                                    elif a != -1:
 -                                        os.dup2(a, b)
 -                                _dup2(p2cread, 0)
 -                                _dup2(c2pwrite, 1)
 -                                _dup2(errwrite, 2)
 -
 -                                # Close pipe fds.  Make sure we don't close the
 -                                # same fd more than once, or standard fds.
 -                                closed = set()
 -                                for fd in [p2cread, c2pwrite, errwrite]:
 -                                    if fd > 2 and fd not in closed:
 -                                        os.close(fd)
 -                                        closed.add(fd)
 -
 -                                # Close all other fds, if asked for
 -                                if close_fds:
 -                                    fds_to_keep = set(pass_fds)
 -                                    fds_to_keep.add(errpipe_write)
 -                                    self._close_fds(fds_to_keep)
 -
 -
 -                                if cwd is not None:
 -                                    os.chdir(cwd)
 -
 -                                # This is a copy of Python/pythonrun.c
 -                                # _Py_RestoreSignals().  If that were exposed
 -                                # as a sys._py_restoresignals func it would be
 -                                # better.. but this pure python implementation
 -                                # isn't likely to be used much anymore.
 -                                if restore_signals:
 -                                    signals = ('SIGPIPE', 'SIGXFZ', 'SIGXFSZ')
 -                                    for sig in signals:
 -                                        if hasattr(signal, sig):
 -                                            signal.signal(getattr(signal, sig),
 -                                                          signal.SIG_DFL)
 -
 -                                if start_new_session and hasattr(os, 'setsid'):
 -                                    os.setsid()
 -
 -                                if preexec_fn:
 -                                    preexec_fn()
 -
 -                                if env is None:
 -                                    os.execvp(executable, args)
 -                                else:
 -                                    os.execvpe(executable, args, env)
 -
 -                            except:
 -                                try:
 -                                    exc_type, exc_value = sys.exc_info()[:2]
 -                                    if isinstance(exc_value, OSError):
 -                                        errno_num = exc_value.errno
 -                                    else:
 -                                        errno_num = 0
 -                                    message = '%s:%x:%s' % (exc_type.__name__,
 -                                                            errno_num, exc_value)
 -                                    message = message.encode(errors="surrogatepass")
 -                                    os.write(errpipe_write, message)
 -                                except Exception:
 -                                    # We MUST not allow anything odd happening
 -                                    # above to prevent us from exiting below.
 -                                    pass
 -
 -                            # 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()
 +                        env_list = None  # Use execv instead of execve.
 +                    executable = os.fsencode(executable)
 +                    if os.path.dirname(executable):
 +                        executable_list = (executable,)
 +                    else:
 +                        # This matches the behavior of os._execvpe().
 +                        executable_list = tuple(
 +                            os.path.join(os.fsencode(dir), executable)
 +                            for dir in os.get_exec_path(env))
 +                    fds_to_keep = set(pass_fds)
 +                    fds_to_keep.add(errpipe_write)
 +                    self.pid = _posixsubprocess.fork_exec(
 +                            args, executable_list,
 +                            close_fds, sorted(fds_to_keep), cwd, env_list,
 +                            p2cread, p2cwrite, c2pread, c2pwrite,
 +                            errread, errwrite,
 +                            errpipe_read, errpipe_write,
 +                            restore_signals, start_new_session, preexec_fn)
++                    self._child_created = True
                  finally:
                      # be sure the FD is closed no matter what
                      os.close(errpipe_write)
Simple merge
diff --cc Misc/NEWS
index 06e44836bd9dd5de3e31e1f4d6c31e85fb27974f,a62b485fc21f10eb11ccbd188e0ceaf421440161..b96bb3da17cb7e1d58a7973c50fd21f600d86224
+++ b/Misc/NEWS
@@@ -262,12 -61,6 +262,15 @@@ Core and Builtin
  Library
  -------
  
++- Issue #12650: Fix a race condition where a subprocess.Popen could leak
++  resources (FD/zombie) when killed at the wrong time.
++
 +- Issue #12744: Fix inefficient representation of integers between 2**31 and
 +  2**63 on systems with a 64-bit C "long".
 +
 +- Issue #12646: Add an 'eof' attribute to zlib.Decompress, to make it easier to
 +  detect truncated input streams.
 +
  - Issue #11513: Fix exception handling ``tarfile.TarFile.gzopen()`` when
    the file cannot be opened.