[vlc-devel] [PATCH 3/6] vout: check pools leaks

[Thomas Guillem]

Show an error message and assert if filters, decoders or vout displays leak
pictures.

There is one limitation, leaks won't be detected for vout displays without
direct rendering.
---
 src/misc/picture_pool.c         | 17 +++++++++++++++--
 src/misc/picture_pool.h         | 11 +++++++++++
 src/video_output/video_output.c | 22 +++++++++++++++++++++-
 src/video_output/vout_wrapper.c |  6 +++++-
 4 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/src/misc/picture_pool.c b/src/misc/picture_pool.c
index 527da051e9..3d5e6a37ff 100644
--- a/src/misc/picture_pool.c
+++ b/src/misc/picture_pool.c
@@ -50,14 +50,17 @@ struct picture_pool_t {
     picture_t  *picture[];
 };
 
-static void picture_pool_Destroy(picture_pool_t *pool)
+static bool picture_pool_Destroy(picture_pool_t *pool)
 {
     if (atomic_fetch_sub(&pool->refs, 1) != 1)
-        return;
+        return false;
+
+    assert(pool->picture_count - popcountll(pool->available) == 0);
 
     vlc_cond_destroy(&pool->wait);
     vlc_mutex_destroy(&pool->lock);
     vlc_free(pool);
+    return true;
 }
 
 void picture_pool_Release(picture_pool_t *pool)
@@ -67,6 +70,16 @@ void picture_pool_Release(picture_pool_t *pool)
     picture_pool_Destroy(pool);
 }
 
+void picture_pool_IncRef(picture_pool_t *pool)
+{
+    atomic_fetch_add(&pool->refs, 1);
+}
+
+bool picture_pool_DecRef(picture_pool_t *pool)
+{
+    return picture_pool_Destroy(pool);
+}
+
 static void picture_pool_ReleasePicture(picture_t *clone)
 {
     picture_priv_t *priv = (picture_priv_t *)clone;
diff --git a/src/misc/picture_pool.h b/src/misc/picture_pool.h
index 35a14e7895..4296970f29 100644
--- a/src/misc/picture_pool.h
+++ b/src/misc/picture_pool.h
@@ -40,3 +40,14 @@ unsigned picture_pool_Reset( picture_pool_t * );
  * picture_pool_Reset will also reset the cancel state to false.
  */
 void picture_pool_Cancel( picture_pool_t *, bool canceled );
+
+/**
+ * Increments the reference count of the pool
+ */
+void picture_pool_IncRef(picture_pool_t *);
+
+/**
+ * Decrements the reference count of the pool
+ * @return true if the pool is destroyed (reference count to 0).
+ */
+bool picture_pool_DecRef(picture_pool_t *);
diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
index 84972dbfda..f9631eb3e9 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 and the vout display modules are closed. The vout thread
+         * should be the only one that still hold references on the private and
+         * the decoder pools. Time to check for leaks. */
+        assert(sys->private_pool);
+
+        bool leak;
+        leak = !picture_pool_DecRef(sys->private_pool);
+        if (leak)
+            msg_Err(vout, "private_pool pictures leaked");
+        assert(!leak);
+
+        leak = !picture_pool_DecRef(sys->decoder_pool);
+        if (leak)
+            msg_Err(vout, "decoder_pool pictures leaked");
+        assert(!leak);
+        sys->private_pool = sys->decoder_pool = NULL;
+    }
 }
 
 static void ThreadInit(vout_thread_t *vout)
diff --git a/src/video_output/vout_wrapper.c b/src/video_output/vout_wrapper.c
index 310f50cdc6..05557cf4f0 100644
--- a/src/video_output/vout_wrapper.c
+++ b/src/video_output/vout_wrapper.c
@@ -35,6 +35,7 @@
 #include <assert.h>
 #include "vout_internal.h"
 #include "display.h"
+#include "../misc/picture_pool.h"
 
 /*****************************************************************************
  * Local prototypes
@@ -94,7 +95,6 @@ void vout_CloseWrapper(vout_thread_t *vout, vout_display_state_t *state)
 #ifdef _WIN32
     var_DelCallback(vout, "video-wallpaper", Forward, NULL);
 #endif
-    sys->decoder_pool = NULL; /* FIXME remove */
 
     vout_DeleteDisplay(sys->display.vd, state);
     free(sys->display.title);
@@ -147,6 +147,7 @@ int vout_InitWrapper(vout_thread_t *vout)
         sys->dpb_size     = picture_pool_GetSize(display_pool) - reserved_picture;
         sys->decoder_pool = display_pool;
         sys->display_pool = display_pool;
+        picture_pool_IncRef(display_pool);
     } else if (!sys->decoder_pool) {
         sys->decoder_pool =
             picture_pool_NewFromFormat(&source,
@@ -160,6 +161,7 @@ int vout_InitWrapper(vout_thread_t *vout)
         } else {
             sys->dpb_size = picture_pool_GetSize(sys->decoder_pool) - reserved_picture;
         }
+        picture_pool_IncRef(sys->decoder_pool);
         NoDrInit(vout);
     }
     sys->private_pool = picture_pool_Reserve(sys->decoder_pool, private_picture);
@@ -167,9 +169,11 @@ int vout_InitWrapper(vout_thread_t *vout)
     {
         if (sys->decoder_pool != sys->display_pool)
             picture_pool_Release(sys->decoder_pool);
+        picture_pool_DecRef(sys->decoder_pool);
         sys->display_pool = sys->decoder_pool = NULL;
         return VLC_EGENERIC;
     }
+    picture_pool_IncRef(sys->private_pool);
     return VLC_SUCCESS;
 }
 
-- 
2.11.0