On Fri, Jul 2, 2021 at 3:51 PM Yann Ylavic <ylavic....@gmail.com> wrote:
>
> On Fri, Jul 2, 2021 at 3:19 PM Joe Orton <jor...@redhat.com> wrote:
> >
> > If pthread_mutex_timedlock() is not supported, apr_thread_mutex_unlock()
> > will take a locked mutex and immediately lock it again:
> >
> > https://github.com/apache/apr/blob/trunk/locks/unix/thread_mutex.c#L297
> >
> > APR_DECLARE(apr_status_t) apr_thread_mutex_unlock(apr_thread_mutex_t *mutex)
> > {
> >     apr_status_t status;
> >
> >     if (mutex->cond) {
> >         status = pthread_mutex_lock(&mutex->mutex);
> >
> > This is undefined behaviour unless APR ensures that mutex is recursive,
> > which it doesn't AFAICT.
>
> When mutex->cond != NULL (ie !HAVE_PTHREAD_MUTEX_TIMEDLOCK),
> apr_thread_mutex_lock() and co don't leave mutex->mutex locked (the
> actual locking happens thanks to the ->cond wait on the ->locked
> flag).
>
> So apr_thread_mutex_unlock() can (and actually must) lock ->mutex to
> signal the ->cond and clear the ->locked.
> What am I missing?

Coverity likely can't figure out without the #ifdefs, so both your
patch and the code look good to me :)

Reply via email to