bpo-38630: Fix subprocess.Popen.send_signal() race condition (GH-16984)

On Unix, subprocess.Popen.send_signal() now polls the process status.
Polling reduces the risk of sending a signal to the wrong process if
the process completed, the Popen.returncode attribute is still None,
and the pid has been reassigned (recycled) to a new different
process.
This commit is contained in:
Victor Stinner 2020-01-15 17:38:55 +01:00 committed by GitHub
parent ed154c387e
commit e85a305503
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 57 additions and 3 deletions

View file

@ -2061,9 +2061,31 @@ class Popen(object):
def send_signal(self, sig):
"""Send a signal to the process."""
# Skip signalling a process that we know has already died.
if self.returncode is None:
os.kill(self.pid, sig)
# bpo-38630: Polling reduces the risk of sending a signal to the
# wrong process if the process completed, the Popen.returncode
# attribute is still None, and the pid has been reassigned
# (recycled) to a new different process. This race condition can
# happens in two cases.
#
# Case 1. Thread A calls Popen.poll(), thread B calls
# Popen.send_signal(). In thread A, waitpid() succeed and returns
# the exit status. Thread B calls kill() because poll() in thread A
# did not set returncode yet. Calling poll() in thread B prevents
# the race condition thanks to Popen._waitpid_lock.
#
# Case 2. waitpid(pid, 0) has been called directly, without
# using Popen methods: returncode is still None is this case.
# Calling Popen.poll() will set returncode to a default value,
# since waitpid() fails with ProcessLookupError.
self.poll()
if self.returncode is not None:
# Skip signalling a process that we know has already died.
return
# The race condition can still happen if the race condition
# described above happens between the returncode test
# and the kill() call.
os.kill(self.pid, sig)
def terminate(self):
"""Terminate the process with SIGTERM