From 5d1013256c133b61587b6a80a0f9d509ac11d123 Mon Sep 17 00:00:00 2001
|
From: Chris Jerdonek <chris.jerdonek@gmail.com>
|
Date: Sat, 16 May 2020 15:57:27 -0700
|
Subject: [PATCH 1/5] 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.
|
---
|
Doc/library/asyncio-eventloop.rst | 4 ++-
|
Doc/library/asyncio-policy.rst | 13 ++++++-
|
Lib/asyncio/unix_events.py | 34 ++++++++++++++-----
|
Lib/test/test_asyncio/test_subprocess.py | 3 +-
|
.../2020-05-16-17-50-10.bpo-38323.Ar35np.rst | 2 ++
|
5 files changed, 44 insertions(+), 12 deletions(-)
|
create mode 100644 Misc/NEWS.d/next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst
|
|
diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py
|
index 19d713545e4cd..d2a32cb879b6b 100644
|
--- a/Lib/asyncio/unix_events.py
|
+++ b/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):
|
@@ -1263,15 +1270,24 @@ def attach_loop(self, loop):
|
# 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 +1330,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):
|
diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py
|
index 6657a88e657c2..b11a31a34a2c6 100644
|
--- a/Lib/test/test_asyncio/test_subprocess.py
|
+++ b/Lib/test/test_asyncio/test_subprocess.py
|
@@ -672,12 +672,13 @@ def setUp(self):
|
policy.set_child_watcher(watcher)
|
|
def tearDown(self):
|
- super().tearDown()
|
policy = asyncio.get_event_loop_policy()
|
watcher = policy.get_child_watcher()
|
policy.set_child_watcher(None)
|
watcher.attach_loop(None)
|
watcher.close()
|
+ # Since setUp() does super().setUp() first, do tearDown() last.
|
+ super().tearDown()
|
|
class SubprocessThreadedWatcherTests(SubprocessWatcherMixin,
|
test_utils.TestCase):
|
diff --git a/Misc/NEWS.d/next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst b/Misc/NEWS.d/next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst
|
new file mode 100644
|
index 0000000000000..556e08c69d7a5
|
--- /dev/null
|
+++ b/Misc/NEWS.d/next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst
|
@@ -0,0 +1,2 @@
|
+Fix rare cases with ``MultiLoopChildWatcher`` where the event loop can
|
+fail to awaken in response to a :py:data:`SIGCHLD` signal.
|
|
From 9618884446dc4a72e401b0f05b2992e34e39d700 Mon Sep 17 00:00:00 2001
|
From: Chris Jerdonek <chris.jerdonek@gmail.com>
|
Date: Sat, 16 May 2020 18:49:59 -0700
|
Subject: [PATCH 2/5] Add docstring.
|
|
---
|
Lib/asyncio/unix_events.py | 5 +++++
|
1 file changed, 5 insertions(+)
|
|
diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py
|
index d2a32cb879b6b..17614c23c984c 100644
|
--- a/Lib/asyncio/unix_events.py
|
+++ b/Lib/asyncio/unix_events.py
|
@@ -1266,6 +1266,11 @@ 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.
|
|
From 4d4c147b9bfe4ce7bb51aa4745ead8a422e98c14 Mon Sep 17 00:00:00 2001
|
From: Chris Jerdonek <chris.jerdonek@gmail.com>
|
Date: Fri, 16 Oct 2020 16:37:11 -0700
|
Subject: [PATCH 3/5] Address a couple review comments.
|
|
---
|
Doc/library/asyncio-policy.rst | 2 +-
|
.../next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst | 4 ++--
|
2 files changed, 3 insertions(+), 3 deletions(-)
|
|
diff --git a/Misc/NEWS.d/next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst b/Misc/NEWS.d/next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst
|
index 556e08c69d7a5..e9401d6a2e486 100644
|
--- a/Misc/NEWS.d/next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst
|
+++ b/Misc/NEWS.d/next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst
|
@@ -1,2 +1,2 @@
|
-Fix rare cases with ``MultiLoopChildWatcher`` where the event loop can
|
-fail to awaken in response to a :py:data:`SIGCHLD` signal.
|
+Fix rare cases with :class:`asyncio.MultiLoopChildWatcher` where the event
|
+loop can fail to awaken in response to a :py:data:`SIGCHLD` signal.
|
|
From 14f6cfc20e77a349a22ced05352afd3ee200b403 Mon Sep 17 00:00:00 2001
|
From: Chris Jerdonek <chris.jerdonek@gmail.com>
|
Date: Fri, 16 Oct 2020 16:46:49 -0700
|
Subject: [PATCH 4/5] Revert tearDown() change.
|
|
---
|
Lib/test/test_asyncio/test_subprocess.py | 3 +--
|
1 file changed, 1 insertion(+), 2 deletions(-)
|
|
diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py
|
index b11a31a34a2c6..6657a88e657c2 100644
|
--- a/Lib/test/test_asyncio/test_subprocess.py
|
+++ b/Lib/test/test_asyncio/test_subprocess.py
|
@@ -672,13 +672,12 @@ def setUp(self):
|
policy.set_child_watcher(watcher)
|
|
def tearDown(self):
|
+ super().tearDown()
|
policy = asyncio.get_event_loop_policy()
|
watcher = policy.get_child_watcher()
|
policy.set_child_watcher(None)
|
watcher.attach_loop(None)
|
watcher.close()
|
- # Since setUp() does super().setUp() first, do tearDown() last.
|
- super().tearDown()
|
|
class SubprocessThreadedWatcherTests(SubprocessWatcherMixin,
|
test_utils.TestCase):
|
