[vlc-devel] [PATCH] Avoid deadlock on UDP stop

[Rémi Denis-Courmont]

Le 2014-05-22 18:00, Romain Vimont a écrit :
> However, (B ==> A) is false:
>   By definition, we consider that we reach EOF once ThreadRead() 
> cannot
>   put more blocks into to fifo queue anymore, i.e. once it calls
>   block_FifoWake().

By definition, an UDP stream *never* reaches EOF. Setting EOF because 
the user requested to stop is already a nasty kludge.

>   The semantic of block_FifoWake() is to make
>   exactly one call to block_FifoGet() nonblocking. During that call, 
> the
>   fifo may not be empty (i.e. p_fifo->p_first != NULL). In that case, 
> it
>   does not return NULL (and p_fifo->b_force_wake will remain unset).

By that line of reasoning, it is wrong to set EOF at all anyway. The 
stream did not end. The user asked to stop reception.

> Therefore, setting b_eof when block_FifoGet() returns NULL is not
> sufficient, it would not fix the deadlock.

It will not fix the deadlock and your patch does not fix the alleged 
deadlock either. Your patch is a kludge because not only it works around 
the alleged bug rather than fix it, but it abuses the EOF flag.

> Instead, the command should be either "do not retrieve any block
> anymore" (what this patch implements), or "retrieve pending blocks, 
> but
> stop (do not block) once the queue is empty".

We already have an atomic variable for the purpose of detecting the 
user stop. I refuse to add another one. Is it that difficult to grasp a 
concept?

> I consider this as a root cause in itself.

We are going to have to disagree then.

If net_Read() failed, then there should not even be no more calls to 
pf_block anyway. So either your alleged bug is non-existent or something 
is wrong *outside* the UDP access.

-- 
Rémi Denis-Courmont