[vlc-devel] [PATCH] HLS: don't signal dl thread out of the blue in Close()

[Hugo Beauzée-Luyssen]

On Mon, Jun 25, 2012 at 8:19 PM, Frederic YHUEL <fyhuel at viotech.net> wrote:
> On Mon, Jun 25, 2012 at 7:08 PM, Hugo Beauzée-Luyssen <hugo at beauzee.fr> wrote:
>> On Mon, Jun 25, 2012 at 8:00 PM, Laurent Aimar <fenrir at elivagar.org> wrote:
>>> On Mon, Jun 25, 2012 at 07:59:04PM +0300, Hugo Beauzée-Luyssen wrote:
>>>> On Mon, Jun 25, 2012 at 10:48 AM, Frederic YHUEL <fyhuel at viotech.net> wrote:
>>>> > On Tue, Jun 19, 2012 at 4:17 PM, Frédéric Yhuel <fyhuel at viotech.net> wrote:
>>>> >> ---
>>>> >>  modules/stream_filter/httplive.c |    5 ++++-
>>>> >>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>> >>
>>>> >> diff --git a/modules/stream_filter/httplive.c b/modules/stream_filter/httplive.c
>>>> >> index 5b15bab..5758189 100644
>>>> >> --- a/modules/stream_filter/httplive.c
>>>> >> +++ b/modules/stream_filter/httplive.c
>>>> >> @@ -2000,8 +2000,11 @@ static void Close(vlc_object_t *p_this)
>>>> >>
>>>> >>     /* */
>>>> >>     vlc_mutex_lock(&p_sys->download.lock_wait);
>>>> >> -    vlc_cond_signal(&p_sys->download.wait);
>>>> >> +    /* negate the condition variable's predicate */
>>>> >> +    p_sys->download.segment = p_sys->playback.segment = 0;
>>>> >> +    p_sys->download.seek = 0; /* better safe than sorry */
>>>> >>     vlc_mutex_unlock(&p_sys->download.lock_wait);
>>>> >> +    vlc_cond_signal(&p_sys->download.wait);
>>>  I am not sure that a vlc_cond_signal outside the lock is valid.
>>>
>>
>> Indeed, missed that :/
>> As far as I know it is "valid", but a bad idea. Will fix the patch locally.
>>
>
> Michael Kerrisk writes in "The Linux Programming Interface" that both
> are valid, according to SUSv3.
>
It depends how many threads are using the wait condition I guess. I
think there are only two in the current case but it's been kind of a
long time I didn't read the code again, so I feel more confident in
keeping the mutex locked before signaling.

> It seems like Butenhof points out that, on some implementations,
> unlocking the mutex and then signaling the condition variable yields
> to better performance. I don't know if is really true though :-)
>

That would be logical, assuming the thread is resumed immediately it
has to wait for the lock to be released.

> The important thing is to change the predicate while the mutex is acquired.
>
>
> Best regards
> Frédéric



-- 
Hugo Beauzée-Luyssen
mail: hugo at beauzee.fr
skype: beauze.h