From 357ae54612e66f720d232f7f2d347e59e6780423 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 1/9] 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 54cff32d3e7e73a23db7a693aa2d9e719ffb7c18 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 2/9] 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 3515604..d78dc78 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; } @@ -261,11 +262,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 9fb44094d2e24684772f45500592505b15bc8485 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 3/9] 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 5bdc32f91e4050df0610c305dd2bd50b79ec360e Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 4/9] 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 7671b2f3ff7ff1a6b6f6259e96872649096d73bb Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 5/9] 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 0acd9164e5425dc5003ec86ba32d20887a72f184 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 6/9] 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 8cf936dfd42222022ab2d846b76a3acba3d26d9b Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 7/9] 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 d78dc78..486420b 100644 --- a/source4/lib/messaging/tests/irpc.c +++ b/source4/lib/messaging/tests/irpc.c @@ -102,6 +102,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"); @@ -136,6 +141,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 71d9c1682bd9e8856471534911b0ada5251c7bd3 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 8/9] s4:imessaging: Remove dcerpc_binding_handle_set_sync_ev() call from irpc_binding_handle() The callers do this explicitly now if required. Change-Id: I0e6f562aac4e3c0a75149c5850eb9f96269a3caf Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher --- source4/lib/messaging/messaging.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c index c33aae3..a466cc0 100644 --- a/source4/lib/messaging/messaging.c +++ b/source4/lib/messaging/messaging.c @@ -1384,8 +1384,6 @@ 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; } -- 1.7.9.5 From b418a6f75539a4c57c9a66c93a44a894af7434a6 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 5 May 2014 16:27:59 +1200 Subject: [PATCH 9/9] 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 | 29 +++++++++++++---------------- 2 files changed, 19 insertions(+), 23 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 a466cc0..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; @@ -1388,9 +1385,9 @@ struct dcerpc_binding_handle *irpc_binding_handle(TALLOC_CTX *mem_ctx, } 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