[PATCH 1/5] server: Remove the async from the queue in async_set_result().
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- In an attempt to make the refcounting logic a bit more idiomatic. server/async.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/server/async.c b/server/async.c index 442d98d8c7a..b290005f9a0 100644 --- a/server/async.c +++ b/server/async.c @@ -159,6 +159,11 @@ void async_terminate( struct async *async, unsigned int status ) async->status = status; if (async->iosb && async->iosb->status == STATUS_PENDING) async->iosb->status = status; + /* if no APC could be queued (e.g. the process is terminated), + * thread_queue_apc() may trigger async_set_result(), which may drop the + * last reference to the async, so grab a temporary reference here */ + grab_object( async ); + if (!async->direct_result) { apc_call_t data; @@ -172,7 +177,8 @@ void async_terminate( struct async *async, unsigned int status ) } async_reselect( async ); - if (async->queue) release_object( async ); /* so that it gets destroyed when the async is done */ + + release_object( async ); } /* callback for timeout on an async request */ @@ -191,7 +197,6 @@ void free_async_queue( struct async_queue *queue ) LIST_FOR_EACH_ENTRY_SAFE( async, next, &queue->queue, struct async, queue_entry ) { - grab_object( &async->obj ); if (!async->completion) async->completion = fd_get_completion( async->fd, &async->comp_key ); async->fd = NULL; async_terminate( async, STATUS_HANDLES_CLOSED ); @@ -384,7 +389,6 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota { status = async->status; async->status = STATUS_PENDING; - grab_object( async ); if (status != STATUS_ALERTED) /* it was terminated in the meantime */ async_terminate( async, status ); @@ -425,6 +429,16 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota if (async->completion_callback) async->completion_callback( async->completion_callback_private ); async->completion_callback = NULL; + + async_reselect( async ); + + if (async->queue) + { + async->fd = NULL; + list_remove( &async->queue_entry ); + async->queue = NULL; + release_object( async ); + } } } -- 2.33.0
At best this is a no-op. At worst this will emit an extra APC_ASYNC_IO where the termination (e.g. cancellation) should already have emitted one. Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- server/async.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/server/async.c b/server/async.c index b290005f9a0..086e5eb2870 100644 --- a/server/async.c +++ b/server/async.c @@ -387,13 +387,11 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota if (status == STATUS_PENDING) /* restart it */ { - status = async->status; - async->status = STATUS_PENDING; - - if (status != STATUS_ALERTED) /* it was terminated in the meantime */ - async_terminate( async, status ); - else + if (async->status == STATUS_ALERTED) + { + async->status = STATUS_PENDING; async_reselect( async ); + } } else { -- 2.33.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- I find the "status" field to be quite confusing; it is in particular difficult to remember what distinguishes async->status from async->iosb->status. Patches 3-5 in this set are an attempt to clarify the code by dividing it up into separate boolean flags. server/async.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/async.c b/server/async.c index 086e5eb2870..7890122969f 100644 --- a/server/async.c +++ b/server/async.c @@ -112,13 +112,16 @@ static void async_satisfied( struct object *obj, struct wait_queue_entry *entry struct async *async = (struct async *)obj; assert( obj->ops == &async_ops ); + /* we only return an async handle for asyncs created via create_request_async() */ + assert( async->iosb ); + if (async->direct_result) { async_set_result( &async->obj, async->iosb->status, async->iosb->result ); async->direct_result = 0; } - set_wait_status( entry, async->status ); + set_wait_status( entry, async->iosb->status ); /* close wait handle here to avoid extra server round trip */ if (async->wait_handle) @@ -398,6 +401,7 @@ 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 (async->iosb) async->iosb->status = status; if (async->data.apc) { -- 2.33.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- server/async.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/server/async.c b/server/async.c index 7890122969f..47225af11c9 100644 --- a/server/async.c +++ b/server/async.c @@ -52,6 +52,7 @@ struct async 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 */ + unsigned int alerted :1; /* fd is signaled, but we are waiting for client-side I/O */ struct completion *completion; /* completion associated with fd */ apc_param_t comp_key; /* completion key associated with fd */ unsigned int comp_flags; /* completion flags */ @@ -161,6 +162,8 @@ void async_terminate( struct async *async, unsigned int status ) async->status = status; if (async->iosb && async->iosb->status == STATUS_PENDING) async->iosb->status = status; + if (status == STATUS_ALERTED) + async->alerted = 1; /* if no APC could be queued (e.g. the process is terminated), * thread_queue_apc() may trigger async_set_result(), which may drop the @@ -246,6 +249,7 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da async->pending = 1; async->wait_handle = 0; async->direct_result = 0; + async->alerted = 0; async->completion = fd_get_completion( fd, &async->comp_key ); async->comp_flags = 0; async->completion_callback = NULL; @@ -390,9 +394,10 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota if (status == STATUS_PENDING) /* restart it */ { - if (async->status == STATUS_ALERTED) + if (async->alerted) { async->status = STATUS_PENDING; + async->alerted = 0; async_reselect( async ); } } -- 2.33.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- server/async.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/server/async.c b/server/async.c index 47225af11c9..1c3082083ed 100644 --- a/server/async.c +++ b/server/async.c @@ -42,7 +42,6 @@ struct async struct list process_entry; /* entry in process list */ struct async_queue *queue; /* queue containing this async */ struct fd *fd; /* fd associated with an unqueued async */ - unsigned int status; /* current status */ struct timeout_user *timeout; unsigned int timeout_status; /* status to report upon timeout */ struct event *event; @@ -53,6 +52,7 @@ struct async 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 */ unsigned int alerted :1; /* fd is signaled, but we are waiting for client-side I/O */ + unsigned int terminated :1; /* async has been terminated */ struct completion *completion; /* completion associated with fd */ apc_param_t comp_key; /* completion key associated with fd */ unsigned int comp_flags; /* completion flags */ @@ -158,9 +158,9 @@ void async_terminate( struct async *async, unsigned int status ) { assert( status != STATUS_PENDING ); - if (async->status != STATUS_PENDING) return; /* already terminated */ + if (async->terminated) return; - async->status = status; + async->terminated = 1; if (async->iosb && async->iosb->status == STATUS_PENDING) async->iosb->status = status; if (status == STATUS_ALERTED) async->alerted = 1; @@ -240,7 +240,6 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da async->thread = (struct thread *)grab_object( thread ); async->event = event; - async->status = STATUS_PENDING; async->data = *data; async->timeout = NULL; async->queue = NULL; @@ -250,6 +249,7 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da async->wait_handle = 0; async->direct_result = 0; async->alerted = 0; + async->terminated = 0; async->completion = fd_get_completion( fd, &async->comp_key ); async->comp_flags = 0; async->completion_callback = NULL; @@ -273,7 +273,7 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da void set_async_pending( struct async *async, int signal ) { - if (async->status == STATUS_PENDING) + if (!async->terminated) { async->pending = 1; if (signal && !async->signaled) @@ -390,13 +390,13 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota if (obj->ops != &async_ops) return; /* in case the client messed up the APC results */ - assert( async->status != STATUS_PENDING ); /* it must have been woken up if we get a result */ + assert( async->terminated ); /* it must have been woken up if we get a result */ if (status == STATUS_PENDING) /* restart it */ { if (async->alerted) { - async->status = STATUS_PENDING; + async->terminated = 0; async->alerted = 0; async_reselect( async ); } @@ -405,7 +405,7 @@ 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; + async->terminated = 1; if (async->iosb) async->iosb->status = status; if (async->data.apc) @@ -457,7 +457,7 @@ int async_waiting( struct async_queue *queue ) if (!(ptr = list_head( &queue->queue ))) return 0; async = LIST_ENTRY( ptr, struct async, queue_entry ); - return async->status == STATUS_PENDING; + return !async->terminated; } static int cancel_async( struct process *process, struct object *obj, struct thread *thread, client_ptr_t iosb ) @@ -468,7 +468,7 @@ static int cancel_async( struct process *process, struct object *obj, struct thr restart: LIST_FOR_EACH_ENTRY( async, &process->asyncs, struct async, process_entry ) { - if (async->status != STATUS_PENDING) continue; + if (async->terminated) continue; if ((!obj || (get_fd_user( async->fd ) == obj)) && (!thread || async->thread == thread) && (!iosb || async->data.iosb == iosb)) @@ -578,7 +578,7 @@ struct async *find_pending_async( struct async_queue *queue ) { struct async *async; LIST_FOR_EACH_ENTRY( async, &queue->queue, struct async, queue_entry ) - if (async->status == STATUS_PENDING) return (struct async *)grab_object( async ); + if (!async->terminated) return (struct async *)grab_object( async ); return NULL; } -- 2.33.0
participants (1)
-
Zebediah Figura