[PATCH v3 0/1] MR4133: mfmediaengine: Don't send event notify when engine is shutdown.

[Yuxuan Shui (＠yshui)]

--
  v3: mfmediaengine: Don't send event notify when engine is shutdown.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4133

[PATCH v3 1/1] mfmediaengine: Don't send event notify when engine is shutdown.

[Yuxuan Shui]

From: Yuxuan Shui <yshui(a)codeweavers.com>

I've also tried saving a cancel cookie from `BeginCreateObject*` and using it to cancel the
operation in engine shutdown. However there are 2 problems:

  1. Callback might still get called after `CancelObjectCreation`. Microsoft's
     [own documentation](https://learn.microsoft.com/en-us/windows/win32/api/mfidl/nf-mfidl-imfsource...)
     says as much:
       > Because these methods are asynchronous, however, they might be completed before the
       > operation can be canceled. Therefore, your callback might still be invoked after you
       > call this method.
  2. Our implementations of `BeginCreateObject*` are not cancellable. They call either
     `MFPutWorkItem` or `IMFByteStream_BeginRead`, neight of which provides cancellation.
     (and our `CancelObjectCreation` doesn't actually work. AsyncResult objects aren't added to
     `handler->results` list until they are completed, which means any attempts to cancel an
     yet-to-be-completed object creation will always fail with `MF_E_UNEXPECTED`)
---
 dlls/mfmediaengine/main.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/dlls/mfmediaengine/main.c b/dlls/mfmediaengine/main.c
index 5a63974967d..6dfe0d6093f 100644
--- a/dlls/mfmediaengine/main.c
+++ b/dlls/mfmediaengine/main.c
@@ -1356,6 +1356,14 @@ static HRESULT WINAPI media_engine_load_handler_Invoke(IMFAsyncCallback *iface,
 
     EnterCriticalSection(&engine->cs);
 
+    if (engine->flags & FLAGS_ENGINE_SHUT_DOWN)
+    {
+        LeaveCriticalSection(&engine->cs);
+        TRACE("Engine is shut down, stopping...\n");
+        IUnknown_Release(object);
+        return S_OK;
+    }
+
     engine->network_state = MF_MEDIA_ENGINE_NETWORK_LOADING;
     IMFMediaEngineNotify_EventNotify(engine->callback, MF_MEDIA_ENGINE_EVENT_LOADSTART, 0, 0);
 
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/4133

[Yuxuan Shui (＠yshui)]

On Tue Oct 24 21:39:06 2023 +0000, Yuxuan Shui wrote:
> changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/4133/diffs?diff_id=78366&start_sha=dad133f637c79a6fdc60d7ca530ead3de289e7ba#238bb11c24dbacb840d4252cb92aa8ea2d26e240_1383_1384)
> oops, sorry, i moved the wrong thing around.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4133#note_49687

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/mfmediaengine/main.c:
> EnterCriticalSection(&engine->cs);
>
> +    if (engine->flags & FLAGS_ENGINE_SHUT_DOWN)
> +    {
> +        LeaveCriticalSection(&engine->cs);
> +        TRACE("Engine is shut down, stopping...\n");
> +        IUnknown_Release(object);
> +        return S_OK;
> +    }

Now this is calling Release() on a null 'object'. I don't know if we need a trace message here, but if we need it, something else, we are not stopping anything here, in playback or thread/process terms.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4133#note_49840