[PATCH v3 0/4] MR3339: server: Cancel asyncs when the last handle in process is closed.
Supersedes !3329. -- v3: server: Cancel pipe asyncs when the last handle in process is closed. server: Cancel socket asyncs when the last handle in process is closed. ws2_32/tests: Add test for async cancel on socket's last process handle close. ntdll/tests: Add test for async cancel on pipe's last process handle close. https://gitlab.winehq.org/wine/wine/-/merge_requests/3339
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/pipe.c | 143 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index 243b2794888..c9cfdb03826 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -131,6 +131,24 @@ static BOOL init_func_ptrs(void) return TRUE; } +static HANDLE create_process(const char *arg) +{ + STARTUPINFOA si = { 0 }; + PROCESS_INFORMATION pi; + char cmdline[MAX_PATH]; + char **argv; + BOOL ret; + + si.cb = sizeof(si); + winetest_get_mainargs(&argv); + sprintf(cmdline, "%s %s %s", argv[0], argv[1], arg); + ret = CreateProcessA(NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + ok(ret, "got %lu.\n", GetLastError()); + ret = CloseHandle(pi.hThread); + ok(ret, "got %lu.\n", GetLastError()); + return pi.hProcess; +} + static inline BOOL is_signaled( HANDLE obj ) { return WaitForSingleObject( obj, 0 ) == WAIT_OBJECT_0; @@ -2971,13 +2989,137 @@ static void test_pipe_names(void) } } +static void test_async_cancel_on_handle_close(void) +{ + static const struct + { + BOOL event; + PIO_APC_ROUTINE apc; + BOOL apc_context; + } + tests[] = + { + {TRUE, NULL}, + {FALSE, NULL}, + {TRUE, ioapc}, + {FALSE, ioapc}, + {TRUE, NULL, TRUE}, + {FALSE, NULL, TRUE}, + {TRUE, ioapc, TRUE}, + {FALSE, ioapc, TRUE}, + }; + + FILE_IO_COMPLETION_NOTIFICATION_INFORMATION info; + char read_buf[16]; + HANDLE port, write, read, event, handle2, process_handle; + IO_STATUS_BLOCK io; + NTSTATUS status; + unsigned int i, other_process; + DWORD ret; + BOOL bret; + + create_pipe_pair(&read, &write, FILE_FLAG_OVERLAPPED | PIPE_ACCESS_DUPLEX, + PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE, 4096); + + status = pNtQueryInformationFile(read, &io, &info, sizeof(info), + FileIoCompletionNotificationInformation); + ok(status == STATUS_SUCCESS || broken(status == STATUS_INVALID_INFO_CLASS), + "status = %lx\n", status); + CloseHandle(read); + CloseHandle(write); + if (status) + { + win_skip("FileIoCompletionNotificationInformation is not supported.\n"); + return; + } + + process_handle = create_process("sleep"); + event = CreateEventW(NULL, FALSE, FALSE, NULL); + + for (other_process = 0; other_process < 2; ++other_process) + { + for (i = 0; i < ARRAY_SIZE(tests); ++i) + { + winetest_push_context("other_process %u, i %u", other_process, i); + create_pipe_pair(&read, &write, FILE_FLAG_OVERLAPPED | PIPE_ACCESS_DUPLEX, + PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE, 4096); + port = CreateIoCompletionPort(read, NULL, 0, 0); + ok(!!port, "got %p.\n", port); + + memset(&io, 0xcc, sizeof(io)); + ResetEvent(event); + status = NtReadFile(read, tests[i].event ? event : NULL, tests[i].apc, tests[i].apc_context ? &io : NULL, &io, + read_buf, 16, NULL, NULL); + if (tests[i].apc) + { + ok(status == STATUS_INVALID_PARAMETER, "got %#lx.\n", status); + CloseHandle(read); + CloseHandle(write); + CloseHandle(port); + winetest_pop_context(); + continue; + } + ok(status == STATUS_PENDING, "got %#lx.\n", status); + ok(io.Status == 0xcccccccc, "got %#lx.\n", io.Status); + + bret = DuplicateHandle(GetCurrentProcess(), read, other_process ? process_handle : GetCurrentProcess(), + &handle2, 0, FALSE, DUPLICATE_SAME_ACCESS); + ok(bret, "failed, error %lu.\n", GetLastError()); + + CloseHandle(read); + ok(io.Status == 0xcccccccc, "got %#lx.\n", io.Status); + if (other_process && tests[i].apc_context && !tests[i].event) + todo_wine test_queued_completion(port, &io, STATUS_CANCELLED, 0); + else + test_no_queued_completion(port); + + ret = WaitForSingleObject(event, 0); + ok(ret == WAIT_TIMEOUT, "got %#lx.\n", ret); + + if (other_process) + { + bret = DuplicateHandle(process_handle, handle2, GetCurrentProcess(), &read, 0, FALSE, + DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE); + ok(bret, "failed, error %lu.\n", GetLastError()); + } + else + { + read = handle2; + } + CloseHandle(read); + CloseHandle(write); + CloseHandle(port); + winetest_pop_context(); + } + } + + CloseHandle(event); + TerminateProcess(process_handle, 0); + WaitForSingleObject(process_handle, INFINITE); + CloseHandle(process_handle); +} + START_TEST(pipe) { + char **argv; + int argc; + if (!init_func_ptrs()) return; if (!pIsWow64Process || !pIsWow64Process( GetCurrentProcess(), &is_wow64 )) is_wow64 = FALSE; + argc = winetest_get_mainargs(&argv); + if (argc >= 3) + { + if (!strcmp(argv[2], "sleep")) + { + Sleep(5000); + return; + } + return; + } + trace("starting invalid create tests\n"); test_create_invalid(); @@ -3031,6 +3173,7 @@ START_TEST(pipe) test_security_info(); test_empty_name(); test_pipe_names(); + test_async_cancel_on_handle_close(); pipe_for_each_state(create_pipe_server, connect_pipe, test_pipe_state); pipe_for_each_state(create_pipe_server, connect_and_write_pipe, test_pipe_with_data_state); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3339
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/tests/afd.c | 184 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 172 insertions(+), 12 deletions(-) diff --git a/dlls/ws2_32/tests/afd.c b/dlls/ws2_32/tests/afd.c index 1644a9dc4ff..ef3e6385c7f 100644 --- a/dlls/ws2_32/tests/afd.c +++ b/dlls/ws2_32/tests/afd.c @@ -33,6 +33,24 @@ #define TIMEOUT_INFINITE _I64_MAX +static HANDLE create_process(const char *arg) +{ + STARTUPINFOA si = { 0 }; + PROCESS_INFORMATION pi; + char cmdline[MAX_PATH]; + char **argv; + BOOL ret; + + si.cb = sizeof(si); + winetest_get_mainargs(&argv); + sprintf(cmdline, "%s %s %s", argv[0], argv[1], arg); + ret = CreateProcessA(NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + ok(ret, "got %lu.\n", GetLastError()); + ret = CloseHandle(pi.hThread); + ok(ret, "got %lu.\n", GetLastError()); + return pi.hProcess; +} + static void tcp_socketpair_flags(SOCKET *src, SOCKET *dst, DWORD flags) { SOCKET server = INVALID_SOCKET; @@ -2464,11 +2482,11 @@ static NTSTATUS WINAPI thread_NtDeviceIoControlFile(BOOL kill_thread, HANDLE han return p.ret; } -static unsigned int test_async_thread_termination_apc_count; +static unsigned int test_apc_count; -static void WINAPI test_async_thread_termination_apc( void *arg, IO_STATUS_BLOCK *iosb, ULONG reserved ) +static void WINAPI test_apc_proc( void *arg, IO_STATUS_BLOCK *iosb, ULONG reserved ) { - ++test_async_thread_termination_apc_count; + ++test_apc_count; } static void test_async_thread_termination(void) @@ -2486,18 +2504,18 @@ static void test_async_thread_termination(void) {TRUE, TRUE, NULL, NULL}, {FALSE, FALSE, NULL, NULL}, {TRUE, FALSE, NULL, NULL}, - {FALSE, TRUE, test_async_thread_termination_apc, NULL}, - {TRUE, TRUE, test_async_thread_termination_apc, NULL}, - {FALSE, FALSE, test_async_thread_termination_apc, NULL}, - {TRUE, FALSE, test_async_thread_termination_apc, NULL}, + {FALSE, TRUE, test_apc_proc, NULL}, + {TRUE, TRUE, test_apc_proc, NULL}, + {FALSE, FALSE, test_apc_proc, NULL}, + {TRUE, FALSE, test_apc_proc, NULL}, {FALSE, TRUE, NULL, (void *)0xdeadbeef}, {TRUE, TRUE, NULL, (void *)0xdeadbeef}, {FALSE, FALSE, NULL, (void *)0xdeadbeef}, {TRUE, FALSE, NULL, (void *)0xdeadbeef}, - {FALSE, TRUE, test_async_thread_termination_apc, (void *)0xdeadbeef}, - {TRUE, TRUE, test_async_thread_termination_apc, (void *)0xdeadbeef}, - {FALSE, FALSE, test_async_thread_termination_apc, (void *)0xdeadbeef}, - {TRUE, FALSE, test_async_thread_termination_apc, (void *)0xdeadbeef}, + {FALSE, TRUE, test_apc_proc, (void *)0xdeadbeef}, + {TRUE, TRUE, test_apc_proc, (void *)0xdeadbeef}, + {FALSE, FALSE, test_apc_proc, (void *)0xdeadbeef}, + {TRUE, FALSE, test_apc_proc, (void *)0xdeadbeef}, }; const struct sockaddr_in bind_addr = {.sin_family = AF_INET, .sin_addr.s_addr = htonl(INADDR_LOOPBACK)}; @@ -2550,7 +2568,7 @@ static void test_async_thread_termination(void) } SleepEx(0, TRUE); - ok(!test_async_thread_termination_apc_count, "got APC.\n"); + ok(!test_apc_count, "got APC.\n"); port = CreateIoCompletionPort((HANDLE)listener, NULL, 0, 0); @@ -2754,12 +2772,153 @@ static void test_read_write(void) CloseHandle(event); } +static void test_async_cancel_on_handle_close(void) +{ + static const struct + { + BOOL event; + PIO_APC_ROUTINE apc; + void *apc_context; + } + tests[] = + { + {TRUE, NULL, NULL}, + {FALSE, NULL, NULL}, + {TRUE, test_apc_proc, NULL}, + {FALSE, test_apc_proc, NULL}, + {TRUE, NULL, (void *)0xdeadbeef}, + {FALSE, NULL, (void *)0xdeadbeef}, + {TRUE, test_apc_proc, (void *)0xdeadbeef}, + {FALSE, test_apc_proc, (void *)0xdeadbeef}, + }; + + const struct sockaddr_in bind_addr = {.sin_family = AF_INET, .sin_addr.s_addr = htonl(INADDR_LOOPBACK)}; + char in_buffer[offsetof(struct afd_poll_params, sockets[3])]; + char out_buffer[offsetof(struct afd_poll_params, sockets[3])]; + struct afd_poll_params *in_params = (struct afd_poll_params *)in_buffer; + struct afd_poll_params *out_params = (struct afd_poll_params *)out_buffer; + unsigned int i, other_process; + LARGE_INTEGER zero = {{0}}; + HANDLE process_handle; + ULONG_PTR key, value; + IO_STATUS_BLOCK io; + HANDLE event, port; + ULONG params_size; + SOCKET listener; + HANDLE handle2; + DWORD ret; + BOOL bret; + + process_handle = create_process("sleep"); + + event = CreateEventW(NULL, FALSE, FALSE, NULL); + + in_params->count = 1; + in_params->exclusive = FALSE; + in_params->sockets[0].flags = ~0; + in_params->sockets[0].status = 0xdeadbeef; + params_size = offsetof(struct afd_poll_params, sockets[1]); + in_params->timeout = -10 * 1000 * 1000 * 5; + + for (other_process = 0; other_process < 2; ++other_process) + { + for (i = 0; i < ARRAY_SIZE(tests); ++i) + { + winetest_push_context("other_process %u, i %u", other_process, i); + + listener = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); + ret = bind(listener, (const struct sockaddr *)&bind_addr, sizeof(bind_addr)); + ok(!ret, "got error %u\n", WSAGetLastError()); + ret = listen(listener, 1); + ok(!ret, "got error %u\n", WSAGetLastError()); + + port = CreateIoCompletionPort((HANDLE)listener, NULL, 0, 0); + ok(!!port, "got %p.\n", port); + + in_params->sockets[0].socket = listener; + + memset(&io, 0xcc, sizeof(io)); + ResetEvent(event); + ret = NtDeviceIoControlFile((HANDLE)listener, tests[i].event ? event : NULL, + tests[i].apc, tests[i].apc_context, &io, IOCTL_AFD_POLL, in_params, params_size, + out_params, params_size); + if (tests[i].apc) + { + ok(ret == STATUS_INVALID_PARAMETER, "got %#lx\n", ret); + winetest_pop_context(); + continue; + } + ok(ret == STATUS_PENDING, "got %#lx.\n", ret); + ok(io.Status == 0xcccccccc, "got %#lx.\n", io.Status); + + bret = DuplicateHandle(GetCurrentProcess(), (HANDLE)listener, + other_process ? process_handle : GetCurrentProcess(), + &handle2, 0, FALSE, DUPLICATE_SAME_ACCESS); + ok(bret, "failed, error %lu.\n", GetLastError()); + + closesocket(listener); + + ok(io.Status == 0xcccccccc, "got %#lx\n", io.Status); + memset(&io, 0xcc, sizeof(io)); + key = 0xcc; + value = 0; + ret = NtRemoveIoCompletion(port, &key, &value, &io, &zero); + if (other_process && tests[i].apc_context && !tests[i].event) + { + todo_wine ok(!ret, "got %#lx\n", ret); + todo_wine ok(!key, "got key %#Ix\n", key); + todo_wine ok(value == 0xdeadbeef, "got value %#Ix\n", value); + todo_wine ok(io.Status == STATUS_CANCELLED, "got %#lx\n", io.Status); + } + else + { + ok(ret == WAIT_TIMEOUT, "got %#lx\n", ret); + } + + ret = WaitForSingleObject(event, 0); + ok(ret == WAIT_TIMEOUT, "got %#lx.\n", ret); + + if (other_process) + { + bret = DuplicateHandle(process_handle, handle2, GetCurrentProcess(), (HANDLE *)&listener, 0, FALSE, + DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE); + ok(bret, "failed, error %lu.\n", GetLastError()); + } + else + { + listener = (SOCKET)handle2; + } + + CloseHandle((HANDLE)listener); + CloseHandle(port); + winetest_pop_context(); + } + } + CloseHandle(event); + TerminateProcess(process_handle, 0); + WaitForSingleObject(process_handle, INFINITE); + CloseHandle(process_handle); +} + START_TEST(afd) { WSADATA data; + char **argv; + int argc; WSAStartup(MAKEWORD(2, 2), &data); + argc = winetest_get_mainargs(&argv); + if (argc >= 3) + { + if (!strcmp(argv[2], "sleep")) + { + Sleep(5000); + return; + } + return; + } + test_open_device(); test_poll(); test_poll_exclusive(); @@ -2773,6 +2932,7 @@ START_TEST(afd) test_getsockname(); test_async_thread_termination(); test_read_write(); + test_async_cancel_on_handle_close(); WSACleanup(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3339
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/tests/afd.c | 11 +++++++---- server/async.c | 22 ++++++++++++++++++++++ server/file.h | 1 + server/handle.c | 15 +++++++++++++++ server/handle.h | 1 + server/sock.c | 3 +-- 6 files changed, 47 insertions(+), 6 deletions(-) diff --git a/dlls/ws2_32/tests/afd.c b/dlls/ws2_32/tests/afd.c index ef3e6385c7f..97139605bf1 100644 --- a/dlls/ws2_32/tests/afd.c +++ b/dlls/ws2_32/tests/afd.c @@ -2858,17 +2858,20 @@ static void test_async_cancel_on_handle_close(void) closesocket(listener); + /* Canceled asyncs with completion port and no event do not update IOSB before removing completion. */ + todo_wine_if(other_process && tests[i].apc_context && !tests[i].event) ok(io.Status == 0xcccccccc, "got %#lx\n", io.Status); + memset(&io, 0xcc, sizeof(io)); key = 0xcc; value = 0; ret = NtRemoveIoCompletion(port, &key, &value, &io, &zero); if (other_process && tests[i].apc_context && !tests[i].event) { - todo_wine ok(!ret, "got %#lx\n", ret); - todo_wine ok(!key, "got key %#Ix\n", key); - todo_wine ok(value == 0xdeadbeef, "got value %#Ix\n", value); - todo_wine ok(io.Status == STATUS_CANCELLED, "got %#lx\n", io.Status); + ok(!ret, "got %#lx\n", ret); + ok(!key, "got key %#Ix\n", key); + ok(value == 0xdeadbeef, "got value %#Ix\n", value); + ok(io.Status == STATUS_CANCELLED, "got %#lx\n", io.Status); } else { diff --git a/server/async.c b/server/async.c index 26946b5f5ce..9cb251df5ce 100644 --- a/server/async.c +++ b/server/async.c @@ -614,6 +614,28 @@ void cancel_process_asyncs( struct process *process ) cancel_async( process, NULL, NULL, 0 ); } +int async_close_obj_handle( struct object *obj, struct process *process, obj_handle_t handle ) +{ + /* Handle a special case when the last object handle in the given process is closed. + * If this is the last object handle overall that is handled in object's close_handle and + * destruction. */ + struct async *async; + + if (obj->handle_count == 1 || get_obj_handle_count( process, obj ) != 1) return 1; + +restart: + LIST_FOR_EACH_ENTRY( async, &process->asyncs, struct async, process_entry ) + { + if (async->terminated || async->canceled || get_fd_user( async->fd ) != obj) continue; + if (!async->completion || !async->data.apc_context || async->event) continue; + + async->canceled = 1; + fd_cancel_async( async->fd, async ); + goto restart; + } + return 1; +} + void cancel_terminating_thread_asyncs( struct thread *thread ) { struct async *async; diff --git a/server/file.h b/server/file.h index 210fcec5e78..a43b4afad08 100644 --- a/server/file.h +++ b/server/file.h @@ -246,6 +246,7 @@ extern struct thread *async_get_thread( struct async *async ); extern struct async *find_pending_async( struct async_queue *queue ); 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 ); static inline void init_async_queue( struct async_queue *queue ) { diff --git a/server/handle.c b/server/handle.c index 38ad80da267..0595fdb403b 100644 --- a/server/handle.c +++ b/server/handle.c @@ -520,6 +520,21 @@ obj_handle_t find_inherited_handle( struct process *process, const struct object return 0; } +/* return number of open handles to the object in the process */ +unsigned int get_obj_handle_count( struct process *process, const struct object *obj ) +{ + struct handle_table *table = process->handles; + struct handle_entry *ptr; + unsigned int count = 0; + int i; + + if (!table) return 0; + + for (i = 0, ptr = table->entries; i <= table->last; i++, ptr++) + if (ptr->ptr == obj) ++count; + return count; +} + /* get/set the handle reserved flags */ /* return the old flags (or -1 on error) */ static int set_handle_flags( struct process *process, obj_handle_t handle, int mask, int flags ) diff --git a/server/handle.h b/server/handle.h index ac3104dc003..1d02e040258 100644 --- a/server/handle.h +++ b/server/handle.h @@ -48,6 +48,7 @@ extern obj_handle_t open_object( struct process *process, obj_handle_t parent, u const struct object_ops *ops, const struct unicode_str *name, unsigned int attr ); extern obj_handle_t find_inherited_handle( struct process *process, const struct object_ops *ops ); +extern unsigned int get_obj_handle_count( struct process *process, const struct object *obj ); extern void close_process_handles( struct process *process ); extern struct handle_table *alloc_handle_table( struct process *process, int count ); extern struct handle_table *copy_handle_table( struct process *process, struct process *parent, diff --git a/server/sock.c b/server/sock.c index 34be6ea22ef..2aae9aefbf0 100644 --- a/server/sock.c +++ b/server/sock.c @@ -1655,8 +1655,7 @@ static int sock_close_handle( struct object *obj, struct process *process, obj_h if (signaled) complete_async_poll( poll_req, STATUS_SUCCESS ); } } - - return 1; + return async_close_obj_handle( obj, process, handle ); } static void sock_destroy( struct object *obj ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3339
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/pipe.c | 5 ++++- server/named_pipe.c | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index c9cfdb03826..a1115bf47b9 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -3067,9 +3067,12 @@ static void test_async_cancel_on_handle_close(void) ok(bret, "failed, error %lu.\n", GetLastError()); CloseHandle(read); + /* Canceled asyncs with completion port and no event do not update IOSB before removing completion. */ + todo_wine_if(other_process && tests[i].apc_context && !tests[i].event) ok(io.Status == 0xcccccccc, "got %#lx.\n", io.Status); + if (other_process && tests[i].apc_context && !tests[i].event) - todo_wine test_queued_completion(port, &io, STATUS_CANCELLED, 0); + test_queued_completion(port, &io, STATUS_CANCELLED, 0); else test_no_queued_completion(port); diff --git a/server/named_pipe.c b/server/named_pipe.c index 9a10c2bf041..3adc79068ab 100644 --- a/server/named_pipe.c +++ b/server/named_pipe.c @@ -179,7 +179,7 @@ static const struct object_ops pipe_server_ops = NULL, /* unlink_name */ pipe_server_open_file, /* open_file */ no_kernel_obj_list, /* get_kernel_obj_list */ - no_close_handle, /* close_handle */ + async_close_obj_handle, /* close_handle */ pipe_server_destroy /* destroy */ }; @@ -223,7 +223,7 @@ static const struct object_ops pipe_client_ops = NULL, /* unlink_name */ no_open_file, /* open_file */ no_kernel_obj_list, /* get_kernel_obj_list */ - no_close_handle, /* close_handle */ + async_close_obj_handle, /* close_handle */ pipe_end_destroy /* destroy */ }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3339
v3: - Handle last process handle close in objects' callback for sockets and pipes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3339#note_39494
participants (2)
-
Paul Gofman -
Paul Gofman (＠gofman)