From: Jinoh Kang <jinoh.kang.kr@gmail.com>
Subject: [RFC PATCH v2 00/11] Fix sock_recv reordering issue (#52401)
Message-Id: <1e0d520b-0f60-1323-3305-4f884fb121a1@gmail.com>
Date: Sat, 22 Jan 2022 23:35:02 +0900

    (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