From ae915d5552227e7e8ced6dde33ea83d3df06845e Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 1/8] s4:auth_winbind: explicitly use dcerpc_binding_handle_set_sync_ev() for irpc This indicates that we're using nested event loops... Andrew Bartlett Pair-Programmed-With: Stefan Metzmacher Change-Id: I08f21876d42197f76fe3ae10b4f464626d70bf5a Signed-off-by: Andrew Bartlett Signed-off-by: Stefan Metzmacher --- source4/auth/ntlm/auth_winbind.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source4/auth/ntlm/auth_winbind.c b/source4/auth/ntlm/auth_winbind.c index 3f470fc..26d2d07 100644 --- a/source4/auth/ntlm/auth_winbind.c +++ b/source4/auth/ntlm/auth_winbind.c @@ -136,6 +136,8 @@ static NTSTATUS winbind_check_password(struct auth_method_context *ctx, s->req.in.validation_level = 3; + /* Note: this makes use of nested event loops... */ + dcerpc_binding_handle_set_sync_ev(irpc_handle, ctx->auth_ctx->event_ctx); status = dcerpc_winbind_SamLogon_r(irpc_handle, s, &s->req); NT_STATUS_NOT_OK_RETURN(status); -- 1.7.9.5 From fc8d90e7febdddf15e81bd11a0914196c345b0f3 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 2/8] s4:rpc_server/netlogon: explicitly use dcerpc_binding_handle_set_sync_ev() for irpc This indicates that we're using nested event loops... Andrew Bartlett Pair-Programmed-With: Stefan Metzmacher Change-Id: I4dcc7bf3c624612980e53b6119a60989fc2ea3b6 Signed-off-by: Andrew Bartlett Signed-off-by: Stefan Metzmacher --- source4/rpc_server/netlogon/dcerpc_netlogon.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c index 50e7cab..c7fed22 100644 --- a/source4/rpc_server/netlogon/dcerpc_netlogon.c +++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c @@ -747,6 +747,12 @@ static NTSTATUS dcesrv_netr_LogonSamLogon_base(struct dcesrv_call_state *dce_cal data_blob_const(r->in.logon->generic->data, r->in.logon->generic->length); + /* + * TODO: make this async and avoid + * dcerpc_binding_handle_set_sync_ev() + */ + dcerpc_binding_handle_set_sync_ev(irpc_handle, + dce_call->event_ctx); status = dcerpc_kdc_check_generic_kerberos_r(irpc_handle, mem_ctx, &check); -- 1.7.9.5 From f1c9cf771a2dab828ecb006dcc144f67d83587b3 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 3/8] s4:service_task: explicitly use dcerpc_binding_handle_set_sync_ev() for irpc This indicates that we're using nested event loops... Andrew Bartlett Pair-Programmed-With: Stefan Metzmacher Change-Id: I7e147850566301a5ef2354b8615a044d121968b5 Signed-off-by: Andrew Bartlett Signed-off-by: Stefan Metzmacher --- source4/smbd/service_task.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source4/smbd/service_task.c b/source4/smbd/service_task.c index 5d14faf..7422f2c 100644 --- a/source4/smbd/service_task.c +++ b/source4/smbd/service_task.c @@ -41,6 +41,8 @@ void task_server_terminate(struct task_server *task, const char *reason, bool fa irpc_handle = irpc_binding_handle_by_name(task, task->msg_ctx, "samba", &ndr_table_irpc); if (irpc_handle != NULL) { + /* Note: this makes use of nested event loops... */ + dcerpc_binding_handle_set_sync_ev(irpc_handle, event_ctx); r.in.reason = reason; dcerpc_samba_terminate_r(irpc_handle, task, &r); } -- 1.7.9.5 From e26a0cd0668e1a7543dea251a1bafc52b0495246 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 4/8] s4:pyrpc: explicitly use dcerpc_binding_handle_set_sync_ev() for irpc This indicates that we may use nested event loops... Andrew Bartlett Pair-Programmed-With: Stefan Metzmacher Change-Id: Id014dcc68699c86cb99015a91a6979e30795f727 Signed-off-by: Andrew Bartlett Signed-off-by: Stefan Metzmacher --- source4/librpc/rpc/pyrpc_util.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source4/librpc/rpc/pyrpc_util.c b/source4/librpc/rpc/pyrpc_util.c index 314ad2c..226c884 100644 --- a/source4/librpc/rpc/pyrpc_util.c +++ b/source4/librpc/rpc/pyrpc_util.c @@ -83,6 +83,12 @@ static NTSTATUS pyrpc_irpc_connect(TALLOC_CTX *mem_ctx, const char *irpc_server, return NT_STATUS_INVALID_PIPE_STATE; } + /* + * Note: this allows nested event loops to happen, + * but as there's no top level event loop it's not that critical. + */ + dcerpc_binding_handle_set_sync_ev(*binding_handle, event_ctx); + return NT_STATUS_OK; } -- 1.7.9.5 From 9163669c694f3c0201e67e875b244003ee9cba8c Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 5/8] s4:irpc/tests: explicitly use dcerpc_binding_handle_set_sync_ev() This indicates that we're using nested event loops... Andrew Bartlett Pair-Programmed-With: Stefan Metzmacher Change-Id: I17d530a1f338cfdbd2e4e755b6f01a44a3e7ba7a Signed-off-by: Andrew Bartlett Signed-off-by: Stefan Metzmacher --- source4/lib/messaging/tests/irpc.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/source4/lib/messaging/tests/irpc.c b/source4/lib/messaging/tests/irpc.c index 3515604..cb69d3a 100644 --- a/source4/lib/messaging/tests/irpc.c +++ b/source4/lib/messaging/tests/irpc.c @@ -101,6 +101,11 @@ static bool test_addone(struct torture_context *test, const void *_data, r.in.in_data = value; test_debug = true; + /* + * Note: this makes use of nested event loops + * as client and server use the same loop. + */ + dcerpc_binding_handle_set_sync_ev(irpc_handle, data->ev); status = dcerpc_echo_AddOne_r(irpc_handle, test, &r); test_debug = false; torture_assert_ntstatus_ok(test, status, "AddOne failed"); @@ -135,6 +140,11 @@ static bool test_echodata(struct torture_context *tctx, r.in.in_data = (unsigned char *)talloc_strdup(mem_ctx, "0123456789"); r.in.len = strlen((char *)r.in.in_data); + /* + * Note: this makes use of nested event loops + * as client and server use the same loop. + */ + dcerpc_binding_handle_set_sync_ev(irpc_handle, data->ev); status = dcerpc_echo_EchoData_r(irpc_handle, mem_ctx, &r); torture_assert_ntstatus_ok(tctx, status, "EchoData failed"); -- 1.7.9.5 From 40e3ab07c470b5a025f089102f7927e67991620a Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 6/8] s4:nbt_server/wins: make use explicit use of the top level event context Andrew Bartlett Change-Id: I4b8e5c16bd03a038da6527cfb4c91fc874626b18 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher --- source4/nbt_server/wins/winswack.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/nbt_server/wins/winswack.c b/source4/nbt_server/wins/winswack.c index 0fd9767..35875aa 100644 --- a/source4/nbt_server/wins/winswack.c +++ b/source4/nbt_server/wins/winswack.c @@ -307,7 +307,7 @@ NTSTATUS nbtd_proxy_wins_challenge(struct irpc_message *msg, s->io.in.nbtd_server = nbtd_server; s->io.in.nbt_port = lpcfg_nbt_port(nbtd_server->task->lp_ctx); - s->io.in.event_ctx = msg->ev; + s->io.in.event_ctx = nbtd_server->task->event_ctx; s->io.in.name = &req->in.name; s->io.in.num_addresses = req->in.num_addrs; s->io.in.addresses = talloc_array(s, const char *, req->in.num_addrs); @@ -366,7 +366,7 @@ NTSTATUS nbtd_proxy_wins_release_demand(struct irpc_message *msg, s->req = req; s->io.in.nbtd_server = nbtd_server; - s->io.in.event_ctx = msg->ev; + s->io.in.event_ctx = nbtd_server->task->event_ctx; s->io.in.name = &req->in.name; s->io.in.num_addresses = req->in.num_addrs; s->io.in.addresses = talloc_array(s, const char *, req->in.num_addrs); -- 1.7.9.5 From 27e737a2bbf0aae01cd9ff05478fc4e814ea1e01 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 7/8] s4:irpc/tests: make use explicit use of the top level event context Andrew Bartlett Change-Id: Ia193f97f62a1cb928aa814679578f90bde212013 Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher --- source4/lib/messaging/tests/irpc.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/source4/lib/messaging/tests/irpc.c b/source4/lib/messaging/tests/irpc.c index cb69d3a..486420b 100644 --- a/source4/lib/messaging/tests/irpc.c +++ b/source4/lib/messaging/tests/irpc.c @@ -74,8 +74,9 @@ static void deferred_echodata(struct tevent_context *ev, struct tevent_timer *te */ static NTSTATUS irpc_EchoData(struct irpc_message *irpc, struct echo_EchoData *r) { + struct irpc_test_data *data = talloc_get_type_abort(irpc->private_data, struct irpc_test_data); irpc->defer_reply = true; - tevent_add_timer(irpc->ev, irpc, timeval_zero(), deferred_echodata, irpc); + tevent_add_timer(data->ev, irpc, timeval_zero(), deferred_echodata, irpc); return NT_STATUS_OK; } @@ -271,11 +272,11 @@ static bool irpc_setup(struct torture_context *tctx, void **_data) "Failed to init second messaging context"); /* register the server side function */ - IRPC_REGISTER(data->msg_ctx1, rpcecho, ECHO_ADDONE, irpc_AddOne, NULL); - IRPC_REGISTER(data->msg_ctx2, rpcecho, ECHO_ADDONE, irpc_AddOne, NULL); + IRPC_REGISTER(data->msg_ctx1, rpcecho, ECHO_ADDONE, irpc_AddOne, data); + IRPC_REGISTER(data->msg_ctx2, rpcecho, ECHO_ADDONE, irpc_AddOne, data); - IRPC_REGISTER(data->msg_ctx1, rpcecho, ECHO_ECHODATA, irpc_EchoData, NULL); - IRPC_REGISTER(data->msg_ctx2, rpcecho, ECHO_ECHODATA, irpc_EchoData, NULL); + IRPC_REGISTER(data->msg_ctx1, rpcecho, ECHO_ECHODATA, irpc_EchoData, data); + IRPC_REGISTER(data->msg_ctx2, rpcecho, ECHO_ECHODATA, irpc_EchoData, data); return true; } -- 1.7.9.5 From 388b5355eeca3c216263050ca8fb18f9b68f36c8 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 8/8] s4:imessaging: Remove event context from irpc and imessaging structures The only part of this code with a stored event context is now the binding_handle created by irpc_binding_handle() when in the client dcerpc_binding_handle_set_sync_ev() is called, otherwise a new nested event context is created for sync calls. Note that the FD event associated with the socket still implies the long term event context passed to imessaging_[client]_init(). Andrew Bartlett Change-Id: I9aeae94b26e3736370f449daa96808e6cdc2d55d Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher --- source4/lib/messaging/irpc.h | 13 ++++++------- source4/lib/messaging/messaging.c | 31 +++++++++++++------------------ 2 files changed, 19 insertions(+), 25 deletions(-) diff --git a/source4/lib/messaging/irpc.h b/source4/lib/messaging/irpc.h index 456d190..96f67e1 100644 --- a/source4/lib/messaging/irpc.h +++ b/source4/lib/messaging/irpc.h @@ -38,7 +38,6 @@ struct irpc_message { struct imessaging_context *msg_ctx; struct irpc_list *irpc; void *data; - struct tevent_context *ev; }; /* don't allow calls to take too long */ @@ -63,13 +62,13 @@ NTSTATUS irpc_register(struct imessaging_context *msg_ctx, int call, irpc_function_t fn, void *private_data); struct dcerpc_binding_handle *irpc_binding_handle(TALLOC_CTX *mem_ctx, - struct imessaging_context *msg_ctx, - struct server_id server_id, - const struct ndr_interface_table *table); + struct imessaging_context *msg_ctx, + struct server_id server_id, + const struct ndr_interface_table *table); struct dcerpc_binding_handle *irpc_binding_handle_by_name(TALLOC_CTX *mem_ctx, - struct imessaging_context *msg_ctx, - const char *dest_task, - const struct ndr_interface_table *table); + struct imessaging_context *msg_ctx, + const char *dest_task, + const struct ndr_interface_table *table); void irpc_binding_handle_add_security_token(struct dcerpc_binding_handle *h, struct security_token *token); diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c index c33aae3..f73b2ba 100644 --- a/source4/lib/messaging/messaging.c +++ b/source4/lib/messaging/messaging.c @@ -71,7 +71,6 @@ struct imessaging_context { struct timeval start_time; struct tevent_timer *retry_te; struct { - struct tevent_context *ev; struct tevent_fd *fde; } event; }; @@ -262,7 +261,7 @@ static void msg_retry_timer(struct tevent_context *ev, struct tevent_timer *te, /* handle a socket write event */ -static void imessaging_send_handler(struct imessaging_context *msg) +static void imessaging_send_handler(struct imessaging_context *msg, struct tevent_context *ev) { while (msg->pending) { struct imessaging_rec *rec = msg->pending; @@ -278,9 +277,9 @@ static void imessaging_send_handler(struct imessaging_context *msg) struct imessaging_rec *); if (msg->retry_te == NULL) { msg->retry_te = - tevent_add_timer(msg->event.ev, msg, - timeval_current_ofs(1, 0), - msg_retry_timer, msg); + tevent_add_timer(ev, msg, + timeval_current_ofs(1, 0), + msg_retry_timer, msg); } } break; @@ -306,7 +305,7 @@ static void imessaging_send_handler(struct imessaging_context *msg) /* handle a new incoming packet */ -static void imessaging_recv_handler(struct imessaging_context *msg) +static void imessaging_recv_handler(struct imessaging_context *msg, struct tevent_context *ev) { struct imessaging_rec *rec; NTSTATUS status; @@ -372,10 +371,10 @@ static void imessaging_handler(struct tevent_context *ev, struct tevent_fd *fde, struct imessaging_context *msg = talloc_get_type(private_data, struct imessaging_context); if (flags & TEVENT_FD_WRITE) { - imessaging_send_handler(msg); + imessaging_send_handler(msg, ev); } if (flags & TEVENT_FD_READ) { - imessaging_recv_handler(msg); + imessaging_recv_handler(msg, ev); } } @@ -655,7 +654,6 @@ struct imessaging_context *imessaging_init(TALLOC_CTX *mem_ctx, /* it needs to be non blocking for sends */ set_blocking(socket_get_fd(msg->sock), false); - msg->event.ev = ev; msg->event.fde = tevent_add_fd(ev, msg, socket_get_fd(msg->sock), TEVENT_FD_READ, imessaging_handler, msg); tevent_fd_set_auto_close(msg->event.fde); @@ -834,7 +832,6 @@ static void irpc_handler_request(struct imessaging_context *msg_ctx, m->msg_ctx = msg_ctx; m->irpc = i; m->data = r; - m->ev = msg_ctx->event.ev; m->header.status = i->fn(m, r); @@ -1362,9 +1359,9 @@ static const struct dcerpc_binding_handle_ops irpc_bh_ops = { /* initialise a irpc binding handle */ struct dcerpc_binding_handle *irpc_binding_handle(TALLOC_CTX *mem_ctx, - struct imessaging_context *msg_ctx, - struct server_id server_id, - const struct ndr_interface_table *table) + struct imessaging_context *msg_ctx, + struct server_id server_id, + const struct ndr_interface_table *table) { struct dcerpc_binding_handle *h; struct irpc_bh_state *hs; @@ -1384,15 +1381,13 @@ struct dcerpc_binding_handle *irpc_binding_handle(TALLOC_CTX *mem_ctx, hs->table = table; hs->timeout = IRPC_CALL_TIMEOUT; - dcerpc_binding_handle_set_sync_ev(h, msg_ctx->event.ev); - return h; } struct dcerpc_binding_handle *irpc_binding_handle_by_name(TALLOC_CTX *mem_ctx, - struct imessaging_context *msg_ctx, - const char *dest_task, - const struct ndr_interface_table *table) + struct imessaging_context *msg_ctx, + const char *dest_task, + const struct ndr_interface_table *table) { struct dcerpc_binding_handle *h; struct server_id *sids; -- 1.7.9.5