[PATCH 0/2] MR210: winhttp: Use completion port for overlapped send.
Paul Gofman (@gofman)
wine at gitlab.winehq.org
Wed Jun 8 16:45:10 CDT 2022
There are a few issues with the way it is done now:
1. We queue overlapped WSARecv from one (user) thread and then await completion on another. As it became apparent the completions queued by a thread are canceled when thread terminates. That is currently not the case in Wine, but I have a pending MR for that: https://gitlab.winehq.org/wine/wine/-/merge_requests/135. User may use WinhttpWebSocketSend() and close the thread, if it gets to async send path that send may be canceled before it is complete. The only condition when the async operation is not canceled is that the queued async IO has a completion port and no event (which becomes the case with these patches). I tested on Windows that terminating the thread which called WinhttpWebSocketSend() doesn't cancel the async send, while I am reproducing this case with Wine and the refernced patch.
2. Using overlapped WSASend without a completion port or an event is fragile: the socket may potentially be signaled by a parallel async operation (or maybe instead not singaled until that operation completes; I think Wine and Windows currently don't agree in lot of details how that works, including maybe the higher level WSAGetOverlappedResult). This is more of theoretical point as currently we don't queue any overlapped receives.
3. WSAGetOverlappedResult is currently buggy in a funny way. It doesn't check for valid socket handle, and so if called with an overlapped without an event it waits on socket, and if socket is -1 it is current process pseudo handle and the wait hangs forever. I've sent a patch for winsock for that: https://gitlab.winehq.org/wine/wine/-/merge_requests/203. But since we need completion port for 1 anyway it also seems to also solve the potential race issue between setting socket to -1 when aborting operation and starting wait for overlapped IO on it without additional complications.
More information about the wine-devel