[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Thu Jun 8 22:46:05 UTC 2017


The branch, master has been updated
       via  1828011 tevent: Fix a race condition in tevent context rundown
       via  00390ae tevent: Fix a memleak on FreeBSD
       via  ca71576 tevent: Add tevent_re_initialise to threaded test
       via  afe026d tevent: Re-init threading in tevent_re_initialise
       via  97d912d tevent: Factor out context initialization
       via  b034750 tevent: Fix a typo
      from  08a21f3 messaging: fix net command failure due to unhandled return code

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


- Log -----------------------------------------------------------------
commit 1828011317b0a8142c3b66fff22661a962760574
Author: Volker Lendecke <vl at samba.org>
Date:   Wed May 24 16:22:34 2017 +0200

    tevent: Fix a race condition in tevent context rundown
    
    We protect setting tctx->event_ctx=NULL with tctx->event_ctx_mutex.
    But in _tevent_threaded_schedule_immediate we have the classic
    TOCTOU race: After we checked "ev==NULL", looking at
    tevent_common_context_destructor the event context can go after
    _tevent_threaded_schedule_immediate checked. We need to serialize
    things a bit by keeping tctx->event_ctx_mutex locked while we
    reference "ev", in particular in the
    
    DLIST_ADD_END(ev->scheduled_immediates,im);
    
    I think the locking hierarchy is still maintained, tevent_atfork_prepare()
    first locks all the tctx locks, and then the scheduled_mutex.  Also,
    I don't think this will impact parallelism too badly: event_ctx_mutex
    is only used to protect setting tctx->ev.
    
    Found by staring at code while fixing the FreeBSD memleak due to
    not destroying scheduled_mutex.
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Fri Jun  9 00:45:26 CEST 2017 on sn-devel-144

commit 00390ae27b6bd207add571d7975c37951e15a3e5
Author: Volker Lendecke <vl at samba.org>
Date:   Wed May 24 16:21:40 2017 +0200

    tevent: Fix a memleak on FreeBSD
    
    FreeBSD has malloc'ed memory attached to mutexes. We need to clean this up.
    
    valgrind really helped here
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit ca715762418284a1a2acc81d40e9e429e407ce14
Author: Volker Lendecke <vl at samba.org>
Date:   Mon Jun 5 07:29:11 2017 +0200

    tevent: Add tevent_re_initialise to threaded test
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit afe026d3030c0c05a31de872dd0d120511ba6652
Author: Volker Lendecke <vl at samba.org>
Date:   Mon Jun 5 07:16:17 2017 +0200

    tevent: Re-init threading in tevent_re_initialise
    
    Without this threading is not usable after that call
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 97d912d99afb115e17f683a55aef447dc92ced49
Author: Volker Lendecke <vl at samba.org>
Date:   Mon Jun 5 06:58:37 2017 +0200

    tevent: Factor out context initialization
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit b03475048a49db78422d1bfc11f2c69d56fbf87f
Author: Volker Lendecke <vl at samba.org>
Date:   Mon Jun 5 07:23:27 2017 +0200

    tevent: Fix a typo
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

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

Summary of changes:
 lib/tevent/testsuite.c      |  8 ++++++
 lib/tevent/tevent.c         | 66 ++++++++++++++++++++++++++++-----------------
 lib/tevent/tevent.h         |  2 +-
 lib/tevent/tevent_threads.c | 14 ++++++----
 4 files changed, 59 insertions(+), 31 deletions(-)


Changeset truncated at 500 lines:

diff --git a/lib/tevent/testsuite.c b/lib/tevent/testsuite.c
index 4783ab4..ee29e5b 100644
--- a/lib/tevent/testsuite.c
+++ b/lib/tevent/testsuite.c
@@ -1207,6 +1207,14 @@ static bool test_multi_tevent_threaded_2(struct torture_context *test,
 	ev = tevent_context_init(test);
 	torture_assert(test, ev != NULL, "tevent_context_init failed");
 
+	/*
+	 * tevent_re_initialise used to have a bug where it did not
+	 * re-initialise the thread support after taking it
+	 * down. Excercise that code path.
+	 */
+	ret = tevent_re_initialise(ev);
+	torture_assert(test, ret == 0, "tevent_re_initialise failed");
+
 	tctx = tevent_threaded_context_create(ev, ev);
 	torture_assert(test, tctx != NULL,
 		       "tevent_threaded_context_create failed");
diff --git a/lib/tevent/tevent.c b/lib/tevent/tevent.c
index 65b101f..f3b18a1 100644
--- a/lib/tevent/tevent.c
+++ b/lib/tevent/tevent.c
@@ -341,6 +341,11 @@ int tevent_common_context_destructor(struct tevent_context *ev)
 
 		DLIST_REMOVE(ev->threaded_contexts, tctx);
 	}
+
+	ret = pthread_mutex_destroy(&ev->scheduled_mutex);
+	if (ret != 0) {
+		abort();
+	}
 #endif
 
 	tevent_common_wakeup_fini(ev);
@@ -392,46 +397,26 @@ int tevent_common_context_destructor(struct tevent_context *ev)
 	return 0;
 }
 
-/*
-  create a event_context structure for a specific implemementation.
-  This must be the first events call, and all subsequent calls pass
-  this event_context as the first element. Event handlers also
-  receive this as their first argument.
-
-  This function is for allowing third-party-applications to hook in gluecode
-  to their own event loop code, so that they can make async usage of our client libs
-
-  NOTE: use tevent_context_init() inside of samba!
-*/
-struct tevent_context *tevent_context_init_ops(TALLOC_CTX *mem_ctx,
-					       const struct tevent_ops *ops,
-					       void *additional_data)
+static int tevent_common_context_constructor(struct tevent_context *ev)
 {
-	struct tevent_context *ev;
 	int ret;
 
-	ev = talloc_zero(mem_ctx, struct tevent_context);
-	if (!ev) return NULL;
-
 #ifdef HAVE_PTHREAD
 
 	ret = pthread_once(&tevent_atfork_initialized, tevent_prep_atfork);
 	if (ret != 0) {
-		talloc_free(ev);
-		return NULL;
+		return ret;
 	}
 
 	ret = pthread_mutex_init(&ev->scheduled_mutex, NULL);
 	if (ret != 0) {
-		talloc_free(ev);
-		return NULL;
+		return ret;
 	}
 
 	ret = pthread_mutex_lock(&tevent_contexts_mutex);
 	if (ret != 0) {
 		pthread_mutex_destroy(&ev->scheduled_mutex);
-		talloc_free(ev);
-		return NULL;
+		return ret;
 	}
 
 	DLIST_ADD(tevent_contexts, ev);
@@ -440,11 +425,40 @@ struct tevent_context *tevent_context_init_ops(TALLOC_CTX *mem_ctx,
 	if (ret != 0) {
 		abort();
 	}
-
 #endif
 
 	talloc_set_destructor(ev, tevent_common_context_destructor);
 
+	return 0;
+}
+
+/*
+  create a event_context structure for a specific implemementation.
+  This must be the first events call, and all subsequent calls pass
+  this event_context as the first element. Event handlers also
+  receive this as their first argument.
+
+  This function is for allowing third-party-applications to hook in gluecode
+  to their own event loop code, so that they can make async usage of our client libs
+
+  NOTE: use tevent_context_init() inside of samba!
+*/
+struct tevent_context *tevent_context_init_ops(TALLOC_CTX *mem_ctx,
+					       const struct tevent_ops *ops,
+					       void *additional_data)
+{
+	struct tevent_context *ev;
+	int ret;
+
+	ev = talloc_zero(mem_ctx, struct tevent_context);
+	if (!ev) return NULL;
+
+	ret = tevent_common_context_constructor(ev);
+	if (ret != 0) {
+		talloc_free(ev);
+		return NULL;
+	}
+
 	ev->ops = ops;
 	ev->additional_data = additional_data;
 
@@ -875,6 +889,8 @@ int tevent_re_initialise(struct tevent_context *ev)
 {
 	tevent_common_context_destructor(ev);
 
+	tevent_common_context_constructor(ev);
+
 	return ev->ops->context_init(ev);
 }
 
diff --git a/lib/tevent/tevent.h b/lib/tevent/tevent.h
index ba4bb4d..728cf62 100644
--- a/lib/tevent/tevent.h
+++ b/lib/tevent/tevent.h
@@ -1777,7 +1777,7 @@ void tevent_thread_proxy_schedule(struct tevent_thread_proxy *tp,
  *
  * It is the duty of the caller of tevent_threaded_context_create() to
  * keep the event context around longer than any
- * tevent_threaded_context. tevent will abort if ev is talllc_free'ed
+ * tevent_threaded_context. tevent will abort if ev is talloc_free'ed
  * with an active tevent_threaded_context.
  *
  * If tevent is build without pthread support, this always returns
diff --git a/lib/tevent/tevent_threads.c b/lib/tevent/tevent_threads.c
index 8197323..8ecda02 100644
--- a/lib/tevent/tevent_threads.c
+++ b/lib/tevent/tevent_threads.c
@@ -443,15 +443,14 @@ void _tevent_threaded_schedule_immediate(struct tevent_threaded_context *tctx,
 
 	ev = tctx->event_ctx;
 
-	ret = pthread_mutex_unlock(&tctx->event_ctx_mutex);
-	if (ret != 0) {
-		abort();
-	}
-
 	if (ev == NULL) {
 		/*
 		 * Our event context is already gone.
 		 */
+		ret = pthread_mutex_unlock(&tctx->event_ctx_mutex);
+		if (ret != 0) {
+			abort();
+		}
 		return;
 	}
 
@@ -479,6 +478,11 @@ void _tevent_threaded_schedule_immediate(struct tevent_threaded_context *tctx,
 		abort();
 	}
 
+	ret = pthread_mutex_unlock(&tctx->event_ctx_mutex);
+	if (ret != 0) {
+		abort();
+	}
+
 	/*
 	 * We might want to wake up the main thread under the lock. We
 	 * had a slightly similar situation in pthreadpool, changed


-- 
Samba Shared Repository



More information about the samba-cvs mailing list