[vlc-devel] [PATCH 4/7] vout: check for pools leaks

[Thomas Guillem]

On Wed, Jan 11, 2017, at 12:09, Rémi Denis-Courmont wrote:
> Le mercredi 11 janvier 2017, 09:13:11 Thomas Guillem a écrit :
> > Show an error message and assert if filters, decoders or vout displays leak
> > pictures.
> > 
> > In order to check for leaks, the vout thread will increment the ref count of
> > the pools during initialisation. Then, pools ref counts will be decremented
> > when the vout thread is stopped, just after the decoder, the vout display,
> > and the filter modules are unloaded. If picture_pool_DecRef() doesn't
> > return true from that point, this means that we are leaking pictures.
> > 
> > There is one limitation, leaks won't be detected for vout displays without
> > direct rendering.
> > ---
> >  src/video_output/video_output.c | 22 +++++++++++++++++++++-
> >  src/video_output/vout_wrapper.c |  6 +++++-
> >  2 files changed, 26 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/video_output/video_output.c
> > b/src/video_output/video_output.c index 84972dbfda..ad668ebb86 100644
> > --- a/src/video_output/video_output.c
> > +++ b/src/video_output/video_output.c
> > @@ -1474,7 +1474,27 @@ static void ThreadStop(vout_thread_t *vout,
> > vout_display_state_t *state)
> > 
> >      if (vout->p->decoder_fifo)
> >          picture_fifo_Delete(vout->p->decoder_fifo);
> > -    assert(!vout->p->decoder_pool);
> > +
> > +    vout_thread_sys_t *sys = vout->p;
> > +    if (sys->decoder_pool)
> > +    {
> > +        /* The decoder, the vout display and the filter modules are closed.
> > The
> > +         * vout thread should be the only one that still hold
> 
> That is simply not true. ThreadStop() gets called when the decoder
> changes its 
> output format, so the decoder is most definitely not closed, and most
> definitely 
> still can hold references to picture in the old format.

ah right,

> 
> And that's the whole point: you cannot solve the halting problem, and
> thus you 
> cannot distinguish a leak from a late reference.
> 
> I am not going to review the rest of the patch (series) because it looks
> like 
> it claims to do something that I believe is logically impossible.

I don't get. OK, patches 01, 02 and 04 are wrong. But we can still
remove the vout_Reset() hack (patches 05, 06, 07). We won't be notified
of leaks anymore but this was not working properly (with asynchronous
vout).

> 
> -- 
> Rémi Denis-Courmont
> http://www.remlab.net/
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel