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
