Re: [Mlt-devel] [PATCH] fix race condition in mlt_consumer

[Dan D. <da...@de...>]

On Mon, Oct 21, 2013 at 10:57 AM, Maksym Veremeyenko <ve...@m1...> wrote:
> 21.10.13 20:39, Dan Dennedy написав(ла):
>>
>> On Mon, Oct 21, 2013 at 12:28 AM, Maksym Veremeyenko<ve...@m1...>
>> wrote:
>>>
>>> 19.10.13 04:39, Dan Dennedy написав(ла):
>>>
>>>> On Thu, Oct 17, 2013 at 9:14 AM, Maksym Veremeyenko<ve...@m1...>
>>>> wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> attached patch prevent start new (or additional)
>>>>> consumer_read_ahead_start
>>>>> from avformat consumer during stopping/closing consumer.
>>>>
>>>>
>>>>
>>>> I do not like that you removed "priv->ahead = 0;" from
>>>> mlt_consumer_stop(). Please explain; was that really necessary to fix
>>>> your problem? The other parts look good.
>>>
>>>
>>>
>>> first it:
>>>
>>> 1598:        // Cancel the read ahead threads
>>> 1598:        priv->ahead = 0;
>>>
>>> and then trying to stop consumer module thread:
>>>
>>> 1609:        // Invoke the child callback
>>> 1610:        if ( self->stop != NULL )
>>> 1611:                self->stop( self );
>>>
>>> and it waiting for thread finished. internal consumer thread is trying to
>>> call mlt_consumer_rt_frame.  that function if priv->ahead is not running
>>> and
>>> trying to call consumer_read_ahead_start
>>>
>>> 1521:                if ( priv->ahead == 0 )
>>> 1522:                {
>>> 1523:                        int buffer = mlt_properties_get_int(
>>> properties, "buffer" );
>>> 1524:                        int prefill = mlt_properties_get_int(
>>> properties, "prefill" );
>>> 1525:                        consumer_read_ahead_start( self );
>>> 1526:                        if ( buffer>  1 )
>>> 1527:                                size = prefill>  0&&  prefill<
>>> buffer
>>>
>>> ? prefill : buffer;
>>> 1528:                }
>>>
>>> that successfully starting consumer_read_ahead_thread again:
>>>
>>> 1089:                if ( pthread_create(&priv->ahead_thread,
>>>
>>> &thread_attributes, consumer_read_ahead_thread, self )<  0 )
>>> 1090:                        pthread_create(&priv->ahead_thread, NULL,
>>> consumer_read_ahead_thread, self );
>>>
>>> or
>>>
>>> 1095:                pthread_create(&priv->ahead_thread, NULL,
>>>
>>> consumer_read_ahead_thread, self );
>>>
>>> my patch trying to prevent that behaviour by /two hands/ and i think
>>> checking priv->started could be enough - at least i will hardly check it
>>> in
>>> a day or two...
>>>
>>
>> Are you calling mlt_consumer_stop() and mlt_consumer_start() from 2
>> different threads?
>>
>
> in my case they been called from different threads but with large time
> interval...
>
> what i wanted to notice is that consumer_read_ahead_thread started twice
> because after resetting priv->ahead is possible mlt_consumer_rt_frame from
> internal consumer thread that check condition:
>
>
> 1521:                if ( priv->ahead == 0 )
>
> for starting thread...
>

Thank you, I understand now. I will do more analysis around the
problem and the patch over the next day or two.

-- 
+-DRD-+