pythongh-113009: Fix multiprocessing Process.terminate() on Windows (python#113128)

On Windows, Process.terminate() no longer sets the returncode
attribute to always call WaitForSingleObject() in Process.wait().
Previously, sometimes the process was still running after
TerminateProcess() even if GetExitCodeProcess() is not STILL_ACTIVE.

Author: vstinner

Lib/multiprocessing/popen_spawn_win32.py

@@ -101,37 +101,43 @@ def duplicate_for_child(self, handle):
         return reduction.duplicate(handle, self.sentinel)
 
     def wait(self, timeout=None):
-        if self.returncode is None:
-            if timeout is None:
-                msecs = _winapi.INFINITE
-            else:
-                msecs = max(0, int(timeout * 1000 + 0.5))
-
-            res = _winapi.WaitForSingleObject(int(self._handle), msecs)
-            if res == _winapi.WAIT_OBJECT_0:
-                code = _winapi.GetExitCodeProcess(self._handle)
-                if code == TERMINATE:
-                    code = -signal.SIGTERM
-                self.returncode = code
+        if self.returncode is not None:
+            return self.returncode
+
+        if timeout is None:
+            msecs = _winapi.INFINITE
+        else:
+            msecs = max(0, int(timeout * 1000 + 0.5))
+
+        res = _winapi.WaitForSingleObject(int(self._handle), msecs)
+        if res == _winapi.WAIT_OBJECT_0:
+            code = _winapi.GetExitCodeProcess(self._handle)
+            if code == TERMINATE:
+                code = -signal.SIGTERM
+            self.returncode = code
 
         return self.returncode
 
     def poll(self):
         return self.wait(timeout=0)
 
     def terminate(self):
-        if self.returncode is None:
-            try:
-                _winapi.TerminateProcess(int(self._handle), TERMINATE)
-            except PermissionError:
-                # ERROR_ACCESS_DENIED (winerror 5) is received when the
-                # process already died.
-                code = _winapi.GetExitCodeProcess(int(self._handle))
-                if code == _winapi.STILL_ACTIVE:
-                    raise
-                self.returncode = code
-            else:
-                self.returncode = -signal.SIGTERM
+        if self.returncode is not None:
+            return
+
+        try:
+            _winapi.TerminateProcess(int(self._handle), TERMINATE)
+        except PermissionError:
+            # ERROR_ACCESS_DENIED (winerror 5) is received when the
+            # process already died.
+            code = _winapi.GetExitCodeProcess(int(self._handle))
+            if code == _winapi.STILL_ACTIVE:
+                raise
+
+        # gh-113009: Don't set self.returncode. Even if GetExitCodeProcess()
+        # returns an exit code different than STILL_ACTIVE, the process can
+        # still be running. Only set self.returncode once WaitForSingleObject()
+        # returns WAIT_OBJECT_0 in wait().
 
     kill = terminate
 

Misc/NEWS.d/next/Windows/2023-12-14-19-00-29.gh-issue-113009.6LNdjz.rst

@@ -0,0 +1,5 @@
+:mod:`multiprocessing`: On Windows, fix a race condition in
+``Process.terminate()``: no longer set the ``returncode`` attribute to
+always call ``WaitForSingleObject()`` in ``Process.wait()``.  Previously,
+sometimes the process was still running after ``TerminateProcess()`` even if
+``GetExitCodeProcess()`` is not ``STILL_ACTIVE``. Patch by Victor Stinner.