This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author njs
Recipients njs
Date 2017-04-24.06:55:41
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1493016943.26.0.351168771487.issue30151@psf.upfronthosting.co.za>
In-reply-to
Content
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.
History
Date User Action Args
2017-04-24 06:55:43njssetrecipients: + njs
2017-04-24 06:55:43njssetmessageid: <1493016943.26.0.351168771487.issue30151@psf.upfronthosting.co.za>
2017-04-24 06:55:43njslinkissue30151 messages
2017-04-24 06:55:41njscreate