Skip to content

Commit 5d10132

Browse files
committed
bpo-38323: Fix rare MultiLoopChildWatcher hangs.
This changes asyncio.MultiLoopChildWatcher's attach_loop() method to call loop.add_signal_handler() instead of calling only signal.signal(). This should eliminate some rare hangs since loop.add_signal_handler() calls signal.set_wakeup_fd(). Without this, the main thread sometimes wasn't getting awakened if a signal occurred during an await.
1 parent 1ce5841 commit 5d10132

File tree

5 files changed

+44
-12
lines changed

5 files changed

+44
-12
lines changed

Doc/library/asyncio-eventloop.rst

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1036,7 +1036,9 @@ Unix signals
10361036
The callback will be invoked by *loop*, along with other queued callbacks
10371037
and runnable coroutines of that event loop. Unlike signal handlers
10381038
registered using :func:`signal.signal`, a callback registered with this
1039-
function is allowed to interact with the event loop.
1039+
method is allowed to interact with the event loop. Using
1040+
:func:`signal.signal` instead of this method can also cause the event
1041+
loop not to awaken in rare situations when a signal is received.
10401042

10411043
Raise :exc:`ValueError` if the signal number is invalid or uncatchable.
10421044
Raise :exc:`RuntimeError` if there is a problem setting up the handler.

Doc/library/asyncio-policy.rst

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ implementation used by the asyncio event loop:
219219

220220
This implementation registers a :py:data:`SIGCHLD` signal handler on
221221
instantiation. That can break third-party code that installs a custom handler for
222-
`SIGCHLD`. signal).
222+
the :py:data:`SIGCHLD` signal).
223223

224224
The watcher avoids disrupting other code spawning processes
225225
by polling every process explicitly on a :py:data:`SIGCHLD` signal.
@@ -233,6 +233,17 @@ implementation used by the asyncio event loop:
233233

234234
.. versionadded:: 3.8
235235

236+
.. method:: attach_loop(loop)
237+
238+
Registers the :py:data:`SIGCHLD` signal handler. Like
239+
:meth:`loop.add_signal_handler`, this method can only be invoked
240+
from the main thread.
241+
242+
.. versionchanged:: 3.9
243+
244+
The method now calls :func:`signal.set_wakeup_fd` as part of the
245+
handler initialization.
246+
236247
.. class:: SafeChildWatcher
237248

238249
This implementation uses active event loop from the main thread to handle

Lib/asyncio/unix_events.py

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ def _process_self_data(self, data):
7878
def add_signal_handler(self, sig, callback, *args):
7979
"""Add a handler for a signal. UNIX only.
8080
81+
This method can only be called from the main thread.
82+
8183
Raise ValueError if the signal number is invalid or uncatchable.
8284
Raise RuntimeError if there is a problem setting up the handler.
8385
"""
@@ -1232,10 +1234,15 @@ def close(self):
12321234
self._callbacks.clear()
12331235
if self._saved_sighandler is not None:
12341236
handler = signal.getsignal(signal.SIGCHLD)
1235-
if handler != self._sig_chld:
1237+
# add_signal_handler() sets the handler to _sighandler_noop.
1238+
if handler != _sighandler_noop:
12361239
logger.warning("SIGCHLD handler was changed by outside code")
12371240
else:
1241+
loop = self._loop
1242+
# This clears the wakeup file descriptor if necessary.
1243+
loop.remove_signal_handler(signal.SIGCHLD)
12381244
signal.signal(signal.SIGCHLD, self._saved_sighandler)
1245+
12391246
self._saved_sighandler = None
12401247

12411248
def __enter__(self):
@@ -1263,15 +1270,24 @@ def attach_loop(self, loop):
12631270
# The reason to do it here is that attach_loop() is called from
12641271
# unix policy only for the main thread.
12651272
# Main thread is required for subscription on SIGCHLD signal
1273+
if loop is None or self._saved_sighandler is not None:
1274+
return
1275+
1276+
self._loop = loop
1277+
self._saved_sighandler = signal.getsignal(signal.SIGCHLD)
12661278
if self._saved_sighandler is None:
1267-
self._saved_sighandler = signal.signal(signal.SIGCHLD, self._sig_chld)
1268-
if self._saved_sighandler is None:
1269-
logger.warning("Previous SIGCHLD handler was set by non-Python code, "
1270-
"restore to default handler on watcher close.")
1271-
self._saved_sighandler = signal.SIG_DFL
1279+
logger.warning("Previous SIGCHLD handler was set by non-Python code, "
1280+
"restore to default handler on watcher close.")
1281+
self._saved_sighandler = signal.SIG_DFL
12721282

1273-
# Set SA_RESTART to limit EINTR occurrences.
1274-
signal.siginterrupt(signal.SIGCHLD, False)
1283+
if self._callbacks:
1284+
warnings.warn(
1285+
'A loop is being detached '
1286+
'from a child watcher with pending handlers',
1287+
RuntimeWarning)
1288+
1289+
# This also sets up the wakeup file descriptor.
1290+
loop.add_signal_handler(signal.SIGCHLD, self._sig_chld)
12751291

12761292
def _do_waitpid_all(self):
12771293
for pid in list(self._callbacks):
@@ -1314,7 +1330,7 @@ def _do_waitpid(self, expected_pid):
13141330
expected_pid, returncode)
13151331
loop.call_soon_threadsafe(callback, pid, returncode, *args)
13161332

1317-
def _sig_chld(self, signum, frame):
1333+
def _sig_chld(self, *args):
13181334
try:
13191335
self._do_waitpid_all()
13201336
except (SystemExit, KeyboardInterrupt):

Lib/test/test_asyncio/test_subprocess.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,12 +672,13 @@ def setUp(self):
672672
policy.set_child_watcher(watcher)
673673

674674
def tearDown(self):
675-
super().tearDown()
676675
policy = asyncio.get_event_loop_policy()
677676
watcher = policy.get_child_watcher()
678677
policy.set_child_watcher(None)
679678
watcher.attach_loop(None)
680679
watcher.close()
680+
# Since setUp() does super().setUp() first, do tearDown() last.
681+
super().tearDown()
681682

682683
class SubprocessThreadedWatcherTests(SubprocessWatcherMixin,
683684
test_utils.TestCase):
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix rare cases with ``MultiLoopChildWatcher`` where the event loop can
2+
fail to awaken in response to a :py:data:`SIGCHLD` signal.

0 commit comments

Comments
 (0)