<preamble>
This is my forth attempt to tackle the issue #52401 [1]: "Improper synchronization in sock_recv/sock_send leads to arbitrary reordering of completion of I/O requests", which was initially reported (outside Bugzilla) by Dongwan Kim [2].
Basically, the old process of sock_recv is:
1. Perform the I/O synchronously. 2. Report the result back to the server, or queue up the request in the server. 3. If blocking, wait for I/O to complete.
The new process of sock_recv would be:
1. Queue up the request in the server. 2. Perform the I/O synchronously. 3. Report the result back to the server, 4. If blocking, wait for I/O to complete.
Everything except the actual I/O requires communicating with wineserver. My goal here is to fix the issue without introducing extra calls to wineserver. (However, if it turns out that this goal is not of utmost significance, then this patch serie can be easily modified so that it issues separate server calls.)
The previous approaches are listed here:
- Add a new select type called SELECT_SIGNAL_WAIT_ASYNC [3].
Zebediah has pointed out [4] that it is not very elegant to (ab-)use the synchronization machinery for communicating the async result.
- Use APC_ASYNC_IO to perform the synchronous I/O [5].
This ended up with a total of 11 patches, and turned out to be rather too complicated for a simple task.
- Add a new wineserver call, and use "add_queue" hook to save a round trip to the server [6].
</preamble>
Each of the above turned out to be either too intrusive, complicated, or hard to verify. Per suggestion by Zebediah [7], this is my (hopefully) last approach: keep the new wineserver call, but keep out the implicit STATUS_PENDING transition from "add_queue"; also, (re-)use the "unknown_status" field.
Changelog: - v1 -> v2: fix async queue hang on synchronous failure - v2 -> v3: rewrite handling of pending/failed asyncs. - v3 -> v4: don't set IOSB on synchronous failure, make more robust - v4 -> v5: address review comments - v5 -> v6: address review comments (mostly cosmetic), drop RFC status
[1] https://bugs.winehq.org/show_bug.cgi?id=52401 [2] https://www.winehq.org/pipermail/wine-devel/2021-May/186454.html [3] https://www.winehq.org/pipermail/wine-devel/2022-January/204695.html [4] https://www.winehq.org/pipermail/wine-devel/2022-January/204710.html [5] https://www.winehq.org/pipermail/wine-devel/2022-January/205168.html [6] https://www.winehq.org/pipermail/wine-devel/2022-January/205193.html [7] https://www.winehq.org/pipermail/wine-devel/2022-January/205738.html
Jinoh Kang (4): server: Allow calling async_handoff() with status code STATUS_ALERTED. server: Attempt to complete I/O request immediately in recv_socket. ntdll: Don't call try_recv before server call in sock_recv. server: Replace redundant recv_socket status fields with force_async boolean field.
dlls/ntdll/unix/socket.c | 36 +++++++++-------- dlls/ntdll/unix/sync.c | 26 ++++++++++++ dlls/ntdll/unix/unix_private.h | 1 + dlls/ws2_32/tests/sock.c | 8 ++-- server/async.c | 72 ++++++++++++++++++++++++++++++++++ server/protocol.def | 14 ++++++- server/sock.c | 47 +++++++++++----------- 7 files changed, 159 insertions(+), 45 deletions(-)