[PATCH] mf: Add missing ::Release of clock timer.
nsivov at codeweavers.com
Tue May 5 09:18:32 CDT 2020
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);
>> 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
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:
It's different because there is no list.
More information about the wine-devel