[PATCH 0/2] MR6783: winegstreamer: Fix a memory/resource leak.
References held by contained stream objects must use an internal refcount, otherwise they prevent release of the media source if Start() and Shutdown() are not called. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6783
From: Conor McCarthy <cmccarthy(a)codeweavers.com> Shows that any media source references held by contained objects do not affect the externally visible refcount. --- dlls/mfplat/tests/mfplat.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 752be9ea260..6eed295d212 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -1319,6 +1319,11 @@ static void test_source_resolver(void) ok(mediasource != NULL, "got %p\n", mediasource); ok(obj_type == MF_OBJECT_MEDIASOURCE, "got %d\n", obj_type); + refcount = IMFMediaSource_AddRef(mediasource); + todo_wine + ok(refcount == 2, "Unexpected refcount %ld\n", refcount); + IMFMediaSource_Release(mediasource); + refcount = IMFMediaSource_Release(mediasource); todo_wine ok(!refcount, "Unexpected refcount %ld\n", refcount); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6783
From: Conor McCarthy <cmccarthy(a)codeweavers.com> References held by contained stream objects must use an internal refcount, otherwise they prevent release of the media source if Start() and Shutdown() are not called. --- dlls/mfplat/tests/mfplat.c | 2 -- dlls/winegstreamer/media_source.c | 34 +++++++++++++++++++++++-------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 6eed295d212..f7921d30662 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -1320,12 +1320,10 @@ static void test_source_resolver(void) ok(obj_type == MF_OBJECT_MEDIASOURCE, "got %d\n", obj_type); refcount = IMFMediaSource_AddRef(mediasource); - todo_wine ok(refcount == 2, "Unexpected refcount %ld\n", refcount); IMFMediaSource_Release(mediasource); refcount = IMFMediaSource_Release(mediasource); - todo_wine ok(!refcount, "Unexpected refcount %ld\n", refcount); IMFByteStream_Release(stream); diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index 030d0c1b9a2..a10731a563d 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -176,6 +176,7 @@ struct media_source IMFRateControl IMFRateControl_iface; IMFAsyncCallback async_commands_callback; LONG ref; + LONG internal_ref; DWORD async_commands_queue; IMFMediaEventQueue *event_queue; IMFByteStream *byte_stream; @@ -239,6 +240,27 @@ static inline struct source_async_command *impl_from_async_command_IUnknown(IUnk return CONTAINING_RECORD(iface, struct source_async_command, IUnknown_iface); } +static void media_source_internal_addref(IMFMediaSource *iface) +{ + struct media_source *source = impl_from_IMFMediaSource(iface); + InterlockedIncrement(&source->internal_ref); +} + +static void media_source_internal_release(IMFMediaSource *iface) +{ + struct media_source *source = impl_from_IMFMediaSource(iface); + + if (!InterlockedDecrement(&source->internal_ref)) + { + IMFMediaEventQueue_Release(source->event_queue); + IMFByteStream_Release(source->byte_stream); + wg_parser_destroy(source->wg_parser); + source->cs.DebugInfo->Spare[0] = 0; + DeleteCriticalSection(&source->cs); + free(source); + } +} + static HRESULT WINAPI source_async_command_QueryInterface(IUnknown *iface, REFIID riid, void **obj) { if (IsEqualIID(riid, &IID_IUnknown)) @@ -962,7 +984,7 @@ static ULONG WINAPI media_stream_Release(IMFMediaStream *iface) if (!ref) { - IMFMediaSource_Release(stream->media_source); + media_source_internal_release(stream->media_source); IMFStreamDescriptor_Release(stream->descriptor); IMFMediaEventQueue_Release(stream->event_queue); flush_token_queue(stream, FALSE); @@ -1123,7 +1145,7 @@ static HRESULT media_stream_create(IMFMediaSource *source, IMFStreamDescriptor * return hr; } - IMFMediaSource_AddRef(source); + media_source_internal_addref(source); object->media_source = source; IMFStreamDescriptor_AddRef(descriptor); object->descriptor = descriptor; @@ -1362,12 +1384,7 @@ static ULONG WINAPI media_source_Release(IMFMediaSource *iface) if (!ref) { IMFMediaSource_Shutdown(iface); - IMFMediaEventQueue_Release(source->event_queue); - IMFByteStream_Release(source->byte_stream); - wg_parser_destroy(source->wg_parser); - source->cs.DebugInfo->Spare[0] = 0; - DeleteCriticalSection(&source->cs); - free(source); + media_source_internal_release(iface); } return ref; @@ -1635,6 +1652,7 @@ static HRESULT media_source_create(struct object_context *context, IMFMediaSourc object->IMFRateControl_iface.lpVtbl = &media_source_rate_control_vtbl; object->async_commands_callback.lpVtbl = &source_async_commands_callback_vtbl; object->ref = 1; + object->internal_ref = 1; object->byte_stream = context->stream; IMFByteStream_AddRef(context->stream); object->file_size = context->file_size; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6783
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=149511 Your paranoid android. === w8 (32 bit report) === mfplat: mfplat.c:1323: Test failed: Unexpected refcount 5 mfplat.c:1327: Test failed: Unexpected refcount 3 === w864 (64 bit report) === mfplat: mfplat.c:1323: Test failed: Unexpected refcount 5
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Marvin