[New-bugs-announce] [issue30151] Race condition in use of _PyOS_SigintEvent on windows

[Nathaniel Smith]

New submission from Nathaniel Smith:

As pointed out in this stackoverflow answer: http://stackoverflow.com/a/43578450/
and since I seem to be collecting signal-handling bugs these days :-), there's a race condition in how the interpreter uses _PyOS_SigintEvent to allow control-C to break out of functions like time.sleep on Windows. Suppose we have a call to time.sleep(), and the user hits control-C while it's running.

What's supposed to happen is:

- the windows implementation of pysleep in Modules/timemodule.c does ResetEvent(hInterruptEvent)
- then it blocks waiting for the interrupt event to be set *or* the timeout to expire
- the C-level signal handler runs in a new thread, which sets the "hey a signal arrived" flag and then sets the event
- the main thread wakes up because the event is set, and runs PyErr_CheckSignals()
- this notices that that the signal has arrived and runs the Python-level handler, all is good

But what can happen instead is:

- before doing CALL_FUNCTION, the eval loop checks to see if any signals have arrived. They haven't.
- then the C implementation of time.sleep starts executing.
- then a signal arrives; the signal handler sets the flag and sets the event
- then the main thread clears the event again
- then it blocks waiting for the event to be set or the timeout to expire. But the C-level signal handler's already done and gone, so we don't realize that the flag is set and we should wake up and run the Python-level signal handler.

The solution is that immediately *after* calling ResetEvent(_PyOS_SigintEvent()) but *before* sleeping, we should call PyErr_CheckSignals(). This catches any signals that arrived before we called ResetEvent, and any signals that arrive after that will cause the event to become set and wake us up, so that eliminates the race condition.

This same race-y pattern seems to apply to appear in Modules/timemodule.c, Modules/_multiprocessing/semaphore.c, and Modules/_winapi.c. _winapi.c also handles the event in a weird way that doesn't make sense to me – if the user hits control-C it raises an OSError instead of running the signal handler? OTOH I *think* Modules/_io/winconsoleio.c already handles the race condition correctly, and I don't dare make a guess about whatever Parser/myreadline.c is doing.

----------
components: Interpreter Core
messages: 292196
nosy: njs
priority: normal
severity: normal
status: open
title: Race condition in use of _PyOS_SigintEvent on windows
versions: Python 3.5, Python 3.6, Python 3.7

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue30151>
_______________________________________