From c28bafcfb6a6b564bbce4105c4ea83fe81865585 Mon Sep 17 00:00:00 2001 From: wm4 Date: Sun, 28 Jul 2013 00:10:58 +0200 Subject: mp_image_pool: make reference counting thread-safe See previous commits. Also simplify this thing: 2 flags per pool image are enough to avoid a weird central refcount and an associated shared object keeping the refcount. We could even just store these two flags in the mp_image itself (like in mp_image.flags or mp_image.priv), but let's not for the sake of readability. --- video/mp_image_pool.c | 102 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 69 insertions(+), 33 deletions(-) (limited to 'video/mp_image_pool.c') diff --git a/video/mp_image_pool.c b/video/mp_image_pool.c index dd424ad2a4..a81a008aa1 100644 --- a/video/mp_image_pool.c +++ b/video/mp_image_pool.c @@ -16,6 +16,10 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include "config.h" + +#include +#include #include #include "talloc.h" @@ -25,18 +29,39 @@ #include "mp_image_pool.h" -struct pool_trampoline { - struct mp_image_pool *pool; - int refcount; +#if HAVE_PTHREADS +#include +static pthread_mutex_t pool_mutex = PTHREAD_MUTEX_INITIALIZER; +#define pool_lock() pthread_mutex_lock(&pool_mutex) +#define pool_unlock() pthread_mutex_unlock(&pool_mutex) +#else +#define pool_lock() 0 +#define pool_unlock() 0 +#endif + +// Thread-safety: the pool itself is not thread-safe, but pool-allocated images +// can be referenced and unreferenced from other threads. (As long as compiled +// with pthreads, and the image destructors are thread-safe.) + +struct mp_image_pool { + int max_count; + + struct mp_image **images; + int num_images; +}; + +// Used to gracefully handle the case when the pool is freed while image +// references allocated from the image pool are still held by someone. +struct image_flags { + // If both of these are false, the image must be freed. + bool referenced; // outside mp_image reference exists + bool pool_alive; // the mp_image_pool references this }; static int image_pool_destructor(void *ptr) { struct mp_image_pool *pool = ptr; mp_image_pool_clear(pool); - pool->trampoline->pool = NULL; - if (!pool->trampoline->refcount) - talloc_free(pool->trampoline); return 0; } @@ -46,9 +71,6 @@ struct mp_image_pool *mp_image_pool_new(int max_count) talloc_set_destructor(pool, image_pool_destructor); *pool = (struct mp_image_pool) { .max_count = max_count, - .trampoline = talloc_struct(NULL, struct pool_trampoline, { - .pool = pool, - }), }; return pool; } @@ -57,32 +79,33 @@ void mp_image_pool_clear(struct mp_image_pool *pool) { for (int n = 0; n < pool->num_images; n++) { struct mp_image *img = pool->images[n]; - if (img->priv) { - // Image data is being used - detach from the pool, and make it - // free itself when pool_free_image() is called - img->priv = talloc_struct(NULL, struct pool_trampoline, { - .refcount = 1, - }); - } else { + struct image_flags *it = img->priv; + bool referenced; + pool_lock(); + assert(it->pool_alive); + it->pool_alive = false; + referenced = it->referenced; + pool_unlock(); + if (!referenced) talloc_free(img); - } } pool->num_images = 0; - pool->trampoline->refcount = 0; } -static void pool_free_image(void *ptr) +// This is the only function that is allowed to run in a different thread. +// (Consider passing an image to another thread, which frees it.) +static void unref_image(void *ptr) { struct mp_image *img = ptr; - struct pool_trampoline *trampoline = img->priv; - trampoline->refcount--; - img->priv = NULL; - if (!trampoline->pool) { - // pool was free'd while image reference was still held - if (trampoline->refcount == 0) - talloc_free(trampoline); + struct image_flags *it = img->priv; + bool alive; + pool_lock(); + assert(it->referenced); + it->referenced = false; + alive = it->pool_alive; + pool_unlock(); + if (!alive) talloc_free(img); - } } // Return a new image of given format/size. The only difference to @@ -93,22 +116,35 @@ struct mp_image *mp_image_pool_get(struct mp_image_pool *pool, unsigned int fmt, int w, int h) { struct mp_image *new = NULL; + + pool_lock(); for (int n = 0; n < pool->num_images; n++) { struct mp_image *img = pool->images[n]; - if (!img->priv && img->imgfmt == fmt && img->w == w && img->h == h) { - new = img; - break; + struct image_flags *it = img->priv; + assert(it->pool_alive); + if (!it->referenced) { + if (img->imgfmt == fmt && img->w == w && img->h == h) { + new = img; + break; + } } } + pool_unlock(); + if (!new) { if (pool->num_images >= pool->max_count) mp_image_pool_clear(pool); new = mp_image_alloc(fmt, w, h); + struct image_flags *it = talloc_ptrtype(new, it); + *it = (struct image_flags) { .pool_alive = true }; + new->priv = it; MP_TARRAY_APPEND(pool, pool->images, pool->num_images, new); } - pool->trampoline->refcount++; - new->priv = pool->trampoline; - return mp_image_new_custom_ref(new, new, pool_free_image); + + struct image_flags *it = new->priv; + assert(!it->referenced && it->pool_alive); + it->referenced = true; + return mp_image_new_custom_ref(new, new, unref_image); } // Like mp_image_new_copy(), but allocate the image out of the pool. -- cgit v1.2.3