Skip to content

Commit bd473aa

Browse files
pythongh-87744: fix waitpid race while calling send_signal in asyncio (python#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.
1 parent 1a84bdc commit bd473aa

File tree

3 files changed

+42
-9
lines changed

3 files changed

+42
-9
lines changed

Lib/asyncio/base_subprocess.py

+26-9
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import collections
22
import subprocess
33
import warnings
4+
import os
5+
import signal
6+
import sys
47

58
from . import protocols
69
from . import transports
@@ -142,17 +145,31 @@ def _check_proc(self):
142145
if self._proc is None:
143146
raise ProcessLookupError()
144147

145-
def send_signal(self, signal):
146-
self._check_proc()
147-
self._proc.send_signal(signal)
148+
if sys.platform == 'win32':
149+
def send_signal(self, signal):
150+
self._check_proc()
151+
self._proc.send_signal(signal)
152+
153+
def terminate(self):
154+
self._check_proc()
155+
self._proc.terminate()
156+
157+
def kill(self):
158+
self._check_proc()
159+
self._proc.kill()
160+
else:
161+
def send_signal(self, signal):
162+
self._check_proc()
163+
try:
164+
os.kill(self._proc.pid, signal)
165+
except ProcessLookupError:
166+
pass
148167

149-
def terminate(self):
150-
self._check_proc()
151-
self._proc.terminate()
168+
def terminate(self):
169+
self.send_signal(signal.SIGTERM)
152170

153-
def kill(self):
154-
self._check_proc()
155-
self._proc.kill()
171+
def kill(self):
172+
self.send_signal(signal.SIGKILL)
156173

157174
async def _connect_pipes(self, waiter):
158175
try:

Lib/test/test_asyncio/test_subprocess.py

+15
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,21 @@ async def main():
864864

865865
self.loop.run_until_complete(main())
866866

867+
@unittest.skipIf(sys.platform != 'linux', "Linux only")
868+
def test_subprocess_send_signal_race(self):
869+
# See https://github.com/python/cpython/issues/87744
870+
async def main():
871+
for _ in range(10):
872+
proc = await asyncio.create_subprocess_exec('sleep', '0.1')
873+
await asyncio.sleep(0.1)
874+
try:
875+
proc.send_signal(signal.SIGUSR1)
876+
except ProcessLookupError:
877+
pass
878+
self.assertNotEqual(await proc.wait(), 255)
879+
880+
self.loop.run_until_complete(main())
881+
867882

868883
if sys.platform != 'win32':
869884
# Unix
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix waitpid race while calling :meth:`~asyncio.subprocess.Process.send_signal` in asyncio. Patch by Kumar Aditya.

0 commit comments

Comments
 (0)