diff options
author | Michel Hidalgo <michel@ekumenlabs.com> | 2022-09-17 15:07:54 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-09-17 15:07:54 (GMT) |
commit | 05878106989c6f5b9dd35a6c15a21bee59312827 (patch) | |
tree | 8d58f02a0cc3e14d4983106033a5d97e4cd49bf1 /Lib | |
parent | 2cd70ffb3fe22d778d0bb6ec220fdf67dccc1be6 (diff) | |
download | cpython-05878106989c6f5b9dd35a6c15a21bee59312827.zip cpython-05878106989c6f5b9dd35a6c15a21bee59312827.tar.gz cpython-05878106989c6f5b9dd35a6c15a21bee59312827.tar.bz2 |
gh-87079: Warn on unintended signal wakeup fd override in `asyncio` (#96807)
Warn on loop initialization, when setting the wakeup fd disturbs a previously set wakeup fd, and on loop closing, when upon resetting the wakeup fd, we find it has been changed by someone else.
Diffstat (limited to 'Lib')
-rw-r--r-- | Lib/asyncio/proactor_events.py | 14 | ||||
-rw-r--r-- | Lib/asyncio/unix_events.py | 19 | ||||
-rw-r--r-- | Lib/test/test_asyncio/test_proactor_events.py | 15 | ||||
-rw-r--r-- | Lib/test/test_asyncio/test_unix_events.py | 24 |
4 files changed, 64 insertions, 8 deletions
diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py index ddb9dac..4808c5d 100644 --- a/Lib/asyncio/proactor_events.py +++ b/Lib/asyncio/proactor_events.py @@ -635,7 +635,12 @@ class BaseProactorEventLoop(base_events.BaseEventLoop): self._make_self_pipe() if threading.current_thread() is threading.main_thread(): # wakeup fd can only be installed to a file descriptor from the main thread - signal.set_wakeup_fd(self._csock.fileno()) + oldfd = signal.set_wakeup_fd(self._csock.fileno()) + if oldfd != -1: + warnings.warn( + "Signal wakeup fd was already set", + ResourceWarning, + source=self) def _make_socket_transport(self, sock, protocol, waiter=None, extra=None, server=None): @@ -684,7 +689,12 @@ class BaseProactorEventLoop(base_events.BaseEventLoop): return if threading.current_thread() is threading.main_thread(): - signal.set_wakeup_fd(-1) + oldfd = signal.set_wakeup_fd(-1) + if oldfd != self._csock.fileno(): + warnings.warn( + "Got unexpected signal wakeup fd", + ResourceWarning, + source=self) # Call these methods before closing the event loop (before calling # BaseEventLoop.close), because they can schedule callbacks with # call_soon(), which is forbidden when the event loop is closed. diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py index cf7683f..6c9a89d 100644 --- a/Lib/asyncio/unix_events.py +++ b/Lib/asyncio/unix_events.py @@ -65,7 +65,9 @@ class _UnixSelectorEventLoop(selector_events.BaseSelectorEventLoop): self._signal_handlers = {} def close(self): - super().close() + # remove signal handlers first to verify + # the loop's signal handling setup has not + # been tampered with if not sys.is_finalizing(): for sig in list(self._signal_handlers): self.remove_signal_handler(sig) @@ -77,6 +79,7 @@ class _UnixSelectorEventLoop(selector_events.BaseSelectorEventLoop): ResourceWarning, source=self) self._signal_handlers.clear() + super().close() def _process_self_data(self, data): for signum in data: @@ -102,7 +105,12 @@ class _UnixSelectorEventLoop(selector_events.BaseSelectorEventLoop): # main thread. By calling it early we ensure that an # event loop running in another thread cannot add a signal # handler. - signal.set_wakeup_fd(self._csock.fileno()) + oldfd = signal.set_wakeup_fd(self._csock.fileno()) + if oldfd != -1 and oldfd != self._csock.fileno(): + warnings.warn( + "Signal wakeup fd was already set", + ResourceWarning, + source=self) except (ValueError, OSError) as exc: raise RuntimeError(str(exc)) @@ -166,7 +174,12 @@ class _UnixSelectorEventLoop(selector_events.BaseSelectorEventLoop): if not self._signal_handlers: try: - signal.set_wakeup_fd(-1) + oldfd = signal.set_wakeup_fd(-1) + if oldfd != -1 and oldfd != self._csock.fileno(): + warnings.warn( + "Got unexpected signal wakeup fd", + ResourceWarning, + source=self) except (ValueError, OSError) as exc: logger.info('set_wakeup_fd(-1) failed: %s', exc) diff --git a/Lib/test/test_asyncio/test_proactor_events.py b/Lib/test/test_asyncio/test_proactor_events.py index 7fca054..6b28348 100644 --- a/Lib/test/test_asyncio/test_proactor_events.py +++ b/Lib/test/test_asyncio/test_proactor_events.py @@ -720,8 +720,12 @@ class BaseProactorEventLoopTests(test_utils.TestCase): def test_ctor(self, socketpair): ssock, csock = socketpair.return_value = ( mock.Mock(), mock.Mock()) - with mock.patch('signal.set_wakeup_fd'): - loop = BaseProactorEventLoop(self.proactor) + with mock.patch('signal.set_wakeup_fd') as set_wakeup_fd: + set_wakeup_fd.return_value = -1000 + with self.assertWarnsRegex( + ResourceWarning, 'Signal wakeup fd was already set' + ): + loop = BaseProactorEventLoop(self.proactor) self.assertIs(loop._ssock, ssock) self.assertIs(loop._csock, csock) self.assertEqual(loop._internal_fds, 1) @@ -740,7 +744,12 @@ class BaseProactorEventLoopTests(test_utils.TestCase): def test_close(self): self.loop._close_self_pipe = mock.Mock() - self.loop.close() + with mock.patch('signal.set_wakeup_fd') as set_wakeup_fd: + set_wakeup_fd.return_value = -1000 + with self.assertWarnsRegex( + ResourceWarning, 'Got unexpected signal wakeup fd' + ): + self.loop.close() self.assertTrue(self.loop._close_self_pipe.called) self.assertTrue(self.proactor.close.called) self.assertIsNone(self.loop._proactor) diff --git a/Lib/test/test_asyncio/test_unix_events.py b/Lib/test/test_asyncio/test_unix_events.py index 5bad21e..1ec627f 100644 --- a/Lib/test/test_asyncio/test_unix_events.py +++ b/Lib/test/test_asyncio/test_unix_events.py @@ -89,6 +89,17 @@ class SelectorEventLoopSignalTests(test_utils.TestCase): signal.SIGINT, lambda: True) @mock.patch('asyncio.unix_events.signal') + def test_add_signal_handler_setup_warn(self, m_signal): + m_signal.NSIG = signal.NSIG + m_signal.valid_signals = signal.valid_signals + m_signal.set_wakeup_fd.return_value = -1000 + + with self.assertWarnsRegex( + ResourceWarning, 'Signal wakeup fd was already set' + ): + self.loop.add_signal_handler(signal.SIGINT, lambda: True) + + @mock.patch('asyncio.unix_events.signal') def test_add_signal_handler_coroutine_error(self, m_signal): m_signal.NSIG = signal.NSIG @@ -214,6 +225,19 @@ class SelectorEventLoopSignalTests(test_utils.TestCase): self.assertTrue(m_logging.info) @mock.patch('asyncio.unix_events.signal') + def test_remove_signal_handler_cleanup_warn(self, m_signal): + m_signal.NSIG = signal.NSIG + m_signal.valid_signals = signal.valid_signals + self.loop.add_signal_handler(signal.SIGHUP, lambda: True) + + m_signal.set_wakeup_fd.return_value = -1000 + + with self.assertWarnsRegex( + ResourceWarning, 'Got unexpected signal wakeup fd' + ): + self.loop.remove_signal_handler(signal.SIGHUP) + + @mock.patch('asyncio.unix_events.signal') def test_remove_signal_handler_error(self, m_signal): m_signal.NSIG = signal.NSIG m_signal.valid_signals = signal.valid_signals |