bpo-30038: fix race condition in signal delivery + wakeup fd (#1082) (#2075)

Before, it was possible to get the following sequence of
events (especially on Windows, where the C-level signal handler for
SIGINT is run in a separate thread):

- SIGINT arrives
- trip_signal is called
- trip_signal writes to the wakeup fd
- the main thread wakes up from select()-or-equivalent
- the main thread checks for pending signals, but doesn't see any
- the main thread drains the wakeup fd
- the main thread goes back to sleep
- trip_signal sets is_tripped=1 and calls Py_AddPendingCall to notify
  the main thread the it should run the Python-level signal handler
- the main thread doesn't notice because it's asleep

This has been causing repeated failures in the Trio test suite:
  https://github.com/python-trio/trio/issues/119
(cherry picked from commit 4ae01496971624c75080431806ed1c08e00f22c7)

diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c
index e27075b200194610d07c0fd3aba3ad1f45b36305..ed2f4e8cf0a2dc18f5fbca419134eda846c4b295 100644 (file)
--- a/Modules/signalmodule.c
+++ b/Modules/signalmodule.c
@@ -244,6 +244,32 @@ trip_signal(int sig_num)
 
     Handlers[sig_num].tripped = 1;
 
+    if (!is_tripped) {
+        /* Set is_tripped after setting .tripped, as it gets
+           cleared in PyErr_CheckSignals() before .tripped. */
+        is_tripped = 1;
+        Py_AddPendingCall(checksignals_witharg, NULL);
+    }
+
+    /* And then write to the wakeup fd *after* setting all the globals and
+       doing the Py_AddPendingCall. We used to write to the wakeup fd and then
+       set the flag, but this allowed the following sequence of events
+       (especially on windows, where trip_signal runs in a new thread):
+
+       - main thread blocks on select([wakeup_fd], ...)
+       - signal arrives
+       - trip_signal writes to the wakeup fd
+       - the main thread wakes up
+       - the main thread checks the signal flags, sees that they're unset
+       - the main thread empties the wakeup fd
+       - the main thread goes back to sleep
+       - trip_signal sets the flags to request the Python-level signal handler
+         be run
+       - the main thread doesn't notice, because it's asleep
+
+       See bpo-30038 for more details.
+    */
+
 #ifdef MS_WINDOWS
     fd = Py_SAFE_DOWNCAST(wakeup.fd, SOCKET_T, int);
 #else
@@ -281,13 +307,6 @@ trip_signal(int sig_num)
             }
         }
     }
-
-    if (!is_tripped) {
-        /* Set is_tripped after setting .tripped, as it gets
-           cleared in PyErr_CheckSignals() before .tripped. */
-        is_tripped = 1;
-        Py_AddPendingCall(checksignals_witharg, NULL);
-    }
 }
 
 static void