[WIP] bpo-38323: Fix MultiLoopChildWatcher hangs

Doc/library/asyncio-eventloop.rst

@@ -1036,7 +1036,10 @@ Unix signals
    The callback will be invoked by *loop*, along with other queued callbacks
    and runnable coroutines of that event loop. Unlike signal handlers
    registered using :func:`signal.signal`, a callback registered with this
-   function is allowed to interact with the event loop.
+   method is allowed to interact with the event loop. Also, if
+   :func:`signal.signal` is used instead of this method, the event loop
+   is not guaranteed to awaken when a signal is received, which can be a
+   cause of application hangs.
 
    Raise :exc:`ValueError` if the signal number is invalid or uncatchable.
    Raise :exc:`RuntimeError` if there is a problem setting up the handler.

Doc/library/asyncio-policy.rst

@@ -219,7 +219,7 @@ implementation used by the asyncio event loop:
 
    This implementation registers a :py:data:`SIGCHLD` signal handler on
    instantiation. That can break third-party code that installs a custom handler for
-   `SIGCHLD`.  signal).
+   the :py:data:`SIGCHLD` signal).
 
    The watcher avoids disrupting other code spawning processes
    by polling every process explicitly on a :py:data:`SIGCHLD` signal.
@@ -233,6 +233,17 @@ implementation used by the asyncio event loop:
 
    .. versionadded:: 3.8
 
+   .. method:: attach_loop(loop)
+
+      Registers the :py:data:`SIGCHLD` signal handler.  Like
+      :meth:`loop.add_signal_handler`, this method can only be invoked
+      from the main thread.
+
+     .. versionchanged:: 3.10
+
+        The method now calls :func:`signal.set_wakeup_fd` as part of the
+        handler initialization.
+
 .. class:: SafeChildWatcher
 
    This implementation uses active event loop from the main thread to handle

Lib/asyncio/unix_events.py

@@ -78,6 +78,8 @@ def _process_self_data(self, data):
     def add_signal_handler(self, sig, callback, *args):
         """Add a handler for a signal.  UNIX only.
 
+        This method can only be called from the main thread.
+
         Raise ValueError if the signal number is invalid or uncatchable.
         Raise RuntimeError if there is a problem setting up the handler.
         """
@@ -1232,10 +1234,15 @@ def close(self):
         self._callbacks.clear()
         if self._saved_sighandler is not None:
             handler = signal.getsignal(signal.SIGCHLD)
-            if handler != self._sig_chld:
+            # add_signal_handler() sets the handler to _sighandler_noop.
+            if handler != _sighandler_noop:
                 logger.warning("SIGCHLD handler was changed by outside code")
             else:
+                loop = self._loop
+                # This clears the wakeup file descriptor if necessary.
+                loop.remove_signal_handler(signal.SIGCHLD)
                 signal.signal(signal.SIGCHLD, self._saved_sighandler)
+
             self._saved_sighandler = None
 
     def __enter__(self):
@@ -1259,19 +1266,33 @@ def remove_child_handler(self, pid):
             return False
 
     def attach_loop(self, loop):
+        """
+        This registers the SIGCHLD signal handler.
+
+        This method can only be called from the main thread.
+        """
         # Don't save the loop but initialize itself if called first time
         # The reason to do it here is that attach_loop() is called from
         # unix policy only for the main thread.
         # Main thread is required for subscription on SIGCHLD signal
+        if loop is None or self._saved_sighandler is not None:
+            return
+
+        self._loop = loop
+        self._saved_sighandler = signal.getsignal(signal.SIGCHLD)
         if self._saved_sighandler is None:
-            self._saved_sighandler = signal.signal(signal.SIGCHLD, self._sig_chld)
-            if self._saved_sighandler is None:
-                logger.warning("Previous SIGCHLD handler was set by non-Python code, "
-                               "restore to default handler on watcher close.")
-                self._saved_sighandler = signal.SIG_DFL
+            logger.warning("Previous SIGCHLD handler was set by non-Python code, "
+                           "restore to default handler on watcher close.")
+            self._saved_sighandler = signal.SIG_DFL
 
-            # Set SA_RESTART to limit EINTR occurrences.
-            signal.siginterrupt(signal.SIGCHLD, False)
+        if self._callbacks:
+            warnings.warn(
+                'A loop is being detached '
+                'from a child watcher with pending handlers',
+                RuntimeWarning)
+
+        # This also sets up the wakeup file descriptor.
+        loop.add_signal_handler(signal.SIGCHLD, self._sig_chld)
 
     def _do_waitpid_all(self):
         for pid in list(self._callbacks):
@@ -1314,7 +1335,7 @@ def _do_waitpid(self, expected_pid):
                                  expected_pid, returncode)
                 loop.call_soon_threadsafe(callback, pid, returncode, *args)
 
-    def _sig_chld(self, signum, frame):
+    def _sig_chld(self, *args):
         try:
             self._do_waitpid_all()
         except (SystemExit, KeyboardInterrupt):

Misc/NEWS.d/next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst

@@ -0,0 +1,2 @@
+Fix rare cases with :class:`asyncio.MultiLoopChildWatcher` where the event
+loop can fail to awaken in response to a :py:data:`SIGCHLD` signal.