On 4/18/22 23:42, Bernhard Kölbl wrote:
Signed-off-by: Bernhard Kölbl besentv@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.