gh-87744: fix waitpid race while calling send_signal in asyncio (#121126)

asyncio earlier relied on subprocess module to send signals to the process, this has some drawbacks one being that subprocess module unnecessarily calls waitpid on child processes and hence it races with asyncio implementation which internally uses child watchers. To mitigate this, now asyncio sends signals directly to the process without going through the subprocess on non windows systems. On Windows it fallbacks to subprocess module handling but on windows there are no child watchers so this issue doesn't exists altogether.

diff --git a/Lib/asyncio/base_subprocess.py b/Lib/asyncio/base_subprocess.py
index 6dbde2b696ad1fd2111c9db788d97991be0cc31b..9c2ba679ce2bf1debf340062b8b0f23c4d373add 100644 (file)
--- a/Lib/asyncio/base_subprocess.py
+++ b/Lib/asyncio/base_subprocess.py
@@ -1,6 +1,9 @@
 import collections
 import subprocess
 import warnings
+import os
+import signal
+import sys
 
 from . import protocols
 from . import transports
@@ -142,17 +145,31 @@ class BaseSubprocessTransport(transports.SubprocessTransport):
         if self._proc is None:
             raise ProcessLookupError()
 
-    def send_signal(self, signal):
-        self._check_proc()
-        self._proc.send_signal(signal)
+    if sys.platform == 'win32':
+        def send_signal(self, signal):
+            self._check_proc()
+            self._proc.send_signal(signal)
+
+        def terminate(self):
+            self._check_proc()
+            self._proc.terminate()
+
+        def kill(self):
+            self._check_proc()
+            self._proc.kill()
+    else:
+        def send_signal(self, signal):
+            self._check_proc()
+            try:
+                os.kill(self._proc.pid, signal)
+            except ProcessLookupError:
+                pass
 
-    def terminate(self):
-        self._check_proc()
-        self._proc.terminate()
+        def terminate(self):
+            self.send_signal(signal.SIGTERM)
 
-    def kill(self):
-        self._check_proc()
-        self._proc.kill()
+        def kill(self):
+            self.send_signal(signal.SIGKILL)
 
     async def _connect_pipes(self, waiter):
         try:

diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py
index 23987c70ca7b6349a8c809097ece993a9754a810..54501300a29cf748be1b95c62637ea2e8ce6143e 100644 (file)
--- a/Lib/test/test_asyncio/test_subprocess.py
+++ b/Lib/test/test_asyncio/test_subprocess.py
@@ -864,6 +864,21 @@ class SubprocessMixin:
 
         self.loop.run_until_complete(main())
 
+    @unittest.skipIf(sys.platform != 'linux', "Linux only")
+    def test_subprocess_send_signal_race(self):
+        # See https://github.com/python/cpython/issues/87744
+        async def main():
+            for _ in range(10):
+                proc = await asyncio.create_subprocess_exec('sleep', '0.1')
+                await asyncio.sleep(0.1)
+                try:
+                    proc.send_signal(signal.SIGUSR1)
+                except ProcessLookupError:
+                    pass
+                self.assertNotEqual(await proc.wait(), 255)
+
+        self.loop.run_until_complete(main())
+
 
 if sys.platform != 'win32':
     # Unix

diff --git a/Misc/NEWS.d/next/Library/2024-06-29-05-08-59.gh-issue-87744.rpF6Jw.rst b/Misc/NEWS.d/next/Library/2024-06-29-05-08-59.gh-issue-87744.rpF6Jw.rst
new file mode 100644 (file)
index 0000000..c0b4f34
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2024-06-29-05-08-59.gh-issue-87744.rpF6Jw.rst
@@ -0,0 +1 @@
+Fix waitpid race while calling :meth:`~asyncio.subprocess.Process.send_signal` in asyncio. Patch by Kumar Aditya.