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

[Rémi Denis-Courmont]

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.

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.

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