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.

1 files changed, 25 insertions, 3 deletions

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 30f0d1be15..79dffd349a 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -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