[PATCH v2 0/4] MR8592: ntdll: Make wait in wait_async() always non-alertable.
-- v2: server: Set IOSB for cancelled asyncs in async_terminate(). ntdll: Make wait in wait_async() always non-alertable. ntdll: Factor out test_alert_with_status(). ntdll/tests: Add more tests for canceling synchronous IO. https://gitlab.winehq.org/wine/wine/-/merge_requests/8592
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/pipe.c | 186 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 175 insertions(+), 11 deletions(-) diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index 14a6148a2c5..fb292a8c5e6 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -1655,19 +1655,27 @@ struct blocking_thread_args enum { BLOCKING_THREAD_WRITE, BLOCKING_THREAD_READ, + BLOCKING_THREAD_USER_APC, + BLOCKING_THREAD_CANCEL_IO, BLOCKING_THREAD_QUIT } cmd; HANDLE client; HANDLE pipe; HANDLE event; + HANDLE io_thread; + unsigned int line; }; +#define blocking_thread_command(command) {ctx.cmd = command; ctx.line = __LINE__; SetEvent(ctx.wait);} + static DWORD WINAPI blocking_thread(void *arg) { struct blocking_thread_args *ctx = arg; static const char buf[] = "testdata"; char read_buf[32]; DWORD res, num_bytes; + IO_STATUS_BLOCK io; + NTSTATUS status; BOOL ret; for (;;) @@ -1675,6 +1683,7 @@ static DWORD WINAPI blocking_thread(void *arg) res = WaitForSingleObject(ctx->wait, 10000); ok(res == WAIT_OBJECT_0, "wait returned %lx\n", res); if (res != WAIT_OBJECT_0) break; + winetest_push_context("test line %d", ctx->line); switch(ctx->cmd) { case BLOCKING_THREAD_WRITE: Sleep(100); @@ -1698,11 +1707,32 @@ static DWORD WINAPI blocking_thread(void *arg) ok(ret, "WriteFile failed, error %lu\n", GetLastError()); ok(is_signaled(ctx->pipe), "pipe is not signaled\n"); break; + case BLOCKING_THREAD_USER_APC: + Sleep(100); + if(ctx->event) + ok(!is_signaled(ctx->event), "event is signaled\n"); + ok(!ioapc_called, "ioapc called\n"); + ok(!userapc_called, "userapc called.\n"); + ok(!is_signaled(ctx->client), "client is signaled\n"); + ok(is_signaled(ctx->pipe), "pipe is not signaled\n"); + pQueueUserAPC(userapc, ctx->io_thread, 0); + break; + case BLOCKING_THREAD_CANCEL_IO: + Sleep(100); + if(ctx->event) + ok(!is_signaled(ctx->event), "event is signaled\n"); + ok(!ioapc_called, "ioapc called\n"); + ok(!is_signaled(ctx->client), "client is signaled\n"); + ok(is_signaled(ctx->pipe), "pipe is not signaled\n"); + status = pNtCancelSynchronousIoFile(ctx->io_thread, NULL, &io); + ok(status == STATUS_SUCCESS, "NtCancelSynchronousIoFile failed, status %#lx.\n", status); + break; case BLOCKING_THREAD_QUIT: return 0; default: ok(0, "unvalid command\n"); } + winetest_pop_context(); SetEvent(ctx->done); } @@ -1712,6 +1742,7 @@ static DWORD WINAPI blocking_thread(void *arg) static void test_blocking(ULONG options) { struct blocking_thread_args ctx; + BOOL completing_canceled; OBJECT_ATTRIBUTES attr; UNICODE_STRING name; char read_buf[16]; @@ -1719,10 +1750,13 @@ static void test_blocking(ULONG options) IO_STATUS_BLOCK io; NTSTATUS status; DWORD res, num_bytes; + HANDLE client_nonalert; BOOL ret; ctx.wait = CreateEventW(NULL, FALSE, FALSE, NULL); ctx.done = CreateEventW(NULL, FALSE, FALSE, NULL); + DuplicateHandle(GetCurrentProcess(), GetCurrentThread(), GetCurrentProcess(), &ctx.io_thread, 0, FALSE, + DUPLICATE_SAME_ACCESS); thread = CreateThread(NULL, 0, blocking_thread, &ctx, 0, 0); ok(thread != INVALID_HANDLE_VALUE, "can't create thread, GetLastError: %lx\n", GetLastError()); @@ -1743,9 +1777,8 @@ static void test_blocking(ULONG options) /* blocking read with no event nor APC */ ioapc_called = FALSE; memset(&io, 0xff, sizeof(io)); - ctx.cmd = BLOCKING_THREAD_WRITE; ctx.event = NULL; - SetEvent(ctx.wait); + blocking_thread_command(BLOCKING_THREAD_WRITE); status = NtReadFile(ctx.client, NULL, NULL, NULL, &io, read_buf, sizeof(read_buf), NULL, NULL); ok(status == STATUS_SUCCESS, "status = %lx\n", status); ok(io.Status == STATUS_SUCCESS, "Status = %lx\n", io.Status); @@ -1758,9 +1791,8 @@ static void test_blocking(ULONG options) /* blocking read with event and APC */ ioapc_called = FALSE; memset(&io, 0xff, sizeof(io)); - ctx.cmd = BLOCKING_THREAD_WRITE; ctx.event = CreateEventW(NULL, TRUE, TRUE, NULL); - SetEvent(ctx.wait); + blocking_thread_command(BLOCKING_THREAD_WRITE); status = NtReadFile(ctx.client, ctx.event, ioapc, &io, &io, read_buf, sizeof(read_buf), NULL, NULL); ok(status == STATUS_SUCCESS, "status = %lx\n", status); @@ -1772,12 +1804,14 @@ static void test_blocking(ULONG options) if (!(options & FILE_SYNCHRONOUS_IO_ALERT)) ok(!ioapc_called, "ioapc called\n"); + else + todo_wine ok(ioapc_called, "ioapc called\n"); SleepEx(0, TRUE); /* alertable wait state */ ok(ioapc_called, "ioapc not called\n"); res = WaitForSingleObject(ctx.done, 10000); ok(res == WAIT_OBJECT_0, "wait returned %lx\n", res); - ioapc_called = FALSE; + CloseHandle(ctx.event); ctx.event = NULL; @@ -1787,8 +1821,7 @@ static void test_blocking(ULONG options) ioapc_called = FALSE; memset(&io, 0xff, sizeof(io)); - ctx.cmd = BLOCKING_THREAD_READ; - SetEvent(ctx.wait); + blocking_thread_command(BLOCKING_THREAD_READ); status = NtFlushBuffersFile(ctx.client, &io); ok(status == STATUS_SUCCESS, "status = %lx\n", status); ok(io.Status == STATUS_SUCCESS, "Status = %lx\n", io.Status); @@ -1814,8 +1847,7 @@ static void test_blocking(ULONG options) ioapc_called = FALSE; memset(&io, 0xff, sizeof(io)); - ctx.cmd = BLOCKING_THREAD_READ; - SetEvent(ctx.wait); + blocking_thread_command(BLOCKING_THREAD_READ); status = NtFlushBuffersFile(ctx.client, &io); ok(status == STATUS_SUCCESS, "status = %lx\n", status); ok(io.Status == STATUS_SUCCESS, "Status = %lx\n", io.Status); @@ -1828,13 +1860,143 @@ static void test_blocking(ULONG options) CloseHandle(ctx.pipe); CloseHandle(ctx.client); - ctx.cmd = BLOCKING_THREAD_QUIT; - SetEvent(ctx.wait); + ctx.event = CreateEventW(NULL, TRUE, TRUE, NULL); + status = create_pipe(&ctx.pipe, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, + options); + ok(status == STATUS_SUCCESS, "NtCreateNamedPipeFile returned %lx\n", status); + pRtlInitUnicodeString(&name, testpipe_nt); + InitializeObjectAttributes( &attr, &name, OBJ_CASE_INSENSITIVE, 0, NULL ); + status = NtCreateFile(&ctx.client, SYNCHRONIZE | GENERIC_READ | GENERIC_WRITE, &attr, &io, + NULL, 0, FILE_SHARE_READ | FILE_SHARE_WRITE, FILE_OPEN, + options, NULL, 0 ); + ok(status == STATUS_SUCCESS, "NtCreateFile returned %lx\n", status); + if (options & FILE_SYNCHRONOUS_IO_ALERT) + { + /* Alertable IO interrupted with pre-queued user APC. */ + io.Status = 0xdeadbeef; + io.Information = 0xdeadbeef; + pQueueUserAPC(userapc, GetCurrentThread(), 0); + userapc_called = FALSE; + ioapc_called = FALSE; + status = NtReadFile(ctx.client, ctx.event, ioapc, &io, &io, read_buf, + sizeof(read_buf), NULL, NULL); + todo_wine ok(status == STATUS_CANCELLED, "status = %lx\n", status); + todo_wine ok(io.Status == STATUS_CANCELLED || broken(io.Status == 0xdeadbeef) /* Before Win11 24H2 */, + "Status = %lx\n", io.Status); + completing_canceled = io.Status != 0xdeadbeef; + todo_wine ok(io.Information == 0 || broken(io.Information == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, + "Information = %Iu\n", io.Information); + todo_wine ok(is_signaled(ctx.event) || broken(!completing_canceled) /* Before Win11 24H2 */, + "event is not signaled\n"); + ok(!ioapc_called, "ioapc called\n"); + ok(userapc_called, "user apc is not called.\n"); + SleepEx(0, TRUE); + ok(!ioapc_called, "ioapc called\n"); + + /* Alertable IO interrupted with pre-queued IO completion user APC. */ + ioapc_called = FALSE; + client_nonalert = CreateFileW(L"test.dat", GENERIC_READ | GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, NULL); + ok(client_nonalert != INVALID_HANDLE_VALUE, "CreateFileW error %lu\n", GetLastError()); + status = NtWriteFile(client_nonalert, ctx.event, ioapc, &io, &io, read_buf, sizeof(read_buf), NULL, NULL); + ok(status == STATUS_SUCCESS, "status = %lx\n", status); + ok(io.Status == STATUS_SUCCESS, "Status = %lx\n", io.Status); + ok(io.Information == sizeof(read_buf), "Information = %Iu\n", io.Information); + ok(!ioapc_called, "ioapc called\n"); + ok(is_signaled(ctx.event), "event is not signaled\n"); + + io.Status = 0xdeadbeef; + io.Information = 0xdeadbeef; + userapc_called = FALSE; + ioapc_called = FALSE; + status = NtReadFile(ctx.client, ctx.event, ioapc, &io, &io, read_buf, + sizeof(read_buf), NULL, NULL); + todo_wine ok(status == STATUS_CANCELLED, "status = %lx\n", status); + todo_wine ok(io.Status == STATUS_CANCELLED || broken(io.Status == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, + "Status = %lx\n", io.Status); + todo_wine ok(io.Information == 0 || broken(io.Information == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, + "Information = %Iu\n", io.Information); + todo_wine ok(is_signaled(ctx.event) || broken(!completing_canceled) /* Before Win11 24H2 */, + "event is not signaled\n"); + ok(ioapc_called, "ioapc called\n"); + ok(!userapc_called, "user apc is not called.\n"); + SleepEx(0, TRUE); + + DeleteFileW(L"test.dat"); + CloseHandle(client_nonalert); + + /* Non-alertable IO function with FILE_SYNCHRONOUS_IO_ALERT handle. */ + io.Status = 0xdeadbeef; + io.Information = 0xdeadbeef; + pQueueUserAPC(userapc, GetCurrentThread(), 0); + userapc_called = FALSE; + ioapc_called = FALSE; + status = NtFlushBuffersFile(ctx.client, &io); + ok(status == STATUS_SUCCESS, "status = %lx\n", status); + ok(io.Status == STATUS_SUCCESS, "Status = %lx\n", io.Status); + ok(io.Information == 0, "Information = %Iu\n", io.Information); + todo_wine ok(is_signaled(ctx.event) || broken(!completing_canceled), "event is not signaled\n"); + ok(!ioapc_called, "ioapc called\n"); + ok(!userapc_called, "user apc is not called.\n"); + SleepEx(0, TRUE); + ok(!ioapc_called, "ioapc called\n"); + ok(userapc_called, "user apc is not called.\n"); + + /* Alertable IO interrupted with user APC during wait. */ + io.Status = 0xdeadbeef; + io.Information = 0xdeadbeef; + userapc_called = FALSE; + ioapc_called = FALSE; + blocking_thread_command(BLOCKING_THREAD_USER_APC); + status = NtReadFile(ctx.client, ctx.event, ioapc, &io, &io, read_buf, + sizeof(read_buf), NULL, NULL); + todo_wine ok(status == STATUS_CANCELLED, "status = %lx\n", status); + todo_wine ok(io.Status == STATUS_CANCELLED || broken(io.Status == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, + "Status = %lx\n", io.Status); + todo_wine ok(io.Information == 0 || broken(io.Information == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, + "Information = %Iu\n", io.Information); + todo_wine ok(is_signaled(ctx.event) || broken(!completing_canceled) /* Before Win11 24H2 */, + "event is not signaled\n"); + ok(!ioapc_called, "ioapc called\n"); + ok(userapc_called, "user apc is not called.\n"); + SleepEx(0, TRUE); + ok(!ioapc_called, "ioapc called\n"); + } + + /* IO interrupted with NtCancelSynchronousIoFile(). */ + io.Status = 0xdeadbeef; + io.Information = 0xdeadbeef; + userapc_called = FALSE; + ioapc_called = FALSE; + blocking_thread_command(BLOCKING_THREAD_CANCEL_IO); + status = NtReadFile(ctx.client, ctx.event, ioapc, &io, &io, read_buf, + sizeof(read_buf), NULL, NULL); + ok(status == STATUS_CANCELLED, "status = %lx\n", status); + todo_wine ok(io.Status == STATUS_CANCELLED || broken(io.Status == 0xdeadbeef) /* Before Win11 24H2 */, + "Status = %lx\n", io.Status); + completing_canceled = io.Status != 0xdeadbeef; + todo_wine ok(io.Information == 0 || broken(io.Information == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, + "Information = %Iu\n", io.Information); + ok(is_signaled(ctx.event) || broken(!completing_canceled) /* Before Win11 24H2 */, + "event is not signaled\n"); + ok(!ioapc_called, "ioapc called\n"); + ok(!userapc_called, "user apc is not called.\n"); + SleepEx(0, TRUE); + todo_wine ok(!ioapc_called, "ioapc called\n"); + + ioapc_called = FALSE; + CloseHandle(ctx.event); + ctx.event = NULL; + + CloseHandle(ctx.pipe); + CloseHandle(ctx.client); + + blocking_thread_command(BLOCKING_THREAD_QUIT); res = WaitForSingleObject(thread, 10000); ok(res == WAIT_OBJECT_0, "wait returned %lx\n", res); CloseHandle(ctx.wait); CloseHandle(ctx.done); + CloseHandle(ctx.io_thread); CloseHandle(thread); } @@ -3118,7 +3280,9 @@ START_TEST(pipe) trace("starting blocking tests\n"); test_blocking(FILE_SYNCHRONOUS_IO_NONALERT); + winetest_push_context("aletrable"); test_blocking(FILE_SYNCHRONOUS_IO_ALERT); + winetest_pop_context(); trace("starting FILE_PIPE_INFORMATION tests\n"); test_filepipeinfo(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8592
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/server.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c index 3969c2bb898..e9d11f12e42 100644 --- a/dlls/ntdll/unix/server.c +++ b/dlls/ntdll/unix/server.c @@ -851,15 +851,24 @@ NTSTATUS WINAPI NtContinueEx( CONTEXT *context, KCONTINUE_ARGUMENT *args ) /*********************************************************************** - * NtTestAlert (NTDLL.@) + * test_alert_with_status */ -NTSTATUS WINAPI NtTestAlert(void) +static void test_alert_with_status( NTSTATUS ret_status ) { struct user_apc apc; NTSTATUS status; status = server_select( NULL, 0, SELECT_INTERRUPTIBLE | SELECT_ALERTABLE, 0, NULL, &apc ); - if (status == STATUS_USER_APC) invoke_user_apc( NULL, &apc, STATUS_SUCCESS ); + if (status == STATUS_USER_APC) invoke_user_apc( NULL, &apc, ret_status ); +} + + +/*********************************************************************** + * NtTestAlert (NTDLL.@) + */ +NTSTATUS WINAPI NtTestAlert(void) +{ + test_alert_with_status( STATUS_SUCCESS ); return STATUS_SUCCESS; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8592
From: Paul Gofman <pgofman(a)codeweavers.com> Cancel async and signal async object on user APC instead. --- dlls/ntdll/tests/pipe.c | 30 +++++++++++----------- dlls/ntdll/unix/server.c | 2 +- dlls/ntdll/unix/unix_private.h | 8 +++++- server/async.c | 46 +++++++++++++++++++++++++++------- server/fd.c | 6 +++++ server/file.h | 2 ++ server/thread.c | 1 + 7 files changed, 69 insertions(+), 26 deletions(-) diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index fb292a8c5e6..1e863cb7b2d 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -477,7 +477,7 @@ static void test_alertable(void) ok(ret, "can't queue user apc, GetLastError: %lx\n", GetLastError()); res = listen_pipe(hPipe, hEvent, &iosb, TRUE); - todo_wine ok(res == STATUS_CANCELLED, "NtFsControlFile returned %lx\n", res); + ok(res == STATUS_CANCELLED, "NtFsControlFile returned %lx\n", res); ok(userapc_called, "user apc didn't run\n"); ok(iosb.Status == 0x55555555 || iosb.Status == STATUS_CANCELLED, "iosb.Status got changed to %lx\n", iosb.Status); @@ -491,7 +491,7 @@ static void test_alertable(void) /* wine_todo: the earlier NtFsControlFile call gets cancelled after the pipe gets set into listen state instead of before, so this NtFsControlFile will fail STATUS_INVALID_HANDLE */ res = listen_pipe(hPipe, hEvent, &iosb, TRUE); - todo_wine ok(res == STATUS_CANCELLED, "NtFsControlFile returned %lx\n", res); + ok(res == STATUS_CANCELLED, "NtFsControlFile returned %lx\n", res); ok(userapc_called, "user apc didn't run\n"); ok(iosb.Status == 0x55555555 || iosb.Status == STATUS_CANCELLED, "iosb.Status got changed to %lx\n", iosb.Status); @@ -1805,7 +1805,7 @@ static void test_blocking(ULONG options) if (!(options & FILE_SYNCHRONOUS_IO_ALERT)) ok(!ioapc_called, "ioapc called\n"); else - todo_wine ok(ioapc_called, "ioapc called\n"); + ok(ioapc_called, "ioapc called\n"); SleepEx(0, TRUE); /* alertable wait state */ ok(ioapc_called, "ioapc not called\n"); @@ -1880,13 +1880,13 @@ static void test_blocking(ULONG options) ioapc_called = FALSE; status = NtReadFile(ctx.client, ctx.event, ioapc, &io, &io, read_buf, sizeof(read_buf), NULL, NULL); - todo_wine ok(status == STATUS_CANCELLED, "status = %lx\n", status); - todo_wine ok(io.Status == STATUS_CANCELLED || broken(io.Status == 0xdeadbeef) /* Before Win11 24H2 */, + ok(status == STATUS_CANCELLED, "status = %lx\n", status); + ok(io.Status == STATUS_CANCELLED || broken(io.Status == 0xdeadbeef) /* Before Win11 24H2 */, "Status = %lx\n", io.Status); completing_canceled = io.Status != 0xdeadbeef; - todo_wine ok(io.Information == 0 || broken(io.Information == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, + ok(io.Information == 0 || broken(io.Information == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, "Information = %Iu\n", io.Information); - todo_wine ok(is_signaled(ctx.event) || broken(!completing_canceled) /* Before Win11 24H2 */, + ok(is_signaled(ctx.event) || broken(!completing_canceled) /* Before Win11 24H2 */, "event is not signaled\n"); ok(!ioapc_called, "ioapc called\n"); ok(userapc_called, "user apc is not called.\n"); @@ -1910,12 +1910,12 @@ static void test_blocking(ULONG options) ioapc_called = FALSE; status = NtReadFile(ctx.client, ctx.event, ioapc, &io, &io, read_buf, sizeof(read_buf), NULL, NULL); - todo_wine ok(status == STATUS_CANCELLED, "status = %lx\n", status); - todo_wine ok(io.Status == STATUS_CANCELLED || broken(io.Status == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, + ok(status == STATUS_CANCELLED, "status = %lx\n", status); + ok(io.Status == STATUS_CANCELLED || broken(io.Status == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, "Status = %lx\n", io.Status); - todo_wine ok(io.Information == 0 || broken(io.Information == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, + ok(io.Information == 0 || broken(io.Information == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, "Information = %Iu\n", io.Information); - todo_wine ok(is_signaled(ctx.event) || broken(!completing_canceled) /* Before Win11 24H2 */, + ok(is_signaled(ctx.event) || broken(!completing_canceled) /* Before Win11 24H2 */, "event is not signaled\n"); ok(ioapc_called, "ioapc called\n"); ok(!userapc_called, "user apc is not called.\n"); @@ -1934,7 +1934,7 @@ static void test_blocking(ULONG options) ok(status == STATUS_SUCCESS, "status = %lx\n", status); ok(io.Status == STATUS_SUCCESS, "Status = %lx\n", io.Status); ok(io.Information == 0, "Information = %Iu\n", io.Information); - todo_wine ok(is_signaled(ctx.event) || broken(!completing_canceled), "event is not signaled\n"); + ok(is_signaled(ctx.event) || broken(!completing_canceled), "event is not signaled\n"); ok(!ioapc_called, "ioapc called\n"); ok(!userapc_called, "user apc is not called.\n"); SleepEx(0, TRUE); @@ -1949,12 +1949,12 @@ static void test_blocking(ULONG options) blocking_thread_command(BLOCKING_THREAD_USER_APC); status = NtReadFile(ctx.client, ctx.event, ioapc, &io, &io, read_buf, sizeof(read_buf), NULL, NULL); - todo_wine ok(status == STATUS_CANCELLED, "status = %lx\n", status); + ok(status == STATUS_CANCELLED, "status = %lx\n", status); todo_wine ok(io.Status == STATUS_CANCELLED || broken(io.Status == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, "Status = %lx\n", io.Status); todo_wine ok(io.Information == 0 || broken(io.Information == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, "Information = %Iu\n", io.Information); - todo_wine ok(is_signaled(ctx.event) || broken(!completing_canceled) /* Before Win11 24H2 */, + ok(is_signaled(ctx.event) || broken(!completing_canceled) /* Before Win11 24H2 */, "event is not signaled\n"); ok(!ioapc_called, "ioapc called\n"); ok(userapc_called, "user apc is not called.\n"); @@ -1981,7 +1981,7 @@ static void test_blocking(ULONG options) ok(!ioapc_called, "ioapc called\n"); ok(!userapc_called, "user apc is not called.\n"); SleepEx(0, TRUE); - todo_wine ok(!ioapc_called, "ioapc called\n"); + ok(!ioapc_called, "ioapc called\n"); ioapc_called = FALSE; CloseHandle(ctx.event); diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c index e9d11f12e42..17531b2a1ca 100644 --- a/dlls/ntdll/unix/server.c +++ b/dlls/ntdll/unix/server.c @@ -853,7 +853,7 @@ NTSTATUS WINAPI NtContinueEx( CONTEXT *context, KCONTINUE_ARGUMENT *args ) /*********************************************************************** * test_alert_with_status */ -static void test_alert_with_status( NTSTATUS ret_status ) +void test_alert_with_status( NTSTATUS ret_status ) { struct user_apc apc; NTSTATUS status; diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index fa38f4b87ca..74fb95644ed 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -393,6 +393,8 @@ extern NTSTATUS call_user_apc_dispatcher( CONTEXT *context_ptr, ULONG_PTR arg1, extern NTSTATUS call_user_exception_dispatcher( EXCEPTION_RECORD *rec, CONTEXT *context ); extern void call_raise_user_exception_dispatcher(void); +extern void test_alert_with_status( NTSTATUS status ); + #define IMAGE_DLLCHARACTERISTICS_PREFER_NATIVE 0x0010 /* Wine extension */ #define TICKSPERSEC 10000000 @@ -467,7 +469,11 @@ static inline struct async_data server_async( HANDLE handle, struct async_fileio static inline NTSTATUS wait_async( HANDLE handle, BOOL alertable ) { - return NtWaitForSingleObject( handle, alertable, NULL ); + NTSTATUS ret; + + ret = NtWaitForSingleObject( handle, FALSE, NULL ); + if (alertable) test_alert_with_status( ret ); + return ret; } static inline BOOL in_wow64_call(void) diff --git a/server/async.c b/server/async.c index 1ed241dcb65..5b99cf00421 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 alertable_wait :1; /* blocking alertable IO */ 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 */ @@ -279,6 +280,7 @@ struct async *create_async( struct fd *fd, struct thread *thread, const struct a async->canceled = 0; async->unknown_status = 0; async->blocking = !is_fd_overlapped( fd ); + async->alertable_wait = 0; async->is_system = 0; async->completion = fd_get_completion( fd, &async->comp_key ); async->comp_flags = 0; @@ -336,6 +338,7 @@ static void async_call_completion_callback( struct async *async ) obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_blocking ) { async->blocking = force_blocking || async->blocking; + async->alertable_wait = async->blocking && async->fd && is_fd_wait_alertable( async->fd ) && !force_blocking; if (async->unknown_status) { @@ -396,6 +399,12 @@ obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_ return 0; } + if (async->alertable_wait && !list_empty( &async->thread->user_apc )) + { + async->canceled = 1; + fd_cancel_async( async->fd, async ); + } + if (async->iosb->status != STATUS_PENDING) { if (result) *result = async->iosb->result; @@ -512,18 +521,21 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota /* don't signal completion if the async failed synchronously * this can happen if the initial status was unknown (i.e. for device files) * note that we check the IOSB status here, not the initial status */ - if (async->pending || !NT_ERROR( status )) + if (async->pending || async->canceled || !NT_ERROR( status )) { if (async->data.apc) { - union apc_call data; - memset( &data, 0, sizeof(data) ); - data.type = APC_USER; - data.user.func = async->data.apc; - data.user.args[0] = async->data.apc_context; - data.user.args[1] = async->data.iosb; - data.user.args[2] = 0; - thread_queue_apc( NULL, async->thread, NULL, &data ); + if (!(async->blocking && async->canceled)) + { + union apc_call data; + memset( &data, 0, sizeof(data) ); + data.type = APC_USER; + data.user.func = async->data.apc; + data.user.args[0] = async->data.apc_context; + data.user.args[1] = async->data.iosb; + data.user.args[2] = 0; + thread_queue_apc( NULL, async->thread, NULL, &data ); + } } else if (async->data.apc_context && (async->pending || !(async->comp_flags & FILE_SKIP_COMPLETION_PORT_ON_SUCCESS))) @@ -675,6 +687,22 @@ restart: } } +void cancel_alerted_thread_async( struct thread *thread ) +{ + struct async *async; + + LIST_FOR_EACH_ENTRY( async, &thread->process->asyncs, struct async, process_entry ) + { + if (async->terminated || async->canceled) continue; + if (async->blocking && async->thread == thread && async->alertable_wait) + { + async->canceled = 1; + fd_cancel_async( async->fd, async ); + return; + } + } +} + /* wake up async operations on the queue */ void async_wake_up( struct async_queue *queue, unsigned int status ) { diff --git a/server/fd.c b/server/fd.c index 663f497b7a9..997e66ca517 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2139,6 +2139,12 @@ int is_fd_overlapped( struct fd *fd ) return !(fd->options & (FILE_SYNCHRONOUS_IO_ALERT | FILE_SYNCHRONOUS_IO_NONALERT)); } +/* check if fd is in synchronous alert mode */ +int is_fd_wait_alertable( struct fd *fd ) +{ + return fd->options & FILE_SYNCHRONOUS_IO_ALERT; +} + /* retrieve the unix fd for an object */ int get_unix_fd( struct fd *fd ) { diff --git a/server/file.h b/server/file.h index 567194bf00a..972e4f54bb4 100644 --- a/server/file.h +++ b/server/file.h @@ -94,6 +94,7 @@ extern void set_fd_user( struct fd *fd, const struct fd_ops *ops, struct object extern unsigned int get_fd_options( struct fd *fd ); extern unsigned int get_fd_comp_flags( struct fd *fd ); extern int is_fd_overlapped( struct fd *fd ); +extern int is_fd_wait_alertable( struct fd *fd ); extern int get_unix_fd( struct fd *fd ); extern client_ptr_t get_fd_map_address( struct fd *fd ); extern void set_fd_map_address( struct fd *fd, client_ptr_t addr, mem_size_t size ); @@ -278,6 +279,7 @@ extern void fd_copy_completion( struct fd *src, struct fd *dst ); extern struct iosb *async_get_iosb( struct async *async ); extern struct thread *async_get_thread( struct async *async ); extern struct async *find_pending_async( struct async_queue *queue ); +extern void cancel_alerted_thread_async( struct thread *thread ); extern void cancel_process_asyncs( struct process *process ); extern void cancel_terminating_thread_asyncs( struct thread *thread ); extern int async_close_obj_handle( struct object *obj, struct process *process, obj_handle_t handle ); diff --git a/server/thread.c b/server/thread.c index 05ec6a4ec00..65feb5ffbb8 100644 --- a/server/thread.c +++ b/server/thread.c @@ -2073,6 +2073,7 @@ DECL_HANDLER(queue_apc) if (thread) { if (!queue_apc( NULL, thread, apc )) set_error( STATUS_UNSUCCESSFUL ); + else if (apc->call.type == APC_USER) cancel_alerted_thread_async( thread ); release_object( thread ); } else if (process) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8592
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/pipe.c | 8 ++++---- server/async.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index 1e863cb7b2d..cf3d523b936 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -1950,9 +1950,9 @@ static void test_blocking(ULONG options) status = NtReadFile(ctx.client, ctx.event, ioapc, &io, &io, read_buf, sizeof(read_buf), NULL, NULL); ok(status == STATUS_CANCELLED, "status = %lx\n", status); - todo_wine ok(io.Status == STATUS_CANCELLED || broken(io.Status == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, + ok(io.Status == STATUS_CANCELLED || broken(io.Status == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, "Status = %lx\n", io.Status); - todo_wine ok(io.Information == 0 || broken(io.Information == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, + ok(io.Information == 0 || broken(io.Information == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, "Information = %Iu\n", io.Information); ok(is_signaled(ctx.event) || broken(!completing_canceled) /* Before Win11 24H2 */, "event is not signaled\n"); @@ -1971,10 +1971,10 @@ static void test_blocking(ULONG options) status = NtReadFile(ctx.client, ctx.event, ioapc, &io, &io, read_buf, sizeof(read_buf), NULL, NULL); ok(status == STATUS_CANCELLED, "status = %lx\n", status); - todo_wine ok(io.Status == STATUS_CANCELLED || broken(io.Status == 0xdeadbeef) /* Before Win11 24H2 */, + ok(io.Status == STATUS_CANCELLED || broken(io.Status == 0xdeadbeef) /* Before Win11 24H2 */, "Status = %lx\n", io.Status); completing_canceled = io.Status != 0xdeadbeef; - todo_wine ok(io.Information == 0 || broken(io.Information == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, + ok(io.Information == 0 || broken(io.Information == 0xdeadbeef && !completing_canceled) /* Before Win11 24H2 */, "Information = %Iu\n", io.Information); ok(is_signaled(ctx.event) || broken(!completing_canceled) /* Before Win11 24H2 */, "event is not signaled\n"); diff --git a/server/async.c b/server/async.c index 5b99cf00421..5bdbfd659e7 100644 --- a/server/async.c +++ b/server/async.c @@ -192,8 +192,8 @@ void async_terminate( struct async *async, unsigned int status ) /* this can happen if the initial status was unknown (i.e. for device * files). the client should not fill the IOSB in this case; pass it as * NULL to communicate that. - * note that we check the IOSB status and not the initial status */ - if (NT_ERROR( status ) && (!is_fd_overlapped( async->fd ) || !async->pending)) + * Windows 11 will fill the IOSB for canceled IO. */ + if (NT_ERROR( status ) && !async->canceled && (!is_fd_overlapped( async->fd ) || !async->pending)) data.async_io.sb = 0; else data.async_io.sb = async->data.iosb; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8592
v2: - Add test for interrupting alertable IO with IO completion APC from previous IO; - Add test for forced blocking IO with pre-queued APC on alertable handle; - Fix typo in test context; - Add explicit alertable_wait parameter and don't ever set it for forced synchronous IO; - Use fd_cancel_async() instead of async_terminate() in async_handoff(); - Comment on the change in async_terminate() ensuring completion on async canceling. I changed the text of comment because the change fixes not only canceling with user APC but also with NtCancelSynchronousIoFile(). - nest 'if' in async_set_result(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8592#note_110357
``` @@ -336,6 +338,7 @@ static void async_call_completion_callback( struct async *async ) obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_blocking ) { async->blocking = force_blocking || async->blocking; + async->alertable_wait = async->blocking && async->fd && is_fd_wait_alertable( async->fd ) && !force_blocking; if (async->unknown_status) { ``` Bit of a nitpick at this point, but we're guaranteed to have an fd here. Also, you don't need the "async->blocking" part, because is_fd_wait_alertable() already communicates that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8592#note_110537
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8592
participants (3)
-
Elizabeth Figura (＠zfigura) -
Paul Gofman
-
Paul Gofman (＠gofman)