[PATCH] Remove "multicasting" inside messages.c

[Volker Lendecke]

Hi!

There's more to come. g_lock_ping_pong (like ctdb's ping_pong on
g_lock) is a fun test to run. It exposes quite a few problems in our
messaging system.

Until then, review appreciated!

Volker

-- 
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 at sernet.de
-------------- next part --------------
From 2cbe1ccd9177116dda52dc014b3cc192a5d63a13 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Jun 2017 08:48:16 +0200
Subject: [PATCH] messaging: Deliver messages only once

This survived an autobuild, so no subsystem strictly needs this anymore. In
particular the notify subsystem has been rewritten.

Why this patch? It removes some complexity from core code, and it reduces the
potential memory overconsumption: Right now I'm working on a g_lock_ping_pong
test. This test does a lot of messaging_filtered_read_send calls in a tight
loop on a nested event context. With the current code we let the
messaging_filtered_read code consume the message that arrives, but it also
posts it for consumption by the main event context attached to the messaging
context with its "classic" callback. This test never comes back to the main
event context, so it accumulates more and more self-posted messages. That's
just unnecessary, given that due to the successful autobuild nothing but the
read1 test makes use of the "multicasting" of messages.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/messages.c                | 24 ++++++++++--------------
 source3/torture/test_messaging_read.c |  6 +++---
 2 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index a38f40e..b0edb30 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -1015,7 +1015,7 @@ static bool messaging_append_new_waiters(struct messaging_context *msg_ctx)
 	return true;
 }
 
-static void messaging_dispatch_classic(struct messaging_context *msg_ctx,
+static bool messaging_dispatch_classic(struct messaging_context *msg_ctx,
 				       struct messaging_rec *rec)
 {
 	struct messaging_callback *cb, *next;
@@ -1041,13 +1041,10 @@ static void messaging_dispatch_classic(struct messaging_context *msg_ctx,
 		cb->fn(msg_ctx, cb->private_data, rec->msg_type,
 		       rec->src, &rec->buf);
 
-		/*
-		 * we continue looking for matching messages after finding
-		 * one. This matters for subsystems like the internal notify
-		 * code which register more than one handler for the same
-		 * message type
-		 */
+		return true;
 	}
+
+	return false;
 }
 
 /*
@@ -1058,9 +1055,13 @@ static void messaging_dispatch_rec(struct messaging_context *msg_ctx,
 				   struct messaging_rec *rec)
 {
 	size_t i;
+	bool consumed;
 
 	if (ev == msg_ctx->event_ctx) {
-		messaging_dispatch_classic(msg_ctx, rec);
+		consumed = messaging_dispatch_classic(msg_ctx, rec);
+		if (consumed) {
+			return;
+		}
 	}
 
 	if (!messaging_append_new_waiters(msg_ctx)) {
@@ -1102,12 +1103,7 @@ static void messaging_dispatch_rec(struct messaging_context *msg_ctx,
 		if ((ev == state->ev) &&
 		    state->filter(rec, state->private_data)) {
 			messaging_filtered_read_done(req, rec);
-
-			/*
-			 * Only the first one gets the fd-array
-			 */
-			rec->num_fds = 0;
-			rec->fds = NULL;
+			return;
 		}
 
 		i += 1;
diff --git a/source3/torture/test_messaging_read.c b/source3/torture/test_messaging_read.c
index 43b4367..d3e4079 100644
--- a/source3/torture/test_messaging_read.c
+++ b/source3/torture/test_messaging_read.c
@@ -122,7 +122,7 @@ bool run_messaging_read1(int dummy)
 		goto fail;
 	}
 
-	for (i=0; i<3; i++) {
+	for (i=0; i<2; i++) {
 		if (tevent_loop_once(ev) != 0) {
 			fprintf(stderr, "tevent_loop_once failed\n");
 			goto fail;
@@ -131,8 +131,8 @@ bool run_messaging_read1(int dummy)
 
 	printf("%u/%u\n", count1, count2);
 
-	if ((count1 != 1) || (count2 != 1)){
-		fprintf(stderr, "Got %u/%u msgs, expected 1 each\n",
+	if ((count1 != 1) || (count2 != 0)) {
+		fprintf(stderr, "Got %u/%u msgs, expected 1/0\n",
 			count1, count2);
 		goto fail;
 	}
-- 
2.1.4