Issue #14252: Fix subprocess.Popen.terminate() to not raise an error under Windows...

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 017f58d7174597a33c607b0f809be2ed3416fb2c..179f41a85f56c82def6489d77e07f0dede23c66c 100644 (file)
--- 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 fb0b8342465f852323c38a19e3254ec00e26d7a1..6150e88c54c1cb6655bf7086e1a818fa308860c6 100644 (file)
--- 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 f2d90fb4ae009c652aa1bda3b095a0d5d9d74856..5d03fee8e74c83bdb7b1159d548481bd43f81b7b 100644 (file)
--- 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 2338f3085b36dece2ea2ca5429491f9bfe329e5c..f9a79a73007aa3990395762c748f77bdfbd3659f 100644 (file)
--- 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;
 }