From b0998edfecf79109a3c808eabb2beb9dbceb3405 Mon Sep 17 00:00:00 2001 From: Christof Schmitt 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 Reviewed-by: Volker Lendecke (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 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 Reviewed-by: Volker Lendecke (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