[FFmpeg-devel] [PATCH 3/3] avutil/buffer: Fix race in pool.
Hendrik Leppkes
h.leppkes at gmail.com
Sun Mar 17 19:05:57 CET 2013
On Sun, Mar 17, 2013 at 6:46 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> This race will always happen sooner or later in a multi-threaded
> environment and it will over time lead to OOM.
> This fix works by spinning, there are other ways by which this
> can be fixed, like simply detecting the issue after it happened
> and freeing the over-allocated memory or simply using a mutex.
>
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
> libavutil/buffer.c | 7 +++++++
> libavutil/buffer_internal.h | 2 ++
> 2 files changed, 9 insertions(+)
>
> diff --git a/libavutil/buffer.c b/libavutil/buffer.c
> index 5c753ab..6639744 100644
> --- a/libavutil/buffer.c
> +++ b/libavutil/buffer.c
> @@ -307,6 +307,7 @@ static AVBufferRef *pool_alloc_buffer(AVBufferPool *pool)
> ret->buffer->free = pool_release_buffer;
>
> avpriv_atomic_int_add_and_fetch(&pool->refcount, 1);
> + avpriv_atomic_int_add_and_fetch(&pool->nb_allocated, 1);
>
> return ret;
> }
> @@ -318,6 +319,12 @@ AVBufferRef *av_buffer_pool_get(AVBufferPool *pool)
>
> /* check whether the pool is empty */
> buf = get_pool(pool);
> + if (!buf && pool->refcount < pool->nb_allocated) {
> + av_log(NULL, AV_LOG_DEBUG, "Pool race dectected, spining to avoid overallocation and eventual OOM\n");
> + while (!buf && avpriv_atomic_int_get(&pool->refcount) < avpriv_atomic_int_get(&pool->nb_allocated))
> + buf = get_pool(pool);
> + }
> +
Any particular reason the first comparison in the if doesn't use the
atomic gets?
Also, reading the code, the pool holds a refcount onto itself as well,
so refcount is always one higher as nb_allocated if all buffers are in
use, so maybe that should be taken into account here?
> if (!buf)
> return pool_alloc_buffer(pool);
>
> diff --git a/libavutil/buffer_internal.h b/libavutil/buffer_internal.h
> index b2602f8..c291908 100644
> --- a/libavutil/buffer_internal.h
> +++ b/libavutil/buffer_internal.h
> @@ -85,6 +85,8 @@ struct AVBufferPool {
> */
> volatile int refcount;
>
> + volatile int nb_allocated;
> +
> int size;
> AVBufferRef* (*alloc)(int size);
> };
> --
> 1.7.9.5
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
More information about the ffmpeg-devel
mailing list