[SCM] Samba Shared Repository - branch master updated

Stefan Metzmacher metze at samba.org
Tue Apr 24 12:31:07 UTC 2018


The branch, master has been updated
       via  8e5cc97 s3:messages: improve tevent_create_immediate recycling
       via  dfb712a s3:messages: check tevent_fd_get_flags() == 0 before using stale event context pointer
       via  fdcc162 s3:messages: check reg->refcount == 0 before accessing other elements
      from  0b04258 winbind: Remove an unused struct declaration

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


- Log -----------------------------------------------------------------
commit 8e5cc9732bb99df912bfd0fa09f7c14068f09874
Author: Ralph Boehme <slow at samba.org>
Date:   Tue Mar 27 16:04:58 2018 +0200

    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>
    
    Autobuild-User(master): Stefan Metzmacher <metze at samba.org>
    Autobuild-Date(master): Tue Apr 24 14:30:20 CEST 2018 on sn-devel-144

commit dfb712a03c2bd36641506ae9cfce1a0820e1a329
Author: Ralph Boehme <slow at samba.org>
Date:   Tue Mar 27 15:27:32 2018 +0200

    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>

commit fdcc1622082eaea3fc03c0346a56afbbff88e6d1
Author: Ralph Boehme <slow at samba.org>
Date:   Tue Mar 27 16:05:30 2018 +0200

    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>

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

Summary of changes:
 source3/lib/messages.c      | 63 +++++++++++++++++++++++++++++++++------------
 source3/lib/messages_ctdb.c | 14 ++++++++--
 source3/lib/messages_dgm.c  | 14 ++++++++--
 3 files changed, 70 insertions(+), 21 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index 5a31f34..82a1778 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -192,19 +192,34 @@ 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;
 		}
 	}
 
 	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);
@@ -214,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;
 }
@@ -231,14 +251,25 @@ 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) {
 				/*
+				 * 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
 				 */
@@ -247,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;
 		}
@@ -320,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,
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;
 		}
 	}


-- 
Samba Shared Repository



More information about the samba-cvs mailing list