[PATCH 1/4] ntoskrnl: Report IRP completion via get_next_device_request if possible.
Although there is arguably an advantage to saving a server request, the impetus for this patch is make it easier for the server to process the IRP return status before (or at the same time as) it processes the IOSB status. This allows simpler handling of the case where the IRP handler returns STATUS_PENDING but completes the IRP before returning. Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ntoskrnl.exe/ntoskrnl.c | 150 +++++++++++++++++++++++++---------- server/device.c | 21 ++--- server/protocol.def | 2 + 3 files changed, 123 insertions(+), 50 deletions(-) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index f7a9d8e8efe..b4d038c299b 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -76,6 +76,14 @@ static void *ldr_notify_cookie; static PLOAD_IMAGE_NOTIFY_ROUTINE load_image_notify_routines[8]; static unsigned int load_image_notify_routine_count; +struct irp_data +{ + HANDLE handle; + IRP *irp; + BOOL async; + BOOL complete; +}; + static int wine_drivers_rb_compare( const void *key, const struct wine_rb_entry *entry ) { const struct wine_driver *driver = WINE_RB_ENTRY_VALUE( entry, const struct wine_driver, entry ); @@ -419,10 +427,23 @@ static void *create_file_object( HANDLE handle ) DECLARE_CRITICAL_SECTION(irp_completion_cs); +static void free_dispatch_irp( struct irp_data *irp_data ) +{ + IRP *irp = irp_data->irp; + + if (irp->UserBuffer != irp->AssociatedIrp.SystemBuffer) + { + HeapFree( GetProcessHeap(), 0, irp->UserBuffer ); + irp->UserBuffer = NULL; + } + + free( irp_data ); +} + /* transfer result of IRP back to wineserver */ static NTSTATUS WINAPI dispatch_irp_completion( DEVICE_OBJECT *device, IRP *irp, void *context ) { - HANDLE irp_handle = context; + struct irp_data *irp_data = context; void *out_buff = irp->UserBuffer; NTSTATUS status; @@ -431,9 +452,17 @@ static NTSTATUS WINAPI dispatch_irp_completion( DEVICE_OBJECT *device, IRP *irp, EnterCriticalSection( &irp_completion_cs ); + irp_data->complete = TRUE; + if (!irp_data->async) + { + /* main loop will report completion via get_next_device_request */ + LeaveCriticalSection( &irp_completion_cs ); + return STATUS_MORE_PROCESSING_REQUIRED; + } + SERVER_START_REQ( set_irp_result ) { - req->handle = wine_server_obj_handle( irp_handle ); + req->handle = wine_server_obj_handle( irp_data->handle ); req->status = irp->IoStatus.u.Status; req->size = irp->IoStatus.Information; if (!NT_ERROR(irp->IoStatus.u.Status)) @@ -444,11 +473,7 @@ static NTSTATUS WINAPI dispatch_irp_completion( DEVICE_OBJECT *device, IRP *irp, } SERVER_END_REQ; - if (irp->UserBuffer != irp->AssociatedIrp.SystemBuffer) - { - HeapFree( GetProcessHeap(), 0, irp->UserBuffer ); - irp->UserBuffer = NULL; - } + free_dispatch_irp( irp_data ); LeaveCriticalSection( &irp_completion_cs ); return status; @@ -458,26 +483,36 @@ struct dispatch_context { irp_params_t params; HANDLE handle; - IRP *irp; + struct irp_data *irp_data; ULONG in_size; void *in_buff; }; -static void dispatch_irp( DEVICE_OBJECT *device, IRP *irp, struct dispatch_context *context ) +static NTSTATUS dispatch_irp( DEVICE_OBJECT *device, IRP *irp, struct dispatch_context *context ) { + struct irp_data *irp_data; LARGE_INTEGER count; - IoSetCompletionRoutine( irp, dispatch_irp_completion, context->handle, TRUE, TRUE, TRUE ); + if (!(irp_data = malloc( sizeof(*irp_data) ))) + return STATUS_NO_MEMORY; + irp_data->handle = context->handle; + irp_data->irp = irp; + irp_data->async = FALSE; + irp_data->complete = FALSE; + + IoSetCompletionRoutine( irp, dispatch_irp_completion, irp_data, TRUE, TRUE, TRUE ); + context->irp_data = irp_data; context->handle = 0; KeQueryTickCount( &count ); /* update the global KeTickCount */ - context->irp = irp; device->CurrentIrp = irp; KeEnterCriticalRegion(); IoCallDriver( device, irp ); KeLeaveCriticalRegion(); device->CurrentIrp = NULL; + + return STATUS_SUCCESS; } /* process a create request for a given file */ @@ -520,9 +555,7 @@ static NTSTATUS dispatch_create( struct dispatch_context *context ) irp->UserEvent = NULL; irp->Flags |= IRP_CREATE_OPERATION; - dispatch_irp( device, irp, context ); - - return STATUS_SUCCESS; + return dispatch_irp( device, irp, context ); } /* process a close request for a given file */ @@ -558,9 +591,7 @@ static NTSTATUS dispatch_close( struct dispatch_context *context ) irp->UserEvent = NULL; irp->Flags |= IRP_CLOSE_OPERATION; - dispatch_irp( device, irp, context ); - - return STATUS_SUCCESS; + return dispatch_irp( device, irp, context ); } /* process a read request for a given device */ @@ -600,9 +631,7 @@ static NTSTATUS dispatch_read( struct dispatch_context *context ) irp->Flags |= IRP_READ_OPERATION; irp->Flags |= IRP_DEALLOCATE_BUFFER; /* deallocate out_buff */ - dispatch_irp( device, irp, context ); - - return STATUS_SUCCESS; + return dispatch_irp( device, irp, context ); } /* process a write request for a given device */ @@ -636,9 +665,7 @@ static NTSTATUS dispatch_write( struct dispatch_context *context ) irp->Flags |= IRP_WRITE_OPERATION; irp->Flags |= IRP_DEALLOCATE_BUFFER; /* deallocate in_buff */ - dispatch_irp( device, irp, context ); - - return STATUS_SUCCESS; + return dispatch_irp( device, irp, context ); } /* process a flush request for a given device */ @@ -665,9 +692,7 @@ static NTSTATUS dispatch_flush( struct dispatch_context *context ) irpsp = IoGetNextIrpStackLocation( irp ); irpsp->FileObject = file; - dispatch_irp( device, irp, context ); - - return STATUS_SUCCESS; + return dispatch_irp( device, irp, context ); } /* process an ioctl request for a given device */ @@ -680,6 +705,7 @@ static NTSTATUS dispatch_ioctl( struct dispatch_context *context ) DEVICE_OBJECT *device; FILE_OBJECT *file = wine_server_get_ptr( context->params.ioctl.file ); ULONG out_size = context->params.ioctl.out_size; + NTSTATUS status; if (!file) return STATUS_INVALID_HANDLE; @@ -728,10 +754,10 @@ static NTSTATUS dispatch_ioctl( struct dispatch_context *context ) context->in_buff = NULL; irp->Flags |= IRP_DEALLOCATE_BUFFER; /* deallocate in_buff */ - dispatch_irp( device, irp, context ); + status = dispatch_irp( device, irp, context ); HeapFree( GetProcessHeap(), 0, to_free ); - return STATUS_SUCCESS; + return status; } /* process a volume information request for a given device */ @@ -778,9 +804,7 @@ static NTSTATUS dispatch_volume( struct dispatch_context *context ) context->in_buff = NULL; irp->Flags |= IRP_DEALLOCATE_BUFFER; /* deallocate out_buff */ - dispatch_irp( device, irp, context ); - - return STATUS_SUCCESS; + return dispatch_irp( device, irp, context ); } static NTSTATUS dispatch_free( struct dispatch_context *context ) @@ -874,16 +898,11 @@ PEPROCESS PsInitialSystemProcess = NULL; NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event ) { HANDLE manager = get_device_manager(); - struct dispatch_context context; + struct dispatch_context context = {.in_size = 4096}; NTSTATUS status = STATUS_SUCCESS; struct wine_driver *driver; HANDLE handles[2]; - context.handle = NULL; - context.irp = NULL; - context.in_size = 4096; - context.in_buff = NULL; - /* Set the system process global before setting up the request thread trickery */ PsInitialSystemProcess = IoGetCurrentProcess(); request_thread = GetCurrentThreadId(); @@ -903,12 +922,47 @@ NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event ) goto done; } + EnterCriticalSection( &irp_completion_cs ); + SERVER_START_REQ( get_next_device_request ) { req->manager = wine_server_obj_handle( manager ); req->prev = wine_server_obj_handle( context.handle ); - req->user_ptr = wine_server_client_ptr( context.irp ); - req->status = status; + + if (context.irp_data) + { + IRP *irp = context.irp_data->irp; + + req->user_ptr = wine_server_client_ptr( irp ); + + if (context.irp_data->complete) + { + /* IRP completed even before we got here; we can report completion now */ + void *out_buff = irp->UserBuffer; + + if (irp->Flags & IRP_WRITE_OPERATION) + out_buff = NULL; /* do not transfer back input buffer */ + + req->prev = wine_server_obj_handle( context.irp_data->handle ); + req->status = irp->IoStatus.u.Status; + req->result = irp->IoStatus.Information; + if (!NT_ERROR(irp->IoStatus.u.Status) && out_buff) + wine_server_add_data( req, out_buff, irp->IoStatus.Information ); + } + else + { + if (status == STATUS_SUCCESS) + status = STATUS_PENDING; + + req->status = status; + } + } + else + { + req->user_ptr = 0; + req->status = status; + } + wine_server_set_reply( req, context.in_buff, context.in_size ); if (!(status = wine_server_call( req ))) { @@ -924,10 +978,26 @@ NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event ) if (status == STATUS_BUFFER_OVERFLOW) context.in_size = reply->in_size; } - context.irp = NULL; } SERVER_END_REQ; + if (context.irp_data) + { + if (context.irp_data->complete) + { + IoCompleteRequest( context.irp_data->irp, IO_NO_INCREMENT ); + free_dispatch_irp( context.irp_data ); + } + else + { + context.irp_data->async = TRUE; + } + } + + LeaveCriticalSection( &irp_completion_cs ); + + context.irp_data = NULL; + switch (status) { case STATUS_SUCCESS: diff --git a/server/device.c b/server/device.c index a2383ecdc7c..b0e417a6473 100644 --- a/server/device.c +++ b/server/device.c @@ -946,22 +946,25 @@ DECL_HANDLER(get_next_device_request) 0, &device_manager_ops ))) return; - if (req->prev) close_handle( current->process, req->prev ); /* avoid an extra round-trip for close */ - /* process result of previous call */ if (manager->current_call) { irp = manager->current_call; irp->user_ptr = req->user_ptr; - if (req->status) - set_irp_result( irp, req->status, NULL, 0, 0 ); - if (irp->canceled) - /* if it was canceled during dispatch, we couldn't queue cancel call without client pointer, - * so we need to do it now */ - cancel_irp_call( irp ); + if (req->prev) + { + set_irp_result( irp, req->status, get_req_data(), get_req_data_size(), req->result ); + close_handle( current->process, req->prev ); /* avoid an extra round-trip for close */ + } else if (irp->async) + { set_async_pending( irp->async ); + if (irp->canceled) + /* if it was canceled during dispatch, we couldn't queue cancel call without client pointer, + * so we need to do it now */ + cancel_irp_call( irp ); + } free_irp_params( irp ); release_object( irp ); @@ -1019,8 +1022,6 @@ DECL_HANDLER(set_irp_result) if ((irp = (struct irp_call *)get_handle_obj( current->process, req->handle, 0, &irp_call_ops ))) { set_irp_result( irp, req->status, get_req_data(), get_req_data_size(), req->size ); - /* we may be still dispatching the IRP. don't bother queuing cancel if it's already complete */ - irp->canceled = 0; close_handle( current->process, req->handle ); /* avoid an extra round-trip for close */ release_object( irp ); } diff --git a/server/protocol.def b/server/protocol.def index 133d6ad0552..fec212a3ea3 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3368,6 +3368,8 @@ struct handle_info obj_handle_t prev; /* handle to the previous irp */ unsigned int status; /* status of the previous irp */ client_ptr_t user_ptr; /* user pointer of the previous irp */ + data_size_t result; /* IOSB result of the previous irp */ + VARARG(data,bytes); /* output data of the previous irp */ @REPLY irp_params_t params; /* irp parameters */ obj_handle_t next; /* handle to the next irp */ -- 2.33.0
Based on a patch by Chip Davis. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=30155 Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ntoskrnl.exe/ntoskrnl.c | 22 ++++++-------- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 49 +++++++++++++----------------- server/async.c | 38 ++++++++++++++++++----- server/device.c | 31 +++++++++++++------ server/file.h | 2 ++ server/protocol.def | 1 + 6 files changed, 86 insertions(+), 57 deletions(-) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index b4d038c299b..42976f4905e 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -492,6 +492,7 @@ static NTSTATUS dispatch_irp( DEVICE_OBJECT *device, IRP *irp, struct dispatch_c { struct irp_data *irp_data; LARGE_INTEGER count; + NTSTATUS status; if (!(irp_data = malloc( sizeof(*irp_data) ))) return STATUS_NO_MEMORY; @@ -508,11 +509,14 @@ static NTSTATUS dispatch_irp( DEVICE_OBJECT *device, IRP *irp, struct dispatch_c device->CurrentIrp = irp; KeEnterCriticalRegion(); - IoCallDriver( device, irp ); + status = IoCallDriver( device, irp ); KeLeaveCriticalRegion(); device->CurrentIrp = NULL; - return STATUS_SUCCESS; + if (status != STATUS_PENDING && !irp_data->complete) + ERR( "dispatch routine returned %#x but didn't complete the IRP\n", status ); + + return status; } /* process a create request for a given file */ @@ -934,6 +938,7 @@ NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event ) IRP *irp = context.irp_data->irp; req->user_ptr = wine_server_client_ptr( irp ); + req->status = status; if (context.irp_data->complete) { @@ -943,19 +948,12 @@ NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event ) if (irp->Flags & IRP_WRITE_OPERATION) out_buff = NULL; /* do not transfer back input buffer */ - req->prev = wine_server_obj_handle( context.irp_data->handle ); - req->status = irp->IoStatus.u.Status; - req->result = irp->IoStatus.Information; + req->prev = wine_server_obj_handle( context.irp_data->handle ); + req->iosb_status = irp->IoStatus.u.Status; + req->result = irp->IoStatus.Information; if (!NT_ERROR(irp->IoStatus.u.Status) && out_buff) wine_server_add_data( req, out_buff, irp->IoStatus.Information ); } - else - { - if (status == STATUS_SUCCESS) - status = STATUS_PENDING; - - req->status = status; - } } else { diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index b736cfff3ba..7c4b0154781 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -663,19 +663,16 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) size = 0xdeadf00d; SetLastError(0xdeadf00d); ret = DeviceIoControl(device, ioctl, params, sizeof(*params), buffer, sizeof(buffer), &size, NULL); - todo_wine_if ((params->iosb_status != STATUS_PENDING && NT_SUCCESS(expect_status) != NT_SUCCESS(params->iosb_status)) - || (params->iosb_status == STATUS_PENDING && NT_SUCCESS(expect_status))) + todo_wine_if (params->ret_status == STATUS_PENDING && params->iosb_status == STATUS_PENDING) ok(ret == NT_SUCCESS(expect_status), "got %d\n", ret); if (NT_SUCCESS(expect_status)) { - todo_wine_if (!NT_SUCCESS(params->iosb_status) || params->iosb_status == STATUS_PENDING) + todo_wine_if (params->ret_status == STATUS_PENDING && params->iosb_status == STATUS_PENDING) ok(GetLastError() == 0xdeadf00d, "got error %u\n", GetLastError()); } else { - todo_wine_if (RtlNtStatusToDosError(expect_status) != RtlNtStatusToDosError(params->iosb_status) - || NT_SUCCESS(params->iosb_status)) - ok(GetLastError() == RtlNtStatusToDosError(expect_status), "got error %u\n", GetLastError()); + ok(GetLastError() == RtlNtStatusToDosError(expect_status), "got error %u\n", GetLastError()); } if (NT_ERROR(expect_status)) todo_wine ok(size == 0xdeadf00d, "got size %u\n", size); @@ -690,8 +687,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) io.Information = 0xdeadf00d; ret = NtDeviceIoControlFile(device, NULL, NULL, NULL, &io, ioctl, params, sizeof(*params), buffer, sizeof(buffer)); - todo_wine_if (params->ret_status != params->iosb_status && params->ret_status != STATUS_PENDING) - ok(ret == expect_status, "got %#x\n", ret); + ok(ret == expect_status, "got %#x\n", ret); if (NT_ERROR(params->iosb_status)) { todo_wine ok(io.Status == 0xdeadf00d, "got %#x\n", io.Status); @@ -725,10 +721,9 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) io.Information = 0xdeadf00d; ret = NtDeviceIoControlFile(file, event, NULL, (void *)456, &io, ioctl, params, sizeof(*params), buffer, sizeof(buffer)); - todo_wine_if (params->ret_status != params->iosb_status) - ok(ret == params->ret_status - || broken(NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* win10 */ - "got %#x\n", ret); + ok(ret == params->ret_status + || broken(NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* win10 */ + "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { todo_wine ok(io.Status == 0xdeadf00d, "got %#x\n", io.Status); @@ -778,10 +773,9 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) io.Information = 0xdeadf00d; ret = NtDeviceIoControlFile(file, event, NULL, NULL, &io, ioctl, params, sizeof(*params), buffer, sizeof(buffer)); - todo_wine_if (params->ret_status != params->iosb_status) - ok(ret == params->ret_status - || broken(NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* win10 */ - "got %#x\n", ret); + ok(ret == params->ret_status + || broken(NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* win10 */ + "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { todo_wine ok(io.Status == 0xdeadf00d, "got %#x\n", io.Status); @@ -811,10 +805,9 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) io.Information = 0xdeadf00d; ret = NtDeviceIoControlFile(file, NULL, NULL, NULL, &io, ioctl, params, sizeof(*params), buffer, sizeof(buffer)); - todo_wine_if (params->ret_status != params->iosb_status) - ok(ret == params->ret_status - || broken(NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* win10 */ - "got %#x\n", ret); + ok(ret == params->ret_status + || broken(NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* win10 */ + "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { todo_wine ok(io.Status == 0xdeadf00d, "got %#x\n", io.Status); @@ -848,10 +841,9 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) io.Information = 0xdeadf00d; ret = NtDeviceIoControlFile(file, event, NULL, (void *)456, &io, ioctl, params, sizeof(*params), buffer, sizeof(buffer)); - todo_wine_if (params->ret_status != params->iosb_status) - ok(ret == params->ret_status - || broken(NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* win10 */ - "got %#x\n", ret); + ok(ret == params->ret_status + || broken(NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* win10 */ + "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { todo_wine ok(io.Status == 0xdeadf00d, "got %#x\n", io.Status); @@ -890,14 +882,16 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) } else { - todo_wine ok(!ret, "got %#x\n", ret); + todo_wine_if (params->ret_status != STATUS_PENDING) + ok(!ret, "got %#x\n", ret); } if (!ret) { ok(key == 123, "got key %Iu\n", key); ok(value == 456, "got value %Iu\n", value); ok(io.Status == params->iosb_status, "got iosb status %#x\n", io.Status); - ok(io.Information == 3, "got information %Iu\n", io.Information); + todo_wine_if (params->iosb_status == STATUS_PENDING) + ok(io.Information == 3, "got information %Iu\n", io.Information); } } @@ -919,8 +913,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) io.Information = 0xdeadf00d; ret = NtDeviceIoControlFile(file, NULL, return_status_apc, (void *)456, &io, ioctl, params, sizeof(*params), buffer, sizeof(buffer)); - todo_wine_if (params->ret_status != params->iosb_status) - ok(ret == params->ret_status, "got %#x\n", ret); + ok(ret == params->ret_status, "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) { todo_wine ok(io.Status == 0xdeadf00d, "got %#x\n", io.Status); diff --git a/server/async.c b/server/async.c index 2dbdfc76a59..df30ae3c7da 100644 --- a/server/async.c +++ b/server/async.c @@ -48,6 +48,7 @@ struct async async_data_t data; /* data for async I/O call */ struct iosb *iosb; /* I/O status block */ obj_handle_t wait_handle; /* pre-allocated wait handle */ + unsigned int initial_status; /* status returned from initial request */ unsigned int signaled :1; unsigned int pending :1; /* request successfully queued, but pending */ unsigned int direct_result :1;/* a flag if we're passing result directly from request instead of APC */ @@ -125,7 +126,10 @@ static void async_satisfied( struct object *obj, struct wait_queue_entry *entry async->direct_result = 0; } - set_wait_status( entry, async->iosb->status ); + if (async->initial_status == STATUS_PENDING && async->blocking) + set_wait_status( entry, async->iosb->status ); + else + set_wait_status( entry, async->initial_status ); /* close wait handle here to avoid extra server round trip */ if (async->wait_handle) @@ -255,6 +259,7 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da async->timeout = NULL; async->queue = NULL; async->fd = (struct fd *)grab_object( fd ); + async->initial_status = STATUS_PENDING; async->signaled = 0; async->pending = 1; async->wait_handle = 0; @@ -285,17 +290,31 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da return async; } +/* set the initial status of an async whose status was previously unknown + * the initial status may be STATUS_PENDING */ +void async_set_initial_status( struct async *async, unsigned int status ) +{ + assert( async->unknown_status ); + if (!async->terminated) + { + async->initial_status = status; + async->unknown_status = 0; + } +} + void set_async_pending( struct async *async ) { if (!async->terminated) - { async->pending = 1; - async->unknown_status = 0; - if (!async->blocking && !async->signaled) - { - async->signaled = 1; - wake_up( &async->obj, 0 ); - } +} + +void async_wake_obj( struct async *async ) +{ + assert( !async->unknown_status ); + if (!async->blocking) + { + async->signaled = 1; + wake_up( &async->obj, 0 ); } } @@ -311,6 +330,8 @@ obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_ return async->wait_handle; } + async->initial_status = get_error(); + if (!async->pending && NT_ERROR( get_error() )) { close_handle( async->thread->process, async->wait_handle ); @@ -348,6 +369,7 @@ obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_ async->wait_handle = 0; } } + async->initial_status = async->iosb->status; set_error( async->iosb->status ); return async->wait_handle; } diff --git a/server/device.c b/server/device.c index b0e417a6473..df1a71a5a4d 100644 --- a/server/device.c +++ b/server/device.c @@ -952,18 +952,31 @@ DECL_HANDLER(get_next_device_request) irp = manager->current_call; irp->user_ptr = req->user_ptr; - if (req->prev) + if (irp->async) { - set_irp_result( irp, req->status, get_req_data(), get_req_data_size(), req->result ); - close_handle( current->process, req->prev ); /* avoid an extra round-trip for close */ + if (req->status == STATUS_PENDING) + set_async_pending( irp->async ); + async_set_initial_status( irp->async, req->status ); + + if (req->prev) + { + set_irp_result( irp, req->iosb_status, get_req_data(), get_req_data_size(), req->result ); + close_handle( current->process, req->prev ); /* avoid an extra round-trip for close */ + } + else + { + async_wake_obj( irp->async ); + if (irp->canceled) + { + /* if it was canceled during dispatch, we couldn't queue cancel + * call without client pointer, so we need to do it now */ + cancel_irp_call( irp ); + } + } } - else if (irp->async) + else { - set_async_pending( irp->async ); - if (irp->canceled) - /* if it was canceled during dispatch, we couldn't queue cancel call without client pointer, - * so we need to do it now */ - cancel_irp_call( irp ); + set_irp_result( irp, req->status, NULL, 0, 0 ); } free_irp_params( irp ); diff --git a/server/file.h b/server/file.h index 80f2191c050..1d830cd3d6f 100644 --- a/server/file.h +++ b/server/file.h @@ -230,6 +230,8 @@ extern void async_set_result( struct object *obj, unsigned int status, apc_param extern void async_set_completion_callback( struct async *async, async_completion_callback func, void *private ); extern void async_set_unknown_status( struct async *async ); extern void set_async_pending( struct async *async ); +extern void async_set_initial_status( struct async *async, unsigned int status ); +extern void async_wake_obj( struct async *async ); extern int async_waiting( struct async_queue *queue ); extern void async_terminate( struct async *async, unsigned int status ); extern void async_request_complete( struct async *async, unsigned int status, data_size_t result, diff --git a/server/protocol.def b/server/protocol.def index fec212a3ea3..608c481db27 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3368,6 +3368,7 @@ struct handle_info obj_handle_t prev; /* handle to the previous irp */ unsigned int status; /* status of the previous irp */ client_ptr_t user_ptr; /* user pointer of the previous irp */ + unsigned int iosb_status; /* IOSB status of the previous irp */ data_size_t result; /* IOSB result of the previous irp */ VARARG(data,bytes); /* output data of the previous irp */ @REPLY -- 2.33.0
Zebediah Figura <zfigura(a)codeweavers.com> writes:
Based on a patch by Chip Davis.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=30155 Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ntoskrnl.exe/ntoskrnl.c | 22 ++++++-------- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 49 +++++++++++++----------------- server/async.c | 38 ++++++++++++++++++----- server/device.c | 31 +++++++++++++------ server/file.h | 2 ++ server/protocol.def | 1 + 6 files changed, 86 insertions(+), 57 deletions(-)
This is breaking the tests: tools/runtest -q -P wine -T . -M iphlpapi.dll -p dlls/iphlpapi/tests/iphlpapi_test.exe iphlpapi && touch dlls/iphlpapi/tests/iphlpapi.ok iphlpapi.c:165: Test failed: GetIfEntry(bogus row) returned 0, expected ERROR_INVALID_DATA or ERROR_FILE_NOT_FOUND iphlpapi.c:902: Tests skipped: ICMP is not available. iphlpapi.c:1718: Test failed: got 0 iphlpapi.c:1719: Test failed: got ffffff iphlpapi.c:1720: Test failed: got 16777215 iphlpapi.c:1721: Test failed: got 65535 iphlpapi.c:1880: Test failed: got 0031F3BA iphlpapi.c:1883: Test failed: got 0031F3BA iphlpapi.c:1718: Test failed: got 0 iphlpapi.c:1719: Test failed: got ffffff iphlpapi.c:1720: Test failed: got 16777215 iphlpapi.c:1721: Test failed: got 65535 iphlpapi.c:1880: Test failed: got 0031F3BA iphlpapi.c:1883: Test failed: got 0031F3BA iphlpapi.c:1718: Test failed: got 0 iphlpapi.c:1719: Test failed: got ffffff iphlpapi.c:1720: Test failed: got 16777215 iphlpapi.c:1721: Test failed: got 65535 iphlpapi.c:1880: Test failed: got 0031F3BA iphlpapi.c:1883: Test failed: got 0031F3BA iphlpapi.c:1718: Test failed: got 0 iphlpapi.c:1719: Test failed: got ffffff iphlpapi.c:1720: Test failed: got 16777215 iphlpapi.c:1721: Test failed: got 65535 iphlpapi.c:1880: Test failed: got 0031F3BA iphlpapi.c:1883: Test failed: got 0031F3BA iphlpapi.c:1986: Test failed: got 0 iphlpapi.c:1999: Test failed: got 0 iphlpapi.c:2007: Test failed: got 0 make: *** [Makefile:65566: dlls/iphlpapi/tests/iphlpapi.ok] Error 28 -- Alexandre Julliard julliard(a)winehq.org
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ntoskrnl.exe/ntoskrnl.c | 5 +++++ dlls/ntoskrnl.exe/tests/ntoskrnl.c | 3 +-- server/device.c | 2 +- server/protocol.def | 1 + 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index 42976f4905e..1bd5d6fb351 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -949,11 +949,16 @@ NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event ) out_buff = NULL; /* do not transfer back input buffer */ req->prev = wine_server_obj_handle( context.irp_data->handle ); + req->pending = irp->PendingReturned; req->iosb_status = irp->IoStatus.u.Status; req->result = irp->IoStatus.Information; if (!NT_ERROR(irp->IoStatus.u.Status) && out_buff) wine_server_add_data( req, out_buff, irp->IoStatus.Information ); } + else + { + req->pending = 1; + } } else { diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 7c4b0154781..f0e8021aa17 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -882,8 +882,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) } else { - todo_wine_if (params->ret_status != STATUS_PENDING) - ok(!ret, "got %#x\n", ret); + ok(!ret, "got %#x\n", ret); } if (!ret) { diff --git a/server/device.c b/server/device.c index df1a71a5a4d..5abe1e3bb00 100644 --- a/server/device.c +++ b/server/device.c @@ -954,7 +954,7 @@ DECL_HANDLER(get_next_device_request) if (irp->async) { - if (req->status == STATUS_PENDING) + if (req->pending) set_async_pending( irp->async ); async_set_initial_status( irp->async, req->status ); diff --git a/server/protocol.def b/server/protocol.def index 608c481db27..421a46c130d 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3366,6 +3366,7 @@ struct handle_info @REQ(get_next_device_request) obj_handle_t manager; /* handle to the device manager */ obj_handle_t prev; /* handle to the previous irp */ + int pending; /* was the previous irp marked pending? */ unsigned int status; /* status of the previous irp */ client_ptr_t user_ptr; /* user pointer of the previous irp */ unsigned int iosb_status; /* IOSB status of the previous irp */ -- 2.33.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 14 +++++------ server/async.c | 39 ++++++++++++++++++------------ 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index f0e8021aa17..fb91783779b 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -729,7 +729,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) todo_wine ok(io.Status == 0xdeadf00d, "got %#x\n", io.Status); todo_wine ok(io.Information == 0xdeadf00d, "got size %Iu\n", io.Information); ret = WaitForSingleObject(event, 0); - todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); } else { @@ -753,7 +753,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) ret = NtRemoveIoCompletion(port, &key, &value, &io, &zero); if (!params->pending && NT_ERROR(params->iosb_status)) { - todo_wine ok(ret == STATUS_TIMEOUT, "got %#x\n", ret); + ok(ret == STATUS_TIMEOUT, "got %#x\n", ret); } else { @@ -781,7 +781,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) todo_wine ok(io.Status == 0xdeadf00d, "got %#x\n", io.Status); todo_wine ok(io.Information == 0xdeadf00d, "got size %Iu\n", io.Information); ret = WaitForSingleObject(event, 0); - todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); } else { @@ -813,7 +813,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) todo_wine ok(io.Status == 0xdeadf00d, "got %#x\n", io.Status); todo_wine ok(io.Information == 0xdeadf00d, "got size %Iu\n", io.Information); ret = WaitForSingleObject(file, 0); - todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); } else { @@ -849,7 +849,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) todo_wine ok(io.Status == 0xdeadf00d, "got %#x\n", io.Status); todo_wine ok(io.Information == 0xdeadf00d, "got size %Iu\n", io.Information); ret = WaitForSingleObject(event, 0); - todo_wine ok(ret == WAIT_TIMEOUT, "got %d\n", ret); + ok(ret == WAIT_TIMEOUT, "got %d\n", ret); } else { @@ -932,8 +932,8 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params) ret = SleepEx(0, TRUE); if (!params->pending && NT_ERROR(params->iosb_status)) { - todo_wine ok(!ret, "got %d\n", ret); - todo_wine ok(!got_return_status_apc, "got %u APC calls\n", got_return_status_apc); + ok(!ret, "got %d\n", ret); + ok(!got_return_status_apc, "got %u APC calls\n", got_return_status_apc); } else { diff --git a/server/async.c b/server/async.c index df30ae3c7da..7cffd24a18b 100644 --- a/server/async.c +++ b/server/async.c @@ -465,25 +465,32 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota async->terminated = 1; if (async->iosb) async->iosb->status = status; - if (async->data.apc) + /* don't signal completion if the async failed synchronously + * this can happen if the initial status was unknown (i.e. for device files) + * note that we check the IOSB status here, not the initial status */ + if (async->pending || !NT_ERROR( status )) { - apc_call_t data; - memset( &data, 0, sizeof(data) ); - data.type = APC_USER; - data.user.func = async->data.apc; - data.user.args[0] = async->data.apc_context; - data.user.args[1] = async->data.iosb; - data.user.args[2] = 0; - thread_queue_apc( NULL, async->thread, NULL, &data ); - } - else if (async->data.apc_context && (async->pending || - !(async->comp_flags & FILE_SKIP_COMPLETION_PORT_ON_SUCCESS))) - { - add_async_completion( async, async->data.apc_context, status, total ); + if (async->data.apc) + { + apc_call_t data; + memset( &data, 0, sizeof(data) ); + data.type = APC_USER; + data.user.func = async->data.apc; + data.user.args[0] = async->data.apc_context; + data.user.args[1] = async->data.iosb; + data.user.args[2] = 0; + thread_queue_apc( NULL, async->thread, NULL, &data ); + } + else if (async->data.apc_context && (async->pending || + !(async->comp_flags & FILE_SKIP_COMPLETION_PORT_ON_SUCCESS))) + { + add_async_completion( async, async->data.apc_context, status, total ); + } + + if (async->event) set_event( async->event ); + else if (async->fd) set_fd_signaled( async->fd, 1 ); } - if (async->event) set_event( async->event ); - else if (async->fd) set_fd_signaled( async->fd, 1 ); if (!async->signaled) { async->signaled = 1; -- 2.33.0
participants (2)
-
Alexandre Julliard -
Zebediah Figura