[PATCH] tevent and threads - infrastructure improvements - version #2
Ralph Böhme
rb at sernet.de
Sun Sep 13 12:34:11 UTC 2015
Hi Jeremy,
On Fri, Jul 24, 2015 at 10:16:15AM -0700, Jeremy Allison wrote:
> On Thu, Jul 23, 2015 at 04:50:37PM -0700, Jeremy Allison wrote:
> >
> > FYI. I now have a working implementation of this
> > API - passes valgrind memcheck and drd !
> >
> > Hurrah for me :-).
> >
> > Will post an updated patch once I've finished
> > updating the tutorial.
>
> Here it is. Passes valgrind --tool=drd and
> valgrind --tool=memcheck.
>
> Metze please let me know if this is what
> you had in mind.
>
> Everyone else just review :-).
a few minor issue:
* always use talloc_get_type_abort() where possible
* reverse the order of signalling and unlocking in
tevent_thread_proxy_schedule()
Fixups attached.
-Ralph
--
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@sernet.de
-------------- next part --------------
From ccb293d69c8321f353339e5d00b8a07639b515e2 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 13 Sep 2015 12:53:32 +0200
Subject: [PATCH 1/5] FIXUP: lib: tevent: Initial checkin of threaded tevent
context
Use talloc_get_type_abort
---
lib/tevent/tevent_threads.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/tevent/tevent_threads.c b/lib/tevent/tevent_threads.c
index 7db8f47..78e4fe2 100644
--- a/lib/tevent/tevent_threads.c
+++ b/lib/tevent/tevent_threads.c
@@ -68,7 +68,7 @@ static void free_list_handler(struct tevent_context *ev,
void *private_ptr)
{
struct tevent_thread_proxy *tp =
- (struct tevent_thread_proxy *)private_ptr;
+ talloc_get_type_abort(private_ptr, struct tevent_thread_proxy);
int ret;
ret = pthread_mutex_lock(&tp->mutex);
@@ -127,7 +127,7 @@ static void pipe_read_handler(struct tevent_context *ev,
void *private_ptr)
{
struct tevent_thread_proxy *tp =
- (struct tevent_thread_proxy *)private_ptr;
+ talloc_get_type_abort(private_ptr, struct tevent_thread_proxy);
ssize_t len = 64;
int ret;
--
2.1.0
From 7d38a7688fc1036f4e66047dd44bf468be1e0f42 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 13 Sep 2015 12:54:04 +0200
Subject: [PATCH 2/5] FIXUP: lib: tevent: Initial checkin of threaded tevent
context
Reverse order of unlocking and notifying in
tevent_thread_proxy_schedule(): unlocking before notifying avoids a
potential wakeup of the signalled thread with the mutex still locked.
---
lib/tevent/tevent_threads.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/lib/tevent/tevent_threads.c b/lib/tevent/tevent_threads.c
index 78e4fe2..11895ed 100644
--- a/lib/tevent/tevent_threads.c
+++ b/lib/tevent/tevent_threads.c
@@ -316,13 +316,13 @@ void tevent_thread_proxy_schedule(struct tevent_thread_proxy *tp,
if (tp->write_fd == -1) {
/* In the process of being destroyed. Ignore. */
- goto end;
+ goto fail;
}
/* Create a new immediate_list entry. MUST BE ON THE NULL CONTEXT */
im_entry = talloc_zero(NULL, struct tevent_immediate_list);
if (im_entry == NULL) {
- goto end;
+ goto fail;
}
im_entry->handler = handler;
@@ -335,12 +335,23 @@ void tevent_thread_proxy_schedule(struct tevent_thread_proxy *tp,
DLIST_ADD(tp->im_list, im_entry);
+ /*
+ * Unlocking before notifying avoids a potential wakeup of the
+ * signalled thread with the mutex still locked
+ */
+ ret = pthread_mutex_unlock(&tp->mutex);
+ if (ret != 0) {
+ abort();
+ /* Notreached. */
+ }
+
/* And notify the dest_ev_ctx to wake up. */
c = '\0';
(void)write(tp->write_fd, &c, 1);
- end:
+ return;
+fail:
ret = pthread_mutex_unlock(&tp->mutex);
if (ret != 0) {
abort();
--
2.1.0
From 3ef9a4c6281351bb17c7e47876d85c657601f163 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 13 Sep 2015 13:24:38 +0200
Subject: [PATCH 3/5] FIXUP: lib: tevent: Initial test of tevent threaded
context code
Use tallo_get_type_abort().
---
lib/tevent/testsuite.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/tevent/testsuite.c b/lib/tevent/testsuite.c
index a43468d..13a61da 100644
--- a/lib/tevent/testsuite.c
+++ b/lib/tevent/testsuite.c
@@ -820,7 +820,8 @@ static void callback_nowait(struct tevent_context *ev,
struct tevent_immediate *im,
void *private_ptr)
{
- pthread_t *thread_id_ptr = (pthread_t *)private_ptr;
+ pthread_t *thread_id_ptr =
+ talloc_get_type_abort(private_ptr, pthread_t);
unsigned i;
for (i = 0; i < NUM_TEVENT_THREADS; i++) {
@@ -840,7 +841,7 @@ static void callback_nowait(struct tevent_context *ev,
static void *thread_fn_nowait(void *private_ptr)
{
struct tevent_thread_proxy *master_tp =
- (struct tevent_thread_proxy *)private_ptr;
+ talloc_get_type_abort(private_ptr, struct tevent_thread_proxy);
struct tevent_immediate *im;
pthread_t *thread_id_ptr;
--
2.1.0
From be46f814d59e2113e45293d3ea84a41e5b548e84 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 13 Sep 2015 13:54:05 +0200
Subject: [PATCH 4/5] FIXUP: lib: tevent: tests: Add a second thread test that
does request/reply.
Use talloc_get_type_abort().
---
lib/tevent/testsuite.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/lib/tevent/testsuite.c b/lib/tevent/testsuite.c
index 13a61da..1937f90 100644
--- a/lib/tevent/testsuite.c
+++ b/lib/tevent/testsuite.c
@@ -951,7 +951,9 @@ static void thread_callback(struct tevent_context *ev,
void *private_ptr)
{
struct reply_state *rsp =
- (struct reply_state *)talloc_move(ev, &private_ptr);
+ talloc_get_type_abort(private_ptr, struct reply_state);
+
+ talloc_steal(ev, rsp);
*rsp->p_finished = 1;
}
@@ -961,9 +963,11 @@ static void master_callback(struct tevent_context *ev,
void *private_ptr)
{
struct reply_state *rsp =
- (struct reply_state *)talloc_move(ev, &private_ptr);
+ talloc_get_type_abort(private_ptr, struct reply_state);
unsigned i;
+ talloc_steal(ev, rsp);
+
for (i = 0; i < NUM_TEVENT_THREADS; i++) {
if (pthread_equal(rsp->thread_id,
thread_map[i])) {
@@ -986,7 +990,7 @@ static void master_callback(struct tevent_context *ev,
static void *thread_fn_1(void *private_ptr)
{
struct tevent_thread_proxy *master_tp =
- (struct tevent_thread_proxy *)private_ptr;
+ talloc_get_type_abort(private_ptr, struct tevent_thread_proxy);
struct tevent_thread_proxy *tp;
struct tevent_immediate *im;
struct tevent_context *ev;
--
2.1.0
From f78a3ca73e97633443a121853b8ff3efd52f454c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 13 Sep 2015 14:02:50 +0200
Subject: [PATCH 5/5] FIXUP: lib: tevent: docs: Add tutorial on thread usage.
Use talloc_get_type_abort().
---
lib/tevent/doc/tevent_thread.dox | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/lib/tevent/doc/tevent_thread.dox b/lib/tevent/doc/tevent_thread.dox
index 91d3f21..c1c2f70 100644
--- a/lib/tevent/doc/tevent_thread.dox
+++ b/lib/tevent/doc/tevent_thread.dox
@@ -211,7 +211,10 @@ static void thread_callback(struct tevent_context *ev,
// points to from the tevent library back to this thread.
struct reply_state *rsp =
- (struct reply_state *)talloc_move(ev, &private_ptr);
+ talloc_get_type_abort(private_ptr, struct reply_state);
+
+ talloc_steal(ev, rsp);
+
*rsp->p_finished = true;
// im will be talloc_freed on return from this call.
@@ -228,8 +231,9 @@ static void master_callback(struct tevent_context *ev,
// points to from the tevent library to this thread.
struct reply_state *rsp =
- (struct reply_state *)talloc_move(ev, &private_ptr);
- }
+ talloc_get_type_abort(private_ptr, struct reply_state);
+
+ talloc_steal(ev, rsp);
printf("Callback from thread %s\n", thread_id_to_string(rsp->thread_id));
@@ -247,7 +251,8 @@ static void master_callback(struct tevent_context *ev,
static void *thread_fn(void *private_ptr)
{
- struct tevent_thread_proxy *master_tp = (struct tevent_thread_proxy *)private_ptr;
+ struct tevent_thread_proxy *master_tp =
+ talloc_get_type_abort(private_ptr, struct tevent_thread_proxy);
bool finished = false;
int ret;
--
2.1.0
More information about the samba-technical
mailing list