[PATCH] Fix pthreadpools with fork()

[Volker Lendecke]

Hi!

Review would be 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 2fdb69cc1d341b769a049aa28d470a3966ab9476 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 3 Mar 2014 12:20:41 +0100
Subject: [PATCH 1/2] pthreadpool: Fix pthreadpools with fork

The current could would crash if a pthreadpool was created, deleted and the
process then fork()s. "pthreadpools" is NULL in this case, but the
pthread_atfork handlers are in place. This fixes walking the pthreadpools list
in reverse.

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

diff --git a/source3/lib/pthreadpool/pthreadpool.c b/source3/lib/pthreadpool/pthreadpool.c
index bd58d62..654d420 100644
--- a/source3/lib/pthreadpool/pthreadpool.c
+++ b/source3/lib/pthreadpool/pthreadpool.c
@@ -188,16 +188,11 @@ static void pthreadpool_parent(void)
 	int ret;
 	struct pthreadpool *pool;
 
-	pool = DLIST_TAIL(pthreadpools);
-
-	while (1) {
+	for (pool = DLIST_TAIL(pthreadpools);
+	     pool != NULL;
+	     pool = DLIST_PREV(pool)) {
 		ret = pthread_mutex_unlock(&pool->mutex);
 		assert(ret == 0);
-
-		if (pool == pthreadpools) {
-			break;
-		}
-		pool = pool->prev;
 	}
 
 	ret = pthread_mutex_unlock(&pthreadpools_mutex);
@@ -209,9 +204,10 @@ static void pthreadpool_child(void)
 	int ret;
 	struct pthreadpool *pool;
 
-	pool = DLIST_TAIL(pthreadpools);
+	for (pool = DLIST_TAIL(pthreadpools);
+	     pool != NULL;
+	     pool = DLIST_PREV(pool)) {
 
-	while (1) {
 		close(pool->sig_pipe[0]);
 		close(pool->sig_pipe[1]);
 
@@ -236,11 +232,6 @@ static void pthreadpool_child(void)
 
 		ret = pthread_mutex_unlock(&pool->mutex);
 		assert(ret == 0);
-
-		if (pool == pthreadpools) {
-			break;
-		}
-		pool = pool->prev;
 	}
 
 	ret = pthread_mutex_unlock(&pthreadpools_mutex);
-- 
1.8.1.2


From 4ed022997ea27391d83c4fc5ff88d35b452967cd Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 3 Mar 2014 11:57:18 +0000
Subject: [PATCH 2/2] pthreadpool: Add test for fork crash

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

diff --git a/source3/lib/pthreadpool/tests.c b/source3/lib/pthreadpool/tests.c
index 95d37b6..170cedf 100644
--- a/source3/lib/pthreadpool/tests.c
+++ b/source3/lib/pthreadpool/tests.c
@@ -5,6 +5,8 @@
 #include <stdlib.h>
 #include <pthread.h>
 #include <unistd.h>
+#include <sys/types.h>
+#include <sys/wait.h>
 #include "pthreadpool.h"
 
 static int test_init(void)
@@ -318,6 +320,46 @@ static int test_threaded_addjob(int num_pools, int num_threads, int poolsize,
 	return 0;
 }
 
+static int test_fork(void)
+{
+	struct pthreadpool *p;
+	pid_t child, waited;
+	int status, ret;
+
+	ret = pthreadpool_init(1, &p);
+	if (ret != 0) {
+		fprintf(stderr, "pthreadpool_init failed: %s\n",
+			strerror(ret));
+		return -1;
+	}
+	ret = pthreadpool_destroy(p);
+	if (ret != 0) {
+		fprintf(stderr, "pthreadpool_destroy failed: %s\n",
+			strerror(ret));
+		return -1;
+	}
+
+	child = fork();
+	if (child < 0) {
+		perror("fork failed");
+		return -1;
+	}
+	if (child == 0) {
+		exit(0);
+	}
+	waited = wait(&status);
+	if (waited == -1) {
+		perror("wait failed");
+		return -1;
+	}
+	if (waited != child) {
+		fprintf(stderr, "expected child %d, got %d\n",
+			(int)child, (int)waited);
+		return -1;
+	}
+	return 0;
+}
+
 int main(void)
 {
 	int ret;
@@ -328,6 +370,12 @@ int main(void)
 		return 1;
 	}
 
+	ret = test_fork();
+	if (ret != 0) {
+		fprintf(stderr, "test_fork failed\n");
+		return 1;
+	}
+
 	ret = test_jobs(10, 10000);
 	if (ret != 0) {
 		fprintf(stderr, "test_jobs failed\n");
-- 
1.8.1.2