Signed-off-by: Derek Lesho dlesho@codeweavers.com --- dlls/mf/session.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/mf/session.c b/dlls/mf/session.c index 75ef600a4df..eb388de62ac 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -4014,6 +4014,7 @@ static HRESULT present_clock_schedule_timer(struct presentation_clock *clock, DW
if (FAILED(hr = MFCreateAsyncResult(&timer->IUnknown_iface, &clock->timer_callback, NULL, &result))) return hr; + IUnknown_Release(&timer->IUnknown_iface);
hr = MFScheduleWorkItemEx(result, -time, &timer->key); IMFAsyncResult_Release(result);
On 5/4/20 10:56 PM, Derek Lesho wrote:
--- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -4014,6 +4014,7 @@ static HRESULT present_clock_schedule_timer(struct presentation_clock *clock, DW
if (FAILED(hr = MFCreateAsyncResult(&timer->IUnknown_iface, &clock->timer_callback, NULL, &result))) return hr;
IUnknown_Release(&timer->IUnknown_iface);
hr = MFScheduleWorkItemEx(result, -time, &timer->key); IMFAsyncResult_Release(result);
This is a wrong place to release it. Creating result object grabs a timer reference, scheduling it grabs result object reference, and original reference is released.
Timer object is created with refcount of 1, additional reference is taken by result object, another one by cancellation object, optionally. It's released on timer callback, on cancel request, or on clock release. The only path it's leaked I think is on error in present_clock_timer_SetTimer(). Does the attached patch work?
On 5/5/20 6:07 AM, Nikolay Sivov wrote:
On 5/4/20 10:56 PM, Derek Lesho wrote:
--- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -4014,6 +4014,7 @@ static HRESULT present_clock_schedule_timer(struct presentation_clock *clock, DW if (FAILED(hr = MFCreateAsyncResult(&timer->IUnknown_iface, &clock->timer_callback, NULL, &result))) return hr; + IUnknown_Release(&timer->IUnknown_iface); hr = MFScheduleWorkItemEx(result, -time, &timer->key); IMFAsyncResult_Release(result);
This is a wrong place to release it. Creating result object grabs a timer reference, scheduling it grabs result object reference, and original reference is released.
Timer object is created with refcount of 1, additional reference is taken by result object, another one by cancellation object, optionally. It's released on timer callback, on cancel request, or on clock release. The only path it's leaked I think is on error in present_clock_timer_SetTimer(). Does the attached patch work?
No, it doesn't. Where is the original reference supposed to be released? The reference for the async result is released, the cancel key reference is released by the sample grabber, but AFAIK there's no code to release that original reference. FWIW, I see a similar Release here: https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/mf/main.c#l758
On 5/5/20 5:09 PM, Derek Lesho wrote:
On 5/5/20 6:07 AM, Nikolay Sivov wrote:
On 5/4/20 10:56 PM, Derek Lesho wrote:
--- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -4014,6 +4014,7 @@ static HRESULT present_clock_schedule_timer(struct presentation_clock *clock, DW if (FAILED(hr = MFCreateAsyncResult(&timer->IUnknown_iface, &clock->timer_callback, NULL, &result))) return hr; + IUnknown_Release(&timer->IUnknown_iface); hr = MFScheduleWorkItemEx(result, -time, &timer->key); IMFAsyncResult_Release(result);
This is a wrong place to release it. Creating result object grabs a timer reference, scheduling it grabs result object reference, and original reference is released.
Timer object is created with refcount of 1, additional reference is taken by result object, another one by cancellation object, optionally. It's released on timer callback, on cancel request, or on clock release. The only path it's leaked I think is on error in present_clock_timer_SetTimer(). Does the attached patch work?
No, it doesn't. Where is the original reference supposed to be released? The reference for the async result is released, the cancel key reference is released by the sample grabber, but AFAIK there's no code to release that original reference.
Release is missing from present_clock_timer_callback_Invoke() I suppose, it should be released after it's removed from that list. That would be original reference. Additional reference is grabbed/released in this function, and reference held by result object will go away together with result, this is managed by async queue logic.
FWIW, I see a similar Release here: https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/mf/main.c#l758
It's different because there is no list.
-
Derek Lesho -
Nikolay Sivov