[vlc-devel] [PATCH] Handle GET_PARAMETER keepalive responses during PAUSE

[Rémi Denis-Courmont]

	Hello,

Le mercredi 24 juin 2015, 18:42:23 Paul Clark a écrit :
> On 24/06/15 18:30, Rémi Denis-Courmont wrote:
> > Le mercredi 24 juin 2015, 18:18:03 Paul Clark a écrit :
> >> +
> >> +            if( !wait_Live555_response( p_demux ) )
> > 
> > Are you sure this function is re-entrant? Seems like nothing good will
> > happen if the timeout thread and the input thread end up waiting at the
> > same time.
> Since there's a single shared live555 RTSPClient and based on other
> existing comments...
> 
>          /* Voodoo (= no) thread safety here! *Ahem* */
> 
> ... I'm pretty sure it *isn't* re-entrant, so yes, that would be Bad if
> it happened.  However this code is only activated during pause and the
> existing code attempts to make sure it doesn't conflict with foreground
> processing through the b_handle_keep_alive flag.

The window for resuming while pause is still being processed is much longer 
with the patch.

> So the best I can claim is that it is no worse than what is there
> already and does fix a nasty lockup...

I agree that the patch is formally no worse. In all defined cases, 
b_handle_keep_alive is false. So the patch is formally moot.

> Ensuring provable thread-safety 
> of this stuff is beyond my understanding of VLC internals I'm afraid.

I believe the patch makes crashes much more likely though. That being said, I 
do not maintain this particular VLC plugin. Its maintainer(s) should decide on 
what to do.

-- 
Rémi Denis-Courmont
http://www.remlab.net/