thread mutex double-locking w/o _timedlock

[Joe Orton]

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.

Current versions of Coverity are smart enough to detect double-locking 
and hence this code triggers a scanner warning for every single call to 
apr_thread_mutex_unlock() within APR.

I'm not sure how to fix the problem, maybe force mutexes to be 
recursive?  But isn't it also true that all this code can be
#if'ed out for platforms which do support pthread_mutex_timedlock()?

e.g. as attached.

Regards, Joe

Index: locks/unix/thread_mutex.c
===================================================================
--- locks/unix/thread_mutex.c   (revision 1891201)
+++ locks/unix/thread_mutex.c   (working copy)
@@ -121,6 +121,7 @@
 {
     apr_status_t rv;
 
+#ifndef HAVE_PTHREAD_MUTEX_TIMEDLOCK
     if (mutex->cond) {
         apr_status_t rv2;
 
@@ -152,7 +153,8 @@
 
         return rv;
     }
-
+#endif
+    
     rv = pthread_mutex_lock(&mutex->mutex);
 #ifdef HAVE_ZOS_PTHREADS
     if (rv) {
@@ -167,6 +169,7 @@
 {
     apr_status_t rv;
 
+#ifndef HAVE_PTHREAD_MUTEX_TIMEDLOCK
     if (mutex->cond) {
         apr_status_t rv2;
 
@@ -196,7 +199,8 @@
 
         return rv;
     }
-
+#endif
+    
     rv = pthread_mutex_trylock(&mutex->mutex);
     if (rv) {
 #ifdef HAVE_ZOS_PTHREADS
@@ -298,7 +302,11 @@
 {
     apr_status_t status;
 
+#ifndef HAVE_PTHREAD_MUTEX_TIMEDLOCK
     if (mutex->cond) {
+        /* ### If the mutex is locked, and there is no guarantee this
+         * is a recursive mutex, double-locking here looks like
+         * undefined behaviour. */
         status = pthread_mutex_lock(&mutex->mutex);
         if (status) {
 #ifdef HAVE_ZOS_PTHREADS
@@ -320,6 +328,7 @@
 
         mutex->locked = 0;
     }
+#endif
 
     status = pthread_mutex_unlock(&mutex->mutex);
 #ifdef HAVE_ZOS_PTHREADS
@@ -335,9 +344,12 @@
 {
     apr_status_t rv, rv2 = APR_SUCCESS;
 
+#ifndef HAVE_PTHREAD_MUTEX_TIMEDLOCK
     if (mutex->cond) {
         rv2 = apr_thread_cond_destroy(mutex->cond);
     }
+#endif
+    
     rv = apr_pool_cleanup_run(mutex->pool, mutex, thread_mutex_cleanup);
     if (rv == APR_SUCCESS) {
         rv = rv2;
Index: include/arch/unix/apr_arch_thread_mutex.h
===================================================================
--- include/arch/unix/apr_arch_thread_mutex.h   (revision 1891201)
+++ include/arch/unix/apr_arch_thread_mutex.h   (working copy)
@@ -33,7 +33,9 @@
 struct apr_thread_mutex_t {
     apr_pool_t *pool;
     pthread_mutex_t mutex;
+#ifndef HAVE_PTHREAD_MUTEX_TIMEDLOCK
     apr_thread_cond_t *cond;
+#endif
     int locked, num_waiters;
 };
 #endif