[PATCH] mf: Add missing ::Release of clock timer.

[Nikolay Sivov]

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.