[PATCH] Cleanups for pthreadpool.c

Volker Lendecke Volker.Lendecke at SerNet.DE
Tue Dec 12 14:53:08 UTC 2017


Hi!

While hunting the deadlock I did the attached small patches.

Review appreciated!

Thanks, Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From fcd201bcfe99562bfecfe51a6058d24008d4352b Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 12 Dec 2017 13:52:56 +0100
Subject: [PATCH 1/2] pthreadpool: Simplify the logic in add_job a bit

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

diff --git a/lib/pthreadpool/pthreadpool.c b/lib/pthreadpool/pthreadpool.c
index b70694a6a1b..2dfda38f144 100644
--- a/lib/pthreadpool/pthreadpool.c
+++ b/lib/pthreadpool/pthreadpool.c
@@ -678,27 +678,33 @@ int pthreadpool_add_job(struct pthreadpool *pool, int job_id,
 	}
 
 	res = pthreadpool_create_thread(pool);
-	if (res != 0) {
-		if (pool->num_threads == 0) {
-			/*
-			 * No thread could be created to run job,
-			 * fallback to sync call.
-			 */
-			pthreadpool_undo_put_job(pool);
-			pthread_mutex_unlock(&pool->mutex);
-
-			fn(private_data);
-			return pool->signal_fn(job_id, fn, private_data,
-					       pool->signal_fn_private_data);
-		}
+	if (res == 0) {
+		res = pthread_mutex_unlock(&pool->mutex);
+		assert(res == 0);
+		return 0;
+	}
 
+	if (pool->num_threads != 0) {
 		/*
 		 * At least one thread is still available, let
 		 * that one run the queued job.
 		 */
-		res = 0;
+		res = pthread_mutex_unlock(&pool->mutex);
+		assert(res == 0);
+		return 0;
 	}
 
-	pthread_mutex_unlock(&pool->mutex);
+	/*
+	 * No thread could be created to run job, fallback to sync
+	 * call.
+	 */
+	pthreadpool_undo_put_job(pool);
+
+	res = pthread_mutex_unlock(&pool->mutex);
+	assert(res == 0);
+
+	fn(private_data);
+	res = pool->signal_fn(job_id, fn, private_data,
+			      pool->signal_fn_private_data);
 	return res;
 }
-- 
2.11.0


From cad890cad3c639fa4b9ce3ed72b33726700f93bf Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 12 Dec 2017 13:58:48 +0100
Subject: [PATCH 2/2] pthreadpool: Add some asserts

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

diff --git a/lib/pthreadpool/pthreadpool.c b/lib/pthreadpool/pthreadpool.c
index 2dfda38f144..92a88c9ca84 100644
--- a/lib/pthreadpool/pthreadpool.c
+++ b/lib/pthreadpool/pthreadpool.c
@@ -652,11 +652,13 @@ int pthreadpool_add_job(struct pthreadpool *pool, int job_id,
 	 * Add job to the end of the queue
 	 */
 	if (!pthreadpool_put_job(pool, job_id, fn, private_data)) {
-		pthread_mutex_unlock(&pool->mutex);
+		res = pthread_mutex_unlock(&pool->mutex);
+		assert(res == 0);
 		return ENOMEM;
 	}
 
 	if (pool->num_idle > 0) {
+		int unlock_res;
 		/*
 		 * We have idle threads, wake one.
 		 */
@@ -664,7 +666,8 @@ int pthreadpool_add_job(struct pthreadpool *pool, int job_id,
 		if (res != 0) {
 			pthreadpool_undo_put_job(pool);
 		}
-		pthread_mutex_unlock(&pool->mutex);
+		unlock_res = pthread_mutex_unlock(&pool->mutex);
+		assert(unlock_res == 0);
 		return res;
 	}
 
@@ -673,7 +676,8 @@ int pthreadpool_add_job(struct pthreadpool *pool, int job_id,
 		/*
 		 * No more new threads, we just queue the request
 		 */
-		pthread_mutex_unlock(&pool->mutex);
+		res = pthread_mutex_unlock(&pool->mutex);
+		assert(res == 0);
 		return 0;
 	}
 
-- 
2.11.0



More information about the samba-technical mailing list