From 1f9a8354002dbb29f38bfbd4273ce62e40ac80ba Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Sun, 11 Mar 2012 19:29:12 +0100 Subject: Issue #14252: Fix subprocess.Popen.terminate() to not raise an error under Windows when the child process has already exited. --- Lib/subprocess.py | 12 +++++++- Lib/test/test_subprocess.py | 67 +++++++++++++++++++++++++++++++++++++++++++++ Misc/NEWS | 3 ++ PC/_subprocess.c | 1 + 4 files changed, 82 insertions(+), 1 deletion(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 017f58d..179f41a 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1075,7 +1075,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 fb0b834..6150e88 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -989,6 +989,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() @@ -1007,6 +1028,18 @@ class POSIXProcessTestCase(BaseTestCase): self.assertStderrEqual(stderr, b'') 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 @@ -1568,6 +1601,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) @@ -1577,6 +1635,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') + # The module says: # "NB This only works (and is only relevant) for UNIX." diff --git a/Misc/NEWS b/Misc/NEWS index f2d90fb..5d03fee 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -22,6 +22,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 2338f30..f9a79a7 100644 --- a/PC/_subprocess.c +++ b/PC/_subprocess.c @@ -684,6 +684,7 @@ PyInit__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); return m; } -- cgit v0.12