From 2d21fe079fb57e55d9bac0c69d8527013bf4fbc7 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Fri, 22 Jul 2011 14:55:32 +1000 Subject: [PATCH] s4-messaging: fixed the removal of messaging sockets in child tasks when a child task exits we were firing a destructor on any inherited messaging contexts, which could trigger a removal of the parents message socket and messaging database entry. This adds a new auto_remove flag to imessaging_init(), and exposes the cleanup code for use by the stream service. Pair-Programmed-With: Andrew Bartlett Autobuild-User: Andrew Tridgell Autobuild-Date: Fri Jul 22 08:09:06 CEST 2011 on sn-devel-104 --- source4/lib/messaging/messaging.c | 23 ++++++++++++++++------- source4/lib/messaging/messaging.h | 8 +++++--- source4/lib/messaging/pymessaging.c | 10 +++++----- source4/lib/messaging/tests/irpc.c | 4 ++-- source4/lib/messaging/tests/messaging.c | 10 +++++----- source4/smbd/server.c | 4 ++-- source4/smbd/service_stream.c | 5 +++-- source4/smbd/service_task.c | 6 +++--- 8 files changed, 41 insertions(+), 29 deletions(-) diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c index 484f22b2ee2..ccc60032a6a 100644 --- a/source4/lib/messaging/messaging.c +++ b/source4/lib/messaging/messaging.c @@ -541,10 +541,11 @@ NTSTATUS imessaging_send_ptr(struct imessaging_context *msg, struct server_id se /* - destroy the messaging context + remove our messaging socket and database entry */ -static int imessaging_destructor(struct imessaging_context *msg) +int imessaging_cleanup(struct imessaging_context *msg) { + DEBUG(5,("imessaging: cleaning up %s\n", msg->path)); unlink(msg->path); while (msg->names && msg->names[0]) { irpc_remove_name(msg, msg->names[0]); @@ -554,11 +555,17 @@ static int imessaging_destructor(struct imessaging_context *msg) /* create the listening socket and setup the dispatcher + + use temporary=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 */ struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx, - const char *dir, - struct server_id server_id, - struct tevent_context *ev) + const char *dir, + struct server_id server_id, + struct tevent_context *ev, + bool auto_remove) { struct imessaging_context *msg; NTSTATUS status; @@ -622,7 +629,9 @@ struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx, EVENT_FD_READ, imessaging_handler, msg); tevent_fd_set_auto_close(msg->event.fde); - talloc_set_destructor(msg, imessaging_destructor); + if (auto_remove) { + talloc_set_destructor(msg, imessaging_cleanup); + } imessaging_register(msg, NULL, MSG_PING, ping_message); imessaging_register(msg, NULL, MSG_IRPC, irpc_handler); @@ -641,7 +650,7 @@ struct imessaging_context *imessaging_client_init(TALLOC_CTX *mem_ctx, struct server_id id; ZERO_STRUCT(id); id.pid = random() % 0x10000000; - return imessaging_init(mem_ctx, dir, id, ev); + return imessaging_init(mem_ctx, dir, id, ev, true); } /* a list of registered irpc server functions diff --git a/source4/lib/messaging/messaging.h b/source4/lib/messaging/messaging.h index eb8a8abc795..e51590fe74a 100644 --- a/source4/lib/messaging/messaging.h +++ b/source4/lib/messaging/messaging.h @@ -54,9 +54,11 @@ NTSTATUS imessaging_register(struct imessaging_context *msg, void *private_data, NTSTATUS imessaging_register_tmp(struct imessaging_context *msg, void *private_data, msg_callback_t fn, uint32_t *msg_type); struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx, - const char *dir, - struct server_id server_id, - struct tevent_context *ev); + const char *dir, + struct server_id server_id, + struct tevent_context *ev, + bool auto_remove); +int imessaging_cleanup(struct imessaging_context *msg); struct imessaging_context *imessaging_client_init(TALLOC_CTX *mem_ctx, const char *dir, struct tevent_context *ev); diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c index cafd45beae3..4f90db59f2d 100644 --- a/source4/lib/messaging/pymessaging.c +++ b/source4/lib/messaging/pymessaging.c @@ -96,13 +96,13 @@ static PyObject *py_imessaging_connect(PyTypeObject *self, PyObject *args, PyObj return NULL; ret->msg_ctx = imessaging_init(ret->mem_ctx, - imessaging_path, - server_id, - ev); + imessaging_path, + server_id, + ev, true); } else { ret->msg_ctx = imessaging_client_init(ret->mem_ctx, - imessaging_path, - ev); + imessaging_path, + ev); } if (ret->msg_ctx == NULL) { diff --git a/source4/lib/messaging/tests/irpc.c b/source4/lib/messaging/tests/irpc.c index cfa2bcb91e1..554a7efa997 100644 --- a/source4/lib/messaging/tests/irpc.c +++ b/source4/lib/messaging/tests/irpc.c @@ -249,14 +249,14 @@ static bool irpc_setup(struct torture_context *tctx, void **_data) imessaging_init(tctx, lpcfg_imessaging_path(tctx, tctx->lp_ctx), cluster_id(0, MSG_ID1), - data->ev), + data->ev, true), "Failed to init first messaging context"); torture_assert(tctx, data->msg_ctx2 = imessaging_init(tctx, lpcfg_imessaging_path(tctx, tctx->lp_ctx), cluster_id(0, MSG_ID2), - data->ev), + data->ev, true), "Failed to init second messaging context"); /* register the server side function */ diff --git a/source4/lib/messaging/tests/messaging.c b/source4/lib/messaging/tests/messaging.c index 38c34fc52ee..c5d37953d05 100644 --- a/source4/lib/messaging/tests/messaging.c +++ b/source4/lib/messaging/tests/messaging.c @@ -72,8 +72,8 @@ static bool test_ping_speed(struct torture_context *tctx) ev = tctx->ev; msg_server_ctx = imessaging_init(tctx, - lpcfg_imessaging_path(tctx, tctx->lp_ctx), cluster_id(0, 1), - ev); + lpcfg_imessaging_path(tctx, tctx->lp_ctx), cluster_id(0, 1), + ev, true); torture_assert(tctx, msg_server_ctx != NULL, "Failed to init ping messaging context"); @@ -81,9 +81,9 @@ static bool test_ping_speed(struct torture_context *tctx) imessaging_register_tmp(msg_server_ctx, tctx, exit_message, &msg_exit); msg_client_ctx = imessaging_init(tctx, - lpcfg_imessaging_path(tctx, tctx->lp_ctx), - cluster_id(0, 2), - ev); + lpcfg_imessaging_path(tctx, tctx->lp_ctx), + cluster_id(0, 2), + ev, true); torture_assert(tctx, msg_client_ctx != NULL, "msg_client_ctx imessaging_init() failed"); diff --git a/source4/smbd/server.c b/source4/smbd/server.c index b0f683ba93f..ba8f8227a93 100644 --- a/source4/smbd/server.c +++ b/source4/smbd/server.c @@ -220,8 +220,8 @@ static NTSTATUS setup_parent_messaging(struct tevent_context *event_ctx, NTSTATUS status; msg = imessaging_init(talloc_autofree_context(), - lpcfg_imessaging_path(event_ctx, lp_ctx), - cluster_id(0, SAMBA_PARENT_TASKID), event_ctx); + lpcfg_imessaging_path(event_ctx, lp_ctx), + cluster_id(0, SAMBA_PARENT_TASKID), event_ctx, false); NT_STATUS_HAVE_NO_MEMORY(msg); irpc_add_name(msg, "samba"); diff --git a/source4/smbd/service_stream.c b/source4/smbd/service_stream.c index 6e65122063b..28159f4f00d 100644 --- a/source4/smbd/service_stream.c +++ b/source4/smbd/service_stream.c @@ -77,6 +77,7 @@ void stream_terminate_connection(struct stream_connection *srv_conn, const char talloc_free(srv_conn->event.fde); srv_conn->event.fde = NULL; + imessaging_cleanup(srv_conn->msg_ctx); model_ops->terminate(event_ctx, srv_conn->lp_ctx, reason); talloc_free(srv_conn); } @@ -188,8 +189,8 @@ static void stream_new_connection(struct tevent_context *ev, /* setup to receive internal messages on this connection */ srv_conn->msg_ctx = imessaging_init(srv_conn, - lpcfg_imessaging_path(srv_conn, lp_ctx), - srv_conn->server_id, ev); + lpcfg_imessaging_path(srv_conn, lp_ctx), + srv_conn->server_id, ev, false); if (!srv_conn->msg_ctx) { stream_terminate_connection(srv_conn, "imessaging_init() failed"); return; diff --git a/source4/smbd/service_task.c b/source4/smbd/service_task.c index 32c44cf6603..f68805fde03 100644 --- a/source4/smbd/service_task.c +++ b/source4/smbd/service_task.c @@ -79,9 +79,9 @@ static void task_server_callback(struct tevent_context *event_ctx, task->lp_ctx = lp_ctx; task->msg_ctx = imessaging_init(task, - lpcfg_imessaging_path(task, task->lp_ctx), - task->server_id, - task->event_ctx); + lpcfg_imessaging_path(task, task->lp_ctx), + task->server_id, + task->event_ctx, false); if (!task->msg_ctx) { task_server_terminate(task, "imessaging_init() failed", true); return; -- 2.34.1