[PATCH v4 0/12] MR6369: Draft: Refactor the async state flags
-- v4: server: Eliminate async_alerted() helper function. server: Eliminate async_unknown_status() helper function. server: Remove "unknown_status" field from struct async. server: Remove "alerted" field from struct async. server: Remove "terminated" field from struct async. server: Remove "signaled" field from struct async. server: Remove "direct_result" field from struct async. server: Signal async unconditionally in async_set_result(). server: Introduce a new async state enum. server: Introduce async_unknown_status() helper to compute the 'unknown_status' flag. server: Introduce async_alerted() helper to compute the 'alerted' flag. server: Introduce async_terminated() helper to compute the 'terminated' flag. https://gitlab.winehq.org/wine/wine/-/merge_requests/6369
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This helper will later be rewritten to compute the flag from a state enum. --- server/async.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/server/async.c b/server/async.c index 749c547af4f..9c7218a2e54 100644 --- a/server/async.c +++ b/server/async.c @@ -161,12 +161,17 @@ static void async_destroy( struct object *obj ) release_object( async->thread ); } +static int async_terminated( const struct async *async ) +{ + return async->terminated; +} + /* notifies client thread of new status of its async request */ void async_terminate( struct async *async, unsigned int status ) { struct iosb *iosb = async->iosb; - if (async->terminated) return; + if (async_terminated( async )) return; async->terminated = 1; if (async->iosb && async->iosb->status == STATUS_PENDING) async->iosb->status = status; @@ -491,7 +496,7 @@ 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->terminated ); /* it must have been woken up if we get a result */ + assert( async_terminated( async ) ); /* it must have been woken up if we get a result */ if (async->unknown_status) async_set_initial_status( async, status ); @@ -571,7 +576,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->terminated; + return !async_terminated( async ); } static int cancel_async( struct process *process, struct object *obj, struct thread *thread, client_ptr_t iosb ) @@ -586,7 +591,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->terminated || async->canceled || async->is_system) continue; + if (async_terminated( async ) || async->canceled || async->is_system) continue; if ((!obj || (get_fd_user( async->fd ) == obj)) && (!thread || async->thread == thread) && (!iosb || async->data.iosb == iosb)) @@ -608,7 +613,7 @@ 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 ) || async->canceled) continue; if (async->blocking && async->thread == thread && (!iosb || async->data.iosb == iosb)) { @@ -628,7 +633,7 @@ void cancel_process_asyncs( struct process *process ) restart: LIST_FOR_EACH_ENTRY( async, &process->asyncs, struct async, process_entry ) { - if (async->terminated || async->canceled) continue; + if (async_terminated( async ) || async->canceled) continue; async->canceled = 1; fd_cancel_async( async->fd, async ); goto restart; @@ -647,7 +652,7 @@ 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 ) || async->canceled || get_fd_user( async->fd ) != obj) continue; if (!async->completion || !async->data.apc_context || async->event) continue; async->canceled = 1; @@ -664,7 +669,7 @@ 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 ) || async->canceled) continue; if (async->completion && async->data.apc_context && !async->event) continue; if (async->is_system) continue; @@ -793,7 +798,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->terminated) return (struct async *)grab_object( async ); + if (!async_terminated( async )) return (struct async *)grab_object( async ); return NULL; } @@ -863,7 +868,7 @@ DECL_HANDLER(set_async_direct_result) if (!async) return; - if (!async->unknown_status || !async->terminated || !async->alerted) + if (!async->unknown_status || !async_terminated( async ) || !async->alerted) { set_error( STATUS_INVALID_PARAMETER ); release_object( &async->obj ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6369
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This helper will later be rewritten to compute the flag from a state enum. --- server/async.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/server/async.c b/server/async.c index 9c7218a2e54..4754d9f2a86 100644 --- a/server/async.c +++ b/server/async.c @@ -166,6 +166,11 @@ static int async_terminated( const struct async *async ) return async->terminated; } +static int async_alerted( const struct async *async ) +{ + return async->alerted; +} + /* notifies client thread of new status of its async request */ void async_terminate( struct async *async, unsigned int status ) { @@ -500,7 +505,7 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota if (async->unknown_status) async_set_initial_status( async, status ); - if (async->alerted && status == STATUS_PENDING) /* restart it */ + if (async_alerted( async ) && status == STATUS_PENDING) /* restart it */ { async->terminated = 0; async->alerted = 0; @@ -868,7 +873,7 @@ DECL_HANDLER(set_async_direct_result) if (!async) return; - if (!async->unknown_status || !async_terminated( async ) || !async->alerted) + if (!async->unknown_status || !async_terminated( async ) || !async_alerted( async )) { set_error( STATUS_INVALID_PARAMETER ); release_object( &async->obj ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6369
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This helper will later be rewritten to compute the flag from a state enum. --- server/async.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/server/async.c b/server/async.c index 4754d9f2a86..a664b6bc93c 100644 --- a/server/async.c +++ b/server/async.c @@ -171,6 +171,11 @@ static int async_alerted( const struct async *async ) return async->alerted; } +static int async_unknown_status( const struct async *async ) +{ + return async->unknown_status; +} + /* notifies client thread of new status of its async request */ void async_terminate( struct async *async, unsigned int status ) { @@ -326,7 +331,7 @@ void set_async_pending( struct async *async ) void async_wake_obj( struct async *async ) { - assert( !async->unknown_status ); + assert( !async_unknown_status( async ) ); if (!async->blocking) { async->signaled = 1; @@ -346,7 +351,7 @@ obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_ { async->blocking = force_blocking || async->blocking; - if (async->unknown_status) + if (async_unknown_status( async )) { /* even the initial status is not known yet */ set_error( STATUS_PENDING ); @@ -503,7 +508,7 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota assert( async_terminated( async ) ); /* it must have been woken up if we get a result */ - if (async->unknown_status) async_set_initial_status( async, status ); + if (async_unknown_status( async )) async_set_initial_status( async, status ); if (async_alerted( async ) && status == STATUS_PENDING) /* restart it */ { @@ -568,7 +573,7 @@ int async_queue_has_waiting_asyncs( struct async_queue *queue ) struct async *async; LIST_FOR_EACH_ENTRY( async, &queue->queue, struct async, queue_entry ) - if (!async->unknown_status) return 1; + if (!async_unknown_status( async )) return 1; return 0; } @@ -873,7 +878,7 @@ DECL_HANDLER(set_async_direct_result) if (!async) return; - if (!async->unknown_status || !async_terminated( async ) || !async_alerted( async )) + if (!async_unknown_status( async ) || !async_terminated( async ) || !async_alerted( async )) { set_error( STATUS_INVALID_PARAMETER ); release_object( &async->obj ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6369
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- server/async.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+) diff --git a/server/async.c b/server/async.c index a664b6bc93c..68c248728d0 100644 --- a/server/async.c +++ b/server/async.c @@ -34,6 +34,48 @@ #include "process.h" #include "handle.h" +enum async_state +{ + /* The I/O operation has just initiated, or initial status is known. */ + ASYNC_INITIAL, + + /* The I/O operation has just initiated via create_request_async(). */ + ASYNC_INITIAL_DIRECT_RESULT, + + /* The initial status is not known yet. It will be determined by the client or winedevice.exe. */ + ASYNC_UNKNOWN_STATUS, + + /* The I/O operation is being processed by the wineserver or winedevice.eze. */ + ASYNC_IN_PROGRESS, + + /* The I/O operation is being processed by the client. */ + ASYNC_ALERTED, + + /* The I/O operation has finished synchronously (APC result) but not been + * acknowledged by the client. + * + * The completion is being delivered to the client indirectly via + * APC_ASYNC_IO. The client had no chance to fill the IOSB synchronously, + * due to unknwon initial status (e.g., processed by winedevice.exe). + */ + ASYNC_FINALIZING_SYNC_APC_RESULT, + + /* The I/O operation has finished synchronously (direct result) but not + * been acknowledged by the client. + * + * The completion is being delivered to the client directly via server + * request return. The client may proceed to fill the IOSB synchronously, + * and notify the server that it has done so. + */ + ASYNC_FINALIZING_SYNC_DIRECT_RESULT, + + /* The I/O operation has finished asynchronously but not been acknowledged by the client. */ + ASYNC_FINALIZING_ASYNC, + + /* The I/O operation has finished and the result has been acknowledged by the client. */ + ASYNC_COMPLETED +}; + struct async { struct object obj; /* object header */ @@ -49,6 +91,7 @@ struct async 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 */ + enum async_state state; 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 */ @@ -110,6 +153,32 @@ static int async_signaled( struct object *obj, struct wait_queue_entry *entry ) { struct async *async = (struct async *)obj; assert( obj->ops == &async_ops ); + switch (async->state) + { + case ASYNC_INITIAL: + case ASYNC_INITIAL_DIRECT_RESULT: + case ASYNC_UNKNOWN_STATUS: + case ASYNC_FINALIZING_SYNC_APC_RESULT: + assert( async->signaled == 0 ); + break; + case ASYNC_FINALIZING_SYNC_DIRECT_RESULT: + /* The client will "wait" on the async to signal completion. */ + assert( async->signaled == 1 ); + break; + case ASYNC_IN_PROGRESS: + case ASYNC_ALERTED: + case ASYNC_FINALIZING_ASYNC: + /* If nonblocking, the client will "wait" on the async to close it. */ + assert( async->signaled == (!async->blocking) ); + break; + case ASYNC_COMPLETED: + /* If there is an open async handle, notify the waiter of completion. */ + assert( async->signaled == 1 ); + break; + default: + assert( 0 ); + break; + } return async->signaled; } @@ -121,6 +190,7 @@ static void async_satisfied( struct object *obj, struct wait_queue_entry *entry /* we only return an async handle for asyncs created via create_request_async() */ assert( async->iosb ); + assert( async->direct_result == (async->state == ASYNC_FINALIZING_SYNC_DIRECT_RESULT) ); if (async->direct_result) { async_set_result( &async->obj, async->iosb->status, async->iosb->result ); @@ -163,16 +233,58 @@ static void async_destroy( struct object *obj ) static int async_terminated( const struct async *async ) { + switch (async->state) + { + case ASYNC_INITIAL: + case ASYNC_INITIAL_DIRECT_RESULT: + case ASYNC_IN_PROGRESS: + assert( async->terminated == 0 ); + break; + case ASYNC_UNKNOWN_STATUS: + case ASYNC_ALERTED: + case ASYNC_FINALIZING_SYNC_APC_RESULT: + case ASYNC_FINALIZING_SYNC_DIRECT_RESULT: + case ASYNC_FINALIZING_ASYNC: + case ASYNC_COMPLETED: + assert( async->terminated == 1 ); + break; + default: + assert( 0 ); + break; + } return async->terminated; } static int async_alerted( const struct async *async ) { + switch (async->state) + { + case ASYNC_INITIAL_DIRECT_RESULT: + case ASYNC_IN_PROGRESS: + case ASYNC_FINALIZING_SYNC_APC_RESULT: + case ASYNC_FINALIZING_SYNC_DIRECT_RESULT: + case ASYNC_FINALIZING_ASYNC: + case ASYNC_COMPLETED: + assert( async->alerted == 0 ); + break; + case ASYNC_INITIAL: + /* initial status set from set_async_direct_result */ + assert( async->alerted == 1 ); + break; + case ASYNC_UNKNOWN_STATUS: + case ASYNC_ALERTED: + assert( async->alerted == 1 ); + break; + default: + assert( 0 ); + break; + } return async->alerted; } static int async_unknown_status( const struct async *async ) { + assert( async->unknown_status == (async->state == ASYNC_UNKNOWN_STATUS) ); return async->unknown_status; } @@ -182,17 +294,50 @@ void async_terminate( struct async *async, unsigned int status ) struct iosb *iosb = async->iosb; if (async_terminated( async )) return; + assert( async->state == ASYNC_INITIAL || async->state == ASYNC_INITIAL_DIRECT_RESULT || async->state == ASYNC_IN_PROGRESS ); + + if (status == STATUS_ALERTED) + { + assert( async->direct_result == (async->state == ASYNC_INITIAL_DIRECT_RESULT) ); + if (async->direct_result) + { + assert( async->unknown_status ); + async->state = ASYNC_UNKNOWN_STATUS; + } + else + { + assert( !async->unknown_status ); + async->state = ASYNC_ALERTED; + } + } + else + { + if (async->state == ASYNC_INITIAL) + async->state = ASYNC_FINALIZING_SYNC_APC_RESULT; + else if (async->state == ASYNC_INITIAL_DIRECT_RESULT) + async->state = ASYNC_FINALIZING_SYNC_DIRECT_RESULT; + else + async->state = ASYNC_FINALIZING_ASYNC; + } async->terminated = 1; if (async->iosb && async->iosb->status == STATUS_PENDING) async->iosb->status = status; if (status == STATUS_ALERTED) async->alerted = 1; + if (async->state == ASYNC_FINALIZING_SYNC_DIRECT_RESULT || + (async->state == ASYNC_ALERTED || async->state == ASYNC_FINALIZING_ASYNC) && !async->blocking) + { + async->signaled = 1; + wake_up( &async->obj, 0 ); + } + /* 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 ); + assert( (!async->direct_result) == (async->state != ASYNC_FINALIZING_SYNC_DIRECT_RESULT && async->state != ASYNC_UNKNOWN_STATUS) ); if (!async->direct_result) { apc_call_t data; @@ -284,6 +429,7 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da async->queue = NULL; async->fd = (struct fd *)grab_object( fd ); async->initial_status = STATUS_PENDING; + async->state = ASYNC_INITIAL; async->signaled = 0; async->pending = 1; async->wait_handle = 0; @@ -321,10 +467,13 @@ void async_set_initial_status( struct async *async, unsigned int status ) { async->initial_status = status; async->unknown_status = 0; + if (async->state == ASYNC_UNKNOWN_STATUS) + async->state = ASYNC_INITIAL; } void set_async_pending( struct async *async ) { + assert( (!async->terminated) == (async->state == ASYNC_INITIAL_DIRECT_RESULT || async->state == ASYNC_UNKNOWN_STATUS) ); if (!async->terminated) async->pending = 1; } @@ -332,6 +481,8 @@ void set_async_pending( struct async *async ) void async_wake_obj( struct async *async ) { assert( !async_unknown_status( async ) ); + assert( async->state == ASYNC_INITIAL ); + async->state = ASYNC_IN_PROGRESS; if (!async->blocking) { async->signaled = 1; @@ -417,6 +568,8 @@ obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_ } else { + assert( async->state == ASYNC_INITIAL_DIRECT_RESULT ); + async->state = ASYNC_IN_PROGRESS; async->direct_result = 0; async->pending = 1; if (!async->blocking) @@ -472,6 +625,8 @@ void async_request_complete_alloc( struct async *async, unsigned int status, dat /* mark an async as having unknown initial status */ void async_set_unknown_status( struct async *async ) { + assert( async->state == ASYNC_INITIAL_DIRECT_RESULT ); + async->state = ASYNC_UNKNOWN_STATUS; async->unknown_status = 1; async->direct_result = 0; } @@ -512,6 +667,8 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota if (async_alerted( async ) && status == STATUS_PENDING) /* restart it */ { + assert( async->state == ASYNC_INITIAL || async->state == ASYNC_ALERTED ); + async->state = ASYNC_IN_PROGRESS; async->terminated = 0; async->alerted = 0; async_reselect( async ); @@ -549,6 +706,7 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota else if (async->fd && !async->is_system) set_fd_signaled( async->fd, 1 ); } + async->state = ASYNC_COMPLETED; if (!async->signaled) { async->signaled = 1; @@ -786,6 +944,7 @@ struct async *create_request_async( struct fd *fd, unsigned int comp_flags, cons return NULL; } async->pending = 0; + async->state = ASYNC_INITIAL_DIRECT_RESULT; async->direct_result = 1; async->is_system = !!is_system; async->comp_flags = comp_flags; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6369
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This helps simplify further refactoring that gets rid of 'signaled' flag entirely. --- server/async.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/server/async.c b/server/async.c index 68c248728d0..336bf10ac1a 100644 --- a/server/async.c +++ b/server/async.c @@ -707,11 +707,8 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota } async->state = ASYNC_COMPLETED; - if (!async->signaled) - { - async->signaled = 1; - wake_up( &async->obj, 0 ); - } + async->signaled = 1; + wake_up( &async->obj, 0 ); async_call_completion_callback( async ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6369
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- server/async.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/server/async.c b/server/async.c index 336bf10ac1a..3a37c105a2b 100644 --- a/server/async.c +++ b/server/async.c @@ -94,7 +94,6 @@ struct async enum async_state state; 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 */ unsigned int terminated :1; /* async has been terminated */ unsigned int canceled :1; /* have we already queued cancellation for this async? */ @@ -190,11 +189,9 @@ static void async_satisfied( struct object *obj, struct wait_queue_entry *entry /* we only return an async handle for asyncs created via create_request_async() */ assert( async->iosb ); - assert( async->direct_result == (async->state == ASYNC_FINALIZING_SYNC_DIRECT_RESULT) ); - if (async->direct_result) + if (async->state == ASYNC_FINALIZING_SYNC_DIRECT_RESULT) { async_set_result( &async->obj, async->iosb->status, async->iosb->result ); - async->direct_result = 0; } if (async->initial_status == STATUS_PENDING && async->blocking) @@ -298,8 +295,7 @@ void async_terminate( struct async *async, unsigned int status ) if (status == STATUS_ALERTED) { - assert( async->direct_result == (async->state == ASYNC_INITIAL_DIRECT_RESULT) ); - if (async->direct_result) + if (async->state == ASYNC_INITIAL_DIRECT_RESULT) { assert( async->unknown_status ); async->state = ASYNC_UNKNOWN_STATUS; @@ -337,8 +333,7 @@ void async_terminate( struct async *async, unsigned int status ) * last reference to the async, so grab a temporary reference here */ grab_object( async ); - assert( (!async->direct_result) == (async->state != ASYNC_FINALIZING_SYNC_DIRECT_RESULT && async->state != ASYNC_UNKNOWN_STATUS) ); - if (!async->direct_result) + if (async->state != ASYNC_FINALIZING_SYNC_DIRECT_RESULT && async->state != ASYNC_UNKNOWN_STATUS) { apc_call_t data; @@ -433,7 +428,6 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da async->signaled = 0; async->pending = 1; async->wait_handle = 0; - async->direct_result = 0; async->alerted = 0; async->terminated = 0; async->canceled = 0; @@ -524,10 +518,10 @@ obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_ * the one responsible for performing the I/O is not the device driver, * but instead the client that requested the I/O in the first place. * - * also, async_set_unknown_status() would set direct_result to zero - * forcing APC_ASYNC_IO to fire in async_terminate(), which is not - * useful due to subtle semantic differences between synchronous and - * asynchronous completion. + * also, async_set_unknown_status() would eventually force APC_ASYNC_IO + * to fire in async_terminate(), which is not useful due to subtle + * semantic differences between synchronous and asynchronous + * completion. */ async->unknown_status = 1; async_terminate( async, STATUS_ALERTED ); @@ -570,7 +564,6 @@ obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_ { assert( async->state == ASYNC_INITIAL_DIRECT_RESULT ); async->state = ASYNC_IN_PROGRESS; - async->direct_result = 0; async->pending = 1; if (!async->blocking) { @@ -628,7 +621,6 @@ void async_set_unknown_status( struct async *async ) assert( async->state == ASYNC_INITIAL_DIRECT_RESULT ); async->state = ASYNC_UNKNOWN_STATUS; async->unknown_status = 1; - async->direct_result = 0; } /* set the timeout of an async operation */ @@ -942,7 +934,6 @@ struct async *create_request_async( struct fd *fd, unsigned int comp_flags, cons } async->pending = 0; async->state = ASYNC_INITIAL_DIRECT_RESULT; - async->direct_result = 1; async->is_system = !!is_system; async->comp_flags = comp_flags; } @@ -1043,7 +1034,6 @@ DECL_HANDLER(set_async_direct_result) if (status == STATUS_PENDING) { - async->direct_result = 0; async->pending = 1; } else if (req->mark_pending) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6369
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- server/async.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/server/async.c b/server/async.c index 3a37c105a2b..7f0970a7db9 100644 --- a/server/async.c +++ b/server/async.c @@ -92,7 +92,6 @@ struct async obj_handle_t wait_handle; /* pre-allocated wait handle */ unsigned int initial_status; /* status returned from initial request */ enum async_state state; - unsigned int signaled :1; unsigned int pending :1; /* request successfully queued, but pending */ unsigned int alerted :1; /* fd is signaled, but we are waiting for client-side I/O */ unsigned int terminated :1; /* async has been terminated */ @@ -158,27 +157,22 @@ static int async_signaled( struct object *obj, struct wait_queue_entry *entry ) case ASYNC_INITIAL_DIRECT_RESULT: case ASYNC_UNKNOWN_STATUS: case ASYNC_FINALIZING_SYNC_APC_RESULT: - assert( async->signaled == 0 ); - break; + return 0; case ASYNC_FINALIZING_SYNC_DIRECT_RESULT: /* The client will "wait" on the async to signal completion. */ - assert( async->signaled == 1 ); - break; + return 1; case ASYNC_IN_PROGRESS: case ASYNC_ALERTED: case ASYNC_FINALIZING_ASYNC: /* If nonblocking, the client will "wait" on the async to close it. */ - assert( async->signaled == (!async->blocking) ); - break; + return !async->blocking; case ASYNC_COMPLETED: /* If there is an open async handle, notify the waiter of completion. */ - assert( async->signaled == 1 ); - break; + return 1; default: assert( 0 ); - break; + return 0; } - return async->signaled; } static void async_satisfied( struct object *obj, struct wait_queue_entry *entry ) @@ -324,7 +318,6 @@ void async_terminate( struct async *async, unsigned int status ) if (async->state == ASYNC_FINALIZING_SYNC_DIRECT_RESULT || (async->state == ASYNC_ALERTED || async->state == ASYNC_FINALIZING_ASYNC) && !async->blocking) { - async->signaled = 1; wake_up( &async->obj, 0 ); } @@ -425,7 +418,6 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da async->fd = (struct fd *)grab_object( fd ); async->initial_status = STATUS_PENDING; async->state = ASYNC_INITIAL; - async->signaled = 0; async->pending = 1; async->wait_handle = 0; async->alerted = 0; @@ -479,7 +471,6 @@ void async_wake_obj( struct async *async ) async->state = ASYNC_IN_PROGRESS; if (!async->blocking) { - async->signaled = 1; wake_up( &async->obj, 0 ); } } @@ -558,7 +549,6 @@ obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_ if (async->iosb->status != STATUS_PENDING) { if (result) *result = async->iosb->result; - async->signaled = 1; } else { @@ -699,7 +689,6 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota } async->state = ASYNC_COMPLETED; - async->signaled = 1; wake_up( &async->obj, 0 ); async_call_completion_callback( async ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6369
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- server/async.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/server/async.c b/server/async.c index 7f0970a7db9..e515629dcda 100644 --- a/server/async.c +++ b/server/async.c @@ -94,7 +94,6 @@ struct async enum async_state state; unsigned int pending :1; /* request successfully queued, but pending */ 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 unknown_status :1; /* initial status is not known yet */ unsigned int blocking :1; /* async is blocking */ @@ -229,21 +228,18 @@ static int async_terminated( const struct async *async ) case ASYNC_INITIAL: case ASYNC_INITIAL_DIRECT_RESULT: case ASYNC_IN_PROGRESS: - assert( async->terminated == 0 ); - break; + return 0; case ASYNC_UNKNOWN_STATUS: case ASYNC_ALERTED: case ASYNC_FINALIZING_SYNC_APC_RESULT: case ASYNC_FINALIZING_SYNC_DIRECT_RESULT: case ASYNC_FINALIZING_ASYNC: case ASYNC_COMPLETED: - assert( async->terminated == 1 ); - break; + return 1; default: assert( 0 ); - break; + return 0; } - return async->terminated; } static int async_alerted( const struct async *async ) @@ -310,7 +306,6 @@ void async_terminate( struct async *async, unsigned int status ) async->state = ASYNC_FINALIZING_ASYNC; } - async->terminated = 1; if (async->iosb && async->iosb->status == STATUS_PENDING) async->iosb->status = status; if (status == STATUS_ALERTED) async->alerted = 1; @@ -421,7 +416,6 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da async->pending = 1; async->wait_handle = 0; async->alerted = 0; - async->terminated = 0; async->canceled = 0; async->unknown_status = 0; async->blocking = !is_fd_overlapped( fd ); @@ -459,8 +453,7 @@ void async_set_initial_status( struct async *async, unsigned int status ) void set_async_pending( struct async *async ) { - assert( (!async->terminated) == (async->state == ASYNC_INITIAL_DIRECT_RESULT || async->state == ASYNC_UNKNOWN_STATUS) ); - if (!async->terminated) + if (async->state == ASYNC_INITIAL_DIRECT_RESULT || async->state == ASYNC_UNKNOWN_STATUS) async->pending = 1; } @@ -651,7 +644,6 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota { assert( async->state == ASYNC_INITIAL || async->state == ASYNC_ALERTED ); async->state = ASYNC_IN_PROGRESS; - async->terminated = 0; async->alerted = 0; async_reselect( async ); } @@ -659,7 +651,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->terminated = 1; if (async->iosb) async->iosb->status = status; /* don't signal completion if the async failed synchronously -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6369
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- server/async.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/server/async.c b/server/async.c index e515629dcda..23bc257a7a5 100644 --- a/server/async.c +++ b/server/async.c @@ -93,7 +93,6 @@ struct async unsigned int initial_status; /* status returned from initial request */ enum async_state state; unsigned int pending :1; /* request successfully queued, but pending */ - unsigned int alerted :1; /* fd is signaled, but we are waiting for client-side I/O */ unsigned int canceled :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 */ @@ -252,21 +251,17 @@ static int async_alerted( const struct async *async ) case ASYNC_FINALIZING_SYNC_DIRECT_RESULT: case ASYNC_FINALIZING_ASYNC: case ASYNC_COMPLETED: - assert( async->alerted == 0 ); - break; + return 0; case ASYNC_INITIAL: /* initial status set from set_async_direct_result */ - assert( async->alerted == 1 ); - break; + return 1; case ASYNC_UNKNOWN_STATUS: case ASYNC_ALERTED: - assert( async->alerted == 1 ); - break; + return 1; default: assert( 0 ); - break; + return 0; } - return async->alerted; } static int async_unknown_status( const struct async *async ) @@ -307,8 +302,6 @@ void async_terminate( struct async *async, unsigned int status ) } if (async->iosb && async->iosb->status == STATUS_PENDING) async->iosb->status = status; - if (status == STATUS_ALERTED) - async->alerted = 1; if (async->state == ASYNC_FINALIZING_SYNC_DIRECT_RESULT || (async->state == ASYNC_ALERTED || async->state == ASYNC_FINALIZING_ASYNC) && !async->blocking) @@ -415,7 +408,6 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da async->state = ASYNC_INITIAL; async->pending = 1; async->wait_handle = 0; - async->alerted = 0; async->canceled = 0; async->unknown_status = 0; async->blocking = !is_fd_overlapped( fd ); @@ -644,7 +636,6 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota { assert( async->state == ASYNC_INITIAL || async->state == ASYNC_ALERTED ); async->state = ASYNC_IN_PROGRESS; - async->alerted = 0; async_reselect( async ); } else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6369
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- server/async.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/server/async.c b/server/async.c index 23bc257a7a5..cbc4607327c 100644 --- a/server/async.c +++ b/server/async.c @@ -94,7 +94,6 @@ struct async enum async_state state; unsigned int pending :1; /* request successfully queued, but pending */ unsigned int canceled :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. */ struct completion *completion; /* completion associated with fd */ @@ -266,8 +265,7 @@ static int async_alerted( const struct async *async ) static int async_unknown_status( const struct async *async ) { - assert( async->unknown_status == (async->state == ASYNC_UNKNOWN_STATUS) ); - return async->unknown_status; + return async->state == ASYNC_UNKNOWN_STATUS; } /* notifies client thread of new status of its async request */ @@ -282,12 +280,10 @@ void async_terminate( struct async *async, unsigned int status ) { if (async->state == ASYNC_INITIAL_DIRECT_RESULT) { - assert( async->unknown_status ); async->state = ASYNC_UNKNOWN_STATUS; } else { - assert( !async->unknown_status ); async->state = ASYNC_ALERTED; } } @@ -409,7 +405,6 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da async->pending = 1; async->wait_handle = 0; async->canceled = 0; - async->unknown_status = 0; async->blocking = !is_fd_overlapped( fd ); async->is_system = 0; async->completion = fd_get_completion( fd, &async->comp_key ); @@ -438,7 +433,6 @@ struct async *create_async( struct fd *fd, struct thread *thread, const async_da void async_set_initial_status( struct async *async, unsigned int status ) { async->initial_status = status; - async->unknown_status = 0; if (async->state == ASYNC_UNKNOWN_STATUS) async->state = ASYNC_INITIAL; } @@ -489,17 +483,16 @@ obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_ * instead. * * since we're deferring the initial I/O (to the client), we mark the - * async as having unknown initial status (unknown_status = 1). note - * that we don't reuse async_set_unknown_status() here. this is because - * the one responsible for performing the I/O is not the device driver, - * but instead the client that requested the I/O in the first place. + * async as having unknown initial status. note that we don't reuse + * async_set_unknown_status() here. this is because the one + * responsible for performing the I/O is not the device driver, but + * instead the client that requested the I/O in the first place. * * also, async_set_unknown_status() would eventually force APC_ASYNC_IO * to fire in async_terminate(), which is not useful due to subtle * semantic differences between synchronous and asynchronous * completion. */ - async->unknown_status = 1; async_terminate( async, STATUS_ALERTED ); return async->wait_handle; } @@ -595,7 +588,6 @@ void async_set_unknown_status( struct async *async ) { assert( async->state == ASYNC_INITIAL_DIRECT_RESULT ); async->state = ASYNC_UNKNOWN_STATUS; - async->unknown_status = 1; } /* set the timeout of an async operation */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6369
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- server/async.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/server/async.c b/server/async.c index cbc4607327c..87fbb9d2ae2 100644 --- a/server/async.c +++ b/server/async.c @@ -263,11 +263,6 @@ static int async_alerted( const struct async *async ) } } -static int async_unknown_status( const struct async *async ) -{ - return async->state == ASYNC_UNKNOWN_STATUS; -} - /* notifies client thread of new status of its async request */ void async_terminate( struct async *async, unsigned int status ) { @@ -445,7 +440,6 @@ void set_async_pending( struct async *async ) void async_wake_obj( struct async *async ) { - assert( !async_unknown_status( async ) ); assert( async->state == ASYNC_INITIAL ); async->state = ASYNC_IN_PROGRESS; if (!async->blocking) @@ -466,7 +460,7 @@ obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_ { async->blocking = force_blocking || async->blocking; - if (async_unknown_status( async )) + if (async->state == ASYNC_UNKNOWN_STATUS) { /* even the initial status is not known yet */ set_error( STATUS_PENDING ); @@ -622,7 +616,7 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota assert( async_terminated( async ) ); /* it must have been woken up if we get a result */ - if (async_unknown_status( async )) async_set_initial_status( async, status ); + if (async->state == ASYNC_UNKNOWN_STATUS) async_set_initial_status( async, status ); if (async_alerted( async ) && status == STATUS_PENDING) /* restart it */ { @@ -683,7 +677,7 @@ int async_queue_has_waiting_asyncs( struct async_queue *queue ) struct async *async; LIST_FOR_EACH_ENTRY( async, &queue->queue, struct async, queue_entry ) - if (!async_unknown_status( async )) return 1; + if (async->state != ASYNC_UNKNOWN_STATUS) return 1; return 0; } @@ -988,7 +982,7 @@ DECL_HANDLER(set_async_direct_result) if (!async) return; - if (!async_unknown_status( async ) || !async_terminated( async ) || !async_alerted( async )) + if (async->state != ASYNC_UNKNOWN_STATUS) { set_error( STATUS_INVALID_PARAMETER ); release_object( &async->obj ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6369
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- server/async.c | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/server/async.c b/server/async.c index 87fbb9d2ae2..74dca104628 100644 --- a/server/async.c +++ b/server/async.c @@ -240,29 +240,6 @@ static int async_terminated( const struct async *async ) } } -static int async_alerted( const struct async *async ) -{ - switch (async->state) - { - case ASYNC_INITIAL_DIRECT_RESULT: - case ASYNC_IN_PROGRESS: - case ASYNC_FINALIZING_SYNC_APC_RESULT: - case ASYNC_FINALIZING_SYNC_DIRECT_RESULT: - case ASYNC_FINALIZING_ASYNC: - case ASYNC_COMPLETED: - return 0; - case ASYNC_INITIAL: - /* initial status set from set_async_direct_result */ - return 1; - case ASYNC_UNKNOWN_STATUS: - case ASYNC_ALERTED: - return 1; - default: - assert( 0 ); - return 0; - } -} - /* notifies client thread of new status of its async request */ void async_terminate( struct async *async, unsigned int status ) { @@ -617,10 +594,11 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota assert( async_terminated( async ) ); /* it must have been woken up if we get a result */ if (async->state == ASYNC_UNKNOWN_STATUS) async_set_initial_status( async, status ); + assert( async->state != ASYNC_UNKNOWN_STATUS ); - if (async_alerted( async ) && status == STATUS_PENDING) /* restart it */ + if ((async->state == ASYNC_INITIAL || async->state == ASYNC_ALERTED) && + status == STATUS_PENDING) /* restart it */ { - assert( async->state == ASYNC_INITIAL || async->state == ASYNC_ALERTED ); async->state = ASYNC_IN_PROGRESS; async_reselect( async ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6369
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=148754 Your paranoid android. === debian11b (64 bit WoW report) === kernel32: comm.c:1574: Test failed: AbortWaitCts hComPortEvent failed comm.c:1586: Test failed: Unexpected time 1001, expected around 500 user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 0000000000D60104, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
participants (3)
-
Jinoh Kang -
Jinoh Kang (＠iamahuman)
-
Marvin