subprocess's Popen.wait() is now thread safe so that multiple threads
may be calling wait() or poll() on a Popen instance at the same time
without losing the Popen.returncode value. Fixes issue #21291.
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 38f0f18..ddc033a 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -405,6 +405,10 @@
import _posixsubprocess
import select
import selectors
+ try:
+ import threading
+ except ImportError:
+ import dummy_threading as threading
# When select or poll has indicated that the file is writable,
# we can write up to _PIPE_BUF bytes without risk of blocking.
@@ -748,6 +752,12 @@
pass_fds=()):
"""Create new Popen instance."""
_cleanup()
+ # Held while anything is calling waitpid before returncode has been
+ # updated to prevent clobbering returncode if wait() or poll() are
+ # called from multiple threads at once. After acquiring the lock,
+ # code must re-check self.returncode to see if another thread just
+ # finished a waitpid() call.
+ self._waitpid_lock = threading.Lock()
self._input = None
self._communication_started = False
@@ -1450,6 +1460,7 @@
def _handle_exitstatus(self, sts, _WIFSIGNALED=os.WIFSIGNALED,
_WTERMSIG=os.WTERMSIG, _WIFEXITED=os.WIFEXITED,
_WEXITSTATUS=os.WEXITSTATUS):
+ """All callers to this function MUST hold self._waitpid_lock."""
# This method is called (indirectly) by __del__, so it cannot
# refer to anything outside of its local scope.
if _WIFSIGNALED(sts):
@@ -1471,7 +1482,13 @@
"""
if self.returncode is None:
+ if not self._waitpid_lock.acquire(False):
+ # Something else is busy calling waitpid. Don't allow two
+ # at once. We know nothing yet.
+ return None
try:
+ if self.returncode is not None:
+ return self.returncode # Another thread waited.
pid, sts = _waitpid(self.pid, _WNOHANG)
if pid == self.pid:
self._handle_exitstatus(sts)
@@ -1485,10 +1502,13 @@
# can't get the status.
# http://bugs.python.org/issue15756
self.returncode = 0
+ finally:
+ self._waitpid_lock.release()
return self.returncode
def _try_wait(self, wait_flags):
+ """All callers to this function MUST hold self._waitpid_lock."""
try:
(pid, sts) = _eintr_retry_call(os.waitpid, self.pid, wait_flags)
except OSError as e:
@@ -1521,11 +1541,17 @@
# cribbed from Lib/threading.py in Thread.wait() at r71065.
delay = 0.0005 # 500 us -> initial delay of 1 ms
while True:
- (pid, sts) = self._try_wait(os.WNOHANG)
- assert pid == self.pid or pid == 0
- if pid == self.pid:
- self._handle_exitstatus(sts)
- break
+ if self._waitpid_lock.acquire(False):
+ try:
+ if self.returncode is not None:
+ break # Another thread waited.
+ (pid, sts) = self._try_wait(os.WNOHANG)
+ assert pid == self.pid or pid == 0
+ if pid == self.pid:
+ self._handle_exitstatus(sts)
+ break
+ finally:
+ self._waitpid_lock.release()
remaining = self._remaining_time(endtime)
if remaining <= 0:
raise TimeoutExpired(self.args, timeout)
@@ -1533,11 +1559,15 @@
time.sleep(delay)
else:
while self.returncode is None:
- (pid, sts) = self._try_wait(0)
- # Check the pid and loop as waitpid has been known to return
- # 0 even without WNOHANG in odd situations. issue14396.
- if pid == self.pid:
- self._handle_exitstatus(sts)
+ with self._waitpid_lock:
+ if self.returncode is not None:
+ break # Another thread waited.
+ (pid, sts) = self._try_wait(0)
+ # Check the pid and loop as waitpid has been known to
+ # return 0 even without WNOHANG in odd situations.
+ # http://bugs.python.org/issue14396.
+ if pid == self.pid:
+ self._handle_exitstatus(sts)
return self.returncode