From d481d4b7676f2e5d33465ca691346f69af655b24 Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Mon, 1 Jul 2024 07:11:47 +0200 Subject: [PATCH] [3.13] gh-87744: fix waitpid race while calling send_signal in asyncio (GH-121126) (#121194) gh-87744: fix waitpid race while calling send_signal in asyncio (GH-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. (cherry picked from commit bd473aa598c5161521a7018896dc124728214a6c) Co-authored-by: Kumar Aditya --- Lib/asyncio/base_subprocess.py | 35 ++++++++++++++----- Lib/test/test_asyncio/test_subprocess.py | 15 ++++++++ ...4-06-29-05-08-59.gh-issue-87744.rpF6Jw.rst | 1 + 3 files changed, 42 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-06-29-05-08-59.gh-issue-87744.rpF6Jw.rst diff --git a/Lib/asyncio/base_subprocess.py b/Lib/asyncio/base_subprocess.py index 6dbde2b696ad..9c2ba679ce2b 100644 --- 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 cf1a1985338e..c822d8045b0c 100644 --- a/Lib/test/test_asyncio/test_subprocess.py +++ b/Lib/test/test_asyncio/test_subprocess.py @@ -873,6 +873,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 index 000000000000..c0b4f349fb6d --- /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. -- 2.47.3