Skip to content

Commit

Permalink
bpo-30038: fix race condition in signal delivery + wakeup fd
Browse files Browse the repository at this point in the history
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:
  python-trio/trio#119
  • Loading branch information
njsmith committed May 16, 2017
1 parent e8a6bb4 commit e357d5d
Showing 1 changed file with 26 additions and 7 deletions.
33 changes: 26 additions & 7 deletions Modules/signalmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit e357d5d

Please sign in to comment.