From cab0864b9560b05bdf6d6899ec18ab21cfd37596 Mon Sep 17 00:00:00 2001
From: Victor Stinner <vstinner@python.org>
Date: Thu, 14 Dec 2023 18:57:22 +0100
Subject: [PATCH] gh-113009: Fix multiprocessing Process.terminate() on Windows

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.
---
 Lib/multiprocessing/popen_spawn_win32.py      | 54 ++++++++++---------
 ...-12-14-19-00-29.gh-issue-113009.6LNdjz.rst |  5 ++
 2 files changed, 35 insertions(+), 24 deletions(-)
 create mode 100644 Misc/NEWS.d/next/Windows/2023-12-14-19-00-29.gh-issue-113009.6LNdjz.rst

diff --git a/Lib/multiprocessing/popen_spawn_win32.py b/Lib/multiprocessing/popen_spawn_win32.py
index af044305709e56..49d4c7eea22411 100644
--- a/Lib/multiprocessing/popen_spawn_win32.py
+++ b/Lib/multiprocessing/popen_spawn_win32.py
@@ -101,18 +101,20 @@ 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
 
@@ -120,18 +122,22 @@ 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
 
diff --git a/Misc/NEWS.d/next/Windows/2023-12-14-19-00-29.gh-issue-113009.6LNdjz.rst b/Misc/NEWS.d/next/Windows/2023-12-14-19-00-29.gh-issue-113009.6LNdjz.rst
new file mode 100644
index 00000000000000..6fd7f7f9afdfa2
--- /dev/null
+++ b/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.