[PATCH v2 0/12] MR6369: Draft: Refactor the async state flags
-- v2: 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..0e815397230 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_INITIAL_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_FINALIZING_SYNC_APC_RESULT && async->state != ASYNC_UNKNOWN_STATUS && !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) ); 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 || 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 0e815397230..6a32de99217 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 6a32de99217..702ded48251 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_INITIAL_DIRECT_RESULT || async->state == ASYNC_FINALIZING_SYNC_DIRECT_RESULT) ); - if (async->direct_result) + if (async->state == ASYNC_INITIAL_DIRECT_RESULT || 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) ); - if (!async->direct_result) + if (async->state != ASYNC_FINALIZING_SYNC_DIRECT_RESULT) { 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 702ded48251..e15c69d8fe5 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_FINALIZING_SYNC_APC_RESULT && async->state != ASYNC_UNKNOWN_STATUS && !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 e15c69d8fe5..0191a9dfdd1 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 || async->state == ASYNC_UNKNOWN_STATUS) ); - if (!async->terminated) + if (async->state == ASYNC_INITIAL || 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 0191a9dfdd1..111f19f87f6 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_FINALIZING_SYNC_APC_RESULT && async->state != ASYNC_UNKNOWN_STATUS && !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 111f19f87f6..239f6642984 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 239f6642984..ee0817afa2e 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 ee0817afa2e..fd9ca1afe81 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=148557 Your paranoid android. === debian11b (64 bit WoW report) === httpapi: httpapi.c:239: Test failed: Got error 5. httpapi.c:240: Test failed: Got size 0. httpapi.c:242: Test failed: Got flags 0xcccccccc. httpapi.c:245: Test failed: Got URL context cccccccccccccccc. httpapi.c:246: Test failed: Got major version 52428. httpapi.c:247: Test failed: Got major version 52428. httpapi.c:248: Test failed: Got verb 3435973836. httpapi.c:249: Test failed: Got unknown verb length 52428. httpapi.c:250: Test failed: Got raw URL length 52428. httpapi.c:251: Test failed: Got unknown verb httpapi:httpapi:002c done (-1073741819) in 0s 515B ieframe: ie: Timeout qmgr: job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:781: Test failed: got 0x800706ba job.c:802: Test failed: BITS job timed out job.c:813: Test failed: got 0x800706ba job.c:817: Test failed: got 0x800706ba job.c:820: Test failed: got 0x800706ba job.c:824: Test failed: got 0x800706ba job.c:828: Test failed: got 0x800706ba job.c:834: Test failed: got 0x800706ba job.c:837: Test failed: got 0x800706ba job.c:838: Test failed: unexpected state: 1 job.c:841: Test failed: got 0x800706ba job.c:846: Test failed: got 0x800706ba qmgr.c:191: Test failed: Failed to create Manager instance, skipping tests rpcrt4: server.c:1337: unhandled exception 00000d68 in child process 0030 Report validation errors: adsldp:ldap crashed (80000101) atl100:atl crashed (80000101) crypt32:cert crashed (80000101) crypt32:chain crashed (80000101) dplayx:dplayx has no test summary line (early exit of the main process?) dplayx:dplayx has unaccounted for todo messages dplayx:dplayx returned a non-zero exit code despite reporting no failures hnetcfg:policy has no test summary line (early exit of the main process?) hnetcfg:policy has unaccounted for todo messages hnetcfg:policy returned a non-zero exit code despite reporting no failures httpapi:httpapi has unaccounted for failure messages httpapi:httpapi has no done line (or it is garbled) ieframe:webbrowser crashed (80000101) kernel32:comm crashed (80000101) mf:mf crashed (80000101) mshtml:activex crashed (80000101) mshtml:dom crashed (80000101) mshtml:events crashed (80000101) mshtml:htmldoc crashed (80000101) mshtml:htmllocation crashed (80000101) mshtml:misc crashed (80000101) mshtml:script crashed (80000101) mshtml:style crashed (80000101) mshtml:xmlhttprequest crashed (80000101) msxml3:httpreq crashed (80000101) msxml3:xmlview crashed (80000101) ntoskrnl.exe:ntoskrnl has no test summary line (early exit of the main process?) ntoskrnl.exe:ntoskrnl returned a non-zero exit code despite reporting no failures rpcrt4:server has no test summary line (early exit of the main process?) secur32:schannel has no test summary line (early exit of the main process?) secur32:schannel has unaccounted for todo messages secur32:schannel returned a non-zero exit code despite reporting no failures urlmon:protocol crashed (80000101) urlmon:url crashed (80000101) webservices:channel crashed (80000101) webservices:proxy crashed (80000101) winhttp:notification crashed (80000101) winhttp:winhttp crashed (80000101) wininet:ftp crashed (80000101) wininet:http crashed (80000101) wldap32:parse crashed (80000101) ws2_32:afd crashed (80000101) ws2_32:sock crashed (80000101) wsdapi:discovery has no test summary line (early exit of the main process?) wsdapi:discovery returned a non-zero exit code despite reporting no failures
participants (3)
-
Jinoh Kang -
Jinoh Kang (＠iamahuman)
-
Marvin