In time.sleep on Windows, if the WaitForSingleObject call fails, the call returns success. It essentially looks like the timeout was 0.

Errors should be highly unlikely, as the event object is leaked (see issue40912), but if they _do_ occur, we should raise them.

Thinking about testing here.. is there any straightforward way to cause WaitForSingleObjectEx to fail?

The code change would be fairly slight and amenable to inspection, but it would be good to actually test it ;)

> is there any straightforward way to cause WaitForSingleObjectEx to fail?

The wait can fail for ERROR_INVALID_HANDLE or ERROR_ACCESS_DENIED (i.e. the handle lacks SYNCHRONIZE access). If _PyOS_SigintEvent were replaced with get/set functions, then a non-waitable handle could be set temporarily via ctypes.

Thanks, Eryk. I've had a couple of casts at this (and also with an eye to https://bugs.python.org/issue40912 in a very similar area).

Trouble is I can't come up with a way of adding a set.. function which doesn't seem wholly artificial, given that it's basically creating an anonymous event and checking its return. I can't come up with a non-testing scenario where the ability to override would be useful.

If I understand your proposal, a tentative "set..." function would have to take a HANDLE parameter so that it could be overridden by a test? That means its normal use would be something like:

setSigintEvent(CreateEvent(NULL, ....));

so either the error checking for that would be inside the function, which feels weird, or would happen outside, which feels like the functions not doing anything. (I appreciate I may be overthinking here).

I'm very much open to suggestions here, but it seems to me that either:

We make the (simple) change without tests;

or we add a setSigintEvent function as above whose only purpose is to be overridden -- and then only for testing.

You could test this by getting the event and CloseHandle-ing it. A function to do this could be added to _testcapimodule. It'd have to run in its own process, but we have (a few) helpers around for this.

Given the concerns, I don't think we should change this in any maintenance releases, so I removed 3.7 and 3.8 from the versions list. I'd be equally happy without an explicit test in

... equally happy without an explicit test in anything that's only prerelease right now.

If WaitForSingleObjectEx fails, do you think the system error code should be raised as an OSError? Probably an invalid-handle or access-denied error is too cryptic here. It may be better to raise something like RuntimeError('time.sleep(): invalid SIGINT event handle'). 

> You could test this by getting the event and CloseHandle-ing it. 

Yes, that's a simpler approach. It should work in a child process that's well controlled. 

I was thinking of a less controlled environment, in which there's a chance that another waitable object will reuse the handle, such as a file object. The test would be reliable regardless of process context (but only in the main thread, of course) if there were a _PyOS_SetSigintEvent function -- or if sigint_event were directly accessible in _testcapimodule. It could just temporarily set a duplicate handle with no access. For example:

    >>> h1 = CreateEvent(None, True, True, None)
    >>> WaitForSingleObject(h1, 0)
    0
    >>> hp = GetCurrentProcess()
    >>> h2 = DuplicateHandle(hp, h1, hp, 0, False, 0)
    >>> WaitForSingleObject(h2, 0)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    pywintypes.error: (5, 'WaitForSingleObject', 'Access is denied.')

> If WaitForSingleObjectEx fails, do you think the system error code should be raised as an OSError?

It's invalid (and unfixable) internal state, so perhaps SystemError makes the most sense? There's no point catching it in most cases.

Or since we don't want to worry too much about formatting the message, why not raise the OSError with the helper function and then immediately chain it with a fixed SystemError?