[SCM] Samba Shared Repository - branch master updated

Christof Schmitt cs at samba.org
Fri Dec 8 12:55:03 UTC 2017


The branch, master has been updated
       via  8cdb399 pthreadpool: Add test for pthread_create failure
       via  8e17be1c wscript: Add check for --wrap linker flag
       via  065fb5d pthreadpool: Undo put_job when returning error
       via  949ccc3 pthreadpool: Move creating of thread to new function
      from  aaa946b s4:kdc: only map SDB_ERR_NOT_FOUND_HERE to HDB_ERR_NOT_FOUND_HERE

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 8cdb3995caf7a21d0c27a56a0bf0c1efd5b491e4
Author: Christof Schmitt <cs at samba.org>
Date:   Wed Dec 6 15:10:23 2017 -0700

    pthreadpool: Add test for pthread_create failure
    
    This is implemented using cmocka and the __wrap override for
    pthread_create.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13170
    
    Signed-off-by: Christof Schmitt <cs at samba.org
    Reviewed-by: Andreas Schneider <asn at samba.org>
    
    Autobuild-User(master): Christof Schmitt <cs at samba.org>
    Autobuild-Date(master): Fri Dec  8 13:54:20 CET 2017 on sn-devel-144

commit 8e17be1c3df09c238560c8a7e62c17e9f9ff9bc7
Author: Christof Schmitt <cs at samba.org>
Date:   Thu Dec 7 10:42:30 2017 -0700

    wscript: Add check for --wrap linker flag
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13170
    
    Signed-off-by: Christof Schmitt <cs at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>

commit 065fb5d94d25d19fc85832bb85aa9e379e8551cc
Author: Christof Schmitt <cs at samba.org>
Date:   Tue Nov 28 10:59:06 2017 -0700

    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 at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 949ccc3ea9073a3d38bff28345f644d39177256f
Author: Christof Schmitt <cs at samba.org>
Date:   Tue Nov 28 10:49:36 2017 -0700

    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>
    Reviewed-by: Volker Lendecke <vl at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 lib/pthreadpool/pthreadpool.c  | 103 ++++++++++++++++--------
 lib/pthreadpool/tests_cmocka.c | 176 +++++++++++++++++++++++++++++++++++++++++
 lib/pthreadpool/wscript_build  |   7 ++
 source3/selftest/tests.py      |   7 ++
 wscript                        |   4 +
 5 files changed, 263 insertions(+), 34 deletions(-)
 create mode 100644 lib/pthreadpool/tests_cmocka.c


Changeset truncated at 500 lines:

diff --git a/lib/pthreadpool/pthreadpool.c b/lib/pthreadpool/pthreadpool.c
index 23885aa..309aba9 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;
@@ -521,14 +526,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;
@@ -557,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;
 	}
@@ -570,43 +620,28 @@ 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;
-	}
+		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);
 
-        res = pthread_sigmask(SIG_BLOCK, &mask, &omask);
-	if (res != 0) {
-		pthread_attr_destroy(&thread_attr);
-		pthread_mutex_unlock(&pool->mutex);
-		return res;
-	}
+			fn(private_data);
+			return pool->signal_fn(job_id, fn, private_data,
+					       pool->signal_fn_private_data);
+		}
 
-	res = pthread_create(&thread_id, &thread_attr, pthreadpool_server,
-			     (void *)pool);
-	if (res == 0) {
-		pool->num_threads += 1;
+		/*
+		 * At least one thread is still available, let
+		 * that one run the queued job.
+		 */
+		res = 0;
 	}
 
-        assert(pthread_sigmask(SIG_SETMASK, &omask, NULL) == 0);
-
-	pthread_attr_destroy(&thread_attr);
-
 	pthread_mutex_unlock(&pool->mutex);
 	return res;
 }
diff --git a/lib/pthreadpool/tests_cmocka.c b/lib/pthreadpool/tests_cmocka.c
new file mode 100644
index 0000000..75a935f
--- /dev/null
+++ b/lib/pthreadpool/tests_cmocka.c
@@ -0,0 +1,176 @@
+/*
+ * Unix SMB/CIFS implementation.
+ * cmocka tests for thread pool implementation
+ * Copyright (C) Christof Schmitt 2017
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <errno.h>
+#include <pthread.h>
+#include <setjmp.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <talloc.h>
+#include <tevent.h>
+#include <pthreadpool_tevent.h>
+
+#include <cmocka.h>
+
+struct pthreadpool_tevent_test {
+	struct tevent_context *ev;
+	struct pthreadpool_tevent *pool;
+};
+
+static int setup_pthreadpool_tevent(void **state)
+{
+	struct pthreadpool_tevent_test *t;
+	int ret;
+
+	t = talloc(NULL, struct pthreadpool_tevent_test);
+	assert_non_null(t);
+
+	t->ev = tevent_context_init(t);
+	assert_non_null(t->ev);
+
+	ret = pthreadpool_tevent_init(t->ev, 0, &t->pool);
+	assert_return_code(ret, 0);
+
+	*state = t;
+
+	return 0;
+}
+
+static int teardown_pthreadpool_tevent(void **state)
+{
+	struct pthreadpool_tevent_test *t = *state;
+
+	TALLOC_FREE(t);
+
+	return 0;
+}
+
+int __wrap_pthread_create(pthread_t *thread, const pthread_attr_t *attr,
+			  void *(*start_routine) (void *), void *arg);
+int __real_pthread_create(pthread_t *thread, const pthread_attr_t *attr,
+			  void *(*start_routine) (void *),  void *arg);
+
+int __wrap_pthread_create(pthread_t *thread, const pthread_attr_t *attr,
+			  void *(*start_routine) (void *), void *arg)
+{
+	int error;
+
+	error = mock_type(int);
+	if (error != 0) {
+		return error;
+	}
+
+	return __real_pthread_create(thread, attr, start_routine, arg);
+}
+
+static void test_job_threadid(void *ptr)
+{
+	pthread_t *threadid = ptr;
+
+	*threadid = pthread_self();
+}
+
+static int test_create_do(struct tevent_context *ev,
+			  struct pthreadpool_tevent *pool,
+			  bool *in_main_thread)
+{
+	struct tevent_req *req;
+	pthread_t main_thread, worker_thread;
+	bool ok;
+	int ret;
+
+	main_thread = pthread_self();
+
+	req = pthreadpool_tevent_job_send(
+		ev, ev, pool, test_job_threadid, &worker_thread);
+	if (req == NULL) {
+		fprintf(stderr, "pthreadpool_tevent_job_send failed\n");
+		TALLOC_FREE(ev);
+		return ENOMEM;
+	}
+
+	ok = tevent_req_poll(req, ev);
+	if (!ok) {
+		ret = errno;
+		fprintf(stderr, "tevent_req_poll failed: %s\n",
+			strerror(ret));
+		TALLOC_FREE(ev);
+		return ret;
+	}
+
+
+	ret = pthreadpool_tevent_job_recv(req);
+	TALLOC_FREE(req);
+	if (ret != 0) {
+		fprintf(stderr, "tevent_req_recv failed: %s\n",
+			strerror(ret));
+		TALLOC_FREE(ev);
+		return ret;
+	}
+	*in_main_thread = pthread_equal(worker_thread, main_thread);
+
+	return 0;
+}
+
+static void test_create(void **state)
+{
+	struct pthreadpool_tevent_test *t = *state;
+	bool in_main_thread;
+	int ret;
+
+	/*
+	 * When pthreadpool cannot create the first worker thread,
+	 * this job will run in the sync fallback in the main thread.
+	 */
+	will_return(__wrap_pthread_create, EAGAIN);
+	ret = test_create_do(t->ev, t->pool, &in_main_thread);
+	assert_return_code(ret, 0);
+	assert_true(in_main_thread);
+
+	/*
+	 * When a thread can be created, the job will run in the worker thread.
+	 */
+	will_return(__wrap_pthread_create, 0);
+	ret = test_create_do(t->ev, t->pool, &in_main_thread);
+	assert_return_code(ret, 0);
+	assert_false(in_main_thread);
+
+	/*
+	 * Workerthread will still be active for a second; immediately
+	 * running another job will also use the worker thread, even
+	 * if a new thread cannot be created.
+	 */
+	ret = test_create_do(t->ev, t->pool, &in_main_thread);
+	assert_return_code(ret, 0);
+	assert_false(in_main_thread);
+}
+
+int main(int argc, char **argv)
+{
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test_setup_teardown(test_create,
+						setup_pthreadpool_tevent,
+						teardown_pthreadpool_tevent),
+	};
+
+	cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
+
+	return cmocka_run_group_tests(tests, NULL, NULL);
+}
diff --git a/lib/pthreadpool/wscript_build b/lib/pthreadpool/wscript_build
index d530463..57df255 100644
--- a/lib/pthreadpool/wscript_build
+++ b/lib/pthreadpool/wscript_build
@@ -21,3 +21,10 @@ bld.SAMBA_BINARY('pthreadpooltest',
                   deps='PTHREADPOOL',
                   enabled=bld.env.WITH_PTHREADPOOL,
                   install=False)
+
+bld.SAMBA_BINARY('pthreadpooltest_cmocka',
+                  source='tests_cmocka.c',
+                  deps='PTHREADPOOL cmocka',
+                  ldflags='-Wl,--wrap=pthread_create',
+                  enabled=bld.env.WITH_PTHREADPOOL and bld.env['HAVE_LDWRAP'],
+                  install=False)
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 8360cea..1b35768 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -57,6 +57,9 @@ finally:
 have_libarchive = ("HAVE_LIBARCHIVE" in config_hash)
 have_linux_kernel_oplocks = ("HAVE_KERNEL_OPLOCKS_LINUX" in config_hash)
 have_inotify = ("HAVE_INOTIFY" in config_hash)
+have_ldwrap = ("HAVE_LDWRAP" in config_hash)
+with_pthreadpool = ("WITH_PTHREADPOOL" in config_hash)
+
 
 plantestsuite("samba3.blackbox.success", "nt4_dc:local", [os.path.join(samba3srcdir, "script/tests/test_success.sh")])
 plantestsuite("samba3.blackbox.failure", "nt4_dc:local", [os.path.join(samba3srcdir, "script/tests/test_failure.sh")])
@@ -332,6 +335,10 @@ plantestsuite(
     "samba3.pthreadpool", "nt4_dc",
     [os.path.join(samba3srcdir, "script/tests/test_pthreadpool.sh")])
 
+if with_pthreadpool and have_ldwrap:
+    plantestsuite("samba3.pthreadpool_cmocka", "nt4_dc",
+                  [os.path.join(bindir(), "pthreadpooltest_cmocka")])
+
 plantestsuite("samba3.async_req", "nt4_dc",
               [os.path.join(samba3srcdir, "script/tests/test_async_req.sh")])
 
diff --git a/wscript b/wscript
index b167102..0ef5f60 100644
--- a/wscript
+++ b/wscript
@@ -183,6 +183,10 @@ def configure(conf):
 
     conf.RECURSE('lib/ldb')
 
+    if conf.CHECK_LDFLAGS(['-Wl,--wrap=test']):
+        conf.env['HAVE_LDWRAP'] = True
+        conf.define('HAVE_LDWRAP', 1)
+
     if not (Options.options.without_ad_dc):
         conf.DEFINE('AD_DC_BUILD_IS_ENABLED', 1)
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list