[PATCH 4/4] dlls/ntdll: Call try_recv inside sock_recv only if it is safe to do so.

[Zebediah Figura (she/her)]

On 1/18/22 13:30, Jinoh Kang wrote:
> Otherwise, try_recv() call from sock_recv() may race against try_recv()
> call from async_recv_proc(), shuffling the packet order.
> 
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52401
> Signed-off-by: Jinoh Kang <jinoh.kang.kr at gmail.com>
> ---
>   dlls/ntdll/unix/socket.c | 38 ++++++++++++++++++-------------------
>   dlls/ws2_32/tests/sock.c |  8 ++++----
>   server/protocol.def      |  4 ++--
>   server/sock.c            | 41 ++++++++++++++++++++++++++++------------
>   4 files changed, 54 insertions(+), 37 deletions(-)

Despite the attempts to split 2/4 and 3/4 out of this patch, it's still 
doing two things at once. Namely, the optimization whereby recv_socket 
returns STATUS_ALERTED should not be part of this patch.

As I discussed in [1] it's not really clear to me that three server 
calls is significantly worse than two. It'd be nice to know that just 
removing the initial try_recv() call is really going to make programs 
worse if we're going to pursue it. I.e. personally I'd rather just leave 
it alone until we find a program that gets worse.

That aside, though, I'm not sure that adding a new select type is really 
the right way to go about this. One approach that occurs to me, which 
might end up being simpler, would be to return an apc_call_t, 
essentially as if the select request had been called immediately 
afterward. (In that case perhaps we should return STATUS_KERNEL_APC 
rather than STATUS_ALERTED).

[1] https://www.winehq.org/pipermail/wine-devel/2021-December/202825.html