[PATCH] Make messaging_context private
Volker Lendecke
Volker.Lendecke at SerNet.DE
Fri May 30 07:54:43 MDT 2014
On Thu, May 29, 2014 at 05:09:13PM +0200, Volker Lendecke wrote:
> Attached find a patchset that makes struct messaging_context
> private to messages.c.
>
> Review would be appreciated!
The attached patchset contains an additional patch: It
enforces just one messaging_context per process again. We
lost this paranoia check when the tdb-based messaging went
away, but I think we should keep it.
Review & push 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 f097fe4f5741a09abe45a4c047fb7e18afb5cbcf Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 29 May 2014 13:10:45 +0200
Subject: [PATCH 1/6] imessaging: Fix a comment
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/lib/messaging/messaging.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c
index f73b2ba..66732ce 100644
--- a/source4/lib/messaging/messaging.c
+++ b/source4/lib/messaging/messaging.c
@@ -564,7 +564,7 @@ int imessaging_cleanup(struct imessaging_context *msg)
/*
create the listening socket and setup the dispatcher
- use temporary=true when you want a destructor to remove the
+ use auto_remove=true when you want a destructor to remove the
associated messaging socket and database entry on talloc free. Don't
use this in processes that may fork and a child may talloc free this
memory
--
1.7.9.5
From ba695be0cf051b979b2bba859901791cd7d01520 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 29 May 2014 14:51:37 +0200
Subject: [PATCH 2/6] messaging3: The backend send_fn doesn't need a
messaging_context
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/include/messages.h | 2 +-
source3/lib/messages.c | 6 +++---
source3/lib/messages_ctdbd.c | 4 ++--
source3/lib/messages_dgm.c | 6 +++---
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/source3/include/messages.h b/source3/include/messages.h
index 18362f9..9a63cd6 100644
--- a/source3/include/messages.h
+++ b/source3/include/messages.h
@@ -87,7 +87,7 @@ struct messaging_context {
};
struct messaging_backend {
- NTSTATUS (*send_fn)(struct messaging_context *msg_ctx,
+ NTSTATUS (*send_fn)(struct server_id src,
struct server_id pid, int msg_type,
const struct iovec *iov, int iovlen,
struct messaging_backend *backend);
diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index 6e2e7ca..9e77009 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -380,7 +380,7 @@ NTSTATUS messaging_send_iov(struct messaging_context *msg_ctx,
}
if (!procid_is_local(&server)) {
- return msg_ctx->remote->send_fn(msg_ctx, server,
+ return msg_ctx->remote->send_fn(msg_ctx->id, server,
msg_type, iov, iovlen,
msg_ctx->remote);
}
@@ -407,8 +407,8 @@ NTSTATUS messaging_send_iov(struct messaging_context *msg_ctx,
return NT_STATUS_OK;
}
- return msg_ctx->local->send_fn(msg_ctx, server, msg_type, iov, iovlen,
- msg_ctx->local);
+ return msg_ctx->local->send_fn(msg_ctx->id, server, msg_type,
+ iov, iovlen, msg_ctx->local);
}
static struct messaging_rec *messaging_rec_dup(TALLOC_CTX *mem_ctx,
diff --git a/source3/lib/messages_ctdbd.c b/source3/lib/messages_ctdbd.c
index 34b3e2a..3b7fa05 100644
--- a/source3/lib/messages_ctdbd.c
+++ b/source3/lib/messages_ctdbd.c
@@ -88,7 +88,7 @@ struct ctdbd_connection *messaging_ctdbd_connection(void)
return global_ctdbd_connection;
}
-static NTSTATUS messaging_ctdb_send(struct messaging_context *msg_ctx,
+static NTSTATUS messaging_ctdb_send(struct server_id src,
struct server_id pid, int msg_type,
const struct iovec *iov, int iovlen,
struct messaging_backend *backend)
@@ -109,7 +109,7 @@ static NTSTATUS messaging_ctdb_send(struct messaging_context *msg_ctx,
msg.msg_version = MESSAGE_VERSION;
msg.msg_type = msg_type;
msg.dest = pid;
- msg.src = msg_ctx->id;
+ msg.src = src;
msg.buf = data_blob_const(buf, talloc_get_size(buf));
status = ctdbd_messaging_send(ctx->conn, pid.vnn, pid.pid, &msg);
diff --git a/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c
index 6912035..2f2647e 100644
--- a/source3/lib/messages_dgm.c
+++ b/source3/lib/messages_dgm.c
@@ -44,7 +44,7 @@ struct messaging_dgm_hdr {
struct server_id src;
};
-static NTSTATUS messaging_dgm_send(struct messaging_context *msg_ctx,
+static NTSTATUS messaging_dgm_send(struct server_id src,
struct server_id pid, int msg_type,
const struct iovec *iov, int iovlen,
struct messaging_backend *backend);
@@ -286,7 +286,7 @@ static int messaging_dgm_context_destructor(struct messaging_dgm_context *c)
return 0;
}
-static NTSTATUS messaging_dgm_send(struct messaging_context *msg_ctx,
+static NTSTATUS messaging_dgm_send(struct server_id src,
struct server_id pid, int msg_type,
const struct iovec *iov, int iovlen,
struct messaging_backend *backend)
@@ -312,7 +312,7 @@ static NTSTATUS messaging_dgm_send(struct messaging_context *msg_ctx,
hdr.msg_version = MESSAGE_VERSION;
hdr.msg_type = msg_type & MSG_TYPE_MASK;
hdr.dst = pid;
- hdr.src = msg_ctx->id;
+ hdr.src = src;
DEBUG(10, ("%s: Sending message 0x%x to %s\n", __func__,
(unsigned)hdr.msg_type,
--
1.7.9.5
From 720e8bfac13e157652f08d3ca00c92adfcc52d80 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 29 May 2014 15:01:03 +0200
Subject: [PATCH 3/6] messaging3: Introduce messaging_local_backend()
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/include/messages.h | 2 ++
source3/lib/messages.c | 6 ++++++
source3/lib/messages_dgm.c | 9 ++++++---
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/source3/include/messages.h b/source3/include/messages.h
index 9a63cd6..bea3fd0 100644
--- a/source3/include/messages.h
+++ b/source3/include/messages.h
@@ -116,6 +116,8 @@ struct messaging_context *messaging_init(TALLOC_CTX *mem_ctx,
struct tevent_context *ev);
struct server_id messaging_server_id(const struct messaging_context *msg_ctx);
+struct messaging_backend *messaging_local_backend(
+ struct messaging_context *msg_ctx);
/*
* re-init after a fork
diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index 9e77009..db7257d 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -862,4 +862,10 @@ static void mess_parent_dgm_cleanup_done(struct tevent_req *req)
tevent_req_set_callback(req, mess_parent_dgm_cleanup_done, msg);
}
+struct messaging_backend *messaging_local_backend(
+ struct messaging_context *msg_ctx)
+{
+ return msg_ctx->local;
+}
+
/** @} **/
diff --git a/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c
index 2f2647e..cf6e190 100644
--- a/source3/lib/messages_dgm.c
+++ b/source3/lib/messages_dgm.c
@@ -369,8 +369,9 @@ static void messaging_dgm_recv(struct unix_msg_ctx *ctx,
NTSTATUS messaging_dgm_cleanup(struct messaging_context *msg_ctx, pid_t pid)
{
+ struct messaging_backend *be = messaging_local_backend(msg_ctx);
struct messaging_dgm_context *ctx = talloc_get_type_abort(
- msg_ctx->local->private_data, struct messaging_dgm_context);
+ be->private_data, struct messaging_dgm_context);
char *lockfile_name, *socket_name;
int fd, ret;
struct flock lck = {};
@@ -421,8 +422,9 @@ NTSTATUS messaging_dgm_cleanup(struct messaging_context *msg_ctx, pid_t pid)
NTSTATUS messaging_dgm_wipe(struct messaging_context *msg_ctx)
{
+ struct messaging_backend *be = messaging_local_backend(msg_ctx);
struct messaging_dgm_context *ctx = talloc_get_type_abort(
- msg_ctx->local->private_data, struct messaging_dgm_context);
+ be->private_data, struct messaging_dgm_context);
char *msgdir_name;
DIR *msgdir;
struct dirent *dp;
@@ -477,7 +479,8 @@ void *messaging_dgm_register_tevent_context(TALLOC_CTX *mem_ctx,
struct messaging_context *msg_ctx,
struct tevent_context *ev)
{
+ struct messaging_backend *be = messaging_local_backend(msg_ctx);
struct messaging_dgm_context *ctx = talloc_get_type_abort(
- msg_ctx->local->private_data, struct messaging_dgm_context);
+ be->private_data, struct messaging_dgm_context);
return poll_funcs_tevent_register(mem_ctx, ctx->msg_callbacks, ev);
}
--
1.7.9.5
From ac507a89befde43bf15e019a9886fbed89ad6a93 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 29 May 2014 16:44:32 +0200
Subject: [PATCH 4/6] messaging3: Add and use messaging_tevent_context()
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/include/messages.h | 2 ++
source3/lib/ctdbd_conn.c | 7 ++++---
source3/lib/messages.c | 6 ++++++
source3/lib/messages_dgm.c | 3 ++-
4 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/source3/include/messages.h b/source3/include/messages.h
index bea3fd0..d029d66 100644
--- a/source3/include/messages.h
+++ b/source3/include/messages.h
@@ -116,6 +116,8 @@ struct messaging_context *messaging_init(TALLOC_CTX *mem_ctx,
struct tevent_context *ev);
struct server_id messaging_server_id(const struct messaging_context *msg_ctx);
+struct tevent_context *messaging_tevent_context(
+ struct messaging_context *msg_ctx);
struct messaging_backend *messaging_local_backend(
struct messaging_context *msg_ctx);
diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
index 35845ed..201c700 100644
--- a/source3/lib/ctdbd_conn.c
+++ b/source3/lib/ctdbd_conn.c
@@ -496,8 +496,8 @@ static NTSTATUS ctdb_read_req(struct ctdbd_connection *conn, uint32_t reqid,
* We're waiting for a call reply, but an async message has
* crossed. Defer dispatching to the toplevel event loop.
*/
- evt = tevent_add_timer(conn->msg_ctx->event_ctx,
- conn->msg_ctx->event_ctx,
+ evt = tevent_add_timer(messaging_tevent_context(conn->msg_ctx),
+ messaging_tevent_context(conn->msg_ctx),
timeval_zero(),
deferred_message_dispatch,
msg_state);
@@ -747,7 +747,8 @@ NTSTATUS ctdbd_register_msg_ctx(struct ctdbd_connection *conn,
SMB_ASSERT(conn->msg_ctx == NULL);
SMB_ASSERT(conn->fde == NULL);
- if (!(conn->fde = tevent_add_fd(msg_ctx->event_ctx, conn,
+ if (!(conn->fde = tevent_add_fd(messaging_tevent_context(msg_ctx),
+ conn,
ctdb_packet_get_fd(conn->pkt),
TEVENT_FD_READ,
ctdbd_socket_handler,
diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index db7257d..3d838fe 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -868,4 +868,10 @@ struct messaging_backend *messaging_local_backend(
return msg_ctx->local;
}
+struct tevent_context *messaging_tevent_context(
+ struct messaging_context *msg_ctx)
+{
+ return msg_ctx->event_ctx;
+}
+
/** @} **/
diff --git a/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c
index cf6e190..9b58d9e 100644
--- a/source3/lib/messages_dgm.c
+++ b/source3/lib/messages_dgm.c
@@ -232,7 +232,8 @@ NTSTATUS messaging_dgm_init(struct messaging_context *msg_ctx,
}
ctx->tevent_handle = poll_funcs_tevent_register(
- ctx, ctx->msg_callbacks, msg_ctx->event_ctx);
+ ctx, ctx->msg_callbacks,
+ messaging_tevent_context(msg_ctx));
if (ctx->tevent_handle == NULL) {
TALLOC_FREE(result);
return NT_STATUS_NO_MEMORY;
--
1.7.9.5
From c8ec13469dd825b80f9f60d664276be6c548abd4 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 29 May 2014 16:44:55 +0200
Subject: [PATCH 5/6] messaging3: Make messaging_context private
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/include/messages.h | 21 ---------------------
source3/lib/messages.c | 15 +++++++++++++++
2 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/source3/include/messages.h b/source3/include/messages.h
index d029d66..5784e41 100644
--- a/source3/include/messages.h
+++ b/source3/include/messages.h
@@ -65,27 +65,6 @@
struct messaging_context;
struct messaging_rec;
-/*
- * struct messaging_context belongs to messages.c, but because we still have
- * messaging_dispatch, we need it here. Once we get rid of signals for
- * notifying processes, this will go.
- */
-
-struct messaging_context {
- struct server_id id;
- struct tevent_context *event_ctx;
- struct messaging_callback *callbacks;
-
- struct tevent_req **new_waiters;
- unsigned num_new_waiters;
-
- struct tevent_req **waiters;
- unsigned num_waiters;
-
- struct messaging_backend *local;
- struct messaging_backend *remote;
-};
-
struct messaging_backend {
NTSTATUS (*send_fn)(struct server_id src,
struct server_id pid, int msg_type,
diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index 3d838fe..364bbbc 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -61,6 +61,21 @@ struct messaging_callback {
void *private_data;
};
+struct messaging_context {
+ struct server_id id;
+ struct tevent_context *event_ctx;
+ struct messaging_callback *callbacks;
+
+ struct tevent_req **new_waiters;
+ unsigned num_new_waiters;
+
+ struct tevent_req **waiters;
+ unsigned num_waiters;
+
+ struct messaging_backend *local;
+ struct messaging_backend *remote;
+};
+
/****************************************************************************
A useful function for testing the message system.
****************************************************************************/
--
1.7.9.5
From 042ee602513c99c3e5059b812dc182323a574b33 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 6/6] messaging3: 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.
This is the same patch as bd55fdb lifted to messaging.c
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/messages.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index 364bbbc..1263bf1 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -74,8 +74,12 @@ struct messaging_context {
struct messaging_backend *local;
struct messaging_backend *remote;
+
+ bool *have_context;
};
+static int messaging_context_destructor(struct messaging_context *msg_ctx);
+
/****************************************************************************
A useful function for testing the message system.
****************************************************************************/
@@ -205,6 +209,13 @@ struct messaging_context *messaging_init(TALLOC_CTX *mem_ctx,
{
struct messaging_context *ctx;
NTSTATUS status;
+ static bool have_context = false;
+
+ if (have_context) {
+ DEBUG(0, ("No two messaging contexts per process\n"));
+ return NULL;
+ }
+
if (!(ctx = talloc_zero(mem_ctx, struct messaging_context))) {
return NULL;
@@ -212,6 +223,7 @@ struct messaging_context *messaging_init(TALLOC_CTX *mem_ctx,
ctx->id = procid_self();
ctx->event_ctx = ev;
+ ctx->have_context = &have_context;
status = messaging_dgm_init(ctx, ctx, &ctx->local);
@@ -242,9 +254,19 @@ struct messaging_context *messaging_init(TALLOC_CTX *mem_ctx,
register_dmalloc_msgs(ctx);
debug_register_msgs(ctx);
+ have_context = true;
+ talloc_set_destructor(ctx, messaging_context_destructor);
+
return ctx;
}
+static int messaging_context_destructor(struct messaging_context *msg_ctx)
+{
+ SMB_ASSERT(*msg_ctx->have_context);
+ *msg_ctx->have_context = false;
+ return 0;
+}
+
struct server_id messaging_server_id(const struct messaging_context *msg_ctx)
{
return msg_ctx->id;
--
1.7.9.5
More information about the samba-technical
mailing list