[PATCH] Enforce only one messaging_context per process

[Volker Lendecke]

Hi!

With the current source3/lib/messaging.c implementation we
run into problems if we have more than one messaging context
per process. Messages might not be delivered correctly, we
only have one tdb record and only one signal per message
sent. Messages might end up with the wrong messaging
context. The attached patchset enforces this and fixes two
places where we did have two messaging contexts.

Review would be appreciated.

Thanks,

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 3b3458c1feac28ef26d22312a26700d9f6b64f82 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 18 Feb 2014 20:51:43 +0100
Subject: [PATCH 1/3] spoolssd: Use only one messaging_context per process

After the fork, the code created a fresh messaging_context before doing the
reinit_after_fork. This means to have two initialized messaging contexts in
that process. This patch aligns spoolssd with lsad.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/printing/spoolssd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/printing/spoolssd.c b/source3/printing/spoolssd.c
index 0b6980a..1bf87b3 100644
--- a/source3/printing/spoolssd.c
+++ b/source3/printing/spoolssd.c
@@ -285,7 +285,7 @@ static bool spoolss_child_init(struct tevent_context *ev_ctx,
 {
 	NTSTATUS status;
 	struct rpc_srv_callbacks spoolss_cb;
-	struct messaging_context *msg_ctx = messaging_init(NULL, ev_ctx);
+	struct messaging_context *msg_ctx = server_messaging_context();
 	bool ok;
 
 	status = reinit_after_fork(msg_ctx, ev_ctx,
-- 
1.7.9.5


From cfb6354e0b8f8379ca368fab71fafb54381c2b90 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 18 Feb 2014 20:51:57 +0100
Subject: [PATCH 2/3] vfstext: Use just one messaging_context in vfstest

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/torture/vfstest.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/source3/torture/vfstest.c b/source3/torture/vfstest.c
index 2cec687..40d44c3 100644
--- a/source3/torture/vfstest.c
+++ b/source3/torture/vfstest.c
@@ -464,7 +464,7 @@ int main(int argc, char *argv[])
 	char *filename = NULL;
 	char cwd[MAXPATHLEN];
 	TALLOC_CTX *frame = talloc_stackframe();
-	struct tevent_context *ev = samba_tevent_context_init(NULL);
+	struct tevent_context *ev;
 	struct auth_session_info *session_info = NULL;
 	NTSTATUS status = NT_STATUS_OK;
 
@@ -537,9 +537,11 @@ int main(int argc, char *argv[])
 		return 1;
 	}
 
+	ev = server_event_context();
+
 	status = create_conn_struct(vfs,
 				ev,
-				messaging_init(vfs, ev),
+				server_messaging_context(),
                                 &vfs->conn,
                                 -1,
                                 getcwd(cwd, sizeof(cwd)),
-- 
1.7.9.5


From 238d1c1cf087bd5ad7fa0fe03d4e0114f8be0243 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 18 Feb 2014 20:51:23 +0100
Subject: [PATCH 3/3] messaging: Enforce just one messaging context

The current messaging implementation is based on a tdb indexed by server_id. If
we have more than one messaging context in a process, messages might not arrive
at the right context and be dropped, depending on which signal handler is
triggered first.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/messages_local.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/source3/lib/messages_local.c b/source3/lib/messages_local.c
index 6b9c251..4f8d81c 100644
--- a/source3/lib/messages_local.c
+++ b/source3/lib/messages_local.c
@@ -53,6 +53,7 @@ struct messaging_tdb_context {
 	struct tdb_wrap *tdb;
 	struct tevent_signal *se;
 	int received_messages;
+	bool *have_context;
 };
 
 static NTSTATUS messaging_tdb_send(struct messaging_context *msg_ctx,
@@ -77,6 +78,8 @@ static void messaging_tdb_signal_handler(struct tevent_context *ev_ctx,
 	message_dispatch(ctx->msg_ctx);
 }
 
+static int messaging_tdb_context_destructor(struct messaging_tdb_context *ctx);
+
 /****************************************************************************
  Initialise the messaging functions. 
 ****************************************************************************/
@@ -88,6 +91,12 @@ NTSTATUS messaging_tdb_init(struct messaging_context *msg_ctx,
 	struct messaging_backend *result;
 	struct messaging_tdb_context *ctx;
 	struct loadparm_context *lp_ctx;
+	static bool have_context = false;
+
+	if (have_context) {
+		DEBUG(0, ("No two messaging contexts per process\n"));
+		return NT_STATUS_OBJECT_NAME_COLLISION;
+	}
 
 	if (!(result = talloc(mem_ctx, struct messaging_backend))) {
 		DEBUG(0, ("talloc failed\n"));
@@ -110,6 +119,7 @@ NTSTATUS messaging_tdb_init(struct messaging_context *msg_ctx,
 	result->send_fn = messaging_tdb_send;
 
 	ctx->msg_ctx = msg_ctx;
+	ctx->have_context = &have_context;
 
 	ctx->tdb = tdb_wrap_open(ctx, lock_path("messages.tdb"), 0,
 				 TDB_CLEAR_IF_FIRST|TDB_DEFAULT|TDB_VOLATILE|TDB_INCOMPATIBLE_HASH,
@@ -139,10 +149,20 @@ NTSTATUS messaging_tdb_init(struct messaging_context *msg_ctx,
 
 	sec_init();
 
+	have_context = true;
+	talloc_set_destructor(ctx, messaging_tdb_context_destructor);
+
 	*presult = result;
 	return NT_STATUS_OK;
 }
 
+static int messaging_tdb_context_destructor(struct messaging_tdb_context *ctx)
+{
+	SMB_ASSERT(*ctx->have_context);
+	*ctx->have_context = false;
+	return 0;
+}
+
 bool messaging_tdb_parent_init(TALLOC_CTX *mem_ctx)
 {
 	struct tdb_wrap *db;
-- 
1.7.9.5