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.

diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py
index ddb9daca0269367901e463fd41a60bbeb5db937b..4808c5dfc458563e9cc83c143877003db6d92af4 100644 (file)
--- 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 cf7683fee6462117d147dd3e702e13801498f474..6c9a89dbc5d09f0d750949d626900fb4683c2bf7 100644 (file)
--- 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 7fca0541ee75a272dd296e05f14574e30b10d536..6b28348a71e26f34b053c7efb39dda4de3b3aff5 100644 (file)
--- 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 5bad21ecbae4afa6512361a4d787299ad46dbf93..1ec627f63eeef0402bc472a4cb71c32f7711d741 100644 (file)
--- a/Lib/test/test_asyncio/test_unix_events.py
+++ b/Lib/test/test_asyncio/test_unix_events.py
@@ -88,6 +88,17 @@ class SelectorEventLoopSignalTests(test_utils.TestCase):
             self.loop.add_signal_handler,
             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
@@ -213,6 +224,19 @@ class SelectorEventLoopSignalTests(test_utils.TestCase):
         self.loop.remove_signal_handler(signal.SIGHUP)
         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

diff --git a/Misc/NEWS.d/next/Library/2022-09-14-19-15-01.gh-issue-87079.0zYmW5.rst b/Misc/NEWS.d/next/Library/2022-09-14-19-15-01.gh-issue-87079.0zYmW5.rst
new file mode 100644 (file)
index 0000000..989c6dd
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2022-09-14-19-15-01.gh-issue-87079.0zYmW5.rst
@@ -0,0 +1,2 @@
+Warn the user whenever asyncio event loops override a signal wake up file
+descriptor that was previously set.