[FFmpeg-devel] [PATCH] lavu: add pthread asserts if ASSERT_LEVEL>1

[Ganesh Ajjanagadde]

On Tue, Dec 22, 2015 at 12:05 PM, Clément Bœsch <u at pkh.me> wrote:
> ---
>  libavutil/thread.h | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 96 insertions(+)
>
> diff --git a/libavutil/thread.h b/libavutil/thread.h
> index 3d15737..0bb745e 100644
> --- a/libavutil/thread.h
> +++ b/libavutil/thread.h
> @@ -30,6 +30,102 @@
>
>  #if HAVE_PTHREADS
>  #include <pthread.h>
> +
> +#if defined(ASSERT_LEVEL) && ASSERT_LEVEL > 0
> +
> +#include "log.h"
> +
> +#define ASSERT_PTHREAD_NORET(func, ...) do {                            \
> +    int ret = func(__VA_ARGS__);                                        \
> +    if (ret) {                                                          \
> +        av_log(NULL, AV_LOG_FATAL, AV_STRINGIFY(func)                   \
> +               " failed with error: %s\n", av_err2str(AVERROR(ret)));   \
> +        abort();                                                        \
> +    }                                                                   \
> +} while (0)
> +
> +#define ASSERT_PTHREAD(func, ...) do {                                  \
> +    ASSERT_PTHREAD_NORET(func, __VA_ARGS__);                            \
> +    return 0;                                                           \
> +} while (0)
> +
> +static inline int strict_pthread_join(pthread_t thread, void **value_ptr)
> +{
> +    ASSERT_PTHREAD(pthread_join, thread, value_ptr);
> +}
> +
> +static inline int strict_pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *attr)
> +{
> +    if (attr) {
> +        ASSERT_PTHREAD_NORET(pthread_mutex_init, mutex, attr);
> +    } else {
> +        pthread_mutexattr_t local_attr;
> +        ASSERT_PTHREAD_NORET(pthread_mutexattr_init, &local_attr);
> +        ASSERT_PTHREAD_NORET(pthread_mutexattr_settype, &local_attr, PTHREAD_MUTEX_ERRORCHECK);
> +        ASSERT_PTHREAD_NORET(pthread_mutex_init, mutex, &local_attr);
> +        ASSERT_PTHREAD_NORET(pthread_mutexattr_destroy, &local_attr);
> +    }
> +    return 0;
> +}
> +
> +static inline int strict_pthread_mutex_destroy(pthread_mutex_t *mutex)
> +{
> +    ASSERT_PTHREAD(pthread_mutex_destroy, mutex);
> +}
> +
> +static inline int strict_pthread_mutex_lock(pthread_mutex_t *mutex)
> +{
> +    ASSERT_PTHREAD(pthread_mutex_lock, mutex);
> +}
> +
> +static inline int strict_pthread_mutex_unlock(pthread_mutex_t *mutex)
> +{
> +    ASSERT_PTHREAD(pthread_mutex_unlock, mutex);
> +}
> +
> +static inline int strict_pthread_cond_init(pthread_cond_t *cond, const pthread_condattr_t *attr)
> +{
> +    ASSERT_PTHREAD(pthread_cond_init, cond, attr);
> +}
> +
> +static inline int strict_pthread_cond_destroy(pthread_cond_t *cond)
> +{
> +    ASSERT_PTHREAD(pthread_cond_destroy, cond);
> +}
> +
> +static inline int strict_pthread_cond_signal(pthread_cond_t *cond)
> +{
> +    ASSERT_PTHREAD(pthread_cond_signal, cond);
> +}
> +
> +static inline int strict_pthread_cond_broadcast(pthread_cond_t *cond)
> +{
> +    ASSERT_PTHREAD(pthread_cond_broadcast, cond);
> +}
> +
> +static inline int strict_pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
> +{
> +    ASSERT_PTHREAD(pthread_cond_wait, cond, mutex);
> +}
> +
> +static inline int strict_pthread_once(pthread_once_t *once_control, void (*init_routine)(void))
> +{
> +    ASSERT_PTHREAD(pthread_once, once_control, init_routine);
> +}
> +
> +#define pthread_join           strict_pthread_join
> +#define pthread_mutex_init     strict_pthread_mutex_init
> +#define pthread_mutex_destroy  strict_pthread_mutex_destroy
> +#define pthread_mutex_lock     strict_pthread_mutex_lock
> +#define pthread_mutex_unlock   strict_pthread_mutex_unlock
> +#define pthread_cond_init      strict_pthread_cond_init
> +#define pthread_cond_destroy   strict_pthread_cond_destroy
> +#define pthread_cond_signal    strict_pthread_cond_signal
> +#define pthread_cond_broadcast strict_pthread_cond_broadcast
> +#define pthread_cond_wait      strict_pthread_cond_wait
> +#define pthread_once           strict_pthread_once
> +#endif

I like the idea here, and it is superior to some thoughts I had on
addressing this. There is a question whether aborting is the right
thing to do here though. Technically, I guess in some instances a
client can do something meaningful if these functions fail, and
aborting may be too drastic. However, this approach has a good
advantage of simplicity, and is reasonable in my view.

One thing I don't like are the macros. Are you sure they are unavoidable?

> +
>  #elif HAVE_OS2THREADS
>  #include "compat/os2threads.h"
>  #else
> --
> 2.6.4
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel