[RFC PATCH v2 00/11] Fix sock_recv reordering issue (#52401)
Jinoh Kang
jinoh.kang.kr at gmail.com
Sat Jan 22 08:35:02 CST 2022
(cc/ Dongwan Kim: This is the successor to the patch serie I just replied.)
As discussed in https://www.winehq.org/pipermail/wine-devel/2022-January/204710.html,
I converted my previous patch serie to use APCs instead of a new select op.
"Talk is cheap. Show me the code." -Linus
I was, well, the original proposer of the APC approach; now, this patch serie
serves as my counterargument to this approach.
I tried my best to minimize the complexity introduced by this patch serie;
however, the APC approach resulted in net increase in the number of patches:
- server: Add a helper function to trigger synchronous completion of I/O via APC_ASYNC_IO.
- include: Add a helper function to test if a status indicates deferred completion.
- server: Add helpers for synchronous APC delivery via ordinary server call reply.
- server: Prevent I/O synchronous completion requests from firing APC interrupts if possible.
- ntdll: Make server_select and server_wait accept extra "inline_apc" argument.
- ntdll: Don't interrupt sock_recv with an I/O synchronous completion APC.
The APC approach *did* enable code reuse of async_recv_proc, albeit with the
following drawbacks:
- We need to modify async_recv_proc anyway.
- Deferring the synchronous I/O to the APC callback makes the synchronous I/O
code flow much less obvious.
(Implicit call flow: sock_recv -> async_recv_proc -> sock_recv)
In the meantime, the following patches are common to both approaches:
- ntdll: Don't call try_recv before server call in sock_recv.
- server: Replace redundant recv_socket status fields with force_async boolean field.
Further notes are placed in each patch.
As always, feedbacks welcome.
Changelog:
- v1 -> v2
- remove autogenerated changes
- server/thread.c: dequeue_synchronous_apc: check for handle allocation failure
Jinoh Kang (11):
server: Allow calling async_handoff() with status code STATUS_ALERTED.
server: Add a helper function to trigger synchronous completion of I/O
via APC_ASYNC_IO.
include: Add a helper function to test if a status indicates deferred
completion.
server: Attempt to complete I/O request immediately in recv_socket.
server: Add helpers for synchronous APC delivery via ordinary server
call reply.
server: Prevent I/O synchronous completion requests from firing APC
interrupts if possible.
ntdll: Make server_select and server_wait accept extra "inline_apc"
argument.
ntdll: Add a helper to process APC and wait for async in one function
call.
ntdll: Don't interrupt sock_recv with an I/O synchronous completion
APC.
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/server.c | 16 ++++++----
dlls/ntdll/unix/socket.c | 33 ++++++++-------------
dlls/ntdll/unix/sync.c | 29 ++++++++++++++----
dlls/ntdll/unix/thread.c | 4 +--
dlls/ntdll/unix/unix_private.h | 6 ++--
dlls/ws2_32/tests/sock.c | 8 ++---
include/wine/afd.h | 17 +++++++++++
include/wine/async.h | 46 +++++++++++++++++++++++++++++
server/async.c | 32 +++++++++++++++++++-
server/file.h | 1 +
server/protocol.def | 11 +++++--
server/sock.c | 54 ++++++++++++++++++++++++++--------
server/thread.c | 49 ++++++++++++++++++++++++++++++
server/thread.h | 3 ++
server/trace.c | 19 ++++++++++++
tools/make_requests | 1 +
16 files changed, 276 insertions(+), 53 deletions(-)
create mode 100644 include/wine/async.h
--
2.31.1
More information about the wine-devel
mailing list