[Python-checkins] bpo-27448: Work around a gc.disable race condition in subprocess. (#1932)
Gregory P. Smith
webhook-mailer at python.org
Tue Sep 5 14:20:05 EDT 2017
https://github.com/python/cpython/commit/5e8e371364ee58dadb9a4e4e51c7e9cf6bedbfae
commit: 5e8e371364ee58dadb9a4e4e51c7e9cf6bedbfae
branch: 2.7
author: Gregory P. Smith <greg at krypto.org>
committer: GitHub <noreply at github.com>
date: 2017-09-05T11:20:02-07:00
summary:
bpo-27448: Work around a gc.disable race condition in subprocess. (#1932)
* bpo-27448: Work around a gc.disable race condition in subprocess.
This works around a gc.isenabled/gc.disable race condition in the 2.7
subprocess module by using a lock for the critical section. It'll
prevent multiple simultaneous subprocess launches from winding up with
gc remaining disabled but it can't fix the ultimate problem: gc enable
and disable is a global setting and a hack.
Users are *strongly encouraged* to use subprocess32 from PyPI instead
of the 2.7 standard library subprocess module. Mixing threads with
subprocess is a recipie for disaster otherwise even with "fixes" to
ameliorate common issues like this.
* Add a blurb!
files:
A Misc/NEWS.d/next/Library/2017-09-05-10-55-50.bpo-27448.QdAqzZ.rst
M Lib/subprocess.py
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 4b41f5ec5c7..1f2da0ffbe8 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -71,6 +71,10 @@ class pywintypes:
else:
import select
_has_poll = hasattr(select, 'poll')
+ try:
+ import threading
+ except ImportError:
+ threading = None
import fcntl
import pickle
@@ -878,6 +882,21 @@ def _close_fds(self, but):
pass
+ # Used as a bandaid workaround for https://bugs.python.org/issue27448
+ # to prevent multiple simultaneous subprocess launches from interfering
+ # with one another and leaving gc disabled.
+ if threading:
+ _disabling_gc_lock = threading.Lock()
+ else:
+ class _noop_context_manager(object):
+ # A dummy context manager that does nothing for the rare
+ # user of a --without-threads build.
+ def __enter__(self): pass
+ def __exit__(self, *args): pass
+
+ _disabling_gc_lock = _noop_context_manager()
+
+
def _execute_child(self, args, executable, preexec_fn, close_fds,
cwd, env, universal_newlines,
startupinfo, creationflags, shell, to_close,
@@ -909,10 +928,12 @@ def _close_in_parent(fd):
errpipe_read, errpipe_write = self.pipe_cloexec()
try:
try:
- 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()
+ with self._disabling_gc_lock:
+ gc_was_enabled = gc.isenabled()
+ # Disable gc to avoid bug where gc -> file_dealloc ->
+ # write to stderr -> hang.
+ # https://bugs.python.org/issue1336
+ gc.disable()
try:
self.pid = os.fork()
except:
@@ -986,9 +1007,10 @@ def _dup2(a, b):
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)
+ finally:
+ # 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:
diff --git a/Misc/NEWS.d/next/Library/2017-09-05-10-55-50.bpo-27448.QdAqzZ.rst b/Misc/NEWS.d/next/Library/2017-09-05-10-55-50.bpo-27448.QdAqzZ.rst
new file mode 100644
index 00000000000..9e269858d4b
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2017-09-05-10-55-50.bpo-27448.QdAqzZ.rst
@@ -0,0 +1,5 @@
+Work around a `gc.disable()` race condition in the `subprocess` module that
+could leave garbage collection disabled when multiple threads are spawning
+subprocesses at once. Users are *strongly encouraged* to use the
+`subprocess32` module from PyPI on Python 2.7 instead, it is much more
+reliable.
More information about the Python-checkins
mailing list