bpo-43406: Fix possible race condition where ``PyErr_CheckSignals`` tries to execute a non-Python signal handler (GH-24756)

We can receive signals (at the C level, in `trip_signal()` in signalmodule.c) while `signal.signal` is being called to modify the corresponding handler.  Later when `PyErr_CheckSignals()` is called to handle the given signal, the handler may be a non-callable object and would raise a cryptic asynchronous exception.

1 files changed, 50 insertions, 0 deletions

diff --git a/Lib/test/test_signal.py b/Lib/test/test_signal.py
index 6a43fe7..c9de4a4 100644
--- a/Lib/test/test_signal.py
+++ b/Lib/test/test_signal.py
@@ -6,6 +6,7 @@ import socket
 import statistics
 import subprocess
 import sys
+import threading
 import time
 import unittest
 from test import support
@@ -1251,6 +1252,55 @@ class StressTest(unittest.TestCase):
         # Python handler
         self.assertEqual(len(sigs), N, "Some signals were lost")
 
+    @unittest.skipUnless(hasattr(signal, "SIGUSR1"),
+                         "test needs SIGUSR1")
+    def test_stress_modifying_handlers(self):
+        # bpo-43406: race condition between trip_signal() and signal.signal
+        signum = signal.SIGUSR1
+        num_sent_signals = 0
+        num_received_signals = 0
+        do_stop = False
+
+        def custom_handler(signum, frame):
+            nonlocal num_received_signals
+            num_received_signals += 1
+
+        def set_interrupts():
+            nonlocal num_sent_signals
+            while not do_stop:
+                signal.raise_signal(signum)
+                num_sent_signals += 1
+
+        def cycle_handlers():
+            while num_sent_signals < 100:
+                for i in range(20000):
+                    # Cycle between a Python-defined and a non-Python handler
+                    for handler in [custom_handler, signal.SIG_IGN]:
+                        signal.signal(signum, handler)
+
+        old_handler = signal.signal(signum, custom_handler)
+        self.addCleanup(signal.signal, signum, old_handler)
+        t = threading.Thread(target=set_interrupts)
+        t.start()
+        try:
+            with support.catch_unraisable_exception() as cm:
+                cycle_handlers()
+                if cm.unraisable is not None:
+                    # An unraisable exception may be printed out when
+                    # a signal is ignored due to the aforementioned
+                    # race condition, check it.
+                    self.assertIsInstance(cm.unraisable.exc_value, OSError)
+                    self.assertIn(
+                        f"Signal {signum} ignored due to race condition",
+                        str(cm.unraisable.exc_value))
+            # Sanity check that some signals were received, but not all
+            self.assertGreater(num_received_signals, 0)
+            self.assertLess(num_received_signals, num_sent_signals)
+        finally:
+            do_stop = True
+            t.join()
+
+
 class RaiseSignalTest(unittest.TestCase):
 
     def test_sigint(self):