[PATCH v2 0/4] MR2958: mf: Various refcount leak fixes for session.
-- v2: mfmediaengine/tests: Add refcount checks for final release of media engine. https://gitlab.winehq.org/wine/wine/-/merge_requests/2958
From: Bernhard Kölbl <besentv(a)gmail.com> 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 <besentv(a)gmail.com> --- 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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2958
From: Bernhard Kölbl <besentv(a)gmail.com> Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com> --- 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); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2958
From: Bernhard Kölbl <besentv(a)gmail.com> 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 <besentv(a)gmail.com> --- 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))) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2958
From: Bernhard Kölbl <besentv(a)gmail.com> Signed-off-by: Bernhard Kölbl <besentv(a)gmail.com> --- 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(¬ify->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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2958
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.
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2958#note_34561
participants (2)
-
Bernhard Kölbl -
Marvin