Calling try_recv could make messages misordered.
Suppose that a server program calls a overlapped WSARecv and its operation is pending. Another program sends a message and wineserver is busy. The server program calls another overlapped WSARecv and it intercepts the message for the pending WSARecv.
The problem already had been discussed here https://www.winehq.org/pipermail/wine-devel/2021-May/186612.html
To avoid this situation, this kind of approach can be applied.
The server program needs to know if there are pending asyncs before calling try_recv. Wineserver notifies it whenever invoking APC_ASYNC_IO. Then, the server program updates it and check it before calling try_recv.
Signed-off-by: Dongwan Kim kdw6485@gmail.com --- dlls/ntdll/unix/server.c | 8 ++++---- dlls/ntdll/unix/socket.c | 25 ++++++++++++++++++++++++- dlls/ws2_32/socket.c | 28 ++++++++++++++++++++++++++++ include/wine/afd.h | 12 +++++++++++- include/wine/server_protocol.h | 1 + server/async.c | 8 ++++++++ 6 files changed, 76 insertions(+), 6 deletions(-)
diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c index a388247beb2..047da6e0974 100644 --- a/dlls/ntdll/unix/server.c +++ b/dlls/ntdll/unix/server.c @@ -368,17 +368,17 @@ static void invoke_system_apc( const apc_call_t *call, apc_result_t *result, BOO case APC_ASYNC_IO: { struct async_fileio *user = wine_server_get_ptr( call->async_io.user ); - ULONG_PTR info = call->async_io.result; + ULONG_PTR info[2] = { call->async_io.result, call->async_io.async_wait }; NTSTATUS status;
result->type = call->type; status = call->async_io.status; - if (user->callback( user, &info, &status )) + if (user->callback( user, info, &status )) { result->async_io.status = status; - result->async_io.total = info; + result->async_io.total = *info; /* the server will pass us NULL if a call failed synchronously */ - set_async_iosb( call->async_io.sb, result->async_io.status, info ); + set_async_iosb( call->async_io.sb, result->async_io.status, *info ); } else result->async_io.status = STATUS_PENDING; /* restart it */ break; diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index f82c7ae1ddf..1066da19bc7 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -651,13 +651,27 @@ static NTSTATUS try_recv( int fd, struct async_recv_ioctl *async, ULONG_PTR *siz return status; }
+struct list *async_queue_list; +static struct afd_async_queue* find_async_queue(HANDLE handle){ + struct afd_async_queue *queue; + + LIST_FOR_EACH_ENTRY(queue, async_queue_list, struct afd_async_queue, entry){ + if(queue->sock == handle) + return queue; + } + return NULL; +} static BOOL async_recv_proc( void *user, ULONG_PTR *info, NTSTATUS *status ) { struct async_recv_ioctl *async = user; + struct afd_async_queue *queue; int fd, needs_close;
TRACE( "%#x\n", *status );
+ queue = find_async_queue(async->io.handle); + if(queue) + queue->read_q = info[1]; //store if waiting asyncs exist if (*status == STATUS_ALERTED) { if ((*status = server_get_unix_fd( async->io.handle, 0, &fd, &needs_close, NULL, NULL ))) @@ -679,6 +693,7 @@ static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi struct WS_sockaddr *addr, int *addr_len, DWORD *ret_flags, int unix_flags, int force_async ) { struct async_recv_ioctl *async; + struct afd_async_queue *queue; ULONG_PTR information; HANDLE wait_handle; DWORD async_size; @@ -735,7 +750,12 @@ static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi } }
- status = try_recv( fd, async, &information ); + queue = find_async_queue(handle); + + if(queue->read_q ) // if there are waiting asyncs, avoid cutting in line. + status = STATUS_DEVICE_NOT_READY; + else + status = try_recv( fd, async, &information );
if (status != STATUS_SUCCESS && status != STATUS_BUFFER_OVERFLOW && status != STATUS_DEVICE_NOT_READY) { @@ -764,6 +784,7 @@ static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi SERVER_END_REQ;
if (status != STATUS_PENDING) release_fileio( &async->io ); + else queue->read_q = 1;
if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT ); return status; @@ -1306,6 +1327,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc unsigned int *ws_flags = u64_to_user_ptr(params->ws_flags_ptr); int unix_flags = 0;
+ async_queue_list = u64_to_user_ptr(params->async_queue_list); + if ((status = server_get_unix_fd( handle, 0, &fd, &needs_close, NULL, NULL ))) return status;
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 648284c10a3..42954537abe 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -157,6 +157,30 @@ DECLARE_CRITICAL_SECTION(cs_socket_list); static SOCKET *socket_list; static unsigned int socket_list_size;
+struct list async_queue_list = LIST_INIT(async_queue_list); + +static struct afd_async_queue* alloc_async_queue(HANDLE handle){ + struct afd_async_queue *queue = + (struct afd_async_queue*)malloc(sizeof(struct afd_async_queue)); + memset(queue, 0, sizeof(struct afd_async_queue)); + + queue->sock = handle; + list_add_head(&async_queue_list, &queue->entry); + return queue; +} + +static void remove_async_queue(HANDLE handle){ + struct afd_async_queue *queue; + + LIST_FOR_EACH_ENTRY(queue, &async_queue_list, struct afd_async_queue, entry){ + if(queue->sock == handle){ + list_remove(&queue->entry); + free(queue); + break; + } + } +} + const char *debugstr_sockaddr( const struct sockaddr *a ) { if (!a) return "(nil)"; @@ -355,6 +379,8 @@ static BOOL socket_list_add(SOCKET socket) SOCKET *new_array;
EnterCriticalSection(&cs_socket_list); + alloc_async_queue((HANDLE)socket); + for (i = 0; i < socket_list_size; ++i) { if (!socket_list[i]) @@ -938,6 +964,7 @@ static int WS2_recv_base( SOCKET s, WSABUF *buffers, DWORD buffer_count, DWORD * params.force_async = !!overlapped; params.count = buffer_count; params.buffers_ptr = u64_from_user_ptr(buffers); + params.async_queue_list = u64_from_user_ptr(&async_queue_list);
status = NtDeviceIoControlFile( (HANDLE)s, event, apc, cvalue, piosb, IOCTL_AFD_WINE_RECVMSG, ¶ms, sizeof(params), NULL, 0 ); @@ -1171,6 +1198,7 @@ int WINAPI closesocket( SOCKET s ) }
CloseHandle( (HANDLE)s ); + remove_async_queue( (HANDLE)s); return 0; }
diff --git a/include/wine/afd.h b/include/wine/afd.h index efd5787e90a..c4ece63138a 100644 --- a/include/wine/afd.h +++ b/include/wine/afd.h @@ -24,6 +24,7 @@ #include <winternl.h> #include <winioctl.h> #include <mswsock.h> +#include "wine/list.h"
struct afd_wsabuf_32 { @@ -322,8 +323,9 @@ struct afd_recvmsg_params int force_async; unsigned int count; ULONGLONG buffers_ptr; /* WSABUF[] */ + ULONGLONG async_queue_list; }; -C_ASSERT( sizeof(struct afd_recvmsg_params) == 48 ); +C_ASSERT( sizeof(struct afd_recvmsg_params) == 56 );
struct afd_sendmsg_params { @@ -365,4 +367,12 @@ struct afd_get_info_params }; C_ASSERT( sizeof(struct afd_get_info_params) == 12 );
+struct afd_async_queue{ + struct list entry; + HANDLE sock; + char read_q; + char write_q; +}; + + #endif diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index 1d17a40530f..ce26240bc9a 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -492,6 +492,7 @@ typedef union client_ptr_t user; client_ptr_t sb; data_size_t result; + unsigned int async_wait; } async_io; struct { diff --git a/server/async.c b/server/async.c index 1a564ff1a69..5d6cc9770c3 100644 --- a/server/async.c +++ b/server/async.c @@ -164,6 +164,7 @@ static void async_destroy( struct object *obj ) void async_terminate( struct async *async, unsigned int status ) { struct iosb *iosb = async->iosb; + struct async* async_waiting;
if (async->terminated) return;
@@ -202,6 +203,13 @@ void async_terminate( struct async *async, unsigned int status ) else data.async_io.status = status;
+ async_waiting = find_pending_async(async->queue); + if(async_waiting) + { + data.async_io.async_wait = 1; + release_object(async_waiting); + } + thread_queue_apc( async->thread->process, async->thread, &async->obj, &data ); }