[FFmpeg-devel] [PATCH] avutil/log: Check and warn for recursive calls

[wm4]

On Thu, 20 Mar 2014 14:51:11 +0100
Michael Niedermayer <michaelni at gmx.at> wrote:

> this only works when a error checking mutex is available.
> an alternative would be to use thread local storage to implement our own checking mutex
> 
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  libavutil/log.c |   10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/libavutil/log.c b/libavutil/log.c
> index a0bb5e4..b32dfe0 100644
> --- a/libavutil/log.c
> +++ b/libavutil/log.c
> @@ -42,7 +42,11 @@
>  
>  #if HAVE_PTHREADS
>  #include <pthread.h>
> +#    ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
> +static pthread_mutex_t mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
> +#    else
>  static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
> +#    endif
>  #endif
>  
>  #define LINE_SZ 1024
> @@ -251,7 +255,11 @@ void av_log_default_callback(void* ptr, int level, const char* fmt, va_list vl)
>      if (level > av_log_level)
>          return;
>  #if HAVE_PTHREADS
> -    pthread_mutex_lock(&mutex);
> +    if (pthread_mutex_lock(&mutex)) {
> +        const char *msg = "av_log is thread safe, but cannot be called from signal handlers\n";
> +        write (2, msg, strlen(msg));
> +        return;
> +    }

So this is just to catch signal handler usage? Calling pthread
functions from signal handlers is AFAIK not safe, even if you attempt
to workaround the fundamental deadlock problems.

I'd say ffmpeg is not responsible for catching basic undefined behavior
(like calling things that are not signal-safe from signal handlers).

Maybe av_log could be explicitly documented as not signal-safe or so to
make sure that users don't run into this accidentally.

>  #endif
>  
>      format_line(ptr, level, fmt, vl, part, &print_prefix, type);