[PATCH v12 0/10] MR7797: Draft: 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. -- v12: 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. 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 958ae9a6937..6803e4ec3ec 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 567194bf00a..7f2d0185bd4 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 8e0f9189a61..28fcfc02f5e 100644 --- a/server/process.c +++ b/server/process.c @@ -987,7 +987,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 ++- include/wine/server_protocol.h | 4 +- server/async.c | 155 +++++++++++++++++++++++++++++++-- server/protocol.def | 2 + server/request_handlers.h | 2 + server/request_trace.h | 7 +- 6 files changed, 169 insertions(+), 15 deletions(-) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 6803e4ec3ec..4cdbbd30ff4 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) + NtWaitForMultipleObjects( 1, &cancel_handle, FALSE, TRUE, NULL ); + else if (status == STATUS_INVALID_HANDLE) + return status; + + io_status->Status = status; + io_status->Information = 0; + return status; } diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index 40f786f1dc8..1ca131c2ced 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -3295,6 +3295,8 @@ struct cancel_async_request struct cancel_async_reply { struct reply_header __header; + obj_handle_t cancel_handle; + char __pad_12[4]; }; @@ -6861,6 +6863,6 @@ union generic_reply struct set_keyboard_repeat_reply set_keyboard_repeat_reply; }; -#define SERVER_PROTOCOL_VERSION 887 +#define SERVER_PROTOCOL_VERSION 888 #endif /* __WINE_WINE_SERVER_PROTOCOL_H */ diff --git a/server/async.c b/server/async.c index 764596642db..f8635520f41 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 event_sync *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_event_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 cf92cdbabaa..af578f96898 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -2473,6 +2473,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 diff --git a/server/request_handlers.h b/server/request_handlers.h index 5acb9ff74ae..631e2f6a24b 100644 --- a/server/request_handlers.h +++ b/server/request_handlers.h @@ -1404,6 +1404,8 @@ C_ASSERT( offsetof(struct cancel_async_request, handle) == 12 ); C_ASSERT( offsetof(struct cancel_async_request, iosb) == 16 ); C_ASSERT( offsetof(struct cancel_async_request, only_thread) == 24 ); C_ASSERT( sizeof(struct cancel_async_request) == 32 ); +C_ASSERT( offsetof(struct cancel_async_reply, cancel_handle) == 8 ); +C_ASSERT( sizeof(struct cancel_async_reply) == 16 ); C_ASSERT( offsetof(struct get_async_result_request, user_arg) == 16 ); C_ASSERT( sizeof(struct get_async_result_request) == 24 ); C_ASSERT( sizeof(struct get_async_result_reply) == 8 ); diff --git a/server/request_trace.h b/server/request_trace.h index 4a859242bd3..9fb7ffe297d 100644 --- a/server/request_trace.h +++ b/server/request_trace.h @@ -1578,6 +1578,11 @@ static void dump_cancel_async_request( const struct cancel_async_request *req ) fprintf( stderr, ", only_thread=%d", req->only_thread ); } +static void dump_cancel_async_reply( const struct cancel_async_reply *req ) +{ + fprintf( stderr, " cancel_handle=%04x", req->cancel_handle ); +} + static void dump_get_async_result_request( const struct get_async_result_request *req ) { dump_uint64( " user_arg=", &req->user_arg ); @@ -3820,7 +3825,7 @@ static const dump_func reply_dumpers[REQ_NB_REQUESTS] = NULL, NULL, NULL, - NULL, + (dump_func)dump_cancel_async_reply, (dump_func)dump_get_async_result_reply, (dump_func)dump_set_async_direct_result_reply, (dump_func)dump_read_reply, -- 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 | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 87f41895511..bd2d490cba6 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -695,15 +695,17 @@ 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 || broken(NT_ERROR(params->iosb_status) && params->pending), "got %#lx\n", io.Status); + todo_wine_if (NT_ERROR(params->iosb_status) && params->pending) + ok(io.Information == 3 || broken(NT_ERROR(params->iosb_status) && params->pending), "got size %Iu\n", io.Information); } ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer); @@ -725,7 +727,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), "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { @@ -772,7 +774,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), "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { @@ -800,7 +802,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), "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { @@ -832,7 +834,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), "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { @@ -1132,8 +1134,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 == 0xc00000a3 || 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 | 39 +++++ dlls/ntoskrnl.exe/tests/driver.h | 21 +-- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 238 ++++++++++++++++++++++++++++- 3 files changed, 287 insertions(+), 11 deletions(-) diff --git a/dlls/ntoskrnl.exe/tests/driver.c b/dlls/ntoskrnl.exe/tests/driver.c index dda8806ba74..220e4fbd190 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 KDEVICE_QUEUE device_queue; +static IRP *current_irp; + static POBJECT_TYPE *pExEventObjectType, *pIoFileObjectType, *pPsThreadType, *pIoDriverObjectType; static PEPROCESS *pPsInitialSystemProcess; static void *create_caller_thread; @@ -2682,10 +2685,18 @@ 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; + KDEVICE_QUEUE_ENTRY *entry; + BOOLEAN ret; switch (stack->Parameters.DeviceIoControl.IoControlCode) { @@ -2706,6 +2717,34 @@ static NTSTATUS WINAPI driver_IoControl(DEVICE_OBJECT *device, IRP *irp) IoSetCancelRoutine(irp, cancel_ioctl_irp); IoMarkIrpPending(irp); return STATUS_PENDING; + case IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE_INIT: + KeInitializeDeviceQueue(&device_queue); + ok(IsListEmpty(&device_queue.DeviceListHead), "expected empty queue list\n"); + status = STATUS_SUCCESS; + break; + case IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE: + /* put the IRP into the queue */ + ret = KeInsertDeviceQueue(&device_queue, &irp->Tail.Overlay.DeviceQueueEntry); + /* or store it elsewhere if it's the one we're supposing to be + * working on "currently" */ + if (!ret) + current_irp = irp; + ok(device_queue.Busy, "expected Busy state\n"); + IoMarkIrpPending(irp); + return STATUS_PENDING; + case IOCTL_WINETEST_TEST_CANCEL_COMPLETE: + /* complete the pending IRPs gathered from + * IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE */ + cancel_complete(current_irp); + current_irp = NULL; + while ((entry = KeRemoveDeviceQueue(&device_queue))) + { + current_irp = CONTAINING_RECORD( entry, IRP, Tail.Overlay.DeviceQueueEntry.DeviceListEntry ); + cancel_complete(current_irp); + current_irp = NULL; + } + 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..4ee97c8d7ce 100644 --- a/dlls/ntoskrnl.exe/tests/driver.h +++ b/dlls/ntoskrnl.exe/tests/driver.h @@ -27,15 +27,18 @@ #define IOCTL_WINETEST_LOAD_DRIVER CTL_CODE(FILE_DEVICE_UNKNOWN, 0x802, METHOD_BUFFERED, FILE_ANY_ACCESS) #define IOCTL_WINETEST_RESET_CANCEL CTL_CODE(FILE_DEVICE_UNKNOWN, 0x803, METHOD_BUFFERED, FILE_ANY_ACCESS) #define IOCTL_WINETEST_TEST_CANCEL CTL_CODE(FILE_DEVICE_UNKNOWN, 0x804, METHOD_BUFFERED, FILE_ANY_ACCESS) -#define IOCTL_WINETEST_GET_CANCEL_COUNT CTL_CODE(FILE_DEVICE_UNKNOWN, 0x805, METHOD_BUFFERED, FILE_ANY_ACCESS) -#define IOCTL_WINETEST_DETACH CTL_CODE(FILE_DEVICE_UNKNOWN, 0x806, METHOD_BUFFERED, FILE_ANY_ACCESS) -#define IOCTL_WINETEST_GET_CREATE_COUNT CTL_CODE(FILE_DEVICE_UNKNOWN, 0x807, METHOD_BUFFERED, FILE_ANY_ACCESS) -#define IOCTL_WINETEST_GET_CLOSE_COUNT CTL_CODE(FILE_DEVICE_UNKNOWN, 0x808, METHOD_BUFFERED, FILE_ANY_ACCESS) -#define IOCTL_WINETEST_GET_FSCONTEXT CTL_CODE(FILE_DEVICE_UNKNOWN, 0x809, METHOD_BUFFERED, FILE_ANY_ACCESS) -#define IOCTL_WINETEST_RETURN_STATUS_BUFFERED CTL_CODE(FILE_DEVICE_UNKNOWN, 0x80a, METHOD_BUFFERED, FILE_ANY_ACCESS) -#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_TEST_CANCEL_UNCANCELLABLE_INIT CTL_CODE(FILE_DEVICE_UNKNOWN, 0x805, METHOD_BUFFERED, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE CTL_CODE(FILE_DEVICE_UNKNOWN, 0x806, METHOD_BUFFERED, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_TEST_CANCEL_COMPLETE CTL_CODE(FILE_DEVICE_UNKNOWN, 0x807, METHOD_BUFFERED, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_GET_CANCEL_COUNT CTL_CODE(FILE_DEVICE_UNKNOWN, 0x808, METHOD_BUFFERED, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_DETACH CTL_CODE(FILE_DEVICE_UNKNOWN, 0x809, METHOD_BUFFERED, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_GET_CREATE_COUNT CTL_CODE(FILE_DEVICE_UNKNOWN, 0x80a, METHOD_BUFFERED, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_GET_CLOSE_COUNT CTL_CODE(FILE_DEVICE_UNKNOWN, 0x80b, METHOD_BUFFERED, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_GET_FSCONTEXT CTL_CODE(FILE_DEVICE_UNKNOWN, 0x80c, METHOD_BUFFERED, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_RETURN_STATUS_BUFFERED CTL_CODE(FILE_DEVICE_UNKNOWN, 0x80d, METHOD_BUFFERED, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_RETURN_STATUS_DIRECT CTL_CODE(FILE_DEVICE_UNKNOWN, 0x80d, METHOD_OUT_DIRECT, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_RETURN_STATUS_NEITHER CTL_CODE(FILE_DEVICE_UNKNOWN, 0x80d, METHOD_NEITHER, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_COMPLETION CTL_CODE(FILE_DEVICE_UNKNOWN, 0x80e, METHOD_NEITHER, 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 bd2d490cba6..1c80bcd3898 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -438,16 +438,65 @@ static void test_basic_ioctl(void) ok(!strcmp(buf, "Wine is no"), "got '%s'\n", buf); } +static BOOL returned; + +struct cancel_thread_ctx +{ + HANDLE file; + BOOL ex; +}; + +static DWORD WINAPI test_cancel_complete_thread(void *parameter) +{ + struct cancel_thread_ctx *ctx = parameter; + OVERLAPPED overlapped; + BOOL res; + + Sleep(200); + ok(!returned == !ctx->ex, "%s() %sreturned\n", + ctx->ex ? "CancelIoEx" : "CancelIo", returned ? "" : "not "); + res = DeviceIoControl(ctx->file, IOCTL_WINETEST_TEST_CANCEL_COMPLETE, NULL, 0, NULL, 0, NULL, &overlapped); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + return res; +} + +static DWORD WINAPI test_cancel_ex_thread(void *parameter) +{ + struct cancel_thread_ctx *ctx = parameter; + BOOL res; + + Sleep(100); + res = CancelIoEx(ctx->file, NULL); + ok(res, "CancelIoEx failed: %lu\n", GetLastError()); + return res; +} + +static DWORD WINAPI test_cancel_sync_thread(void *parameter) +{ + HANDLE tid = parameter; + BOOL res; + + Sleep(100); + ok(!returned, "Synchronous DeviceIoControl() already returned\n"); + res = CancelSynchronousIo(tid); + ok(res, "CancelSynchronousIo failed: %lu\n", GetLastError()); + Sleep(0); + todo_wine ok(returned, "Synchronous DeviceIoControl() hasn't returned yet\n"); + return res; +} + static void test_overlapped(void) { - OVERLAPPED overlapped, overlapped2, *o; + HANDLE file, port, thread, thread2, thread_current; + OVERLAPPED overlapped, overlapped2, o3, *o; + struct cancel_thread_ctx ctx; DWORD cancel_cnt, size; - HANDLE file, port; ULONG_PTR key; BOOL res; overlapped.hEvent = CreateEventW(NULL, TRUE, FALSE, NULL); overlapped2.hEvent = CreateEventW(NULL, TRUE, FALSE, NULL); + o3.hEvent = CreateEventW(NULL, TRUE, FALSE, NULL); file = CreateFileA("\\\\.\\WineTestDriver", FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES, 0, NULL, OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL); @@ -502,6 +551,189 @@ static void test_overlapped(void) ok(cancel_cnt == 2, "cancel_cnt = %lu\n", cancel_cnt); } + /* test cancelling uncancellable requests - CancelIo */ + res = DeviceIoControl(file, IOCTL_WINETEST_RESET_CANCEL, NULL, 0, NULL, 0, NULL, &overlapped); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(file, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE_INIT, NULL, 0, NULL, 0, NULL, &overlapped); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(file, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE, NULL, 0, NULL, 0, NULL, &overlapped); + ok(!res && GetLastError() == ERROR_IO_PENDING, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(file, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE, NULL, 0, NULL, 0, NULL, &overlapped2); + ok(!res && GetLastError() == ERROR_IO_PENDING, "DeviceIoControl failed: %lu\n", GetLastError()); + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(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); + + returned = FALSE; + ctx.file = file; + ctx.ex = FALSE; + thread = CreateThread(NULL, 0, test_cancel_complete_thread, &ctx, 0, NULL); + res = CancelIo(file); + ok(res, "CancelIo failed: %lu\n", GetLastError()); + returned = TRUE; + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + + res = CancelIo(file); + ok(res, "CancelIo failed: %lu\n", GetLastError()); + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(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); + + /* test cancelling uncancellable requests - CancelIoEx */ + res = DeviceIoControl(file, IOCTL_WINETEST_RESET_CANCEL, NULL, 0, NULL, 0, NULL, &overlapped); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(file, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE_INIT, NULL, 0, NULL, 0, NULL, &overlapped); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(file, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE, NULL, 0, NULL, 0, NULL, &overlapped); + ok(!res && GetLastError() == ERROR_IO_PENDING, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(file, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE, NULL, 0, NULL, 0, NULL, &overlapped2); + ok(!res && GetLastError() == ERROR_IO_PENDING, "DeviceIoControl failed: %lu\n", GetLastError()); + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(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); + + returned = FALSE; + ctx.file = file; + ctx.ex = TRUE; + thread = CreateThread(NULL, 0, test_cancel_complete_thread, &ctx, 0, NULL); + res = CancelIoEx(file, NULL); + ok(res, "CancelIoEx failed: %lu\n", GetLastError()); + res = CancelIoEx(file, NULL); + ok(res, "CancelIoEx failed: %lu\n", GetLastError()); + returned = TRUE; + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + + res = CancelIoEx(file, NULL); + ok(!res && GetLastError() == ERROR_NOT_FOUND, "CancelIoEx failed: %lu\n", GetLastError()); + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(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); + + /* test cancelling uncancellable requests - CancelIoEx + CancelIo */ + res = DeviceIoControl(file, IOCTL_WINETEST_RESET_CANCEL, NULL, 0, NULL, 0, NULL, &overlapped); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(file, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE_INIT, NULL, 0, NULL, 0, NULL, &overlapped); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(file, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE, NULL, 0, NULL, 0, NULL, &overlapped); + ok(!res && GetLastError() == ERROR_IO_PENDING, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(file, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE, NULL, 0, NULL, 0, NULL, &overlapped2); + ok(!res && GetLastError() == ERROR_IO_PENDING, "DeviceIoControl failed: %lu\n", GetLastError()); + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(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); + + returned = FALSE; + ctx.file = file; + ctx.ex = FALSE; + thread = CreateThread(NULL, 0, test_cancel_complete_thread, &ctx, 0, NULL); + res = CancelIoEx(file, NULL); + ok(res, "CancelIoEx failed: %lu\n", GetLastError()); + res = CancelIo(file); + ok(res, "CancelIo failed: %lu\n", GetLastError()); + returned = TRUE; + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(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); + + /* test cancelling uncancellable requests - CancelIo + CancelIoEx */ + res = DeviceIoControl(file, IOCTL_WINETEST_RESET_CANCEL, NULL, 0, NULL, 0, NULL, &overlapped); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(file, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE_INIT, NULL, 0, NULL, 0, NULL, &overlapped); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(file, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE, NULL, 0, NULL, 0, NULL, &overlapped); + ok(!res && GetLastError() == ERROR_IO_PENDING, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(file, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE, NULL, 0, NULL, 0, NULL, &overlapped2); + ok(!res && GetLastError() == ERROR_IO_PENDING, "DeviceIoControl failed: %lu\n", GetLastError()); + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(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); + + returned = FALSE; + ctx.file = file; + ctx.ex = FALSE; + thread = CreateThread(NULL, 0, test_cancel_complete_thread, &ctx, 0, NULL); + thread2 = CreateThread(NULL, 0, test_cancel_ex_thread, &ctx, 0, NULL); + res = CancelIo(file); + ok(res, "CancelIo failed: %lu\n", GetLastError()); + returned = TRUE; + WaitForSingleObject(thread, INFINITE); + WaitForSingleObject(thread2, INFINITE); + CloseHandle(thread); + + cancel_cnt = 0xdeadbeef; + res = DeviceIoControl(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); + + /* test cancelling uncancellable requests - CancelSynchronousIo */ + res = DeviceIoControl(device, IOCTL_WINETEST_RESET_CANCEL, NULL, 0, NULL, 0, &size, NULL); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + + res = DeviceIoControl(device, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE_INIT, NULL, 0, NULL, 0, &size, NULL); + ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); + + 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 == 0, "cancel_cnt = %lu\n", cancel_cnt); + + returned = FALSE; + res = DuplicateHandle(GetCurrentProcess(), GetCurrentThread(), GetCurrentProcess(), &thread_current, 0, FALSE, DUPLICATE_SAME_ACCESS); + ok(res, "DuplicateHandle failed: %lu\n", GetLastError()); + thread2 = CreateThread(NULL, 0, test_cancel_sync_thread, thread_current, 0, NULL); + /* This hangs */ + if (0) + { + ctx.file = device; + thread = CreateThread(NULL, 0, test_cancel_complete_thread, &ctx, 0, NULL); + res = DeviceIoControl(device, IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE, 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(device, IOCTL_WINETEST_TEST_CANCEL, NULL, 0, NULL, 0, &size, NULL); + ok(!res && GetLastError() == ERROR_OPERATION_ABORTED, "DeviceIoControl failed: %lu\n", GetLastError()); + returned = TRUE; + WaitForSingleObject(thread2, INFINITE); + CloseHandle(thread2); + CloseHandle(thread_current); + + 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); @@ -527,6 +759,7 @@ static void test_overlapped(void) CloseHandle(port); CloseHandle(overlapped.hEvent); CloseHandle(overlapped2.hEvent); + CloseHandle(o3.hEvent); CloseHandle(file); } @@ -1261,6 +1494,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 220e4fbd190..01d9c871ce9 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 | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 1c80bcd3898..7078e122e4f 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -490,6 +490,7 @@ static void test_overlapped(void) HANDLE file, port, thread, thread2, thread_current; OVERLAPPED overlapped, overlapped2, o3, *o; struct cancel_thread_ctx ctx; + IO_STATUS_BLOCK cancel_sb; DWORD cancel_cnt, size; ULONG_PTR key; BOOL res; @@ -573,8 +574,10 @@ static void test_overlapped(void) ctx.file = file; ctx.ex = FALSE; thread = CreateThread(NULL, 0, test_cancel_complete_thread, &ctx, 0, NULL); - res = CancelIo(file); - ok(res, "CancelIo failed: %lu\n", GetLastError()); + res = NtCancelIoFile(file, &cancel_sb); + ok(!res, "Unexpected result %u\n", res); + ok(!cancel_sb.Status, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); returned = TRUE; WaitForSingleObject(thread, INFINITE); CloseHandle(thread); @@ -609,16 +612,22 @@ static void test_overlapped(void) ctx.file = file; ctx.ex = TRUE; thread = CreateThread(NULL, 0, test_cancel_complete_thread, &ctx, 0, NULL); - res = CancelIoEx(file, NULL); - ok(res, "CancelIoEx failed: %lu\n", GetLastError()); - res = CancelIoEx(file, NULL); - ok(res, "CancelIoEx failed: %lu\n", GetLastError()); + res = NtCancelIoFileEx(file, NULL, &cancel_sb); + ok(!res, "Unexpected result %u\n", res); + ok(!cancel_sb.Status, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); + res = NtCancelIoFileEx(file, NULL, &cancel_sb); + ok(!res, "Unexpected result %u\n", res); + ok(!cancel_sb.Status, "Unexpected Status %lx\n", cancel_sb.Status); + ok(!cancel_sb.Information, "Unexpected Information %Iu\n", cancel_sb.Information); returned = TRUE; WaitForSingleObject(thread, INFINITE); CloseHandle(thread); - res = CancelIoEx(file, NULL); - ok(!res && GetLastError() == ERROR_NOT_FOUND, "CancelIoEx failed: %lu\n", GetLastError()); + res = NtCancelIoFileEx(file, NULL, &cancel_sb); + ok(res, "Unexpected result %u\n", res); + 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); cancel_cnt = 0xdeadbeef; res = DeviceIoControl(file, IOCTL_WINETEST_GET_CANCEL_COUNT, NULL, 0, &cancel_cnt, sizeof(cancel_cnt), NULL, &o3); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7797
On Mon Sep 1 17:50:28 2025 +0000, Matteo Bruni wrote:
I'm attaching some new ntoskrnl tests which pass for me on up to date Windows 10 and 11. There's room for cleanup but I think they answer most of the questions you had; I'm curious to know if you think this is convincing enough or otherwise you'd like some other piece tested. Or anything else, really (e.g. maybe I'm taking the wrong conclusions from those test results...) It looks like only `CancelIo()`, not `CancelIoEx()`, waits for the operations to complete. It does wait for completion even if the operations aren't actually cancelled but eventually end regularly on their own. On the other hand, `CancelSynchronousIo()` really wants the completion routine itself to call `IoCompleteRequest()`. The values set in the IOSB `Status` and `Information` fields are apparently not particularly important, or at least nothing breaks immediately if those are set to unexpected values. The thread ID check I added in `cancel_ioctl_irp()` does seem to strongly suggest that the cancel routine is called synchronously. Not sure if this test in particular is "safe" for upstream. Assuming all this makes sense, it looks to me that the approach from this MR is okay once I limit it to `CancelIo()`, leaving `CancelIoEx()` to the old mechanism. Luckily there is no concern with multiple "cancels" having to wait on the same operations. I guess I could also use `async_cancel` for `CancelSynchronousIo()` and get closer to native, although that's never going to be quite right until the whole async cancellation / completion becomes more synchronous. [ntoskrnl-cancel-tests.txt](/uploads/6067830a1d7b7355df5c048a5d67c4df/ntoskrnl-cancel-tests.txt) I just pushed a new revision with a bunch of changes, including those already discussed. I didn't touch `CancelSynchronousIo()`'s implementation at all.
In the end I also essentially reverted the change splitting cancelled asyncs to separate lists. That means some trickery in `cancel_process_async()` becomes necessary to avoid looping forever, but it seems a positive overall as it cuts a lot of looping around the various lists. WRT the old ntdll tests, now I'm not checking for completion after `CancelIoEx()` at all. That in practice seems to happen consistently on Windows, probably because the cancel routine is called synchronously. I could restore those checks and mark them as `todo_wine` (or maybe even `flaky_wine`) if preferred. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7797#note_114941
7/10: ``` - 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 || broken(NT_ERROR(params->iosb_status) && params->pending), "got %#lx\n", io.Status); + todo_wine_if (NT_ERROR(params->iosb_status) && params->pending) + ok(io.Information == 3 || broken(NT_ERROR(params->iosb_status) && params->pending), "got size %Iu\n", io.Information); ``` We should also be testing the actual values in those broken() blocks. Also, can we please mark which versions have which results in a comment? ``` 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), "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { ``` Similarly, no objection to removing broken(), but can we please keep the comment? ``` - 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 == 0xc00000a3 || 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); ``` c00000a3 is STATUS_DEVICE_NOT_READY. 8/10: ``` + case IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE_INIT: + KeInitializeDeviceQueue(&device_queue); + ok(IsListEmpty(&device_queue.DeviceListHead), "expected empty queue list\n"); + status = STATUS_SUCCESS; + break; ``` Or just initialize it once in DriverEntry(), and then you don't need a dedicated ioctl. ``` +#define IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE CTL_CODE(FILE_DEVICE_UNKNOWN, 0x806, METHOD_BUFFERED, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_TEST_CANCEL_COMPLETE CTL_CODE(FILE_DEVICE_UNKNOWN, 0x807, METHOD_BUFFERED, FILE_ANY_ACCESS) ``` I'd be inclined to just make these IOCTL_WINETEST_QUEUE_ASYNC and IOCTL_WINETEST_COMPLETE_ASYNC, since they don't necessarily have anything to do with cancellation, and not upend the rest of the definitions... ``` + /* put the IRP into the queue */ + ret = KeInsertDeviceQueue(&device_queue, &irp->Tail.Overlay.DeviceQueueEntry); + /* or store it elsewhere if it's the one we're supposing to be + * working on "currently" */ + if (!ret) + current_irp = irp; ``` KDEVICE_QUEUE is an odd API and I feel like it honestly makes this test more confusing. I'd just have a simple "queued_async_irps[2]" array and stash them there. Incidentally, because there is some loose threading involved I would be inclined to validate everything we can from the kernel side. Make sure there really is an IRP queued before we complete it, etc. I know I usually say "there's no use avoiding a crash; a failure is a failure", but bluescreens are a different story, because they make iterating a pain. ``` + returned = FALSE; + ctx.file = file; + ctx.ex = FALSE; + thread = CreateThread(NULL, 0, test_cancel_complete_thread, &ctx, 0, NULL); + res = CancelIo(file); + ok(res, "CancelIo failed: %lu\n", GetLastError()); + returned = TRUE; + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); ``` The way I tend to prefer writing this kind of test is to do the blocking action from the thread, check (from the main thread) that the thread doesn't complete immediately (e.g. WaitForSingleObject(200)), and then perform the action from the main thread that should complete it and check that it completes within another reasonable amount of time. I think it's a little easier to read. If you feel this is fine though I won't object. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7797#note_115350
participants (4)
-
Elizabeth Figura (＠zfigura) -
Matteo Bruni
-
Matteo Bruni (＠Mystral)
-
Rémi Bernon