[PATCH] small fixes for pthreadpool

[Volker Lendecke]

Hi!

Review appreciated!

Thanks, Volker
-------------- next part --------------
>From 8332db8c45629394cd609d2abd847ae56ea7f8b7 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 29 Aug 2016 11:35:20 +0200
Subject: [PATCH 1/2] pthreadpool: We always want asserts to abort()

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/pthreadpool/pthreadpool.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/source3/lib/pthreadpool/pthreadpool.c b/source3/lib/pthreadpool/pthreadpool.c
index fc21d43..2e9f42c 100644
--- a/source3/lib/pthreadpool/pthreadpool.c
+++ b/source3/lib/pthreadpool/pthreadpool.c
@@ -23,6 +23,11 @@
 #include "system/threads.h"
 #include "pthreadpool.h"
 #include "lib/util/dlinklist.h"
+
+#ifdef NDEBUG
+#undef NDEBUG
+#endif
+
 #include <assert.h>
 
 struct pthreadpool_job {
-- 
2.1.4


>From 49a3bfac6c5bf3c44c7b9484e81940d9a6a023e2 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 29 Aug 2016 11:35:39 +0200
Subject: [PATCH 2/2] pthreadpool: Signal job completion without the pool mutex

This essentially reverts 1c4284c7395f23. We now call an alien function from
within pthreadpool, and we should not hold a mutex during that call. The alien
function could (and pthreadpool_tevent_job_signal actually does) lock a mutex.
We can't guarantee proper lock ordering here, so in theory we could deadlock. I
haven't seen it in the wild yet, but I could imagine that both _parent pieces
in pthreadpool and tevent could trigger such a deadlock.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/pthreadpool/pthreadpool.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/source3/lib/pthreadpool/pthreadpool.c b/source3/lib/pthreadpool/pthreadpool.c
index 2e9f42c..ee59cc4 100644
--- a/source3/lib/pthreadpool/pthreadpool.c
+++ b/source3/lib/pthreadpool/pthreadpool.c
@@ -491,12 +491,13 @@ static void *pthreadpool_server(void *arg)
 
 			job.fn(job.private_data);
 
-			res = pthread_mutex_lock(&pool->mutex);
-			assert(res == 0);
-
 			ret = pool->signal_fn(job.id,
 					      job.fn, job.private_data,
 					      pool->signal_fn_private_data);
+
+			res = pthread_mutex_lock(&pool->mutex);
+			assert(res == 0);
+
 			if (ret != 0) {
 				pthreadpool_server_exit(pool);
 				pthread_mutex_unlock(&pool->mutex);
-- 
2.1.4