[RFC PATCH 0/6] Fix sock_recv reordering issue (#52401): new wineserver call approach

[Jinoh Kang]

    (cc/ Dongwan Kim: This is the 2nd successor to the patch serie I just replied.)

This is my third 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.)

My 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.

So here comes my third approach, which hooks the "add_queue" method of
async objects.  Quoting [6]:

> If we are not going with SIGNAL_WAIT_ASYNC, there's an alternate approach
> (avoiding three server round trips):
>
> 1. Add a new wineserver request that calls async_set_result(), as in [1].
> 2. Re-implement async's add_queue to switch to pending mode before calling the
>    original add_queue.
>
> In this approach, Unix side will handle synchronous I/O operation as follows:
>
> 1. If the operation is completed, we request wineserver to call async_set_result() directly, and don't wait *at all*.
> 2. If the operation needs to be queued (STATUS_PENDING), we do async_wait() as before.
>    This will "magically" switch the async to STATUS_PENDING and continue polling.
>
> Note "magically" -- we're injecting impure semantics into async's pre-wait stage.
> I don't think this would be a significant problem, since the "async" object type
> is internal to Wine anyway.

[1] https://bugs.winehq.org/show_bug.cgi?id=52401
[2] https://www.winehq.org/pipermail/wine-devel/2021-December/202546.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

Jinoh Kang (6):
  server: Allow calling async_handoff() with status code STATUS_ALERTED.
  server: Allow setting async result directly from Unix side.
  ntdll: Add a Unix-side wrapper function for
    "notify_async_direct_result".
  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       | 39 ++++++++++---------
 dlls/ntdll/unix/sync.c         | 21 ++++++++++
 dlls/ntdll/unix/unix_private.h |  1 +
 dlls/ws2_32/tests/sock.c       |  8 ++--
 server/async.c                 | 70 +++++++++++++++++++++++++++++-----
 server/protocol.def            | 12 +++++-
 server/sock.c                  | 40 +++++++++++++------
 7 files changed, 144 insertions(+), 47 deletions(-)

-- 
2.31.1