[PATCH v23 0/10] MR7797: ntdll: Wait for all pending operations to be cancelled in NtCancelIoFile[Ex]().
This fixes Trials Fusion often crashing when disconnecting a controller while there are more still connected. -- v23: ntoskrnl/tests: Use the 'Nt' version of the CancelIo APIs. ntoskrnl/tests: Test the thread ID the cancellation routine runs from. ntoskrnl/tests: Add more cancellation tests. ntoskrnl/tests: Fix tests on current Windows 10 / 11. ntdll/tests: Test IOSB values of the cancel operation. ntdll/tests: Add more NtCancelIoFile[Ex]() tests. ntdll: Wait for all asyncs to handle cancel in NtCancelIoFile(). server: Factor out a cancel_async() function. server: Introduce a find_async_from_user helper. ntdll: Factor out a cancel_io() function. https://gitlab.winehq.org/wine/wine/-/merge_requests/7797
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/ntdll/unix/file.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index bb74b975c8a..1a2072a57cc 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -6476,19 +6476,16 @@ NTSTATUS WINAPI NtFlushBuffersFileEx( HANDLE handle, ULONG flags, void *params, } -/************************************************************************** - * NtCancelIoFile (NTDLL.@) - */ -NTSTATUS WINAPI NtCancelIoFile( HANDLE handle, IO_STATUS_BLOCK *io_status ) +static NTSTATUS cancel_io( HANDLE handle, IO_STATUS_BLOCK *io, IO_STATUS_BLOCK *io_status, + BOOL only_thread ) { unsigned int status; - TRACE( "%p %p\n", handle, io_status ); - SERVER_START_REQ( cancel_async ) { req->handle = wine_server_obj_handle( handle ); - req->only_thread = TRUE; + req->iosb = wine_server_client_ptr( io ); + req->only_thread = only_thread; if (!(status = wine_server_call( req ))) { io_status->Status = status; @@ -6501,28 +6498,25 @@ NTSTATUS WINAPI NtCancelIoFile( HANDLE handle, IO_STATUS_BLOCK *io_status ) } +/************************************************************************** + * NtCancelIoFile (NTDLL.@) + */ +NTSTATUS WINAPI NtCancelIoFile( HANDLE handle, IO_STATUS_BLOCK *io_status ) +{ + TRACE( "%p %p\n", handle, io_status ); + + return cancel_io( handle, NULL, io_status, TRUE ); +} + + /************************************************************************** * NtCancelIoFileEx (NTDLL.@) */ NTSTATUS WINAPI NtCancelIoFileEx( HANDLE handle, IO_STATUS_BLOCK *io, IO_STATUS_BLOCK *io_status ) { - unsigned int status; - TRACE( "%p %p %p\n", handle, io, io_status ); - SERVER_START_REQ( cancel_async ) - { - req->handle = wine_server_obj_handle( handle ); - req->iosb = wine_server_client_ptr( io ); - if (!(status = wine_server_call( req ))) - { - io_status->Status = status; - io_status->Information = 0; - } - } - SERVER_END_REQ; - - return status; + return cancel_io( handle, io, io_status, FALSE ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7797
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/async.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/server/async.c b/server/async.c index 4068f744567..1602746a969 100644 --- a/server/async.c +++ b/server/async.c @@ -576,6 +576,16 @@ int async_waiting( struct async_queue *queue ) return !async->terminated; } +static struct async *find_async_from_user( struct process *process, client_ptr_t user ) +{ + struct async *async; + + LIST_FOR_EACH_ENTRY( async, &process->asyncs, struct async, process_entry ) + if (async->data.user == user) return async; + + return NULL; +} + static int cancel_async( struct process *process, struct object *obj, struct thread *thread, client_ptr_t iosb ) { struct async *async; @@ -830,17 +840,10 @@ DECL_HANDLER(cancel_async) /* get async result from associated iosb */ DECL_HANDLER(get_async_result) { - struct iosb *iosb = NULL; struct async *async; + struct iosb *iosb; - LIST_FOR_EACH_ENTRY( async, ¤t->process->asyncs, struct async, process_entry ) - if (async->data.user == req->user_arg) - { - iosb = async->iosb; - break; - } - - if (!iosb) + if (!(async = find_async_from_user( current->process, req->user_arg )) || !(iosb = async->iosb)) { set_error( STATUS_INVALID_PARAMETER ); return; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7797
From: Matteo Bruni <mbruni(a)codeweavers.com> --- server/async.c | 49 ++++++++++++++++++++++++------------------------ server/file.h | 2 +- server/process.c | 2 +- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/server/async.c b/server/async.c index 1602746a969..764596642db 100644 --- a/server/async.c +++ b/server/async.c @@ -54,7 +54,7 @@ struct async unsigned int direct_result :1;/* a flag if we're passing result directly from request instead of APC */ unsigned int alerted :1; /* fd is signaled, but we are waiting for client-side I/O */ unsigned int terminated :1; /* async has been terminated */ - unsigned int canceled :1; /* have we already queued cancellation for this async? */ + unsigned int cancelled :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. */ @@ -276,7 +276,7 @@ struct async *create_async( struct fd *fd, struct thread *thread, const struct a async->direct_result = 0; async->alerted = 0; async->terminated = 0; - async->canceled = 0; + async->cancelled = 0; async->unknown_status = 0; async->blocking = !is_fd_overlapped( fd ); async->is_system = 0; @@ -576,6 +576,12 @@ int async_waiting( struct async_queue *queue ) return !async->terminated; } +static void cancel_async( struct async *async ) +{ + async->cancelled = 1; + fd_cancel_async( async->fd, async ); +} + static struct async *find_async_from_user( struct process *process, client_ptr_t user ) { struct async *async; @@ -586,25 +592,24 @@ static struct async *find_async_from_user( struct process *process, client_ptr_t return NULL; } -static int cancel_async( struct process *process, struct object *obj, struct thread *thread, client_ptr_t iosb ) +static int cancel_process_async( struct process *process, struct object *obj, struct thread *thread, client_ptr_t iosb ) { struct async *async; int woken = 0; - /* FIXME: it would probably be nice to replace the "canceled" flag with a - * single LIST_FOR_EACH_ENTRY_SAFE, but currently cancelling an async can - * cause other asyncs to be removed via async_reselect() */ + /* We can't simply use LIST_FOR_EACH_ENTRY_SAFE here, because currently + * cancelling an async can cause other asyncs to be removed via + * async_reselect() */ restart: LIST_FOR_EACH_ENTRY( async, &process->asyncs, struct async, process_entry ) { - if (async->terminated || async->canceled || async->is_system) continue; + if (async->terminated || async->cancelled || async->is_system) continue; if ((!obj || (get_fd_user( async->fd ) == obj)) && (!thread || async->thread == thread) && (!iosb || async->data.iosb == iosb)) { - async->canceled = 1; - fd_cancel_async( async->fd, async ); + cancel_async( async ); woken++; goto restart; } @@ -620,12 +625,11 @@ static int cancel_blocking( struct process *process, struct thread *thread, clie restart: LIST_FOR_EACH_ENTRY( async, &process->asyncs, struct async, process_entry ) { - if (async->terminated || async->canceled) continue; + if (async->terminated || async->cancelled) continue; if (async->blocking && async->thread == thread && (!iosb || async->data.iosb == iosb)) { - async->canceled = 1; - fd_cancel_async( async->fd, async ); + cancel_async( async ); woken++; goto restart; } @@ -633,16 +637,15 @@ restart: return woken; } -void cancel_process_asyncs( struct process *process ) +void cancel_terminating_process_asyncs( struct process *process ) { 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 ); + if (async->terminated || async->cancelled) continue; + cancel_async( async ); goto restart; } } @@ -659,11 +662,9 @@ int async_close_obj_handle( struct object *obj, struct process *process, obj_han 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->terminated || async->cancelled || 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 ); + cancel_async( async ); goto restart; } return 1; @@ -676,12 +677,10 @@ void cancel_terminating_thread_asyncs( struct thread *thread ) restart: LIST_FOR_EACH_ENTRY( async, &thread->process->asyncs, struct async, process_entry ) { - if (async->thread != thread || async->terminated || async->canceled) continue; + if (async->thread != thread || async->terminated || async->cancelled) 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 ); + cancel_async( async ); goto restart; } } @@ -831,7 +830,7 @@ DECL_HANDLER(cancel_async) if (obj) { - int count = cancel_async( current->process, obj, thread, req->iosb ); + int count = cancel_process_async( current->process, obj, thread, req->iosb ); if (!count && !thread) set_error( STATUS_NOT_FOUND ); release_object( obj ); } diff --git a/server/file.h b/server/file.h index db89602441f..e1485e4d5fc 100644 --- a/server/file.h +++ b/server/file.h @@ -278,7 +278,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_process_asyncs( struct process *process ); +extern void cancel_terminating_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/process.c b/server/process.c index 52abaa21d75..b30c835f74f 100644 --- a/server/process.c +++ b/server/process.c @@ -986,7 +986,7 @@ static void process_killed( struct process *process ) close_process_desktop( process ); process->winstation = 0; process->desktop = 0; - cancel_process_asyncs( process ); + cancel_terminating_process_asyncs( process ); close_process_handles( process ); if (process->idle_event) release_object( process->idle_event ); process->idle_event = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7797
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/ntdll/unix/file.c | 14 ++-- server/async.c | 155 ++++++++++++++++++++++++++++++++++++++--- server/protocol.def | 2 + 3 files changed, 158 insertions(+), 13 deletions(-) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 1a2072a57cc..0fbf9b8ea1b 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -6479,6 +6479,7 @@ NTSTATUS WINAPI NtFlushBuffersFileEx( HANDLE handle, ULONG flags, void *params, static NTSTATUS cancel_io( HANDLE handle, IO_STATUS_BLOCK *io, IO_STATUS_BLOCK *io_status, BOOL only_thread ) { + HANDLE cancel_handle; unsigned int status; SERVER_START_REQ( cancel_async ) @@ -6487,13 +6488,18 @@ static NTSTATUS cancel_io( HANDLE handle, IO_STATUS_BLOCK *io, IO_STATUS_BLOCK * req->iosb = wine_server_client_ptr( io ); req->only_thread = only_thread; if (!(status = wine_server_call( req ))) - { - io_status->Status = status; - io_status->Information = 0; - } + cancel_handle = wine_server_ptr_handle( reply->cancel_handle ); } SERVER_END_REQ; + if (!status && cancel_handle) + NtWaitForSingleObject( cancel_handle, TRUE, NULL ); + else if (status == STATUS_INVALID_HANDLE) + return status; + + io_status->Status = status; + io_status->Information = 0; + return status; } diff --git a/server/async.c b/server/async.c index 764596642db..771540c3815 100644 --- a/server/async.c +++ b/server/async.c @@ -34,6 +34,96 @@ #include "process.h" #include "handle.h" +struct async_cancel +{ + struct object obj; /* object header */ + struct object *sync; /* sync object for wait/signal */ + unsigned int count; /* count of the asyncs in the cancel group */ + obj_handle_t wait_handle; /* handle to wait for all the cancels to complete */ + struct process *process; /* referenced process, only used to close wait_handle */ +}; + +static void async_cancel_dump( struct object *obj, int verbose ); +static void async_cancel_satisfied( struct object *obj, struct wait_queue_entry *entry ); +static struct object *async_cancel_get_sync( struct object *obj ); +static void async_cancel_destroy( struct object *obj ); + +static const struct object_ops async_cancel_ops = +{ + sizeof(struct async_cancel), /* size */ + &no_type, /* type */ + async_cancel_dump, /* dump */ + NULL, /* add_queue */ + NULL, /* remove_queue */ + NULL, /* signaled */ + async_cancel_satisfied, /* satisfied */ + no_signal, /* signal */ + no_get_fd, /* get_fd */ + async_cancel_get_sync, /* get_sync */ + default_map_access, /* map_access */ + default_get_sd, /* get_sd */ + default_set_sd, /* set_sd */ + no_get_full_name, /* get_full_name */ + no_lookup_name, /* lookup_name */ + no_link_name, /* link_name */ + NULL, /* unlink_name */ + no_open_file, /* open_file */ + no_kernel_obj_list, /* get_kernel_obj_list */ + no_close_handle, /* close_handle */ + async_cancel_destroy /* destroy */ +}; + +static void async_cancel_dump( struct object *obj, int verbose ) +{ + struct async_cancel *cancel = (struct async_cancel *)obj; + assert( obj->ops == &async_cancel_ops ); + fprintf( stderr, "async_cancel %p\n", cancel ); +} + +static void async_cancel_satisfied( struct object *obj, struct wait_queue_entry *entry ) +{ + struct async_cancel *cancel = (struct async_cancel *)obj; + assert( obj->ops == &async_cancel_ops ); + + /* close wait handle here to avoid extra server round trip */ + close_handle( cancel->process, cancel->wait_handle ); + cancel->wait_handle = 0; +} + +static struct object *async_cancel_get_sync( struct object *obj ) +{ + struct async_cancel *cancel = (struct async_cancel *)obj; + + assert( obj->ops == &async_cancel_ops ); + return grab_object( cancel->sync ); +} + +static void async_cancel_destroy( struct object *obj ) +{ + struct async_cancel *cancel = (struct async_cancel *)obj; + + assert( obj->ops == &async_cancel_ops ); + if (cancel->sync) release_object( cancel->sync ); +} + +static struct async_cancel *create_async_cancel( struct process *process ) +{ + struct async_cancel *cancel; + + if (!(cancel = alloc_object( &async_cancel_ops ))) return NULL; + cancel->sync = NULL; + cancel->count = 0; + cancel->wait_handle = 0; + cancel->process = process; + + if (!(cancel->sync = create_internal_sync( 1, 0 ))) + { + release_object( cancel ); + return NULL; + } + return cancel; +} + struct async { struct object obj; /* object header */ @@ -63,6 +153,7 @@ struct async unsigned int comp_flags; /* completion flags */ async_completion_callback completion_callback; /* callback to be called on completion */ void *completion_callback_private; /* argument to completion_callback */ + struct async_cancel *async_cancel; /* cancel object if async is being cancelled */ }; static void async_dump( struct object *obj, int verbose ); @@ -146,6 +237,7 @@ static void async_destroy( struct object *obj ) struct async *async = (struct async *)obj; assert( obj->ops == &async_ops ); + assert( !async->async_cancel ); list_remove( &async->process_entry ); if (async->queue) @@ -284,6 +376,7 @@ struct async *create_async( struct fd *fd, struct thread *thread, const struct a async->comp_flags = 0; async->completion_callback = NULL; async->completion_callback_private = NULL; + async->async_cancel = NULL; if (iosb) async->iosb = (struct iosb *)grab_object( iosb ); else async->iosb = NULL; @@ -485,6 +578,20 @@ static void add_async_completion( struct async *async, apc_param_t cvalue, unsig if (async->completion) add_completion( async->completion, async->comp_key, cvalue, status, information ); } +static void async_complete_cancel( struct async *async ) +{ + struct async_cancel *cancel; + + if (!(cancel = async->async_cancel)) return; + async->async_cancel = NULL; + + if (!--cancel->count) + { + signal_sync( cancel->sync ); + release_object( cancel ); + } +} + /* store the result of the client-side async callback */ void async_set_result( struct object *obj, unsigned int status, apc_param_t total ) { @@ -543,6 +650,7 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota } async_call_completion_callback( async ); + async_complete_cancel( async ); if (async->queue) { @@ -592,10 +700,16 @@ static struct async *find_async_from_user( struct process *process, client_ptr_t return NULL; } -static int cancel_process_async( struct process *process, struct object *obj, struct thread *thread, client_ptr_t iosb ) +static int cancel_process_async( struct process *process, struct object *obj, struct thread *thread, client_ptr_t iosb, obj_handle_t *wait_handle ) { - struct async *async; - int woken = 0; + struct async_cancel *cancel = NULL; + struct async *async, *next_async; + struct list tracked; + int count = 0; + + if (thread && !(cancel = create_async_cancel( process ))) return 0; + + list_init( &tracked ); /* We can't simply use LIST_FOR_EACH_ENTRY_SAFE here, because currently * cancelling an async can cause other asyncs to be removed via @@ -604,17 +718,37 @@ static int cancel_process_async( struct process *process, struct object *obj, st restart: LIST_FOR_EACH_ENTRY( async, &process->asyncs, struct async, process_entry ) { - if (async->terminated || async->cancelled || async->is_system) continue; + if (async->terminated || async->is_system) continue; if ((!obj || (get_fd_user( async->fd ) == obj)) && (!thread || async->thread == thread) && (!iosb || async->data.iosb == iosb)) { - cancel_async( async ); - woken++; + if (!async->cancelled) cancel_async( async ); + if (cancel) + { + assert( !async->async_cancel ); + async->async_cancel = cancel; + cancel->count++; + } + list_remove( &async->process_entry ); + list_add_tail( &tracked, &async->process_entry ); + count++; goto restart; } } - return woken; + /* Put the asyncs back into the process list */ + LIST_FOR_EACH_ENTRY_SAFE( async, next_async, &tracked, struct async, process_entry ) + { + list_remove( &async->process_entry ); + list_add_tail( &process->asyncs, &async->process_entry ); + } + if (cancel) + { + if (!cancel->count) release_object( cancel ); + else cancel->wait_handle = alloc_handle( process, cancel, SYNCHRONIZE, 0 ); + *wait_handle = cancel->wait_handle; + } + return count; } static int cancel_blocking( struct process *process, struct thread *thread, client_ptr_t iosb ) @@ -672,10 +806,11 @@ restart: void cancel_terminating_thread_asyncs( struct thread *thread ) { + struct process *process = thread->process; struct async *async; restart: - LIST_FOR_EACH_ENTRY( async, &thread->process->asyncs, struct async, process_entry ) + LIST_FOR_EACH_ENTRY( async, &process->asyncs, struct async, process_entry ) { if (async->thread != thread || async->terminated || async->cancelled) continue; if (async->completion && async->data.apc_context && !async->event) continue; @@ -827,11 +962,13 @@ DECL_HANDLER(cancel_async) { struct object *obj = get_handle_obj( current->process, req->handle, 0, NULL ); struct thread *thread = req->only_thread ? current : NULL; + obj_handle_t wait_handle = 0; if (obj) { - int count = cancel_process_async( current->process, obj, thread, req->iosb ); + int count = cancel_process_async( current->process, obj, thread, req->iosb, &wait_handle ); if (!count && !thread) set_error( STATUS_NOT_FOUND ); + else reply->cancel_handle = wait_handle; release_object( obj ); } } diff --git a/server/protocol.def b/server/protocol.def index 1881f0a628a..3798462b81d 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -2502,6 +2502,8 @@ enum message_type obj_handle_t handle; /* handle to comm port, socket or file */ client_ptr_t iosb; /* I/O status block (NULL=all) */ int only_thread; /* cancel matching this thread */ +(a)REPLY + obj_handle_t cancel_handle; @END -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7797
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/ntdll/tests/pipe.c | 57 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index 14a6148a2c5..64bdb7e3b66 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -569,14 +569,33 @@ static void test_nonalertable(void) CloseHandle(hPipe); } -static void test_cancelio(void) +struct cancelio_ctx { - IO_STATUS_BLOCK iosb; + HANDLE pipe; + HANDLE event; + IO_STATUS_BLOCK *iosb; + BOOL null_iosb; +}; + +static DWORD WINAPI cancelioex_thread_func(void *arg) +{ + struct cancelio_ctx *ctx = arg; IO_STATUS_BLOCK cancel_sb; - HANDLE hEvent; - HANDLE hPipe; NTSTATUS res; + res = pNtCancelIoFileEx(ctx->pipe, ctx->null_iosb ? NULL : ctx->iosb, &cancel_sb); + ok(!res, "NtCancelIoFileEx returned %lx\n", res); + + return 0; +} + +static void test_cancelio(void) +{ + IO_STATUS_BLOCK cancel_sb, iosb; + HANDLE hEvent, hPipe, thread; + struct cancelio_ctx ctx; + NTSTATUS res, status; + hEvent = CreateEventW(NULL, TRUE, FALSE, NULL); ok(hEvent != INVALID_HANDLE_VALUE, "can't create event, GetLastError: %lx\n", GetLastError()); @@ -589,9 +608,13 @@ static void test_cancelio(void) ok(res == STATUS_PENDING, "NtFsControlFile returned %lx\n", res); res = pNtCancelIoFile(hPipe, &cancel_sb); + /* Save Status first thing after the call. We want to make very unlikely + * that the kernel APC updating Status could be executed after the call + * but before peeking at Status here. */ + status = iosb.Status; ok(!res, "NtCancelIoFile returned %lx\n", res); - ok(iosb.Status == STATUS_CANCELLED, "Wrong iostatus %lx\n", iosb.Status); + ok(status == STATUS_CANCELLED, "Wrong iostatus %lx\n", status); ok(WaitForSingleObject(hEvent, 0) == 0, "hEvent not signaled\n"); ok(!ioapc_called, "IOAPC ran too early\n"); @@ -616,9 +639,10 @@ static void test_cancelio(void) ok(res == STATUS_PENDING, "NtFsControlFile returned %lx\n", res); res = pNtCancelIoFileEx(hPipe, &iosb, &cancel_sb); + status = iosb.Status; ok(!res, "NtCancelIoFileEx returned %lx\n", res); - ok(iosb.Status == STATUS_CANCELLED, "Wrong iostatus %lx\n", iosb.Status); + ok(status == STATUS_CANCELLED, "Wrong iostatus %lx\n", status); ok(WaitForSingleObject(hEvent, 0) == 0, "hEvent not signaled\n"); iosb.Status = 0xdeadbeef; @@ -626,6 +650,27 @@ static void test_cancelio(void) ok(res == STATUS_NOT_FOUND, "NtCancelIoFileEx returned %lx\n", res); ok(iosb.Status == 0xdeadbeef, "Wrong iostatus %lx\n", iosb.Status); + memset(&iosb, 0x55, sizeof(iosb)); + res = listen_pipe(hPipe, hEvent, &iosb, FALSE); + ok(res == STATUS_PENDING, "NtFsControlFile returned %lx\n", res); + + ctx.pipe = hPipe; + ctx.event = hEvent; + ctx.iosb = &iosb; + ctx.null_iosb = FALSE; + thread = CreateThread(NULL, 0, cancelioex_thread_func, &ctx, 0, NULL); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + + memset(&iosb, 0x55, sizeof(iosb)); + res = listen_pipe(hPipe, hEvent, &iosb, FALSE); + ok(res == STATUS_PENDING, "NtFsControlFile returned %lx\n", res); + + ctx.null_iosb = TRUE; + thread = CreateThread(NULL, 0, cancelioex_thread_func, &ctx, 0, NULL); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + CloseHandle(hPipe); } else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7797
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/ntdll/tests/pipe.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index 64bdb7e3b66..5efb969bcf0 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -586,6 +586,9 @@ static DWORD WINAPI cancelioex_thread_func(void *arg) res = pNtCancelIoFileEx(ctx->pipe, ctx->null_iosb ? NULL : ctx->iosb, &cancel_sb); ok(!res, "NtCancelIoFileEx returned %lx\n", res); + ok(!cancel_sb.Status, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); + return 0; } @@ -613,6 +616,8 @@ static void test_cancelio(void) * but before peeking at Status here. */ status = iosb.Status; ok(!res, "NtCancelIoFile returned %lx\n", res); + ok(!cancel_sb.Status, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); ok(status == STATUS_CANCELLED, "Wrong iostatus %lx\n", status); ok(WaitForSingleObject(hEvent, 0) == 0, "hEvent not signaled\n"); @@ -626,6 +631,8 @@ static void test_cancelio(void) res = pNtCancelIoFile(hPipe, &cancel_sb); ok(!res, "NtCancelIoFile returned %lx\n", res); ok(iosb.Status == STATUS_CANCELLED, "Wrong iostatus %lx\n", iosb.Status); + ok(!cancel_sb.Status, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); CloseHandle(hPipe); @@ -641,6 +648,8 @@ static void test_cancelio(void) res = pNtCancelIoFileEx(hPipe, &iosb, &cancel_sb); status = iosb.Status; ok(!res, "NtCancelIoFileEx returned %lx\n", res); + ok(!cancel_sb.Status, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); ok(status == STATUS_CANCELLED, "Wrong iostatus %lx\n", status); ok(WaitForSingleObject(hEvent, 0) == 0, "hEvent not signaled\n"); @@ -649,6 +658,8 @@ static void test_cancelio(void) 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); + ok(cancel_sb.Status == STATUS_NOT_FOUND, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); memset(&iosb, 0x55, sizeof(iosb)); res = listen_pipe(hPipe, hEvent, &iosb, FALSE); @@ -735,6 +746,8 @@ static void test_cancelsynchronousio(void) ok(ret == WAIT_TIMEOUT, "WaitForSingleObject returned %lu (error %lu)\n", ret, GetLastError()); memset(&iosb, 0x55, sizeof(iosb)); res = pNtCancelSynchronousIoFile(thread, NULL, &iosb); + ok(ctx.iosb.Status == 0xdeadbabe, "Unexpected Status %lx\n", ctx.iosb.Status); + ok(ctx.iosb.Information == 0xdeadbeef, "Unexpected Information %Iu\n", ctx.iosb.Information); ok(res == STATUS_SUCCESS, "Failed to cancel I/O\n"); ok(iosb.Status == STATUS_SUCCESS, "iosb.Status got changed to %lx\n", iosb.Status); ok(iosb.Information == 0, "iosb.Information got changed to %Iu\n", iosb.Information); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7797
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 87f41895511..c1902ba5de9 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -695,15 +695,21 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) ret = NtDeviceIoControlFile(device, NULL, NULL, NULL, &io, ioctl, params, sizeof(*params), buffer, sizeof(buffer)); ok(ret == expect_status, "got %#x\n", ret); - if (NT_ERROR(params->iosb_status)) + if (NT_ERROR(params->iosb_status) && !params->pending) { ok(io.Status == 0xdeadf00d, "got %#lx\n", io.Status); ok(io.Information == 0xdeadf00d, "got size %Iu\n", io.Information); } else { - ok(io.Status == params->iosb_status, "got %#lx\n", io.Status); - ok(io.Information == 3, "got size %Iu\n", io.Information); + todo_wine_if (NT_ERROR(params->iosb_status) && params->pending) + ok(io.Status == params->iosb_status /* newer win10/11 */ + || broken(NT_ERROR(params->iosb_status) && params->pending && io.Status == 0xdeadf00d), /* older win10 */ + "got %#lx\n", io.Status); + todo_wine_if (NT_ERROR(params->iosb_status) && params->pending) + ok(io.Information == 3 /* newer win10/11 */ + || broken(NT_ERROR(params->iosb_status) && params->pending && io.Information == 0xdeadf00d), /* older win10 */ + "got size %Iu\n", io.Information); } ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer); @@ -725,7 +731,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) ret = NtDeviceIoControlFile(file, event, NULL, (void *)456, &io, ioctl, params, sizeof(*params), buffer, sizeof(buffer)); ok(ret == params->ret_status - || broken(NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* win10 */ + || (NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* newer win10/11 */ "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { @@ -772,7 +778,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) ret = NtDeviceIoControlFile(file, event, NULL, NULL, &io, ioctl, params, sizeof(*params), buffer, sizeof(buffer)); ok(ret == params->ret_status - || broken(NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* win10 */ + || (NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* newer win10/11 */ "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { @@ -800,7 +806,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) ret = NtDeviceIoControlFile(file, NULL, NULL, NULL, &io, ioctl, params, sizeof(*params), buffer, sizeof(buffer)); ok(ret == params->ret_status - || broken(NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* win10 */ + || (NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* newer win10/11 */ "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { @@ -832,7 +838,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) ret = NtDeviceIoControlFile(file, event, NULL, (void *)456, &io, ioctl, params, sizeof(*params), buffer, sizeof(buffer)); ok(ret == params->ret_status - || broken(NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* win10 */ + || (NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* newer win10/11 */ "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { @@ -1132,8 +1138,10 @@ static void test_blocking_irp(void) io.Information = 0xdeadf00d; status = NtQueryVolumeInformationFile(file, &io, buffer, sizeof(buffer), FileFsFullSizeInformation); ok(status == STATUS_DEVICE_NOT_READY, "got %#lx\n", status); - ok(io.Status == 0xdeadf00d, "got iosb status %#lx\n", io.Status); - ok(io.Information == 0xdeadf00d, "got information %#Ix\n", io.Information); + todo_wine ok(io.Status == STATUS_DEVICE_NOT_READY || broken(io.Status == 0xdeadf00d), /* older win10 */ + "got iosb status %#lx\n", io.Status); + todo_wine ok(io.Information == 0 || broken(io.Information == 0xdeadf00d), /* older win10 */ + "got information %#Ix\n", io.Information); CloseHandle(file); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7797
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/driver.c | 32 ++++++ dlls/ntoskrnl.exe/tests/driver.h | 2 + dlls/ntoskrnl.exe/tests/ntoskrnl.c | 178 ++++++++++++++++++++++++++++- 3 files changed, 211 insertions(+), 1 deletion(-) diff --git a/dlls/ntoskrnl.exe/tests/driver.c b/dlls/ntoskrnl.exe/tests/driver.c index dda8806ba74..326af858260 100644 --- a/dlls/ntoskrnl.exe/tests/driver.c +++ b/dlls/ntoskrnl.exe/tests/driver.c @@ -58,6 +58,9 @@ static int kmemcmp( const void *ptr1, const void *ptr2, size_t n ) static DRIVER_OBJECT *driver_obj; static DEVICE_OBJECT *lower_device, *upper_device; +static IRP *queued_async_irps[2]; +static unsigned int queued_async_count; + static POBJECT_TYPE *pExEventObjectType, *pIoFileObjectType, *pPsThreadType, *pIoDriverObjectType; static PEPROCESS *pPsInitialSystemProcess; static void *create_caller_thread; @@ -2682,10 +2685,17 @@ static NTSTATUS WINAPI driver_Create(DEVICE_OBJECT *device, IRP *irp) return STATUS_SUCCESS; } +static void cancel_complete(IRP *irp) +{ + irp->IoStatus.Status = STATUS_SUCCESS; + IoCompleteRequest(irp, IO_NO_INCREMENT); +} + static NTSTATUS WINAPI driver_IoControl(DEVICE_OBJECT *device, IRP *irp) { IO_STACK_LOCATION *stack = IoGetCurrentIrpStackLocation(irp); NTSTATUS status = STATUS_NOT_SUPPORTED; + unsigned int i; switch (stack->Parameters.DeviceIoControl.IoControlCode) { @@ -2699,6 +2709,7 @@ static NTSTATUS WINAPI driver_IoControl(DEVICE_OBJECT *device, IRP *irp) status = test_load_driver_ioctl(irp, stack, &irp->IoStatus.Information); break; case IOCTL_WINETEST_RESET_CANCEL: + ok(!queued_async_count, "expected no queued asyncs left\n"); cancel_cnt = 0; status = STATUS_SUCCESS; break; @@ -2706,6 +2717,27 @@ static NTSTATUS WINAPI driver_IoControl(DEVICE_OBJECT *device, IRP *irp) IoSetCancelRoutine(irp, cancel_ioctl_irp); IoMarkIrpPending(irp); return STATUS_PENDING; + case IOCTL_WINETEST_QUEUE_ASYNC: + if (queued_async_count >= ARRAY_SIZE(queued_async_irps)) + { + ok(FALSE, "unexpected queued_async_count %u\n", queued_async_count); + status = ERROR_TOO_MANY_CMDS; + break; + } + queued_async_irps[queued_async_count++] = irp; + IoMarkIrpPending(irp); + return STATUS_PENDING; + case IOCTL_WINETEST_COMPLETE_ASYNC: + /* complete the pending IRPs gathered from + * IOCTL_WINETEST_QUEUE_ASYNC */ + for (i = 0; i < queued_async_count; i++) + { + cancel_complete(queued_async_irps[i]); + queued_async_irps[i] = NULL; + } + queued_async_count = 0; + status = STATUS_SUCCESS; + break; case IOCTL_WINETEST_GET_CANCEL_COUNT: status = get_dword(irp, stack, &irp->IoStatus.Information, cancel_cnt); break; diff --git a/dlls/ntoskrnl.exe/tests/driver.h b/dlls/ntoskrnl.exe/tests/driver.h index e22d756e69e..524ba6abeed 100644 --- a/dlls/ntoskrnl.exe/tests/driver.h +++ b/dlls/ntoskrnl.exe/tests/driver.h @@ -36,6 +36,8 @@ #define IOCTL_WINETEST_RETURN_STATUS_DIRECT CTL_CODE(FILE_DEVICE_UNKNOWN, 0x80a, METHOD_OUT_DIRECT, FILE_ANY_ACCESS) #define IOCTL_WINETEST_RETURN_STATUS_NEITHER CTL_CODE(FILE_DEVICE_UNKNOWN, 0x80a, METHOD_NEITHER, FILE_ANY_ACCESS) #define IOCTL_WINETEST_COMPLETION CTL_CODE(FILE_DEVICE_UNKNOWN, 0x80c, METHOD_NEITHER, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_QUEUE_ASYNC CTL_CODE(FILE_DEVICE_UNKNOWN, 0x80d, METHOD_BUFFERED, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_COMPLETE_ASYNC CTL_CODE(FILE_DEVICE_UNKNOWN, 0x80e, METHOD_BUFFERED, FILE_ANY_ACCESS) #define IOCTL_WINETEST_BUS_MAIN CTL_CODE(FILE_DEVICE_BUS_EXTENDER, 0x800, METHOD_BUFFERED, FILE_ANY_ACCESS) #define IOCTL_WINETEST_BUS_REGISTER_IFACE CTL_CODE(FILE_DEVICE_BUS_EXTENDER, 0x801, METHOD_BUFFERED, FILE_ANY_ACCESS) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index c1902ba5de9..1c59bb411ff 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -438,12 +438,95 @@ static void test_basic_ioctl(void) ok(!strcmp(buf, "Wine is no"), "got '%s'\n", buf); } +static BOOL cancelioex_returned; + +enum cancel_test +{ + cancel, + cancelex, + cancelex_cancel, + cancel_cancelex, + cancelsync, +}; + +struct cancel_thread_ctx +{ + HANDLE file; + enum cancel_test test; +}; + +static DWORD WINAPI test_cancel_thread(void *param) +{ + struct cancel_thread_ctx *ctx = param; + DWORD cancel_cnt, size; + OVERLAPPED o, o2, o3; + BOOL res; + + res = DeviceIoControl(ctx->file, IOCTL_WINETEST_RESET_CANCEL, NULL, 0, NULL, 0, NULL, &o); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + + if (ctx->test != cancelsync) + { + res = DeviceIoControl(ctx->file, IOCTL_WINETEST_QUEUE_ASYNC, NULL, 0, NULL, 0, NULL, &o); + ok(!res && GetLastError() == ERROR_IO_PENDING, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(ctx->file, IOCTL_WINETEST_QUEUE_ASYNC, NULL, 0, NULL, 0, NULL, &o2); + ok(!res && GetLastError() == ERROR_IO_PENDING, "DeviceIoControl failed: %lu\n", GetLastError()); + } + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(ctx->file, IOCTL_WINETEST_GET_CANCEL_COUNT, NULL, 0, &cancel_cnt, sizeof(cancel_cnt), NULL, &o3); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + ok(cancel_cnt == 0, "cancel_cnt = %lu\n", cancel_cnt); + + if (ctx->test == cancelsync) + { + /* This hangs */ + if (0) + { + res = DeviceIoControl(ctx->file, IOCTL_WINETEST_QUEUE_ASYNC, NULL, 0, NULL, 0, &size, NULL); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + } + /* This hangs if the cancel routine doesn't call IoCompleteRequest() + * (regardless of Status / Information values) */ + res = DeviceIoControl(ctx->file, IOCTL_WINETEST_TEST_CANCEL, NULL, 0, NULL, 0, &size, NULL); + ok(!res && GetLastError() == ERROR_OPERATION_ABORTED, "DeviceIoControl failed: %lu\n", GetLastError()); + } + + if (ctx->test == cancel || ctx->test == cancel_cancelex) + { + res = CancelIo(ctx->file); + ok(res, "CancelIo failed: %lu\n", GetLastError()); + } + else if (ctx->test == cancelex || ctx->test == cancelex_cancel) + { + res = CancelIoEx(ctx->file, NULL); + ok(res, "CancelIoEx failed: %lu\n", GetLastError()); + res = CancelIoEx(ctx->file, NULL); + ok(res, "CancelIoEx failed: %lu\n", GetLastError()); + cancelioex_returned = TRUE; + } + if (ctx->test == cancelex_cancel) + { + res = CancelIo(ctx->file); + ok(res, "CancelIo failed: %lu\n", GetLastError()); + } + else if (ctx->test == cancel_cancelex) + { + res = CancelIoEx(ctx->file, NULL); + ok(!res && GetLastError() == ERROR_NOT_FOUND, "CancelIoEx failed: %lu\n", GetLastError()); + } + return res; +} + static void test_overlapped(void) { OVERLAPPED overlapped, overlapped2, *o; + struct cancel_thread_ctx ctx; + HANDLE file, port, thread; DWORD cancel_cnt, size; - HANDLE file, port; ULONG_PTR key; + DWORD ret; BOOL res; overlapped.hEvent = CreateEventW(NULL, TRUE, FALSE, NULL); @@ -502,6 +585,98 @@ static void test_overlapped(void) ok(cancel_cnt == 2, "cancel_cnt = %lu\n", cancel_cnt); } + /* test cancelling uncancellable requests - CancelIo */ + ctx.file = file; + ctx.test = cancel; + thread = CreateThread(NULL, 0, test_cancel_thread, &ctx, 0, NULL); + ret = WaitForSingleObject(thread, 100); + ok(ret == WAIT_TIMEOUT, "CancelIo didn't block\n"); + res = DeviceIoControl(file, IOCTL_WINETEST_COMPLETE_ASYNC, NULL, 0, NULL, 0, NULL, &overlapped); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(file, IOCTL_WINETEST_GET_CANCEL_COUNT, NULL, 0, &cancel_cnt, sizeof(cancel_cnt), NULL, &overlapped2); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + ok(cancel_cnt == 0, "cancel_cnt = %lu\n", cancel_cnt); + + /* test cancelling uncancellable requests - CancelIoEx */ + ctx.file = file; + ctx.test = cancelex; + cancelioex_returned = FALSE; + thread = CreateThread(NULL, 0, test_cancel_thread, &ctx, 0, NULL); + /* CancelIoEx() doesn't block, but the thread doesn't terminate either + * until the pending IOCTL is completed */ + ret = WaitForSingleObject(thread, 100); + todo_wine ok(ret == WAIT_TIMEOUT, "Thread already terminated\n"); + ok(cancelioex_returned, "CancelIoEx did block\n"); + res = DeviceIoControl(file, IOCTL_WINETEST_COMPLETE_ASYNC, NULL, 0, NULL, 0, NULL, &overlapped); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(file, IOCTL_WINETEST_GET_CANCEL_COUNT, NULL, 0, &cancel_cnt, sizeof(cancel_cnt), NULL, &overlapped2); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + ok(cancel_cnt == 0, "cancel_cnt = %lu\n", cancel_cnt); + + /* test cancelling uncancellable requests - CancelIoEx + CancelIo */ + ctx.file = file; + ctx.test = cancelex_cancel; + cancelioex_returned = FALSE; + thread = CreateThread(NULL, 0, test_cancel_thread, &ctx, 0, NULL); + ret = WaitForSingleObject(thread, 100); + ok(ret == WAIT_TIMEOUT, "Thread already terminated\n"); + ok(cancelioex_returned, "CancelIoEx did block\n"); + res = DeviceIoControl(file, IOCTL_WINETEST_COMPLETE_ASYNC, NULL, 0, NULL, 0, NULL, &overlapped); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(file, IOCTL_WINETEST_GET_CANCEL_COUNT, NULL, 0, &cancel_cnt, sizeof(cancel_cnt), NULL, &overlapped2); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + ok(cancel_cnt == 0, "cancel_cnt = %lu\n", cancel_cnt); + + /* test cancelling uncancellable requests - CancelIo + CancelIoEx */ + ctx.file = file; + ctx.test = cancel_cancelex; + thread = CreateThread(NULL, 0, test_cancel_thread, &ctx, 0, NULL); + ret = WaitForSingleObject(thread, 100); + ok(ret == WAIT_TIMEOUT, "CancelIo didn't block\n"); + res = CancelIoEx(file, NULL); + ok(res, "CancelIoEx failed: %lu\n", GetLastError()); + ret = WaitForSingleObject(thread, 100); + ok(ret == WAIT_TIMEOUT, "CancelIo didn't block\n"); + res = DeviceIoControl(file, IOCTL_WINETEST_COMPLETE_ASYNC, NULL, 0, NULL, 0, NULL, &overlapped); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(file, IOCTL_WINETEST_GET_CANCEL_COUNT, NULL, 0, &cancel_cnt, sizeof(cancel_cnt), NULL, &overlapped2); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + ok(cancel_cnt == 0, "cancel_cnt = %lu\n", cancel_cnt); + + /* test cancelling uncancellable requests - CancelSynchronousIo */ + ctx.file = device; + ctx.test = cancelsync; + thread = CreateThread(NULL, 0, test_cancel_thread, &ctx, 0, NULL); + WaitForSingleObject(thread, 100); + ok(ret == WAIT_TIMEOUT, "Synchronous DeviceIoControl() already returned\n"); + res = CancelSynchronousIo(thread); + ok(res, "CancelSynchronousIo failed: %lu\n", GetLastError()); + ret = WaitForSingleObject(thread, INFINITE); + ok(ret == WAIT_OBJECT_0, "CancelIoEx did block\n"); + CloseHandle(thread); + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(device, IOCTL_WINETEST_GET_CANCEL_COUNT, NULL, 0, &cancel_cnt, sizeof(cancel_cnt), &size, NULL); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + ok(cancel_cnt == 1, "cancel_cnt = %lu\n", cancel_cnt); + + /* test completion port */ port = CreateIoCompletionPort(file, NULL, 0xdeadbeef, 0); ok(port != NULL, "CreateIoCompletionPort failed, error %lu\n", GetLastError()); res = GetQueuedCompletionStatus(port, &size, &key, &o, 0); @@ -1265,6 +1440,7 @@ static void test_driver_netio(struct testsign_context *ctx) hthread = CreateThread(NULL, 0, wsk_test_thread, NULL, 0, NULL); main_test(); WaitForSingleObject(hthread, INFINITE); + CloseHandle(hthread); CloseHandle(device); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7797
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/driver.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dlls/ntoskrnl.exe/tests/driver.c b/dlls/ntoskrnl.exe/tests/driver.c index 326af858260..f30c340e9bd 100644 --- a/dlls/ntoskrnl.exe/tests/driver.c +++ b/dlls/ntoskrnl.exe/tests/driver.c @@ -1081,12 +1081,14 @@ static void test_call_driver(DEVICE_OBJECT *device) } static int cancel_cnt; +static HANDLE thread_id; static void WINAPI cancel_irp(DEVICE_OBJECT *device, IRP *irp) { IoReleaseCancelSpinLock(irp->CancelIrql); ok(irp->Cancel == TRUE, "Cancel = %x\n", irp->Cancel); ok(!irp->CancelRoutine, "CancelRoutine = %p\n", irp->CancelRoutine); + ok(PsGetCurrentThreadId() == thread_id, "Unexpected thread_id %p\n", PsGetCurrentThreadId()); irp->IoStatus.Status = STATUS_CANCELLED; irp->IoStatus.Information = 0; cancel_cnt++; @@ -1095,6 +1097,7 @@ static void WINAPI cancel_irp(DEVICE_OBJECT *device, IRP *irp) static void WINAPI cancel_ioctl_irp(DEVICE_OBJECT *device, IRP *irp) { IoReleaseCancelSpinLock(irp->CancelIrql); + todo_wine ok(PsGetCurrentThreadId() == thread_id, "Unexpected thread_id %p\n", PsGetCurrentThreadId()); irp->IoStatus.Status = STATUS_CANCELLED; irp->IoStatus.Information = 0; cancel_cnt++; @@ -1117,6 +1120,8 @@ static void test_cancel_irp(DEVICE_OBJECT *device) BOOLEAN r; NTSTATUS status; + thread_id = PsGetCurrentThreadId(); + /* cancel IRP with no cancel routine */ irp = IoBuildAsynchronousFsdRequest(IRP_MJ_FLUSH_BUFFERS, device, NULL, 0, NULL, &iosb); @@ -2714,6 +2719,7 @@ static NTSTATUS WINAPI driver_IoControl(DEVICE_OBJECT *device, IRP *irp) status = STATUS_SUCCESS; break; case IOCTL_WINETEST_TEST_CANCEL: + thread_id = PsGetCurrentThreadId(); IoSetCancelRoutine(irp, cancel_ioctl_irp); IoMarkIrpPending(irp); return STATUS_PENDING; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7797
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 48 ++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 1c59bb411ff..0f0caf76132 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -458,6 +458,8 @@ struct cancel_thread_ctx static DWORD WINAPI test_cancel_thread(void *param) { struct cancel_thread_ctx *ctx = param; + NTSTATUS status = STATUS_SUCCESS; + IO_STATUS_BLOCK cancel_sb; DWORD cancel_cnt, size; OVERLAPPED o, o2, o3; BOOL res; @@ -495,28 +497,38 @@ static DWORD WINAPI test_cancel_thread(void *param) if (ctx->test == cancel || ctx->test == cancel_cancelex) { - res = CancelIo(ctx->file); - ok(res, "CancelIo failed: %lu\n", GetLastError()); + status = NtCancelIoFile(ctx->file, &cancel_sb); + ok(!status, "Unexpected status %lu\n", status); + ok(!cancel_sb.Status, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); } else if (ctx->test == cancelex || ctx->test == cancelex_cancel) { - res = CancelIoEx(ctx->file, NULL); - ok(res, "CancelIoEx failed: %lu\n", GetLastError()); - res = CancelIoEx(ctx->file, NULL); - ok(res, "CancelIoEx failed: %lu\n", GetLastError()); + status = NtCancelIoFileEx(ctx->file, NULL, &cancel_sb); + ok(!status, "Unexpected status %lu\n", status); + ok(!cancel_sb.Status, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); + status = NtCancelIoFileEx(ctx->file, NULL, &cancel_sb); + ok(!status, "Unexpected status %lu\n", status); + ok(!cancel_sb.Status, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); cancelioex_returned = TRUE; } if (ctx->test == cancelex_cancel) { - res = CancelIo(ctx->file); - ok(res, "CancelIo failed: %lu\n", GetLastError()); + status = NtCancelIoFile(ctx->file, &cancel_sb); + ok(!status, "Unexpected status %lu\n", status); + ok(!cancel_sb.Status, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); } else if (ctx->test == cancel_cancelex) { - res = CancelIoEx(ctx->file, NULL); - ok(!res && GetLastError() == ERROR_NOT_FOUND, "CancelIoEx failed: %lu\n", GetLastError()); + status = NtCancelIoFileEx(ctx->file, NULL, &cancel_sb); + ok(status, "Unexpected status %lu\n", status); + ok(cancel_sb.Status == STATUS_NOT_FOUND, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); } - return res; + return status; } static void test_overlapped(void) @@ -524,7 +536,9 @@ static void test_overlapped(void) OVERLAPPED overlapped, overlapped2, *o; struct cancel_thread_ctx ctx; HANDLE file, port, thread; + IO_STATUS_BLOCK cancel_sb; DWORD cancel_cnt, size; + NTSTATUS status; ULONG_PTR key; DWORD ret; BOOL res; @@ -645,8 +659,10 @@ static void test_overlapped(void) thread = CreateThread(NULL, 0, test_cancel_thread, &ctx, 0, NULL); ret = WaitForSingleObject(thread, 100); ok(ret == WAIT_TIMEOUT, "CancelIo didn't block\n"); - res = CancelIoEx(file, NULL); - ok(res, "CancelIoEx failed: %lu\n", GetLastError()); + status = NtCancelIoFileEx(file, NULL, &cancel_sb); + ok(!status, "Unexpected status %lu\n", status); + ok(!cancel_sb.Status, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); ret = WaitForSingleObject(thread, 100); ok(ret == WAIT_TIMEOUT, "CancelIo didn't block\n"); res = DeviceIoControl(file, IOCTL_WINETEST_COMPLETE_ASYNC, NULL, 0, NULL, 0, NULL, &overlapped); @@ -665,8 +681,10 @@ static void test_overlapped(void) thread = CreateThread(NULL, 0, test_cancel_thread, &ctx, 0, NULL); WaitForSingleObject(thread, 100); ok(ret == WAIT_TIMEOUT, "Synchronous DeviceIoControl() already returned\n"); - res = CancelSynchronousIo(thread); - ok(res, "CancelSynchronousIo failed: %lu\n", GetLastError()); + status = NtCancelSynchronousIoFile(thread, NULL, &cancel_sb); + ok(!status, "Unexpected status %lu\n", status); + ok(!cancel_sb.Status, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); ret = WaitForSingleObject(thread, INFINITE); ok(ret == WAIT_OBJECT_0, "CancelIoEx did block\n"); CloseHandle(thread); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7797
New rebase, now dropping again that final tiny patch since it was indirectly fixed by 8fb53129f958791afaa9a8dbbb4b8c9fe0859479. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7797#note_121157
Anything I can do to help move this forward? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7797#note_122552
Rémi Bernon (@rbernon) commented about server/async.c:
+static void async_cancel_dump( struct object *obj, int verbose ) +{ + struct async_cancel *cancel = (struct async_cancel *)obj; + assert( obj->ops == &async_cancel_ops ); + fprintf( stderr, "async_cancel %p\n", cancel ); +} + +static void async_cancel_satisfied( struct object *obj, struct wait_queue_entry *entry ) +{ + struct async_cancel *cancel = (struct async_cancel *)obj; + assert( obj->ops == &async_cancel_ops ); + + /* close wait handle here to avoid extra server round trip */ + close_handle( cancel->process, cancel->wait_handle ); + cancel->wait_handle = 0; +} As we now use a sync object, and with ntsync anyway, this will never be called. You need to close the handle properly on the client side. The `async_cancel_satisfied` and `wait_handle` members also need to be removed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7797#note_122747
participants (3)
-
Matteo Bruni -
Matteo Bruni (＠Mystral)
-
Rémi Bernon