From e85a305503bf83c5a8ffb3a988dfe7b67461cbee Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 15 Jan 2020 17:38:55 +0100 Subject: 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. --- Lib/test/test_subprocess.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'Lib/test/test_subprocess.py') diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 2073fd1461..f1fb93455d 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -3120,6 +3120,31 @@ class POSIXProcessTestCase(BaseTestCase): self.assertEqual(returncode, -3) + def test_send_signal_race(self): + # bpo-38630: send_signal() must poll the process exit status to reduce + # the risk of sending the signal to the wrong process. + proc = subprocess.Popen(ZERO_RETURN_CMD) + + # wait until the process completes without using the Popen APIs. + pid, status = os.waitpid(proc.pid, 0) + self.assertEqual(pid, proc.pid) + self.assertTrue(os.WIFEXITED(status), status) + self.assertEqual(os.WEXITSTATUS(status), 0) + + # returncode is still None but the process completed. + self.assertIsNone(proc.returncode) + + with mock.patch("os.kill") as mock_kill: + proc.send_signal(signal.SIGTERM) + + # send_signal() didn't call os.kill() since the process already + # completed. + mock_kill.assert_not_called() + + # Don't check the returncode value: the test reads the exit status, + # so Popen failed to read it and uses a default returncode instead. + self.assertIsNotNone(proc.returncode) + @unittest.skipUnless(mswindows, "Windows specific tests") class Win32ProcessTestCase(BaseTestCase): -- cgit v1.2.1