1 From 5d1013256c133b61587b6a80a0f9d509ac11d123 Mon Sep 17 00:00:00 2001
2 From: Chris Jerdonek <chris.jerdonek@gmail.com>
3 Date: Sat, 16 May 2020 15:57:27 -0700
4 Subject: [PATCH 1/5] bpo-38323: Fix rare MultiLoopChildWatcher hangs.
6 This changes asyncio.MultiLoopChildWatcher's attach_loop() method
7 to call loop.add_signal_handler() instead of calling only
8 signal.signal(). This should eliminate some rare hangs since
9 loop.add_signal_handler() calls signal.set_wakeup_fd(). Without
10 this, the main thread sometimes wasn't getting awakened if a
11 signal occurred during an await.
13 Doc/library/asyncio-eventloop.rst | 4 ++-
14 Doc/library/asyncio-policy.rst | 13 ++++++-
15 Lib/asyncio/unix_events.py | 34 ++++++++++++++-----
16 Lib/test/test_asyncio/test_subprocess.py | 3 +-
17 .../2020-05-16-17-50-10.bpo-38323.Ar35np.rst | 2 ++
18 5 files changed, 44 insertions(+), 12 deletions(-)
19 create mode 100644 Misc/NEWS.d/next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst
21 --- Python-3.9.2/Lib/asyncio/unix_events.py.orig 2021-02-19 13:31:44.000000000 +0000
22 +++ Python-3.9.2/Lib/asyncio/unix_events.py 2021-02-27 22:27:23.974509830 +0000
24 def add_signal_handler(self, sig, callback, *args):
25 """Add a handler for a signal. UNIX only.
27 + This method can only be called from the main thread.
29 Raise ValueError if the signal number is invalid or uncatchable.
30 Raise RuntimeError if there is a problem setting up the handler.
32 @@ -1234,10 +1236,15 @@
35 handler = signal.getsignal(signal.SIGCHLD)
36 - if handler != self._sig_chld:
37 + # add_signal_handler() sets the handler to _sighandler_noop.
38 + if handler != _sighandler_noop:
39 logger.warning("SIGCHLD handler was changed by outside code")
42 + # This clears the wakeup file descriptor if necessary.
43 + loop.remove_signal_handler(signal.SIGCHLD)
44 signal.signal(signal.SIGCHLD, self._saved_sighandler)
46 self._saved_sighandler = None
49 @@ -1265,6 +1272,11 @@
50 # The reason to do it here is that attach_loop() is called from
51 # unix policy only for the main thread.
52 # Main thread is required for subscription on SIGCHLD signal
53 + if loop is None or self._saved_sighandler is not None:
57 + self._saved_sighandler = signal.getsignal(signal.SIGCHLD)
58 if self._saved_sighandler is not None:
61 @@ -1274,8 +1286,14 @@
62 "restore to default handler on watcher close.")
63 self._saved_sighandler = signal.SIG_DFL
65 - # Set SA_RESTART to limit EINTR occurrences.
66 - signal.siginterrupt(signal.SIGCHLD, False)
69 + 'A loop is being detached '
70 + 'from a child watcher with pending handlers',
73 + # This also sets up the wakeup file descriptor.
74 + loop.add_signal_handler(signal.SIGCHLD, self._sig_chld)
76 def _do_waitpid_all(self):
77 for pid in list(self._callbacks):
79 expected_pid, returncode)
80 loop.call_soon_threadsafe(callback, pid, returncode, *args)
82 - def _sig_chld(self, signum, frame):
83 + def _sig_chld(self, *args):
85 self._do_waitpid_all()
86 except (SystemExit, KeyboardInterrupt):
87 diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py
88 index 6657a88e657c2..b11a31a34a2c6 100644
89 --- a/Lib/test/test_asyncio/test_subprocess.py
90 +++ b/Lib/test/test_asyncio/test_subprocess.py
91 @@ -672,12 +672,13 @@ def setUp(self):
92 policy.set_child_watcher(watcher)
96 policy = asyncio.get_event_loop_policy()
97 watcher = policy.get_child_watcher()
98 policy.set_child_watcher(None)
99 watcher.attach_loop(None)
101 + # Since setUp() does super().setUp() first, do tearDown() last.
104 class SubprocessThreadedWatcherTests(SubprocessWatcherMixin,
105 test_utils.TestCase):
106 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
108 index 0000000000000..556e08c69d7a5
110 +++ b/Misc/NEWS.d/next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst
112 +Fix rare cases with ``MultiLoopChildWatcher`` where the event loop can
113 +fail to awaken in response to a :py:data:`SIGCHLD` signal.
115 From 9618884446dc4a72e401b0f05b2992e34e39d700 Mon Sep 17 00:00:00 2001
116 From: Chris Jerdonek <chris.jerdonek@gmail.com>
117 Date: Sat, 16 May 2020 18:49:59 -0700
118 Subject: [PATCH 2/5] Add docstring.
121 Lib/asyncio/unix_events.py | 5 +++++
122 1 file changed, 5 insertions(+)
124 diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py
125 index d2a32cb879b6b..17614c23c984c 100644
126 --- a/Lib/asyncio/unix_events.py
127 +++ b/Lib/asyncio/unix_events.py
128 @@ -1266,6 +1266,11 @@ def remove_child_handler(self, pid):
131 def attach_loop(self, loop):
133 + This registers the SIGCHLD signal handler.
135 + This method can only be called from the main thread.
137 # Don't save the loop but initialize itself if called first time
138 # The reason to do it here is that attach_loop() is called from
139 # unix policy only for the main thread.
141 From 4d4c147b9bfe4ce7bb51aa4745ead8a422e98c14 Mon Sep 17 00:00:00 2001
142 From: Chris Jerdonek <chris.jerdonek@gmail.com>
143 Date: Fri, 16 Oct 2020 16:37:11 -0700
144 Subject: [PATCH 3/5] Address a couple review comments.
147 Doc/library/asyncio-policy.rst | 2 +-
148 .../next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst | 4 ++--
149 2 files changed, 3 insertions(+), 3 deletions(-)
151 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
152 index 556e08c69d7a5..e9401d6a2e486 100644
153 --- a/Misc/NEWS.d/next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst
154 +++ b/Misc/NEWS.d/next/Library/2020-05-16-17-50-10.bpo-38323.Ar35np.rst
156 -Fix rare cases with ``MultiLoopChildWatcher`` where the event loop can
157 -fail to awaken in response to a :py:data:`SIGCHLD` signal.
158 +Fix rare cases with :class:`asyncio.MultiLoopChildWatcher` where the event
159 +loop can fail to awaken in response to a :py:data:`SIGCHLD` signal.
161 From 14f6cfc20e77a349a22ced05352afd3ee200b403 Mon Sep 17 00:00:00 2001
162 From: Chris Jerdonek <chris.jerdonek@gmail.com>
163 Date: Fri, 16 Oct 2020 16:46:49 -0700
164 Subject: [PATCH 4/5] Revert tearDown() change.
167 Lib/test/test_asyncio/test_subprocess.py | 3 +--
168 1 file changed, 1 insertion(+), 2 deletions(-)
170 diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py
171 index b11a31a34a2c6..6657a88e657c2 100644
172 --- a/Lib/test/test_asyncio/test_subprocess.py
173 +++ b/Lib/test/test_asyncio/test_subprocess.py
174 @@ -672,13 +672,12 @@ def setUp(self):
175 policy.set_child_watcher(watcher)
179 policy = asyncio.get_event_loop_policy()
180 watcher = policy.get_child_watcher()
181 policy.set_child_watcher(None)
182 watcher.attach_loop(None)
184 - # Since setUp() does super().setUp() first, do tearDown() last.
187 class SubprocessThreadedWatcherTests(SubprocessWatcherMixin,
188 test_utils.TestCase):