[PATCH v3 0/5] MR6490: winsock: Avoid short writes on nonblocking sockets.
-- v3: server: Correct STATUS_NOT_FOUND return from (cancel_async). ntdll: Don't cancel background socket sends. ntdll: Locally duplicated socket fd for background send. ntdll: Avoid short writes on nonblocking sockets. ws2_32/Tests: Add tests for send buffering. https://gitlab.winehq.org/wine/wine/-/merge_requests/6490
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/tests/sock.c | 117 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 114 insertions(+), 3 deletions(-) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 437ee813611..87b87727b60 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -6883,9 +6883,9 @@ static void test_write_events(struct event_test_ctx *ctx) if (!broken(1)) { - /* Windows will never send less than buffer_size bytes here, but Linux - * may do a short write. */ - while ((ret = send(server, buffer, buffer_size, 0)) > 0); + /* Windows will never send less than buffer_size bytes here. */ + while ((ret = send(server, buffer, buffer_size, 0)) > 0) + ok(ret == buffer_size, "got %d.\n", ret); ok(ret == -1, "got %d\n", ret); ok(WSAGetLastError() == WSAEWOULDBLOCK, "got error %u\n", WSAGetLastError()); @@ -14263,6 +14263,116 @@ static void test_broadcast(void) closesocket(s); } +struct test_send_buffering_data +{ + int buffer_size; + int sent_size; + SOCKET server; + char *buffer; +}; + +static DWORD WINAPI test_send_buffering_thread(void *arg) +{ + struct test_send_buffering_data *d = arg; + int ret; + + d->sent_size = 0; + while ((ret = send(d->server, d->buffer, d->buffer_size, 0)) > 0) + { + todo_wine ok(ret == d->buffer_size, "got %d.\n", ret); + d->sent_size += ret; + } + ok(ret == -1, "got %d\n", ret); + ok(WSAGetLastError() == WSAEWOULDBLOCK, "got error %u.\n", WSAGetLastError()); + ok(d->sent_size, "got 0.\n"); + ret = CancelIoEx((HANDLE)d->server, NULL); + todo_wine ok(!ret && GetLastError() == ERROR_NOT_FOUND, "got ret %d, error %lu.\n", ret, GetLastError()); + ret = CancelIo((HANDLE)d->server); + ok(ret, "got error %lu.\n", GetLastError()); + shutdown(d->server, SD_BOTH); + closesocket(d->server); + return 0; +} + +static void test_send_buffering(void) +{ + static const char test_data[] = "abcdefg01234567"; + + struct test_send_buffering_data d; + int ret, recv_size, i; + SOCKET client; + HANDLE thread; + + d.buffer_size = 1024 * 1024 * 50; + d.buffer = malloc(d.buffer_size); + + for (i = 0; i < d.buffer_size; ++i) + d.buffer[i] = test_data[i % sizeof(test_data)]; + + tcp_socketpair(&client, &d.server); + set_blocking(client, FALSE); + set_blocking(d.server, FALSE); + + d.sent_size = 0; + while ((ret = send(d.server, d.buffer, d.buffer_size, 0)) > 0) + { + todo_wine ok(ret == d.buffer_size, "got %d.\n", ret); + d.sent_size += ret; + } + ok(ret == -1, "got %d\n", ret); + ok(WSAGetLastError() == WSAEWOULDBLOCK, "got error %u.\n", WSAGetLastError()); + ok(d.sent_size, "got 0.\n"); + closesocket(d.server); + + recv_size = 0; + while ((ret = recv(client, d.buffer, d.buffer_size, 0)) > 0 || WSAGetLastError() == WSAEWOULDBLOCK) + { + if (ret < 0) + continue; + for (i = 0; i < ret; ++i) + { + if (d.buffer[i] != test_data[(recv_size + i) % sizeof(test_data)]) + break; + } + ok(i == ret, "data mismatch.\n"); + recv_size += ret; + ok(recv_size <= d.sent_size, "got ret %d, recv_size %d, sent_size %d.\n", ret, recv_size, d.sent_size); + } + ok(!ret && !WSAGetLastError(), "got ret %d, error %u.\n", ret, WSAGetLastError()); + ok(recv_size == d.sent_size, "got %d, expected %d.\n", recv_size, d.sent_size); + closesocket(client); + + /* Test with the other thread which terminates before the data is actually sent. */ + for (i = 0; i < d.buffer_size; ++i) + d.buffer[i] = test_data[i % sizeof(test_data)]; + + tcp_socketpair(&client, &d.server); + set_blocking(client, FALSE); + set_blocking(d.server, FALSE); + + thread = CreateThread(NULL, 0, test_send_buffering_thread, &d, 0, NULL); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + + recv_size = 0; + while ((ret = recv(client, d.buffer, d.buffer_size, 0)) > 0 || WSAGetLastError() == WSAEWOULDBLOCK) + { + if (ret < 0) + continue; + for (i = 0; i < ret; ++i) + { + if (d.buffer[i] != test_data[(recv_size + i) % sizeof(test_data)]) + break; + } + ok(i == ret, "data mismatch.\n"); + recv_size += ret; + ok(recv_size <= d.sent_size, "got ret %d, recv_size %d, sent_size %d.\n", ret, recv_size, d.sent_size); + } + ok(!ret && !WSAGetLastError(), "got ret %d, error %u.\n", ret, WSAGetLastError()); + ok(recv_size == d.sent_size, "got %d, expected %d.\n", recv_size, d.sent_size); + closesocket(client); +} + START_TEST( sock ) { int i; @@ -14346,6 +14456,7 @@ START_TEST( sock ) test_connect_udp(); test_tcp_sendto_recvfrom(); test_broadcast(); + test_send_buffering(); /* There is apparently an obscure interaction between this test and * test_WSAGetOverlappedResult(). -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6490
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/socket.c | 62 +++++++++++++++++++++++++++++++++++++--- dlls/ws2_32/tests/sock.c | 8 +++--- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index 577b48b1336..c82d8b04a28 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1134,10 +1134,64 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi /* If we had a short write and the socket is nonblocking (and we are * not trying to force the operation to be asynchronous), return - * success. Windows actually refuses to send any data in this case, - * and returns EWOULDBLOCK, but we have no way of doing that. */ - if (status == STATUS_DEVICE_NOT_READY && async->sent_len) - status = STATUS_SUCCESS; + * success, pretened we've written everything to the socket and queue writing + * remaining data. Windows never reports partial write in this case and queues + * virtually unlimited amount of data for background write in this case. */ + if (status == STATUS_DEVICE_NOT_READY && async->sent_len && async->iov_cursor < async->count) + { + struct iovec *iov = async->iov + async->iov_cursor; + SIZE_T data_size, async_size, addr_size; + struct async_send_ioctl *rem_async; + unsigned int i, iov_count; + IO_STATUS_BLOCK *rem_io; + char *p; + + TRACE( "Short write, queueing remaining data.\n" ); + data_size = 0; + iov_count = async->count - async->iov_cursor; + for (i = 0; i < iov_count; ++i) + data_size += iov[i].iov_len; + + addr_size = max( 0, async->addr_len ); + async_size = offsetof( struct async_send_ioctl, iov[1] ) + data_size + addr_size + + sizeof(IO_STATUS_BLOCK) + sizeof(IO_STATUS_BLOCK32); + if (!(rem_async = (struct async_send_ioctl *)alloc_fileio( async_size, async_send_proc, handle ))) + { + status = STATUS_NO_MEMORY; + } + else + { + /* Use a local copy of socket fd so the async send works after socket handle is closed. */ + rem_async->count = 1; + p = (char *)rem_async + offsetof( struct async_send_ioctl, iov[1] ); + rem_async->iov[0].iov_base = p; + rem_async->iov[0].iov_len = data_size; + for (i = 0; i < iov_count; ++i) + { + memcpy( p, iov[i].iov_base, iov[i].iov_len ); + p += iov[i].iov_len; + } + rem_async->unix_flags = async->unix_flags; + memcpy( p, async->addr, addr_size ); + rem_async->addr = (const struct WS_sockaddr *)p; + p += addr_size; + rem_async->addr_len = async->addr_len; + rem_async->iov_cursor = 0; + rem_async->sent_len = 0; + rem_io = (IO_STATUS_BLOCK *)p; + p += sizeof(IO_STATUS_BLOCK); + rem_io->Pointer = p; + p += sizeof(IO_STATUS_BLOCK32); + status = sock_send( handle, NULL, NULL, NULL, rem_io, fd, rem_async, TRUE ); + if (status == STATUS_PENDING) status = STATUS_SUCCESS; + if (!status) + { + async->sent_len += data_size; + async->iov_cursor = async->count; + } + else ERR( "Remaining write queue failed, status %#x.\n", status ); + } + } set_async_direct_result( &wait_handle, options, io, status, async->sent_len, FALSE ); } diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 87b87727b60..deb162c6451 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -14279,7 +14279,7 @@ static DWORD WINAPI test_send_buffering_thread(void *arg) d->sent_size = 0; while ((ret = send(d->server, d->buffer, d->buffer_size, 0)) > 0) { - todo_wine ok(ret == d->buffer_size, "got %d.\n", ret); + ok(ret == d->buffer_size, "got %d.\n", ret); d->sent_size += ret; } ok(ret == -1, "got %d\n", ret); @@ -14316,7 +14316,7 @@ static void test_send_buffering(void) d.sent_size = 0; while ((ret = send(d.server, d.buffer, d.buffer_size, 0)) > 0) { - todo_wine ok(ret == d.buffer_size, "got %d.\n", ret); + ok(ret == d.buffer_size, "got %d.\n", ret); d.sent_size += ret; } ok(ret == -1, "got %d\n", ret); @@ -14339,7 +14339,7 @@ static void test_send_buffering(void) ok(recv_size <= d.sent_size, "got ret %d, recv_size %d, sent_size %d.\n", ret, recv_size, d.sent_size); } ok(!ret && !WSAGetLastError(), "got ret %d, error %u.\n", ret, WSAGetLastError()); - ok(recv_size == d.sent_size, "got %d, expected %d.\n", recv_size, d.sent_size); + todo_wine ok(recv_size == d.sent_size, "got %d, expected %d.\n", recv_size, d.sent_size); closesocket(client); /* Test with the other thread which terminates before the data is actually sent. */ @@ -14369,7 +14369,7 @@ static void test_send_buffering(void) ok(recv_size <= d.sent_size, "got ret %d, recv_size %d, sent_size %d.\n", ret, recv_size, d.sent_size); } ok(!ret && !WSAGetLastError(), "got ret %d, error %u.\n", ret, WSAGetLastError()); - ok(recv_size == d.sent_size, "got %d, expected %d.\n", recv_size, d.sent_size); + todo_wine ok(recv_size == d.sent_size, "got %d, expected %d.\n", recv_size, d.sent_size); closesocket(client); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6490
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/socket.c | 11 ++++++++++- dlls/ws2_32/tests/sock.c | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index c82d8b04a28..e1cbf65079b 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -130,6 +130,7 @@ struct async_send_ioctl unsigned int sent_len; unsigned int count; unsigned int iov_cursor; + int fd; struct iovec iov[1]; }; @@ -1060,7 +1061,8 @@ static BOOL async_send_proc( void *user, ULONG_PTR *info, unsigned int *status ) if (*status == STATUS_ALERTED) { - if ((*status = server_get_unix_fd( async->io.handle, 0, &fd, &needs_close, NULL, NULL ))) + needs_close = FALSE; + if ((fd = async->fd) == -1 && (*status = server_get_unix_fd( async->io.handle, 0, &fd, &needs_close, NULL, NULL ))) return TRUE; *status = try_send( fd, async ); @@ -1072,6 +1074,7 @@ static BOOL async_send_proc( void *user, ULONG_PTR *info, unsigned int *status ) return FALSE; } *info = async->sent_len; + if (async->fd != -1) close( async->fd ); release_fileio( &async->io ); return TRUE; } @@ -1162,6 +1165,7 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi else { /* Use a local copy of socket fd so the async send works after socket handle is closed. */ + rem_async->fd = dup( fd ); rem_async->count = 1; p = (char *)rem_async + offsetof( struct async_send_ioctl, iov[1] ); rem_async->iov[0].iov_base = p; @@ -1197,7 +1201,10 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi } if (status != STATUS_PENDING) + { + if (async->fd != -1) close( async->fd ); release_fileio( &async->io ); + } if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT ); return status; @@ -1237,6 +1244,7 @@ static NTSTATUS sock_ioctl_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE ap async->iov[i].iov_len = buffers[i].len; } } + async->fd = -1; async->unix_flags = unix_flags; async->addr = addr; async->addr_len = addr_len; @@ -1256,6 +1264,7 @@ NTSTATUS sock_write( HANDLE handle, int fd, HANDLE event, PIO_APC_ROUTINE apc, if (!(async = (struct async_send_ioctl *)alloc_fileio( async_size, async_send_proc, handle ))) return STATUS_NO_MEMORY; + async->fd = -1; async->count = 1; async->iov[0].iov_base = (void *)buffer; async->iov[0].iov_len = length; diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index deb162c6451..1d50515ee8e 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -14339,7 +14339,7 @@ static void test_send_buffering(void) ok(recv_size <= d.sent_size, "got ret %d, recv_size %d, sent_size %d.\n", ret, recv_size, d.sent_size); } ok(!ret && !WSAGetLastError(), "got ret %d, error %u.\n", ret, WSAGetLastError()); - todo_wine ok(recv_size == d.sent_size, "got %d, expected %d.\n", recv_size, d.sent_size); + ok(recv_size == d.sent_size, "got %d, expected %d.\n", recv_size, d.sent_size); closesocket(client); /* Test with the other thread which terminates before the data is actually sent. */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6490
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/socket.c | 15 ++++++++------- dlls/ws2_32/tests/sock.c | 2 +- server/async.c | 23 ++++++++++++++++++----- server/fd.c | 10 +++++----- server/file.h | 3 ++- server/protocol.def | 4 +++- server/sock.c | 12 +++++++----- 7 files changed, 44 insertions(+), 25 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index e1cbf65079b..a02194f843f 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1105,7 +1105,7 @@ static void sock_save_icmp_id( struct async_send_ioctl *async ) } static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc_user, - IO_STATUS_BLOCK *io, int fd, struct async_send_ioctl *async, int force_async ) + IO_STATUS_BLOCK *io, int fd, struct async_send_ioctl *async, unsigned int server_flags ) { HANDLE wait_handle; BOOL nonblocking; @@ -1114,7 +1114,7 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi SERVER_START_REQ( send_socket ) { - req->force_async = force_async; + req->flags = server_flags; req->async = server_async( handle, &async->io, event, apc, apc_user, iosb_client_ptr(io) ); status = wine_server_call( req ); wait_handle = wine_server_ptr_handle( reply->wait ); @@ -1132,7 +1132,7 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi if (status == STATUS_ALERTED) { status = try_send( fd, async ); - if (status == STATUS_DEVICE_NOT_READY && (force_async || !nonblocking)) + if (status == STATUS_DEVICE_NOT_READY && ((server_flags & SERVER_SOCKET_IO_FORCE_ASYNC) || !nonblocking)) status = STATUS_PENDING; /* If we had a short write and the socket is nonblocking (and we are @@ -1186,7 +1186,8 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi p += sizeof(IO_STATUS_BLOCK); rem_io->Pointer = p; p += sizeof(IO_STATUS_BLOCK32); - status = sock_send( handle, NULL, NULL, NULL, rem_io, fd, rem_async, TRUE ); + status = sock_send( handle, NULL, NULL, NULL, rem_io, fd, rem_async, + SERVER_SOCKET_IO_FORCE_ASYNC | SERVER_SOCKET_IO_SYSTEM ); if (status == STATUS_PENDING) status = STATUS_SUCCESS; if (!status) { @@ -1251,7 +1252,7 @@ static NTSTATUS sock_ioctl_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE ap async->iov_cursor = 0; async->sent_len = 0; - return sock_send( handle, event, apc, apc_user, io, fd, async, force_async ); + return sock_send( handle, event, apc, apc_user, io, fd, async, force_async ? SERVER_SOCKET_IO_FORCE_ASYNC : 0 ); } @@ -1274,7 +1275,7 @@ NTSTATUS sock_write( HANDLE handle, int fd, HANDLE event, PIO_APC_ROUTINE apc, async->iov_cursor = 0; async->sent_len = 0; - return sock_send( handle, event, apc, apc_user, io, fd, async, 1 ); + return sock_send( handle, event, apc, apc_user, io, fd, async, SERVER_SOCKET_IO_FORCE_ASYNC ); } @@ -1438,7 +1439,7 @@ static NTSTATUS sock_transmit( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, SERVER_START_REQ( send_socket ) { - req->force_async = 1; + req->flags = SERVER_SOCKET_IO_FORCE_ASYNC; req->async = server_async( handle, &async->io, event, apc, apc_user, iosb_client_ptr(io) ); status = wine_server_call( req ); wait_handle = wine_server_ptr_handle( reply->wait ); diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 1d50515ee8e..30ce3db307f 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -14369,7 +14369,7 @@ static void test_send_buffering(void) ok(recv_size <= d.sent_size, "got ret %d, recv_size %d, sent_size %d.\n", ret, recv_size, d.sent_size); } ok(!ret && !WSAGetLastError(), "got ret %d, error %u.\n", ret, WSAGetLastError()); - todo_wine ok(recv_size == d.sent_size, "got %d, expected %d.\n", recv_size, d.sent_size); + ok(recv_size == d.sent_size, "got %d, expected %d.\n", recv_size, d.sent_size); closesocket(client); } diff --git a/server/async.c b/server/async.c index 3c9b9588c6e..c81957d214e 100644 --- a/server/async.c +++ b/server/async.c @@ -57,6 +57,7 @@ struct async unsigned int canceled :1; /* have we already queued cancellation for this async? */ unsigned int unknown_status :1; /* initial status is not known yet */ unsigned int blocking :1; /* async is blocking */ + unsigned int is_system :1; /* background system operation not affecting userspace visible state. */ struct completion *completion; /* completion associated with fd */ apc_param_t comp_key; /* completion key associated with fd */ unsigned int comp_flags; /* completion flags */ @@ -243,7 +244,7 @@ void queue_async( struct async_queue *queue, struct async *async ) grab_object( async ); list_add_tail( &queue->queue, &async->queue_entry ); - set_fd_signaled( async->fd, 0 ); + if (!async->is_system) set_fd_signaled( async->fd, 0 ); } /* create an async on a given queue of a fd */ @@ -277,6 +278,7 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da async->canceled = 0; async->unknown_status = 0; async->blocking = !is_fd_overlapped( fd ); + async->is_system = 0; async->completion = fd_get_completion( fd, &async->comp_key ); async->comp_flags = 0; async->completion_callback = NULL; @@ -529,7 +531,7 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota } if (async->event) set_event( async->event ); - else if (async->fd) set_fd_signaled( async->fd, 1 ); + else if (async->fd && !async->is_system) set_fd_signaled( async->fd, 1 ); } if (!async->signaled) @@ -584,7 +586,7 @@ static int cancel_async( struct process *process, struct object *obj, struct thr restart: LIST_FOR_EACH_ENTRY( async, &process->asyncs, struct async, process_entry ) { - if (async->terminated || async->canceled) continue; + if (async->terminated || async->canceled || async->is_system) continue; if ((!obj || (get_fd_user( async->fd ) == obj)) && (!thread || async->thread == thread) && (!iosb || async->data.iosb == iosb)) @@ -621,7 +623,16 @@ restart: void cancel_process_asyncs( struct process *process ) { - cancel_async( process, NULL, NULL, 0 ); + struct async *async; + +restart: + LIST_FOR_EACH_ENTRY( async, &process->asyncs, struct async, process_entry ) + { + if (async->terminated || async->canceled) continue; + async->canceled = 1; + fd_cancel_async( async->fd, async ); + goto restart; + } } int async_close_obj_handle( struct object *obj, struct process *process, obj_handle_t handle ) @@ -655,6 +666,7 @@ restart: { if (async->thread != thread || async->terminated || async->canceled) continue; if (async->completion && async->data.apc_context && !async->event) continue; + if (async->is_system) continue; async->canceled = 1; fd_cancel_async( async->fd, async ); @@ -741,7 +753,7 @@ static struct iosb *create_iosb( const void *in_data, data_size_t in_size, data_ /* create an async associated with iosb for async-based requests * returned async must be passed to async_handoff */ -struct async *create_request_async( struct fd *fd, unsigned int comp_flags, const async_data_t *data ) +struct async *create_request_async( struct fd *fd, unsigned int comp_flags, const async_data_t *data, int is_system ) { struct async *async; struct iosb *iosb; @@ -760,6 +772,7 @@ struct async *create_request_async( struct fd *fd, unsigned int comp_flags, cons } async->pending = 0; async->direct_result = 1; + async->is_system = !!is_system; async->comp_flags = comp_flags; } return async; diff --git a/server/fd.c b/server/fd.c index f62e7b60efe..bed6575ab4c 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2719,7 +2719,7 @@ DECL_HANDLER(flush) if (!fd) return; - if ((async = create_request_async( fd, fd->comp_flags, &req->async ))) + if ((async = create_request_async( fd, fd->comp_flags, &req->async, 0 ))) { fd->fd_ops->flush( fd, async ); reply->event = async_handoff( async, NULL, 1 ); @@ -2748,7 +2748,7 @@ DECL_HANDLER(get_volume_info) if (!fd) return; - if ((async = create_request_async( fd, fd->comp_flags, &req->async ))) + if ((async = create_request_async( fd, fd->comp_flags, &req->async, 0 ))) { fd->fd_ops->get_volume_info( fd, async, req->info_class ); reply->wait = async_handoff( async, NULL, 1 ); @@ -2824,7 +2824,7 @@ DECL_HANDLER(read) if (!fd) return; - if ((async = create_request_async( fd, fd->comp_flags, &req->async ))) + if ((async = create_request_async( fd, fd->comp_flags, &req->async, 0 ))) { fd->fd_ops->read( fd, async, req->pos ); reply->wait = async_handoff( async, NULL, 0 ); @@ -2842,7 +2842,7 @@ DECL_HANDLER(write) if (!fd) return; - if ((async = create_request_async( fd, fd->comp_flags, &req->async ))) + if ((async = create_request_async( fd, fd->comp_flags, &req->async, 0 ))) { fd->fd_ops->write( fd, async, req->pos ); reply->wait = async_handoff( async, &reply->size, 0 ); @@ -2861,7 +2861,7 @@ DECL_HANDLER(ioctl) if (!fd) return; - if ((async = create_request_async( fd, fd->comp_flags, &req->async ))) + if ((async = create_request_async( fd, fd->comp_flags, &req->async, 0 ))) { fd->fd_ops->ioctl( fd, req->code, async ); reply->wait = async_handoff( async, NULL, 0 ); diff --git a/server/file.h b/server/file.h index 006b5a8e324..f486f823f25 100644 --- a/server/file.h +++ b/server/file.h @@ -250,7 +250,8 @@ typedef void (*async_completion_callback)( void *private ); extern void free_async_queue( struct async_queue *queue ); extern struct async *create_async( struct fd *fd, struct thread *thread, const async_data_t *data, struct iosb *iosb ); -extern struct async *create_request_async( struct fd *fd, unsigned int comp_flags, const async_data_t *data ); +extern struct async *create_request_async( struct fd *fd, unsigned int comp_flags, const async_data_t *data, + int is_system ); extern obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_blocking ); extern void queue_async( struct async_queue *queue, struct async *async ); extern void async_set_timeout( struct async *async, timeout_t timeout, unsigned int status ); diff --git a/server/protocol.def b/server/protocol.def index de3f9f1b764..e9c213d344e 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1578,13 +1578,15 @@ enum server_fd_type /* Perform a send on a socket */ @REQ(send_socket) async_data_t async; /* async I/O parameters */ - int force_async; /* Force asynchronous mode? */ + unsigned int flags; /* SERVER_SOCKET_IO_* flags */ @REPLY obj_handle_t wait; /* handle to wait on for blocking send */ unsigned int options; /* device open options */ int nonblocking; /* is socket non-blocking? */ @END +#define SERVER_SOCKET_IO_FORCE_ASYNC 0x01 +#define SERVER_SOCKET_IO_SYSTEM 0x02 /* Get socket event flags */ @REQ(socket_get_events) diff --git a/server/sock.c b/server/sock.c index f80f833c302..d2ec882554f 100644 --- a/server/sock.c +++ b/server/sock.c @@ -3915,7 +3915,7 @@ DECL_HANDLER(recv_socket) sock->pending_events &= ~(req->oob ? AFD_POLL_OOB : AFD_POLL_READ); sock->reported_events &= ~(req->oob ? AFD_POLL_OOB : AFD_POLL_READ); - if ((async = create_request_async( fd, get_fd_comp_flags( fd ), &req->async ))) + if ((async = create_request_async( fd, get_fd_comp_flags( fd ), &req->async, 0 ))) { set_error( status ); @@ -3963,6 +3963,7 @@ DECL_HANDLER(send_socket) struct async *async; struct fd *fd; int bind_errno = 0; + BOOL force_async = req->flags & SERVER_SOCKET_IO_FORCE_ASYNC; if (!sock) return; fd = sock->fd; @@ -3985,7 +3986,7 @@ DECL_HANDLER(send_socket) else if (!bind_errno) bind_errno = errno; } - if (!req->force_async && !sock->nonblocking && is_fd_overlapped( fd )) + if (!force_async && !sock->nonblocking && is_fd_overlapped( fd )) timeout = (timeout_t)sock->sndtimeo * -10000; if (bind_errno) status = sock_get_ntstatus( bind_errno ); @@ -3996,7 +3997,7 @@ DECL_HANDLER(send_socket) * asyncs will not consume all available space; if there's no space * available, the current request won't be immediately satiable. */ - if ((!req->force_async && sock->nonblocking) || check_fd_events( sock->fd, POLLOUT )) + if ((!force_async && sock->nonblocking) || check_fd_events( sock->fd, POLLOUT )) { /* Give the client opportunity to complete synchronously. * If it turns out that the I/O request is not actually immediately satiable, @@ -4021,10 +4022,11 @@ DECL_HANDLER(send_socket) } } - if (status == STATUS_PENDING && !req->force_async && sock->nonblocking) + if (status == STATUS_PENDING && !force_async && sock->nonblocking) status = STATUS_DEVICE_NOT_READY; - if ((async = create_request_async( fd, get_fd_comp_flags( fd ), &req->async ))) + if ((async = create_request_async( fd, get_fd_comp_flags( fd ), &req->async, + req->flags & SERVER_SOCKET_IO_SYSTEM ))) { struct send_req *send_req; struct iosb *iosb = async_get_iosb( async ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6490
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/pipe.c | 9 +++++++++ dlls/ws2_32/tests/sock.c | 2 +- server/async.c | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index 697774dfa09..cdf5cdb4151 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -614,6 +614,10 @@ static void test_cancelio(void) ok(ioapc_called, "IOAPC didn't run\n"); + res = pNtCancelIoFile(hPipe, &cancel_sb); + ok(!res, "NtCancelIoFile returned %lx\n", res); + ok(iosb.Status == STATUS_CANCELLED, "Wrong iostatus %lx\n", iosb.Status); + CloseHandle(hPipe); if (pNtCancelIoFileEx) @@ -631,6 +635,11 @@ static void test_cancelio(void) ok(iosb.Status == STATUS_CANCELLED, "Wrong iostatus %lx\n", iosb.Status); ok(WaitForSingleObject(hEvent, 0) == 0, "hEvent not signaled\n"); + iosb.Status = 0xdeadbeef; + res = pNtCancelIoFileEx(hPipe, NULL, &cancel_sb); + ok(res == STATUS_NOT_FOUND, "NtCancelIoFileEx returned %lx\n", res); + ok(iosb.Status == 0xdeadbeef, "Wrong iostatus %lx\n", iosb.Status); + CloseHandle(hPipe); } else diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 30ce3db307f..4fd5fc00cdd 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -14286,7 +14286,7 @@ static DWORD WINAPI test_send_buffering_thread(void *arg) ok(WSAGetLastError() == WSAEWOULDBLOCK, "got error %u.\n", WSAGetLastError()); ok(d->sent_size, "got 0.\n"); ret = CancelIoEx((HANDLE)d->server, NULL); - todo_wine ok(!ret && GetLastError() == ERROR_NOT_FOUND, "got ret %d, error %lu.\n", ret, GetLastError()); + ok(!ret && GetLastError() == ERROR_NOT_FOUND, "got ret %d, error %lu.\n", ret, GetLastError()); ret = CancelIo((HANDLE)d->server); ok(ret, "got error %lu.\n", GetLastError()); shutdown(d->server, SD_BOTH); diff --git a/server/async.c b/server/async.c index c81957d214e..749c547af4f 100644 --- a/server/async.c +++ b/server/async.c @@ -819,7 +819,7 @@ DECL_HANDLER(cancel_async) if (obj) { int count = cancel_async( current->process, obj, thread, req->iosb ); - if (!count && req->iosb) set_error( STATUS_NOT_FOUND ); + if (!count && !thread) set_error( STATUS_NOT_FOUND ); release_object( obj ); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6490
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=148423 Your paranoid android. === w1064_adm (64 bit report) === ws2_32: sock.c:13198: Test failed: wait timed out === w10pro64_ja (64 bit report) === ws2_32: sock.c:14323: Test failed: got error 10055. sock.c:14286: Test failed: got error 10055. === debian11b (64 bit WoW report) === user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000010A00EC, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
v3: - Handle 32 bit iosb; - Rename no_cancel to is_system; - Don't signal or unsignal fd for system asyncs. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6490#note_82206
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6490
participants (4)
-
Elizabeth Figura (＠zfigura) -
Marvin -
Paul Gofman
-
Paul Gofman (＠gofman)