Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide an API that does WaitForSingleObject on Windows #233

Closed
njsmith opened this issue Jun 22, 2017 · 4 comments
Closed

Provide an API that does WaitForSingleObject on Windows #233

njsmith opened this issue Jun 22, 2017 · 4 comments

Comments

@njsmith
Copy link
Member

njsmith commented Jun 22, 2017

As noted in #52, there are some events on Windows that can only be detected by calling one of the WaitFor family of functions (WaitForSingleObject, WaitForSingleObjectEx, WaitForMultipleObjects, etc. etc.). Some examples include child process exit (required for #4), and detecting console input (required for #174).

To resolve this issue, we should provide a trio.hazmat.WaitForSingleObject function that takes a HANDLE, and waits for it, and is cancellable.

The most straightforward way to do this is to allocate a thread to sit in WaitForSingleObjectEx, which can do an "alertable wait", which means that it's possible to cause the function to wake up early by calling QueueUserAPC. So we would use QueueUserAPC from inside our abort callback to implement cancellation. There are some more notes in _io_windows.py.

@sorcio
Copy link
Contributor

sorcio commented Jul 29, 2018

Looks like Windows already implements an asynchronous signaling mechanism around WaitForSingleObject with RegisterWaitForSingleObject. If I understand the documentation correctly, it will implicitly use threads from a thread pool. The cool thing is that you get a wait handle back, which is cancellable with a call to UnregisterWait. If we can pass a callback that can call into trio (through a portal?) then this could be the "fancy solution".

@njsmith
Copy link
Member Author

njsmith commented Jul 29, 2018

Yeah, there are lots of options for how to tackle this. In fact Windows actually has two different thread pool APIs that both have special support for delegating WaitForSingleObject to a thread – RegisterWaitForSingleObject is the older API, and SetThreadpoolWait is the newer API. Also I notice that at the top of this issue I suggest cancelling WaitForSingleObjectEx using APC, while in #4 (comment) I suggested doing cancellation by passing a dedicated Event to WaitForMultipleObjects.

Some considerations:

With the Windows thread pool APIs, I'm having a lot of trouble figuring out what exactly happens when you cancel a wait. Ideally, you call UnregisterWait or CloseThreadpoolWait and then you can pretend like the thread has just disappeared and forget about it. But what if the callback is currently running, and thus uncancellable – do they block and wait for it to finish? What if the wait has completed, and the callback is queued to run, but hasn't actually started yet – do they unqueue it? It would be very bad if we cancelled a wait, carried on, and then suddenly the callback happened anyway and tried to wake up the task that was already off doing something else. It sounds like maybe calling CloseThreadpoolWait and then WaitForThreadpoolWaitCallbacks might take care of this?

With the Windows thread pool APIs, or with QueueUserAPC, we have to provide a callback as a C function pointer. This is possible with cffi – you can generate a C function pointer that calls a Python function – but it's kind of messy. It requires either distributing binary extensions – which we've tried to avoid so far – or else cffi can actually generate some x86 code on the fly, which requires writable executable memory and has weird bugs on some platforms. There are also complications around running Python code on a non-Python thread – I think cffi might take care of this as well, but it's just a lot of complex moving parts, and that makes me nervous.

And in general, with the Windows thread pool APIs, we have very little visibility or insight into what they're doing. Maybe it's great, but if someone does have a problem it might be impossible to debug or fix. Our thread APIs are probably not the best possible (e.g. I'm sure the Windows ones are faster), but having 1 thread pool seems simpler than having 2, and if ours runs into problems we can fix them.

So that's why I've been leaning towards using run_sync_in_worker_thread to do a blocking WaitForMultipleObjects on the actual waitable object + a cancellation event object. I'm not set on this – if someone figures out another approach that works, that's cool. But right now my impression is that this approach has the lowest complexity all around.

@njsmith
Copy link
Member Author

njsmith commented Sep 18, 2018

This was fixed in #575

@njsmith njsmith closed this as completed Sep 18, 2018
@GSPP
Copy link

GSPP commented Oct 27, 2019

https://github.com/python-trio/trio/pull/575/files#diff-ed34813c30595dc83bb2a66e0973e22dR54

This appears to have a race condition. The new thread might start waiting only after the handle has been closed. This could be fixed by making the thread close the cancel_handle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants