Attachment 13853 Details for Bug 13170 – Patches for 4.6

[patch] Patches for 4.6

patches-v4-6 (text/plain), 5.60 KB, created by Christof Schmitt on 2017-12-08 16:28:50 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Christof Schmitt

Created: 2017-12-08 16:28:50 UTC

Size: 5.60 KB

patch

obsolete

>From b0998edfecf79109a3c808eabb2beb9dbceb3405 Mon Sep 17 00:00:00 2001
>From: Christof Schmitt <cs@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@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(cherry picked from commit 949ccc3ea9073a3d38bff28345f644d39177256f)
>---
> source3/lib/pthreadpool/pthreadpool.c | 79 ++++++++++++++++++++---------------
> 1 file changed, 45 insertions(+), 34 deletions(-)
>
>diff --git a/source3/lib/pthreadpool/pthreadpool.c b/source3/lib/pthreadpool/pthreadpool.c
>index 23885aa..4e1e5d4 100644
>--- a/source3/lib/pthreadpool/pthreadpool.c
>+++ b/source3/lib/pthreadpool/pthreadpool.c
>@@ -521,14 +521,56 @@ 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);
>+
>+	if (res == 0) {
>+		pool->num_threads += 1;
>+	}
>+
>+	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,43 +612,12 @@ 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);
>-	if (res != 0) {
>-		pthread_mutex_unlock(&pool->mutex);
>-		return res;
>-	}
>-
>-	res = pthread_attr_setdetachstate(
>-		&thread_attr, PTHREAD_CREATE_DETACHED);
>+	res = pthreadpool_create_thread(pool);
> 	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);
>-
> 	pthread_mutex_unlock(&pool->mutex);
> 	return res;
> }
>-- 
>1.8.3.1
>
>
>From ce267f0e61fec07e19f1cd7390618a095e4e7160 Mon Sep 17 00:00:00 2001
>From: Christof Schmitt <cs@samba.org>
>Date: Tue, 28 Nov 2017 10:59:06 -0700
>Subject: [PATCH 2/2] pthreadpool: Undo 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.
>
>When it is not possible to create a single worker thread, the system
>might be out of resources or hitting a configured limit. In this case
>fall back to calling the job function synchronously instead of raising
>the error to the caller and possibly back to the SMB client.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13170
>
>Signed-off-by: Christof Schmitt <cs@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(cherry picked from commit 065fb5d94d25d19fc85832bb85aa9e379e8551cc)
>---
> source3/lib/pthreadpool/pthreadpool.c | 28 ++++++++++++++++++++++++++--
> 1 file changed, 26 insertions(+), 2 deletions(-)
>
>diff --git a/source3/lib/pthreadpool/pthreadpool.c b/source3/lib/pthreadpool/pthreadpool.c
>index 4e1e5d4..309aba9 100644
>--- a/source3/lib/pthreadpool/pthreadpool.c
>+++ b/source3/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;
>@@ -599,6 +604,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;
> 	}
>@@ -614,8 +622,24 @@ int pthreadpool_add_job(struct pthreadpool *pool, int job_id,
> 
> 	res = pthreadpool_create_thread(pool);
> 	if (res != 0) {
>-		pthread_mutex_unlock(&pool->mutex);
>-		return res;
>+		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);
>+		}
>+
>+		/*
>+		 * At least one thread is still available, let
>+		 * that one run the queued job.
>+		 */
>+		res = 0;
> 	}
> 
> 	pthread_mutex_unlock(&pool->mutex);
>-- 
>1.8.3.1
>

Flags:

vl: review+

Actions: View

Attachments on bug 13170: 13852 | 13853 | 13855