diff --git a/Lib/subprocess.py b/Lib/subprocess.py index f0ef30e7fc7..6cc3fa13128 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1016,7 +1016,17 @@ class Popen(object): def terminate(self): """Terminates the process """ - _subprocess.TerminateProcess(self._handle, 1) + try: + _subprocess.TerminateProcess(self._handle, 1) + except OSError as e: + # ERROR_ACCESS_DENIED (winerror 5) is received when the + # process already died. + if e.winerror != 5: + raise + rc = _subprocess.GetExitCodeProcess(self._handle) + if rc == _subprocess.STILL_ACTIVE: + raise + self.returncode = rc kill = terminate diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index b66356ddbf8..20b1ade753d 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -812,6 +812,27 @@ class POSIXProcessTestCase(BaseTestCase): getattr(p, method)(*args) return p + def _kill_dead_process(self, method, *args): + # Do not inherit file handles from the parent. + # It should fix failures on some platforms. + p = subprocess.Popen([sys.executable, "-c", """if 1: + import sys, time + sys.stdout.write('x\\n') + sys.stdout.flush() + """], + close_fds=True, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + # Wait for the interpreter to be completely initialized before + # sending any signal. + p.stdout.read(1) + # The process should end after this + time.sleep(1) + # This shouldn't raise even though the child is now dead + getattr(p, method)(*args) + p.communicate() + def test_send_signal(self): p = self._kill_process('send_signal', signal.SIGINT) _, stderr = p.communicate() @@ -830,6 +851,18 @@ class POSIXProcessTestCase(BaseTestCase): self.assertStderrEqual(stderr, '') self.assertEqual(p.wait(), -signal.SIGTERM) + def test_send_signal_dead(self): + # Sending a signal to a dead process + self._kill_dead_process('send_signal', signal.SIGINT) + + def test_kill_dead(self): + # Killing a dead process + self._kill_dead_process('kill') + + def test_terminate_dead(self): + # Terminating a dead process + self._kill_dead_process('terminate') + def check_close_std_fds(self, fds): # Issue #9905: test that subprocess pipes still work properly with # some standard fds closed @@ -1126,6 +1159,31 @@ class Win32ProcessTestCase(BaseTestCase): returncode = p.wait() self.assertNotEqual(returncode, 0) + def _kill_dead_process(self, method, *args): + p = subprocess.Popen([sys.executable, "-c", """if 1: + import sys, time + sys.stdout.write('x\\n') + sys.stdout.flush() + sys.exit(42) + """], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + self.addCleanup(p.stdout.close) + self.addCleanup(p.stderr.close) + self.addCleanup(p.stdin.close) + # Wait for the interpreter to be completely initialized before + # sending any signal. + p.stdout.read(1) + # The process should end after this + time.sleep(1) + # This shouldn't raise even though the child is now dead + getattr(p, method)(*args) + _, stderr = p.communicate() + self.assertStderrEqual(stderr, b'') + rc = p.wait() + self.assertEqual(rc, 42) + def test_send_signal(self): self._kill_process('send_signal', signal.SIGTERM) @@ -1135,6 +1193,15 @@ class Win32ProcessTestCase(BaseTestCase): def test_terminate(self): self._kill_process('terminate') + def test_send_signal_dead(self): + self._kill_dead_process('send_signal', signal.SIGTERM) + + def test_kill_dead(self): + self._kill_dead_process('kill') + + def test_terminate_dead(self): + self._kill_dead_process('terminate') + @unittest.skipUnless(getattr(subprocess, '_has_poll', False), "poll system call not supported") diff --git a/Misc/NEWS b/Misc/NEWS index 0e06a1a41d7..c5c2c8225a6 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -18,6 +18,9 @@ Core and Builtins Library ------- +- Issue #14252: Fix subprocess.Popen.terminate() to not raise an error under + Windows when the child process has already exited. + - Issue #14195: An issue that caused weakref.WeakSet instances to incorrectly return True for a WeakSet instance 'a' in both 'a < a' and 'a > a' has been fixed. diff --git a/PC/_subprocess.c b/PC/_subprocess.c index 6780382e6ea..689b0c8b433 100644 --- a/PC/_subprocess.c +++ b/PC/_subprocess.c @@ -670,4 +670,5 @@ init_subprocess() defint(d, "WAIT_OBJECT_0", WAIT_OBJECT_0); defint(d, "CREATE_NEW_CONSOLE", CREATE_NEW_CONSOLE); defint(d, "CREATE_NEW_PROCESS_GROUP", CREATE_NEW_PROCESS_GROUP); + defint(d, "STILL_ACTIVE", STILL_ACTIVE); }