[vlc-devel] [PATCH] HLS: remove some useless mutex locks

[Jean-Paul Saman]

On Tue, Feb 7, 2012 at 3:26 PM, Hugo Beauzée-Luyssen <beauze.h at gmail.com> wrote:
> 2012/2/7 Frédéric Yhuel <fyhuel at viotech.net>:
>> ---
>>  modules/stream_filter/httplive.c |    4 ----
>>  1 files changed, 0 insertions(+), 4 deletions(-)
>>
>> diff --git a/modules/stream_filter/httplive.c b/modules/stream_filter/httplive.c
>> index c03061b..9afd5c0 100644
>> --- a/modules/stream_filter/httplive.c
>> +++ b/modules/stream_filter/httplive.c
>> @@ -2070,9 +2070,7 @@ static void Close(vlc_object_t *p_this)
>>     assert(p_sys->hls_stream);
>>
>>     /* */
>> -    vlc_mutex_lock(&p_sys->download.lock_wait);
>>     vlc_cond_signal(&p_sys->download.wait);
>> -    vlc_mutex_unlock(&p_sys->download.lock_wait);
>>
>
> This mutex isn't useless, if you don't hold it before calling
> vlc_cond_signal there is no way of being sure the thread is actually
> waiting.
> If the mutex is held by the download thread, it's likely to enter the
> wait condition, thus the call to vlc_thread_join would stall.
> Moreover, I think there are some slight chances of deadlock when
> calling Close(), and some non thread-safe access to variables such as
> as p_sys.(download|playback).segment...
> Anyway, I'm pretty sure this mutex is not useless.
>
>>     /* */
>>     if (p_sys->b_live)
>> @@ -2223,9 +2221,7 @@ static ssize_t hls_Read(stream_t *s, uint8_t *p_read, unsigned int i_read)
>>             vlc_mutex_unlock(&segment->lock);
>>
>>             /* signal download thread */
>> -            vlc_mutex_lock(&p_sys->download.lock_wait);
>>             vlc_cond_signal(&p_sys->download.wait);
>> -            vlc_mutex_unlock(&p_sys->download.lock_wait);
>
> The same applies here, if the hls_Thread is about to enter
> vlc_cond_wait (ie. having the download.lock held) if you don't acquire
> the mutex first in the main thread, you might signal the thread before
> it enters the vlc_cond_wait, thus leading to unexpected behavior.
>
>>             continue;
>>         }
>>
>> --
>> 1.7.5.4
>>
>
> Please note that I am no threading expert, and therefore I'd love to
> have a second opinion.
>
> Best regards,
>
> --
> Hugo Beauzée-Luyssen

Hugo,

You are correct.

Kind regards

Jean-Paul Saman
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel