[PATCHES][BUG 13170] Fix crash in pthreadpool thread after failure from pthread_create

[Christof Schmitt]

From 62a2a4e2afdc5bdfdf7687400b0eed46c833fb07 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Tue, 28 Nov 2017 10:49:36 -0700
Subject: [PATCH 1/2] pthreadpool: Move creating of thread to new function

No functional change, but this simplifies error handling.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13170

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 lib/pthreadpool/pthreadpool.c | 75 ++++++++++++++++++++++++-------------------
 1 file changed, 42 insertions(+), 33 deletions(-)

diff --git a/lib/pthreadpool/pthreadpool.c b/lib/pthreadpool/pthreadpool.c
index 23885aa..3b20e37 100644
--- a/lib/pthreadpool/pthreadpool.c
+++ b/lib/pthreadpool/pthreadpool.c
@@ -521,14 +521,52 @@ static void *pthreadpool_server(void *arg)
 	}
 }
 
-int pthreadpool_add_job(struct pthreadpool *pool, int job_id,
-			void (*fn)(void *private_data), void *private_data)
+static int pthreadpool_create_thread(struct pthreadpool *pool)
 {
 	pthread_attr_t thread_attr;
 	pthread_t thread_id;
 	int res;
 	sigset_t mask, omask;
 
+	/*
+	 * Create a new worker thread. It should not receive any signals.
+	 */
+
+	sigfillset(&mask);
+
+	res = pthread_attr_init(&thread_attr);
+	if (res != 0) {
+		return res;
+	}
+
+	res = pthread_attr_setdetachstate(
+		&thread_attr, PTHREAD_CREATE_DETACHED);
+	if (res != 0) {
+		pthread_attr_destroy(&thread_attr);
+		return res;
+	}
+
+	res = pthread_sigmask(SIG_BLOCK, &mask, &omask);
+	if (res != 0) {
+		pthread_attr_destroy(&thread_attr);
+		return res;
+	}
+
+	res = pthread_create(&thread_id, &thread_attr, pthreadpool_server,
+			     (void *)pool);
+
+	assert(pthread_sigmask(SIG_SETMASK, &omask, NULL) == 0);
+
+	pthread_attr_destroy(&thread_attr);
+
+	return res;
+}
+
+int pthreadpool_add_job(struct pthreadpool *pool, int job_id,
+			void (*fn)(void *private_data), void *private_data)
+{
+	int res;
+
 	res = pthread_mutex_lock(&pool->mutex);
 	if (res != 0) {
 		return res;
@@ -570,42 +608,13 @@ int pthreadpool_add_job(struct pthreadpool *pool, int job_id,
 		return 0;
 	}
 
-	/*
-	 * Create a new worker thread. It should not receive any signals.
-	 */
-
-	sigfillset(&mask);
-
-	res = pthread_attr_init(&thread_attr);
+	res = pthreadpool_create_thread(pool);
 	if (res != 0) {
 		pthread_mutex_unlock(&pool->mutex);
 		return res;
 	}
 
-	res = pthread_attr_setdetachstate(
-		&thread_attr, PTHREAD_CREATE_DETACHED);
-	if (res != 0) {
-		pthread_attr_destroy(&thread_attr);
-		pthread_mutex_unlock(&pool->mutex);
-		return res;
-	}
-
-        res = pthread_sigmask(SIG_BLOCK, &mask, &omask);
-	if (res != 0) {
-		pthread_attr_destroy(&thread_attr);
-		pthread_mutex_unlock(&pool->mutex);
-		return res;
-	}
-
-	res = pthread_create(&thread_id, &thread_attr, pthreadpool_server,
-			     (void *)pool);
-	if (res == 0) {
-		pool->num_threads += 1;
-	}
-
-        assert(pthread_sigmask(SIG_SETMASK, &omask, NULL) == 0);
-
-	pthread_attr_destroy(&thread_attr);
+	pool->num_threads += 1;
 
 	pthread_mutex_unlock(&pool->mutex);
 	return res;
-- 
1.8.3.1


From 53fb74bc002aede82d54f70f27fd9979d0ec99f9 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Tue, 28 Nov 2017 10:59:06 -0700
Subject: [PATCH 2/2] pthreadpool: Undo pthreadpool_put_job when returning
 error

When an error is returned to the caller of pthreadpool_add_job, the job
should not be kept in the internal job array. Otherwise the caller might
free the data structure and a later worker thread would still reference
it.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13170

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 lib/pthreadpool/pthreadpool.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/pthreadpool/pthreadpool.c b/lib/pthreadpool/pthreadpool.c
index 3b20e37..8ef3be2 100644
--- a/lib/pthreadpool/pthreadpool.c
+++ b/lib/pthreadpool/pthreadpool.c
@@ -429,6 +429,11 @@ static bool pthreadpool_put_job(struct pthreadpool *p,
 	return true;
 }
 
+static void pthreadpool_undo_put_job(struct pthreadpool *p)
+{
+	p->num_jobs -= 1;
+}
+
 static void *pthreadpool_server(void *arg)
 {
 	struct pthreadpool *pool = (struct pthreadpool *)arg;
@@ -595,6 +600,9 @@ int pthreadpool_add_job(struct pthreadpool *pool, int job_id,
 		 * We have idle threads, wake one.
 		 */
 		res = pthread_cond_signal(&pool->condvar);
+		if (res != 0) {
+			pthreadpool_undo_put_job(pool);
+		}
 		pthread_mutex_unlock(&pool->mutex);
 		return res;
 	}
@@ -610,6 +618,7 @@ int pthreadpool_add_job(struct pthreadpool *pool, int job_id,
 
 	res = pthreadpool_create_thread(pool);
 	if (res != 0) {
+		pthreadpool_undo_put_job(pool);
 		pthread_mutex_unlock(&pool->mutex);
 		return res;
 	}
-- 
1.8.3.1