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

[wm4]

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?