From: "Zebediah Figura (she/her)" Subject: Re: [PATCH 4/4] dlls/ntdll: Call try_recv inside sock_recv only if it is safe to do so. Message-Id: Date: Tue, 18 Jan 2022 17:08:22 -0600 In-Reply-To: References: 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 > --- > 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