[PATCH 1/4] server: Don't check the user data for NULL in async_terminate().
This semantically reverts 481517178ff7dd542966d7f11f3d61b3fc8a0776. That commit was used to implement NtFlushBuffersFile, which at the time didn't use a callback function. 9050b58f0769fca75c4cebed7be3cce51989814e changed it to use irp_completion(), since the result of a blocking flush needed to be taken from the IOSB. As of 97afac469fbe012e22acc1f1045c88b1004a241f that's not true anymore, but on the other hand it is theoretically possible for a device driver to touch the Information member of the IOSB, and we don't seem to lose anything by making all asyncs take a common path. Since all asyncs pass user data and there's no clear reason for them not to, let's get rid of a bit of extra code complexity that's no longer used. Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- server/async.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/server/async.c b/server/async.c index 4dedb27f3d8..0425e4eddf7 100644 --- a/server/async.c +++ b/server/async.c @@ -161,18 +161,14 @@ void async_terminate( struct async *async, unsigned int status ) if (!async->direct_result) { - if (async->data.user) - { - apc_call_t data; + apc_call_t data; - memset( &data, 0, sizeof(data) ); - data.type = APC_ASYNC_IO; - data.async_io.user = async->data.user; - data.async_io.sb = async->data.iosb; - data.async_io.status = status; - thread_queue_apc( async->thread->process, async->thread, &async->obj, &data ); - } - else async_set_result( &async->obj, STATUS_SUCCESS, 0 ); + memset( &data, 0, sizeof(data) ); + data.type = APC_ASYNC_IO; + data.async_io.user = async->data.user; + data.async_io.sb = async->data.iosb; + data.async_io.status = status; + thread_queue_apc( async->thread->process, async->thread, &async->obj, &data ); } async_reselect( async ); -- 2.30.2
Several server objects check if the last handle is being closed in their close_handle callback. If a process holds the last two handles to an object, this code path currently won't be triggered. Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- server/handle.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/server/handle.c b/server/handle.c index 6efd82f2ff9..15da701ee99 100644 --- a/server/handle.c +++ b/server/handle.c @@ -174,21 +174,16 @@ static void handle_table_destroy( struct object *obj ) assert( obj->ops == &handle_table_ops ); - /* first notify all objects that handles are being closed */ - if (table->process) - { - for (i = 0, entry = table->entries; i <= table->last; i++, entry++) - { - struct object *obj = entry->ptr; - if (obj) obj->ops->close_handle( obj, table->process, index_to_handle(i) ); - } - } - for (i = 0, entry = table->entries; i <= table->last; i++, entry++) { struct object *obj = entry->ptr; entry->ptr = NULL; - if (obj) release_object_from_handle( obj ); + if (obj) + { + if (table->process) + obj->ops->close_handle( obj, table->process, index_to_handle(i) ); + release_object_from_handle( obj ); + } } free( table->entries ); } -- 2.30.2
This fixes a regression introduced by 97afac469fbe012e22acc1f1045c88b1004a241f. If we make a request on an asynchronous device handle, and the IRP handler returns STATUS_PENDING, wait_async() will return STATUS_PENDING, as intended. However, if the async object is signaled before the user has a chance to call wait_async() [e.g. if get_next_device_request is called quickly enough], select will return STATUS_PENDING immediately, which causes server_select() to think the object is not signaled, and wait for a select reply forever. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51277 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51295 Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ntdll/unix/server.c | 4 +++- server/protocol.def | 1 + server/thread.c | 32 ++++++++++++++++---------------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c index 567f50d0ad7..1d82e806e17 100644 --- a/dlls/ntdll/unix/server.c +++ b/dlls/ntdll/unix/server.c @@ -603,6 +603,7 @@ unsigned int server_select( const select_op_t *select_op, data_size_t size, UINT apc_call_t call; apc_result_t result; sigset_t old_set; + int signaled; memset( &result, 0, sizeof(result) ); @@ -628,6 +629,7 @@ unsigned int server_select( const select_op_t *select_op, data_size_t size, UINT } if (context) wine_server_set_reply( req, context, 2 * sizeof(*context) ); ret = server_call_unlocked( req ); + signaled = reply->signaled; apc_handle = reply->apc_handle; call = reply->call; } @@ -646,7 +648,7 @@ unsigned int server_select( const select_op_t *select_op, data_size_t size, UINT mutex_unlock( mutex ); mutex = NULL; } - if (ret != STATUS_PENDING) break; + if (signaled) break; ret = wait_select_reply( &cookie ); } diff --git a/server/protocol.def b/server/protocol.def index 10402ca2db3..b4049eb90e7 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1160,6 +1160,7 @@ typedef struct @REPLY apc_call_t call; /* APC call arguments */ obj_handle_t apc_handle; /* handle to next APC */ + int signaled; /* were the handles immediately signaled? */ VARARG(contexts,contexts); /* suspend context(s) */ @END #define SELECT_ALERTABLE 1 diff --git a/server/thread.c b/server/thread.c index e0c520ee37c..e23412c6033 100644 --- a/server/thread.c +++ b/server/thread.c @@ -974,8 +974,8 @@ static int signal_object( obj_handle_t handle ) } /* select on a list of handles */ -static void select_on( const select_op_t *select_op, data_size_t op_size, client_ptr_t cookie, - int flags, abstime_t when ) +static int select_on( const select_op_t *select_op, data_size_t op_size, client_ptr_t cookie, + int flags, abstime_t when ) { int ret; unsigned int count; @@ -984,7 +984,7 @@ static void select_on( const select_op_t *select_op, data_size_t op_size, client switch (select_op->op) { case SELECT_NONE: - if (!wait_on( select_op, 0, NULL, flags, when )) return; + if (!wait_on( select_op, 0, NULL, flags, when )) return 1; break; case SELECT_WAIT: @@ -993,24 +993,24 @@ static void select_on( const select_op_t *select_op, data_size_t op_size, client if (op_size < offsetof( select_op_t, wait.handles ) || count > MAXIMUM_WAIT_OBJECTS) { set_error( STATUS_INVALID_PARAMETER ); - return; + return 1; } if (!wait_on_handles( select_op, count, select_op->wait.handles, flags, when )) - return; + return 1; break; case SELECT_SIGNAL_AND_WAIT: if (!wait_on_handles( select_op, 1, &select_op->signal_and_wait.wait, flags, when )) - return; + return 1; if (select_op->signal_and_wait.signal) { if (!signal_object( select_op->signal_and_wait.signal )) { end_wait( current, get_error() ); - return; + return 1; } /* check if we woke ourselves up */ - if (!current->wait) return; + if (!current->wait) return 1; } break; @@ -1018,23 +1018,23 @@ static void select_on( const select_op_t *select_op, data_size_t op_size, client case SELECT_KEYED_EVENT_RELEASE: object = (struct object *)get_keyed_event_obj( current->process, select_op->keyed_event.handle, select_op->op == SELECT_KEYED_EVENT_WAIT ? KEYEDEVENT_WAIT : KEYEDEVENT_WAKE ); - if (!object) return; + if (!object) return 1; ret = wait_on( select_op, 1, &object, flags, when ); release_object( object ); - if (!ret) return; + if (!ret) return 1; current->wait->key = select_op->keyed_event.key; break; default: set_error( STATUS_INVALID_PARAMETER ); - return; + return 1; } if ((ret = check_wait( current )) != -1) { /* condition is already satisfied */ set_error( end_wait( current, ret )); - return; + return 1; } /* now we need to wait */ @@ -1044,12 +1044,12 @@ static void select_on( const select_op_t *select_op, data_size_t op_size, client thread_timeout, current->wait ))) { end_wait( current, get_error() ); - return; + return 1; } } current->wait->cookie = cookie; set_error( STATUS_PENDING ); - return; + return 0; } /* attempt to wake threads sleeping on the object wait queue */ @@ -1664,7 +1664,7 @@ DECL_HANDLER(select) release_object( apc ); } - select_on( &select_op, op_size, req->cookie, req->flags, req->timeout ); + reply->signaled = select_on( &select_op, op_size, req->cookie, req->flags, req->timeout ); if (get_error() == STATUS_USER_APC) { @@ -1684,7 +1684,7 @@ DECL_HANDLER(select) } release_object( apc ); } - else if (get_error() != STATUS_PENDING && get_reply_max_size() >= sizeof(context_t) && + else if (reply->signaled && get_reply_max_size() >= sizeof(context_t) && current->context && current->suspend_cookie == req->cookie) { ctx = current->context; -- 2.30.2
This was used to implement AcceptEx() using multiple APCs, and made obsolete by 0bbd3f66178abdc6ea15d30d2eb3b033367f8407. Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- server/async.c | 1 - 1 file changed, 1 deletion(-) diff --git a/server/async.c b/server/async.c index 0425e4eddf7..dbd4859edc4 100644 --- a/server/async.c +++ b/server/async.c @@ -396,7 +396,6 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota if (async->timeout) remove_timeout_user( async->timeout ); async->timeout = NULL; async->status = status; - if (status == STATUS_MORE_PROCESSING_REQUIRED) return; /* don't report the completion */ if (async->data.apc) { -- 2.30.2
participants (1)
-
Zebediah Figura