[vlc-devel] [PATCH] decoder: fix occasional deadlock on seek

[Rémi Denis-Courmont]

Le 10 octobre 2017 12:27:49 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>
>
>On Mon, Oct 9, 2017, at 22:25, Rémi Denis-Courmont wrote:
>> Le maanantaina 9. lokakuuta 2017, 18.06.46 EEST Thomas Guillem a
>écrit :
>> > This commit fixes the following deadlock:
>> > 
>> > The input thread (from input_DecoderWait()) reads p_owner->b_idle,
>and wait
>> > on the wait_acknowledge condition variable because p_owner->b_idle
>is false
>> > (and because p_owner->b_has_data is false too). The decoder thread
>sets
>> > p_owner->b_idle to true because the fifo is empty and wait on the
>fifo
>> > condition variable. Both threads are waiting each others, hence the
>> > deadlock.
>> > 
>> > To fix this issue, signal the input thread from the decoder thread
>after
>> > setting p_owner->b_idle to true.
>> > 
>> > Since predictable scheduling behavior is not required,
>p_owner->lock doesn't
>> > have to be held while signaling p_owner->wait_acknowledge.
>> > 
>> > Fixes #16100
>> 
>> I don't know if this is right or wrong. Or rather, I know that the
>> current 
>> buffering algorithm is fundamentally flawed regardless of this patch.
>So
>> I 
>> find it impossible to reason about.
>
>We'll fix one the buffering algorith but not today, not for 3.0.
>This is more an hack on top of an existing hack but this really fix the
>last deadlock that I often see(I hope).
>
>Having 2 different mutexes in this part of the code make the things
>really more complex to me. I'll try to merge them, don't know if it can
>be merged for 3.0.
>
>> 
>> -- 
>> Rémi
>> https://www.remlab.net/
>> 
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

Merging the locks does not make much sense. There is one lock for input (FIFO) and for lock for output. Merging them only reinforces the design bug.
-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.