[PATCH v2 0/4] MR2958: mf: Various refcount leak fixes for session.

[Bernhard Kölbl (＠besentv)]

--
  v2: mfmediaengine/tests: Add refcount checks for final release of media engine.

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

[PATCH v2 1/4] mf: Clear queued topologies on session shutdown.

[Bernhard Kölbl]

From: Bernhard Kölbl

This prevents media engine from leaking one reference to itself,
due to sharing its refcount with the sample grabber sink calback,
which was in turned gets refrenced through nodes in the queued topoligies.

Signed-off-by: Bernhard Kölbl 
---
 dlls/mf/session.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dlls/mf/session.c b/dlls/mf/session.c
index 8b29c8487fb..3641cc74b83 100644
--- a/dlls/mf/session.c
+++ b/dlls/mf/session.c
@@ -2225,6 +2225,7 @@ static HRESULT WINAPI mfsession_Shutdown(IMFMediaSession *iface)
         IMFPresentationClock_Release(session->clock);
         session->clock = NULL;
         session_clear_presentation(session);
+        session_clear_queued_topologies(session);
         session_submit_simple_command(session, SESSION_CMD_SHUTDOWN);
     }
     LeaveCriticalSection(&session->cs);

[PATCH v2 2/4] mf: Don't leak sink in session_get_renderer_node_service().

[Bernhard Kölbl]

From: Bernhard Kölbl

Signed-off-by: Bernhard Kölbl 
---
 dlls/mf/session.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dlls/mf/session.c b/dlls/mf/session.c
index 3641cc74b83..555a71dcdf7 100644
--- a/dlls/mf/session.c
+++ b/dlls/mf/session.c
@@ -2397,6 +2397,7 @@ static HRESULT session_get_renderer_node_service(struct media_session *session,
                             if (FAILED(hr = MFGetService((IUnknown *)sink, service, riid, obj)))
                                 WARN("Failed to get service from renderer node, %#lx.\n", hr);
                         }
+                        IMFMediaSink_Release(sink);
                     }
                     IMFStreamSink_Release(stream_sink);
                 }

[PATCH v2 3/4] mf: Don't make stream sink shutdown dependent on IMFActivate presence in node.

[Bernhard Kölbl]

From: Bernhard Kölbl

Nodes only hold a ref to an activate object when they are a sink anyway
(see session_bind_output_nodes() for reference).
This prevents media engine from getting references to its samplegrabber sink callback leaked.

Signed-off-by: Bernhard Kölbl 
---
 dlls/mf/session.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/mf/session.c b/dlls/mf/session.c
index 555a71dcdf7..efe568f987f 100644
--- a/dlls/mf/session.c
+++ b/dlls/mf/session.c
@@ -762,7 +762,7 @@ static void session_shutdown_current_topology(struct media_session *session)
                         WARN("Failed to shut down activation object for the sink, hr %#lx.\n", hr);
                     IMFActivate_Release(activate);
                 }
-                else if (SUCCEEDED(topology_node_get_object(node, &IID_IMFStreamSink, (void **)&stream_sink)))
+                if (SUCCEEDED(topology_node_get_object(node, &IID_IMFStreamSink, (void **)&stream_sink)))
                 {
                     if (SUCCEEDED(IMFStreamSink_GetMediaSink(stream_sink, &sink)))
                     {

[PATCH v2 4/4] mfmediaengine/tests: Add refcount checks for final release of media engine.

[Bernhard Kölbl]

From: Bernhard Kölbl

Signed-off-by: Bernhard Kölbl 
---
 dlls/mfmediaengine/tests/mfmediaengine.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/dlls/mfmediaengine/tests/mfmediaengine.c b/dlls/mfmediaengine/tests/mfmediaengine.c
index 655b19af050..b09cc45f434 100644
--- a/dlls/mfmediaengine/tests/mfmediaengine.c
+++ b/dlls/mfmediaengine/tests/mfmediaengine.c
@@ -449,6 +449,7 @@ static void test_Shutdown(void)
     UINT32 value;
     double val;
     HRESULT hr;
+    ULONG ref;
     BSTR str;
     BOOL ret;

@@ -625,7 +626,9 @@ static void test_Shutdown(void)
         IMFMediaEngineEx_Release(media_engine_ex);
     }

-    IMFMediaEngine_Release(media_engine);
+    ref = IMFMediaEngine_Release(media_engine);
+    ok(!ref, "Unexpected ref %lu.\n", ref);
+
     IMFMediaEngineNotify_Release(&notify->IMFMediaEngineNotify_iface);
 }

@@ -1227,6 +1230,7 @@ static void test_TransferVideoFrame(void)
     UINT token;
     HRESULT hr;
     DWORD res;
+    ULONG ref;

stream = load_resource(L"i420-64x64.avi", L"video/avi");

@@ -1318,7 +1322,8 @@ static void test_TransferVideoFrame(void)

done:
     IMFMediaEngineEx_Shutdown(media_engine);
-    IMFMediaEngineEx_Release(media_engine);
+    ref = IMFMediaEngineEx_Release(media_engine);
+    ok(!ref, "Unexpected ref %lu.\n", ref);

ID3D11Texture2D_Release(texture);
     ID3D11Device_Release(device);

[PATCH v2 4/4] mfmediaengine/tests: Add refcount checks for final release of media engine.

[Marvin]

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=133373

Your paranoid android.

=== w8 (32 bit report) ===

mfmediaengine:
mfmediaengine.c:630: Test failed: Unexpected ref 2.

=== w8adm (32 bit report) ===

mfmediaengine:
mfmediaengine.c:630: Test failed: Unexpected ref 2.

=== w864 (32 bit report) ===

mfmediaengine:
mfmediaengine.c:630: Test failed: Unexpected ref 2.

=== w1064_adm (64 bit report) ===

mfmediaengine:
mfmediaengine.c:630: Test failed: Unexpected ref 2.

=== w1064_tsign (64 bit report) ===

mfmediaengine:
mfmediaengine.c:630: Test failed: Unexpected ref 2.

=== w10pro64_ja (64 bit report) ===

mfmediaengine:
mfmediaengine.c:630: Test failed: Unexpected ref 2.

=== w10pro64_zh_CN (64 bit report) ===

mfmediaengine:
mfmediaengine.c:630: Test failed: Unexpected ref 2.

[Bernhard Kölbl (＠besentv)]

On Sat Jun  3 13:59:01 2023 +0000, **** wrote:
> Marvin replied on the mailing list:
> 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=133373
> Your paranoid android.
> === w8 (32 bit report) ===
> mfmediaengine:
> mfmediaengine.c:630: Test failed: Unexpected ref 2.
> === w8adm (32 bit report) ===
> mfmediaengine:
> mfmediaengine.c:630: Test failed: Unexpected ref 2.
> === w864 (32 bit report) ===
> mfmediaengine:
> mfmediaengine.c:630: Test failed: Unexpected ref 2.
> === w1064_adm (64 bit report) ===
> mfmediaengine:
> mfmediaengine.c:630: Test failed: Unexpected ref 2.
> === w1064_tsign (64 bit report) ===
> mfmediaengine:
> mfmediaengine.c:630: Test failed: Unexpected ref 2.
> === w10pro64_ja (64 bit report) ===
> mfmediaengine:
> mfmediaengine.c:630: Test failed: Unexpected ref 2.
> === w10pro64_zh_CN (64 bit report) ===
> mfmediaengine:
> mfmediaengine.c:630: Test failed: Unexpected ref 2.

Hm, I guess some Windows versions don't like getting so much stuff called after Media Engine shutdown.