[Patches] messaging cleanups
Stefan Metzmacher
metze at samba.org
Tue Apr 24 09:11:53 UTC 2018
Hi,
here're some minor cleanup patches for the messaging / tevent glue.
These prepare my work on getting a generic impersonation via tevent
wrapper contexts, this will allow to safely hook async path based
functions into the VFS layer later. See
https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-impersonate
The patches are made by Ralph and myself, already reviewed by Volker.
I'll push them shortly...
metze
-------------- next part --------------
From b2f50ba2cc3f415433a81b1f10f5834daaa4bbaf Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 27 Mar 2018 16:05:30 +0200
Subject: [PATCH 1/3] s3:messages: check reg->refcount == 0 before accessing
other elements
Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
source3/lib/messages.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index 5a31f34..45e210f 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -192,15 +192,23 @@ static bool messaging_register_event_context(struct messaging_context *ctx,
for (i=0; i<num_event_contexts; i++) {
struct messaging_registered_ev *reg = &ctx->event_contexts[i];
- if (reg->ev == ev) {
- reg->refcount += 1;
- return true;
- }
if (reg->refcount == 0) {
if (reg->ev != NULL) {
abort();
}
free_reg = reg;
+ /*
+ * We continue here and may find another
+ * free_req, but the important thing is
+ * that we continue to search for an
+ * existing registration in the loop.
+ */
+ continue;
+ }
+
+ if (reg->ev == ev) {
+ reg->refcount += 1;
+ return true;
}
}
@@ -231,10 +239,11 @@ static bool messaging_deregister_event_context(struct messaging_context *ctx,
for (i=0; i<num_event_contexts; i++) {
struct messaging_registered_ev *reg = &ctx->event_contexts[i];
+ if (reg->refcount == 0) {
+ continue;
+ }
+
if (reg->ev == ev) {
- if (reg->refcount == 0) {
- return false;
- }
reg->refcount -= 1;
if (reg->refcount == 0) {
--
1.9.1
From 131861d9f76182f9cc7fcefb001ed5efbd01496f Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 27 Mar 2018 15:27:32 +0200
Subject: [PATCH 2/3] s3:messages: check tevent_fd_get_flags() == 0 before
using stale event context pointer
If the event context got deleted, tevent_fd_get_flags() will return 0
for the stale fde. In that case we should not use fde_ev->ev anymore.
Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
source3/lib/messages_ctdb.c | 14 ++++++++++++--
source3/lib/messages_dgm.c | 14 ++++++++++++--
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/source3/lib/messages_ctdb.c b/source3/lib/messages_ctdb.c
index 66b9f55..d3e2e3f 100644
--- a/source3/lib/messages_ctdb.c
+++ b/source3/lib/messages_ctdb.c
@@ -215,8 +215,18 @@ struct messaging_ctdb_fde *messaging_ctdb_register_tevent_context(
}
for (fde_ev = ctx->fde_evs; fde_ev != NULL; fde_ev = fde_ev->next) {
- if ((fde_ev->ev == ev) &&
- (tevent_fd_get_flags(fde_ev->fde) != 0)) {
+ if (tevent_fd_get_flags(fde_ev->fde) == 0) {
+ /*
+ * If the event context got deleted,
+ * tevent_fd_get_flags() will return 0
+ * for the stale fde.
+ *
+ * In that case we should not
+ * use fde_ev->ev anymore.
+ */
+ continue;
+ }
+ if (fde_ev->ev == ev) {
break;
}
}
diff --git a/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c
index b9cddc2..b8878b6 100644
--- a/source3/lib/messages_dgm.c
+++ b/source3/lib/messages_dgm.c
@@ -1679,8 +1679,18 @@ struct messaging_dgm_fde *messaging_dgm_register_tevent_context(
}
for (fde_ev = ctx->fde_evs; fde_ev != NULL; fde_ev = fde_ev->next) {
- if ((fde_ev->ev == ev) &&
- (tevent_fd_get_flags(fde_ev->fde) != 0)) {
+ if (tevent_fd_get_flags(fde_ev->fde) == 0) {
+ /*
+ * If the event context got deleted,
+ * tevent_fd_get_flags() will return 0
+ * for the stale fde.
+ *
+ * In that case we should not
+ * use fde_ev->ev anymore.
+ */
+ continue;
+ }
+ if (fde_ev->ev == ev) {
break;
}
}
--
1.9.1
From ee8cfc4777ed0c14e1472c9e9d719cafedc463b8 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 27 Mar 2018 16:04:58 +0200
Subject: [PATCH 3/3] s3:messages: improve tevent_create_immediate recycling
We should create the immediate event at the beginning
were we have a chance to return an error, rather than
ignoring a failure later.
As a side effect this also reuses the immediate event
after the refcount went to 0 and up again.
Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
source3/lib/messages.c | 40 ++++++++++++++++++++++++++++++----------
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index 45e210f..82a1778 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -213,6 +213,13 @@ static bool messaging_register_event_context(struct messaging_context *ctx,
}
if (free_reg == NULL) {
+ struct tevent_immediate *im = NULL;
+
+ im = tevent_create_immediate(ctx);
+ if (im == NULL) {
+ return false;
+ }
+
tmp = talloc_realloc(ctx, ctx->event_contexts,
struct messaging_registered_ev,
num_event_contexts+1);
@@ -222,9 +229,14 @@ static bool messaging_register_event_context(struct messaging_context *ctx,
ctx->event_contexts = tmp;
free_reg = &ctx->event_contexts[num_event_contexts];
+ free_reg->im = talloc_move(ctx->event_contexts, &im);
}
- *free_reg = (struct messaging_registered_ev) { .ev = ev, .refcount = 1 };
+ /*
+ * free_reg->im might be cached
+ */
+ free_reg->ev = ev;
+ free_reg->refcount = 1;
return true;
}
@@ -248,6 +260,16 @@ static bool messaging_deregister_event_context(struct messaging_context *ctx,
if (reg->refcount == 0) {
/*
+ * The primary event context
+ * is never unregistered using
+ * messaging_deregister_event_context()
+ * it's only registered using
+ * messaging_register_event_context().
+ */
+ SMB_ASSERT(ev != ctx->event_ctx);
+ SMB_ASSERT(reg->ev != ctx->event_ctx);
+
+ /*
* Not strictly necessary, just
* paranoia
*/
@@ -256,7 +278,14 @@ static bool messaging_deregister_event_context(struct messaging_context *ctx,
/*
* Do not talloc_free(reg->im),
* recycle immediates events.
+ *
+ * We just invalidate it using
+ * the primary event context,
+ * which is never unregistered.
*/
+ tevent_schedule_immediate(reg->im,
+ ctx->event_ctx,
+ NULL, NULL);
}
return true;
}
@@ -329,15 +358,6 @@ static bool messaging_alert_event_contexts(struct messaging_context *ctx)
continue;
}
- if (reg->im == NULL) {
- reg->im = tevent_create_immediate(
- ctx->event_contexts);
- }
- if (reg->im == NULL) {
- DBG_WARNING("Could not create immediate\n");
- continue;
- }
-
/*
* We depend on schedule_immediate to work
* multiple times. Might be a bit inefficient,
--
1.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180424/616e71cc/signature.sig>
More information about the samba-technical
mailing list