Re: [RFC PATCH v2 2/5] server: Add a new server request "notify_async_direct_result."

[Jinoh Kang]

On 1/28/22 04:05, Jinoh Kang wrote:
> Some I/O operations need a way to "queue" the async to the target object
> first, even if the operation itself is to be completed synchronously.
> After synchronous completion, it needs a way to notify the
> IO_STATUS_BLOCK values back to the server.
>
> Add a new wineserver request, "notify_async_direct_result", which
> notifies direct (i.e. synchronous) completion of async from the same
> thread.
> Signed-off-by: Jinoh Kang
>
> Notes:
>     v1 -> v2: dequeue async after failed completion
>
> server/async.c      | 84 ++++++++++++++++++++++++++++++++++++++++-----
>  server/protocol.def | 10 ++++++
>  2 files changed, 86 insertions(+), 8 deletions(-)
>
> diff --git a/server/async.c b/server/async.c
> index 6373e8c1ae4..714e82362c6 100644
> --- a/server/async.c
> +++ b/server/async.c
> @@ -473,6 +473,17 @@ static void add_async_completion( struct async *async, apc_param_t cvalue, unsig
>      if (async->completion) add_completion( async->completion, async->comp_key, cvalue, status, information );
>  }
>
> +static void async_dequeue( struct async *async )
> +{
> - if (!async->queue) return;
> -
> - list_remove( &async->queue_entry );
> - async_reselect( async );
> - async->fd = NULL;
> - async->queue = NULL;
> - release_object( async );
>
> +}
> -
>
> /* store the result of the client-side async callback */
>  void async_set_result( struct object *obj, unsigned int status, apc_param_t total )
>  {
> @@ -531,14 +542,7 @@ void async_set_result( struct object *obj, unsigned int status, apc_param_t tota
>              async->completion_callback( async->completion_callback_private );
>          async->completion_callback = NULL;
> -
>    if (async->queue)
> -
>    {
> -
>        list_remove( &async->queue_entry );
> -
>        async_reselect( async );
> -
>        async->fd = NULL;
> -
>        async->queue = NULL;
> -
>        release_object( async );
> -
>    }
> -
>    async_dequeue( async );
> }
>
> }
>
> @@ -750,3 +754,67 @@ DECL_HANDLER(get_async_result)
>      }
>      set_error( iosb->status );
>  }
> -
>
> +/* Notify direct completion of async and close the wait handle */
> +DECL_HANDLER(notify_async_direct_result)
> +{
> - struct async *async = (struct async *)get_handle_obj( current->process, req->handle, 0, &async_ops );
> -
> - if (!async) return;
> -
> - if (async->iosb && async->unknown_status && !async->pending && async->terminated)
> - {
> -
>    /* Reactivate async.  We call async_reselect() later. */
> -
>    async->terminated = 0;
> -
> -
>    /* Set result for async_handoff(). */
> -
>    set_error( req->status );
> -
>    async->iosb->result = req->information;
> -
> -
>    /* The async_handoff() call prior to the current server request was
> -
>     * effectively a no-op since async->unknown_status is 1.  Calling it
> -
>     * again with async->unknown_status = 0 will do the remaining steps.
> -
>     */
> -
>    async->unknown_status = 0;
> -
>    async_handoff( async, NULL, 0 );
> -
> -
>    if (get_error() == STATUS_PENDING)
> -
>    {
> -
>        async_reselect( async );
> -
>    }
> -
>    else if (NT_ERROR( get_error() ))
> -
>    {
> -
>        /* synchronous I/O failure: don't invoke callbacks, only dequeue it. */
> -
>        async_dequeue( async );

Note: we can't use async_set_result() here.  async_handoff() leaves async->terminated as 0, and async_set_result() has "assert( async->terminated )".

Thus, reusing async_set_result() here requires either:

1. Setting async->pending to 0 so that async_handoff() will call async_terminate().
   This is obviously incorrect since unwanted IOCP packets and APCs will fire on synchronous failure.

2. Not using async_handoff().  We have to copy a lot of code (particulary setting pending and direct_result flags) out of async_handoff() to do this.
> -
>    }
> -
>    else
> -
>    {
> -
>        /* I/O completed successfully.  The client has already set the IOSB,
> -
>         * so we can skip waiting on wait_handle and do async_set_result()
> -
>         * directly.
> -
>         *
> -
>         * If !async->direct_result, an APC_ASYNC_IO has been fired.
> -
>         * async_set_result() will be called when the APC returns.
> -
>         */
> -
>        if (async->direct_result)
> -
>        {
> -
>            async_set_result( &async->obj, async->iosb->status, async->iosb->result );
> -
>            async->direct_result = 0;
> -
>        }
> -
> -
>        /* close wait handle here to avoid extra server round trip */
> -
>        if (async->wait_handle)
> -
>        {
> -
>            close_handle( async->thread->process, async->wait_handle );
> -
>            async->wait_handle = 0;
> -
>        }
> -
>    }
> -
> -
>    /* The wait handle is preserved only when the status is STATUS_PENDING
> -
>     * and async->blocking is set (i.e. we're going to block on it). */
> -
>    reply->handle = async->wait_handle;
> - }
> - else set_error( STATUS_ACCESS_DENIED );
> -
> - release_object( &async->obj );
>
> +}
> diff --git a/server/protocol.def b/server/protocol.def
> index 02e73047f9b..2c3b8dbc619 100644
> --- a/server/protocol.def
> +++ b/server/protocol.def
> @@ -2163,6 +2163,16 @@ enum message_type
>  @END
>
> +/* Notify direct completion of async and close the wait handle */
> +@REQ(notify_async_direct_result)
> - obj_handle_t   handle;        /* wait handle */
> - unsigned int   status;        /* completion status */
> - apc_param_t    information;   /* IO_STATUS_BLOCK Information */
>
> +@REPLY
> - obj_handle_t   handle;        /* wait handle, or NULL if closed */
>
> +@END
> -
> -
>
> /* Perform a read on a file object */
>  @REQ(read)
>      async_data_t   async;         /* async I/O parameters */