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

[Ganesh Ajjanagadde]

On Wed, Dec 23, 2015 at 7:59 PM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> 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?

Please ignore the comment regarding the macros: I thought over it, and
I am pretty sure they can't be avoided.

>
>> +
>>  #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