[PATCH v4 3/4] windows.media.speech: Implement concurrency in IAsyncOperation.
Rémi Bernon
rbernon at codeweavers.com
Tue Apr 19 08:08:06 CDT 2022
On 4/18/22 23:42, Bernhard Kölbl wrote:
> Signed-off-by: Bernhard Kölbl <besentv at gmail.com>
> ---
> dlls/windows.media.speech/async.c | 123 ++++++++++++++++++++++-
> dlls/windows.media.speech/private.h | 4 +-
> dlls/windows.media.speech/recognizer.c | 7 +-
> dlls/windows.media.speech/tests/speech.c | 22 ++--
> 4 files changed, 141 insertions(+), 15 deletions(-)
>
> diff --git a/dlls/windows.media.speech/async.c b/dlls/windows.media.speech/async.c
> index cf0da23e493..66cba46d85b 100644
> --- a/dlls/windows.media.speech/async.c
> +++ b/dlls/windows.media.speech/async.c
> @@ -40,7 +40,13 @@ struct async_operation
> LONG ref;
>
> IAsyncOperationCompletedHandler_IInspectable *handler;
> + IInspectable *result;
>
> + async_operation_worker worker;
IMHO "callback" is more commonly used and more appropriate name than
"worker".
> + TP_WORK *async_run_work;
> + IInspectable *invoker;
> +
> + CRITICAL_SECTION cs;
> AsyncStatus status;
> HRESULT hr;
> };
> @@ -299,7 +345,65 @@ static const struct IAsyncInfoVtbl async_operation_info_vtbl =
> async_operation_info_Close
> };
>
> -HRESULT async_operation_create( const GUID *iid, IAsyncOperation_IInspectable **out )
> +static void CALLBACK async_run_cb(TP_CALLBACK_INSTANCE *instance, void *data, TP_WORK *work)
> +{
> + IAsyncOperation_IInspectable *operation = data;
> + struct async_operation *impl = impl_from_IAsyncOperation_IInspectable(operation);
> + IInspectable *result = NULL, *invoker = NULL;
> + async_operation_worker worker;
> + HRESULT hr;
> +
> + EnterCriticalSection(&impl->cs);
> + if (impl->status == Canceled)
> + goto abort;
> +
> + impl->status = Started;
Should it really ignore and overwrite the Closed status here?
Or perhaps instead treat it as cancellation, as it means the client
probably doesn't care about the result anymore?
> +
> + invoker = impl->invoker;
> + worker = impl->worker;
> + LeaveCriticalSection(&impl->cs);
> +
> + hr = worker(invoker, &result);
> +
> + EnterCriticalSection(&impl->cs);
> + if (FAILED(hr))
> + impl->status = Error;
> + else
> + impl->status = Completed;
> +
> + impl->result = result;
> + impl->hr = hr;
> +
> + if (impl->status == Canceled)
> + goto abort;
This condition will never be true as you just wrote the status above. I
think you can consider that if the callback had been called it's too
late for it to be cancelled.
> +
> + if (impl->handler != NULL && impl->handler != HANDLER_NOT_SET)
> + {
> + IAsyncOperationCompletedHandler_IInspectable *handler = impl->handler;
> + AsyncStatus status = impl->status;
> +
> + IAsyncOperation_IInspectable_AddRef(operation);
> + IAsyncOperationCompletedHandler_IInspectable_AddRef(handler);
> + LeaveCriticalSection(&impl->cs);
> +
> + IAsyncOperationCompletedHandler_IInspectable_Invoke(handler, operation, status);
> +
> + IAsyncOperationCompletedHandler_IInspectable_Release(handler);
> + IAsyncOperation_IInspectable_Release(operation);
I don't think you need to add all these references here, you already
hold one on the operation and on the handler.
> + }
> + else
> + LeaveCriticalSection(&impl->cs);
> +
> + IAsyncOperation_IInspectable_Release(operation);
> + return;
> +
> +abort:
> + impl->hr = E_FAIL;
> + LeaveCriticalSection(&impl->cs);
> + IAsyncOperation_IInspectable_Release(operation);
> +}
> +
It will be hard to check, but I think you need to invoke the handler
even in the Cancel case, as MSDN suggests:
> If you're implementing IAsyncOperation<TResult>, then the set
> implementation of Completed should store the handler, and the
> surrounding logic should invoke it when Close is called. The
> implementation should set the asyncStatus parameter of invoked
> callbacks appropriately if there is a Cancel call, Status is not
> Completed, errors occurred, and so on.
--
Rémi Bernon <rbernon at codeweavers.com>
More information about the wine-devel
mailing list