lib: Fix a pthreadpool race condition

Yes, there is one.... I've seen two flaky builds on sn-devel with
pthreadpool after the coverity checks went in. They were in the

		ret = pthread_mutex_unlock(&pool->mutex);
		assert(ret == 0);

in pthreadpool_parent() and pthreadpool_child(). No idea what that was,
I could not really reproduce that. A build attempt on FreeBSD also gave
an erratic error, this time it was an EINVAL in

		ret = pthread_mutex_lock(&pool->mutex);
		assert(ret == 0);

pthreadpool_parent(). EINVAL means that the mutex is not a proper
mutex. What happened: Someone (a detached thread) does the
pthreadpool_free behind our back, while we are in pthreadpool_parent,
preparing the fork. Unfortunately the mutex was already destroyed before
we came to lock it.

The fix is simple: Remove the obsolete struct pthreadpool from the
linked list before the mutex is destroyed.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

diff --git a/source3/lib/pthreadpool/pthreadpool.c b/source3/lib/pthreadpool/pthreadpool.c
index f34e2197da19f7edef3caf0837d37a07c22e6e4d..eaddd4426d2caa597fd3941849d709386684cd32 100644 (file)
--- a/source3/lib/pthreadpool/pthreadpool.c
+++ b/source3/lib/pthreadpool/pthreadpool.c
@@ -234,6 +234,14 @@ static int pthreadpool_free(struct pthreadpool *pool)
 {
        int ret, ret1;
 
+       ret = pthread_mutex_lock(&pthreadpools_mutex);
+       if (ret != 0) {
+               return ret;
+       }
+       DLIST_REMOVE(pthreadpools, pool);
+       ret = pthread_mutex_unlock(&pthreadpools_mutex);
+       assert(ret == 0);
+
        ret = pthread_mutex_destroy(&pool->mutex);
        ret1 = pthread_cond_destroy(&pool->condvar);
 
@@ -244,14 +252,6 @@ static int pthreadpool_free(struct pthreadpool *pool)
                return ret1;
        }
 
-       ret = pthread_mutex_lock(&pthreadpools_mutex);
-       if (ret != 0) {
-               return ret;
-       }
-       DLIST_REMOVE(pthreadpools, pool);
-       ret = pthread_mutex_unlock(&pthreadpools_mutex);
-       assert(ret == 0);
-
        free(pool->jobs);
        free(pool);