[PATCH v4 0/7] MR871: ntdll: Fill the IOSB only inside the if (alerted) block in multiple functions.
-- v4: ntdll: Fill the IOSB in sock_transmit() only inside the "if (alerted)" block. ntdll: Combine the "if (alerted)" blocks in sock_send(). ntdll: Fill the IOSB in sock_send() only inside the "if (alerted)" block. ntdll: Combine the "if (alerted)" blocks in sock_recv(). ntdll: Fill the IOSB in sock_recv() only inside the "if (alerted)" block. ntdll: The async handle passed to set_async_direct_result() cannot be NULL. ws2_32/tests: Add more tests for iosb contents while a recv is pending.
From: Zebediah Figura zfigura@codeweavers.com
--- dlls/ws2_32/tests/afd.c | 7 ++++--- dlls/ws2_32/tests/sock.c | 4 ++++ 2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/dlls/ws2_32/tests/afd.c b/dlls/ws2_32/tests/afd.c index 21e7a50aef3..3215ddaef62 100644 --- a/dlls/ws2_32/tests/afd.c +++ b/dlls/ws2_32/tests/afd.c @@ -1466,13 +1466,14 @@ static void test_recv(void) IOCTL_AFD_RECV, ¶ms, sizeof(params) - 1, NULL, 0); ok(ret == STATUS_INVALID_PARAMETER, "got %#x\n", ret);
- memset(&io, 0, sizeof(io)); + io.Status = 0xdeadbeef; + io.Information = 0xdeadbeef; memset(buffer, 0xcc, sizeof(buffer)); ret = NtDeviceIoControlFile((HANDLE)client, event, NULL, NULL, &io, IOCTL_AFD_RECV, ¶ms, sizeof(params), NULL, 0); ok(ret == STATUS_PENDING, "got %#x\n", ret); - ok(!io.Status, "got status %#lx\n", io.Status); - ok(!io.Information, "got information %#Ix\n", io.Information); + ok(io.Status == 0xdeadbeef, "got status %#lx\n", io.Status); + ok(io.Information == 0xdeadbeef, "got information %#Ix\n", io.Information);
/* These structures need not remain valid. */ memset(¶ms, 0xcc, sizeof(params)); diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index d4f430a29d2..adf16012951 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -12208,11 +12208,15 @@ static void test_nonblocking_async_recv(void)
memset(buffer, 0, sizeof(buffer)); WSASetLastError(0xdeadbeef); + overlapped.Internal = 0xdeadbeef; + overlapped.InternalHigh = 0xdeadbeef; ret = WSARecv(client, &wsabuf, 1, NULL, &flags, &overlapped, NULL); ok(ret == -1, "got %d\n", ret); ok(WSAGetLastError() == ERROR_IO_PENDING, "got error %u\n", WSAGetLastError()); ret = WaitForSingleObject((HANDLE)client, 0); ok(ret == WAIT_TIMEOUT, "expected timeout\n"); + ok(overlapped.Internal == STATUS_PENDING, "got status %#lx\n", (NTSTATUS)overlapped.Internal); + ok(overlapped.InternalHigh == 0xdeadbeef, "got size %Iu\n", overlapped.InternalHigh);
ret = send(server, "data", 4, 0); ok(ret == 4, "got %d\n", ret);
From: Zebediah Figura zfigura@codeweavers.com
--- dlls/ntdll/unix/sync.c | 9 +++++---- dlls/ntdll/unix/unix_private.h | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index 1194ee514b5..821a1f08311 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -2533,21 +2533,22 @@ NTSTATUS WINAPI NtWaitForAlertByThreadId( const void *address, const LARGE_INTEG /* Notify direct completion of async and close the wait handle if it is no longer needed. * This function is a no-op (returns status as-is) if the supplied handle is NULL. */ -void set_async_direct_result( HANDLE *optional_handle, NTSTATUS status, ULONG_PTR information, BOOL mark_pending ) +void set_async_direct_result( HANDLE *async_handle, NTSTATUS status, ULONG_PTR information, BOOL mark_pending ) { NTSTATUS ret;
- if (!*optional_handle) return; + /* if we got STATUS_ALERTED, we must have a valid async handle */ + assert( *async_handle );
SERVER_START_REQ( set_async_direct_result ) { - req->handle = wine_server_obj_handle( *optional_handle ); + req->handle = wine_server_obj_handle( *async_handle ); req->status = status; req->information = information; req->mark_pending = mark_pending; ret = wine_server_call( req ); if (ret == STATUS_SUCCESS) - *optional_handle = wine_server_ptr_handle( reply->handle ); + *async_handle = wine_server_ptr_handle( reply->handle ); } SERVER_END_REQ;
diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 47f0f9c56a9..73b9ed76de0 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -278,7 +278,7 @@ extern NTSTATUS get_device_info( int fd, struct _FILE_FS_DEVICE_INFORMATION *inf extern void init_files(void) DECLSPEC_HIDDEN; extern void init_cpu_info(void) DECLSPEC_HIDDEN; extern void add_completion( HANDLE handle, ULONG_PTR value, NTSTATUS status, ULONG info, BOOL async ) DECLSPEC_HIDDEN; -extern void set_async_direct_result( HANDLE *optional_handle, NTSTATUS status, ULONG_PTR information, BOOL mark_pending ); +extern void set_async_direct_result( HANDLE *async_handle, NTSTATUS status, ULONG_PTR information, BOOL mark_pending ) DECLSPEC_HIDDEN;
extern void dbg_init(void) DECLSPEC_HIDDEN;
From: Zebediah Figura zfigura@codeweavers.com
We can only get a successful status that way. This avoids an uninitialized variable warning with gcc 12.2. --- dlls/ntdll/unix/socket.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index f8ed9f6f854..8ff58f7566e 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -23,6 +23,7 @@ #endif
#include "config.h" +#include <assert.h> #include <errno.h> #include <sys/types.h> #include <sys/socket.h> @@ -879,24 +880,25 @@ static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi } SERVER_END_REQ;
+ /* the server currently will never succeed immediately */ + assert(status == STATUS_ALERTED || status == STATUS_PENDING || NT_ERROR(status)); + alerted = status == STATUS_ALERTED; if (alerted) { status = try_recv( fd, async, &information ); if (status == STATUS_DEVICE_NOT_READY && (force_async || !nonblocking)) status = STATUS_PENDING; - } - - if (status != STATUS_PENDING) - { - if (!NT_ERROR(status) || (wait_handle && !alerted)) + if (!NT_ERROR(status) && status != STATUS_PENDING) { io->Status = status; io->Information = information; } - release_fileio( &async->io ); }
+ if (status != STATUS_PENDING) + release_fileio( &async->io ); + if (alerted) set_async_direct_result( &wait_handle, status, information, FALSE ); if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT ); return status;
From: Zebediah Figura zfigura@codeweavers.com
There is no need to release the async_fileio structure before calling set_async_direct_result(). --- dlls/ntdll/unix/socket.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index 8ff58f7566e..44694f8c404 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -852,9 +852,8 @@ static BOOL is_icmp_over_dgram( int fd ) static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc_user, IO_STATUS_BLOCK *io, int fd, struct async_recv_ioctl *async, int force_async ) { - BOOL nonblocking, alerted; - ULONG_PTR information; HANDLE wait_handle; + BOOL nonblocking; NTSTATUS status; unsigned int i; ULONG options; @@ -883,9 +882,10 @@ static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi /* the server currently will never succeed immediately */ assert(status == STATUS_ALERTED || status == STATUS_PENDING || NT_ERROR(status));
- alerted = status == STATUS_ALERTED; - if (alerted) + if (status == STATUS_ALERTED) { + ULONG_PTR information; + status = try_recv( fd, async, &information ); if (status == STATUS_DEVICE_NOT_READY && (force_async || !nonblocking)) status = STATUS_PENDING; @@ -894,12 +894,12 @@ static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi io->Status = status; io->Information = information; } + set_async_direct_result( &wait_handle, status, information, FALSE ); }
if (status != STATUS_PENDING) release_fileio( &async->io );
- if (alerted) set_async_direct_result( &wait_handle, status, information, FALSE ); if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT ); return status; }
From: Zebediah Figura zfigura@codeweavers.com
We can only get a successful status that way. This matches sock_recv(). --- dlls/ntdll/unix/socket.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index 44694f8c404..cc3c898d464 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1116,6 +1116,9 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi } SERVER_END_REQ;
+ /* the server currently will never succeed immediately */ + assert(status == STATUS_ALERTED || status == STATUS_PENDING || NT_ERROR(status)); + if (!NT_ERROR(status) && is_icmp_over_dgram( fd )) sock_save_icmp_id( async );
@@ -1132,19 +1135,17 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi * and returns EWOULDBLOCK, but we have no way of doing that. */ if (status == STATUS_DEVICE_NOT_READY && async->sent_len) status = STATUS_SUCCESS; - }
- if (status != STATUS_PENDING) - { information = async->sent_len; - if (!NT_ERROR(status) || (wait_handle && !alerted)) + if (!NT_ERROR(status) && status != STATUS_PENDING) { io->Status = status; io->Information = information; } - release_fileio( &async->io ); } - else information = 0; + + if (status != STATUS_PENDING) + release_fileio( &async->io );
if (alerted) set_async_direct_result( &wait_handle, status, information, FALSE ); if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT );
From: Zebediah Figura zfigura@codeweavers.com
There is no need to release the async_fileio structure before calling set_async_direct_result(). --- dlls/ntdll/unix/socket.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index cc3c898d464..e53ad4b76be 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1099,9 +1099,8 @@ 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 ) { - BOOL nonblocking, alerted; - ULONG_PTR information; HANDLE wait_handle; + BOOL nonblocking; NTSTATUS status; ULONG options;
@@ -1122,9 +1121,10 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi if (!NT_ERROR(status) && is_icmp_over_dgram( fd )) sock_save_icmp_id( async );
- alerted = status == STATUS_ALERTED; - if (alerted) + if (status == STATUS_ALERTED) { + ULONG_PTR information; + status = try_send( fd, async ); if (status == STATUS_DEVICE_NOT_READY && (force_async || !nonblocking)) status = STATUS_PENDING; @@ -1142,12 +1142,13 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi io->Status = status; io->Information = information; } + + set_async_direct_result( &wait_handle, status, information, FALSE ); }
if (status != STATUS_PENDING) release_fileio( &async->io );
- if (alerted) set_async_direct_result( &wait_handle, status, information, FALSE ); if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT ); return status; }
From: Zebediah Figura zfigura@codeweavers.com
We can only get a successful status that way. This matches sock_recv().
Also combine the if (alerted) blocks. Unlike the previous commits, this cannot be separated, as it causes warnings with some versions of gcc. --- dlls/ntdll/unix/socket.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index e53ad4b76be..b502d3a03cb 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1335,9 +1335,7 @@ static NTSTATUS sock_transmit( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, socklen_t addr_len; HANDLE wait_handle; NTSTATUS status; - ULONG_PTR information; ULONG options; - BOOL alerted;
addr_len = sizeof(addr); if (getpeername( fd, &addr.addr, &addr_len ) != 0) @@ -1389,37 +1387,38 @@ static NTSTATUS sock_transmit( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, } SERVER_END_REQ;
- alerted = status == STATUS_ALERTED; - if (alerted) + /* the server currently will never succeed immediately */ + assert(status == STATUS_ALERTED || status == STATUS_PENDING || NT_ERROR(status)); + + if (status == STATUS_ALERTED) { + ULONG_PTR information; + status = try_transmit( fd, file_fd, async ); if (status == STATUS_DEVICE_NOT_READY) status = STATUS_PENDING; - }
- if (status != STATUS_PENDING) - { information = async->head_cursor + async->file_cursor + async->tail_cursor; - if (!NT_ERROR(status) || wait_handle) + if (!NT_ERROR(status) && status != STATUS_PENDING) { io->Status = status; io->Information = information; } - release_fileio( &async->io ); + + set_async_direct_result( &wait_handle, status, information, TRUE ); } - else information = 0;
- if (alerted) + if (status != STATUS_PENDING) + release_fileio( &async->io ); + + if (!status && !(options & (FILE_SYNCHRONOUS_IO_ALERT | FILE_SYNCHRONOUS_IO_NONALERT))) { - set_async_direct_result( &wait_handle, status, information, TRUE ); - if (!(options & (FILE_SYNCHRONOUS_IO_ALERT | FILE_SYNCHRONOUS_IO_NONALERT))) - { - /* Pretend we always do async I/O. The client can always retrieve - * the actual I/O status via the IO_STATUS_BLOCK. - */ - status = STATUS_PENDING; - } + /* Pretend we always do async I/O. The client can always retrieve + * the actual I/O status via the IO_STATUS_BLOCK. + */ + status = STATUS_PENDING; } + if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT ); return status; }
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 full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=125232
Your paranoid android.
=== debian11 (build log) ===
Task: Could not create the win32 wineprefix: Failed to disable the crash dialogs: Task: WineTest did not produce the win32 report
-
Marvin -
Zebediah Figura -
Zebediah Figura (＠zfigura)