[FFmpeg-devel] [PATCH] [Trac ticket #3194] libavcodec/utils: Statically initialize global mutexes to avoid a leak.

Hendrik Leppkes h.leppkes at gmail.com
Sun Dec 15 13:44:51 CET 2013


On Sun, Dec 15, 2013 at 1:38 PM, wm4 <nfxjfg at googlemail.com> wrote:
> On Sun, 15 Dec 2013 13:16:06 +0100
> Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>
>> On Sun, Dec 15, 2013 at 1:10 PM, wm4 <nfxjfg at googlemail.com> wrote:
>> > On Sun, 15 Dec 2013 12:59:58 +0100
>> > Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>> >
>> >> On Sun, Dec 15, 2013 at 12:17 PM, Tomer Barletz <barletz at gmail.com> wrote:
>> >> > ---
>> >> >  libavcodec/utils.c | 25 +++++++------------------
>> >> >  1 file changed, 7 insertions(+), 18 deletions(-)
>> >> >
>> >> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> >> > index 73370fe..0653acc 100644
>> >> > --- a/libavcodec/utils.c
>> >> > +++ b/libavcodec/utils.c
>> >> > @@ -74,20 +74,6 @@ static int default_lockmgr_cb(void **arg, enum AVLockOp op)
>> >> >      case AV_LOCK_CREATE:
>> >> >          return 0;
>> >> >      case AV_LOCK_OBTAIN:
>> >> > -        if (!*mutex) {
>> >> > -            pthread_mutex_t *tmp = av_malloc(sizeof(pthread_mutex_t));
>> >> > -            if (!tmp)
>> >> > -                return AVERROR(ENOMEM);
>> >> > -            if ((err = pthread_mutex_init(tmp, NULL))) {
>> >> > -                av_free(tmp);
>> >> > -                return AVERROR(err);
>> >> > -            }
>> >> > -            if (avpriv_atomic_ptr_cas(mutex, NULL, tmp)) {
>> >> > -                pthread_mutex_destroy(tmp);
>> >> > -                av_free(tmp);
>> >> > -            }
>> >> > -        }
>> >> > -
>> >> >          if ((err = pthread_mutex_lock(*mutex)))
>> >> >              return AVERROR(err);
>> >> >
>> >> > @@ -100,8 +86,7 @@ static int default_lockmgr_cb(void **arg, enum AVLockOp op)
>> >> >      case AV_LOCK_DESTROY:
>> >> >          if (*mutex)
>> >> >              pthread_mutex_destroy(*mutex);
>> >> > -        av_free(*mutex);
>> >> > -        avpriv_atomic_ptr_cas(mutex, *mutex, NULL);
>> >> > +
>> >> >          return 0;
>> >> >      }
>> >> >      return 1;
>> >> > @@ -114,8 +99,12 @@ static int (*lockmgr_cb)(void **mutex, enum AVLockOp op) = NULL;
>> >> >
>> >> >  volatile int ff_avcodec_locked;
>> >> >  static int volatile entangled_thread_counter = 0;
>> >> > -static void *codec_mutex;
>> >> > -static void *avformat_mutex;
>> >> > +
>> >> > +static pthread_mutex_t codec_mutex_real = PTHREAD_MUTEX_INITIALIZER;
>> >> > +static void *codec_mutex = &codec_mutex_real;
>> >> > +
>> >> > +static pthread_mutex_t avformat_mutex_real = PTHREAD_MUTEX_INITIALIZER;
>> >> > +static void *avformat_mutex = &avformat_mutex_real;
>> >> >
>> >> >  #if FF_API_FAST_MALLOC && CONFIG_SHARED && HAVE_SYMVER
>> >> >  FF_SYMVER(void*, av_fast_realloc, (void *ptr, unsigned int *size, size_t min_size), "LIBAVCODEC_55")
>> >>
>> >> Static initialization doesn't work with the w32threads compat wrapper,
>> >> and as such, this patch would break this use-case.
>> >
>> > So why doesn't libavcodec use one of the pthread implementations for
>> > windows?
>>
>> It can, but it also can use native win32 threads.
>> The only complaint is lack of static initializers, no reason to force
>> pthreads just for that.
>
> Maybe the w32threads wrapper could implement static initializers
> instead of requiring dirty hacks in libavcodec? I'm not sure how to do
> that, though. But maybe implementing pthread_once() would be possible.
> That brings me to the question, how do Windows DLLs handle locking on
> initialization?

The real problem here is that apparently you can't rely on the close
callback being called on the lockmgr. If an user app provides its own,
it could also leak.
So why is this not called?

- Hendrik


More information about the ffmpeg-devel mailing list