[FFmpeg-devel] [PATCH] avutil/log: make av_log/av_vlog() thread safe

[Michael Niedermayer]

On Thu, Oct 17, 2013 at 12:04:11AM +0200, wm4 wrote:
> On Wed, 16 Oct 2013 23:43:28 +0200
> Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> > This uses a spinlock
> > 
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> >  libavutil/log.c |    6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/libavutil/log.c b/libavutil/log.c
> > index 53be3ea..b871c8f 100644
> > --- a/libavutil/log.c
> > +++ b/libavutil/log.c
> > @@ -34,6 +34,7 @@
> >  #endif
> >  #include <stdarg.h>
> >  #include <stdlib.h>
> > +#include "atomic.h"
> >  #include "avutil.h"
> >  #include "bprint.h"
> >  #include "common.h"
> > @@ -268,8 +269,13 @@ void av_log(void* avcl, int level, const char *fmt, ...)
> >  
> >  void av_vlog(void* avcl, int level, const char *fmt, va_list vl)
> >  {
> > +    static void * volatile state;
> > +    while (avpriv_atomic_ptr_cas(&state, NULL, (void*)fmt))
> > +        ;
> >      if(av_log_callback)
> >          av_log_callback(avcl, level, fmt, vl);
> > +    if (!fmt || avpriv_atomic_ptr_cas(&state, (void*)fmt, NULL) != (void*)fmt)
> > +        abort(); //cant use av_assert*() here due to that using av_log
> >  }
> >  
> >  int av_log_get_level(void)
> 
> What problem is that trying to solve?

av_log() gets called from multiple decoders or threads within a
decoder. The results are sometimes intermixed log messages, above
patch fixes that. Yes ive confirmed the bug as well as that this
fixes it.


> There could be a race condition
> between checking and using av_log_callback,

uhm, right, fixed, that was independant of this patch though


> but av_log_callback itself
> is user-provided. So, as far as I can see it, it doesn't sound like a
> good idea to call a user-provided callback under a spinlock. Instead,
> you should document that the user-provided callback is expected to be
> thread-safe.

If thats preferred, i can move the lock to our default callback
but all applications need
a lock or thread safe callback if they provide a callback then, even
ones that have a single thread and process only. This doesnt sound
very nice for apps that use no locking primitives anywhere else

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

What does censorship reveal? It reveals fear. -- Julian Assange
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131017/ef9b6a5b/attachment.asc>