On Fri, Jul 02, 2021 at 04:10:08PM +0200, Yann Ylavic wrote: > 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?
Ahhhh, that makes sense. Sorry, I missed that, you're not missing anything, tracing through pages of Coverity output dulls my remaining brain capacity... > Coverity likely can't figure out without the #ifdefs, so both your > patch and the code look good to me :) Great, thanks Yann. I removed the other fields too and dropped the bogus comment, -> r1891204. Regards, Joe
