From 8e8fd89fafbab00bcb91c032692978320b8b1e6b Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 10 Feb 2015 17:02:08 -0800 Subject: Allow two completion queues on request call One for the new rpc notification, the other is bound to the new call. This will make async c++ soooo much easier. --- test/core/end2end/tests/cancel_after_accept.c | 3 ++- .../end2end/tests/request_response_with_binary_metadata_and_payload.c | 3 ++- test/core/end2end/tests/request_response_with_metadata_and_payload.c | 3 ++- test/core/end2end/tests/request_response_with_payload.c | 3 ++- .../tests/request_response_with_trailing_metadata_and_payload.c | 3 ++- test/core/end2end/tests/request_with_large_metadata.c | 3 ++- test/core/end2end/tests/request_with_payload.c | 3 ++- test/core/end2end/tests/simple_delayed_request.c | 3 ++- test/core/end2end/tests/simple_request.c | 3 ++- 9 files changed, 18 insertions(+), 9 deletions(-) (limited to 'test/core/end2end') diff --git a/test/core/end2end/tests/cancel_after_accept.c b/test/core/end2end/tests/cancel_after_accept.c index eb26ff14f0..ab7c683e45 100644 --- a/test/core/end2end/tests/cancel_after_accept.c +++ b/test/core/end2end/tests/cancel_after_accept.c @@ -166,7 +166,8 @@ static void test_cancel_after_accept(grpc_end2end_test_config config, GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, tag(2))); + f.server_cq, f.server_cq, + tag(2))); cq_expect_completion(v_server, tag(2), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/request_response_with_binary_metadata_and_payload.c b/test/core/end2end/tests/request_response_with_binary_metadata_and_payload.c index fa5df5f526..cb477144d3 100644 --- a/test/core/end2end/tests/request_response_with_binary_metadata_and_payload.c +++ b/test/core/end2end/tests/request_response_with_binary_metadata_and_payload.c @@ -175,7 +175,8 @@ static void test_request_response_with_metadata_and_payload( GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, tag(101))); + f.server_cq, f.server_cq, + tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/request_response_with_metadata_and_payload.c b/test/core/end2end/tests/request_response_with_metadata_and_payload.c index ad01fe7081..0d4822ec91 100644 --- a/test/core/end2end/tests/request_response_with_metadata_and_payload.c +++ b/test/core/end2end/tests/request_response_with_metadata_and_payload.c @@ -168,7 +168,8 @@ static void test_request_response_with_metadata_and_payload( GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, tag(101))); + f.server_cq, f.server_cq, + tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/request_response_with_payload.c b/test/core/end2end/tests/request_response_with_payload.c index 6b60c4da65..fe3f05fa95 100644 --- a/test/core/end2end/tests/request_response_with_payload.c +++ b/test/core/end2end/tests/request_response_with_payload.c @@ -162,7 +162,8 @@ static void request_response_with_payload(grpc_end2end_test_fixture f) { GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, tag(101))); + f.server_cq, f.server_cq, + tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/request_response_with_trailing_metadata_and_payload.c b/test/core/end2end/tests/request_response_with_trailing_metadata_and_payload.c index 5878058c98..86ee405964 100644 --- a/test/core/end2end/tests/request_response_with_trailing_metadata_and_payload.c +++ b/test/core/end2end/tests/request_response_with_trailing_metadata_and_payload.c @@ -169,7 +169,8 @@ static void test_request_response_with_metadata_and_payload( GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, tag(101))); + f.server_cq, f.server_cq, + tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/request_with_large_metadata.c b/test/core/end2end/tests/request_with_large_metadata.c index 7e7bec0160..8e5b1014f5 100644 --- a/test/core/end2end/tests/request_with_large_metadata.c +++ b/test/core/end2end/tests/request_with_large_metadata.c @@ -166,7 +166,8 @@ static void test_request_with_large_metadata(grpc_end2end_test_config config) { GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, tag(101))); + f.server_cq, f.server_cq, + tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/request_with_payload.c b/test/core/end2end/tests/request_with_payload.c index 2c23f37e0c..67b1577014 100644 --- a/test/core/end2end/tests/request_with_payload.c +++ b/test/core/end2end/tests/request_with_payload.c @@ -157,7 +157,8 @@ static void test_invoke_request_with_payload(grpc_end2end_test_config config) { GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, tag(101))); + f.server_cq, f.server_cq, + tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/simple_delayed_request.c b/test/core/end2end/tests/simple_delayed_request.c index 99d1a26386..5c9109f962 100644 --- a/test/core/end2end/tests/simple_delayed_request.c +++ b/test/core/end2end/tests/simple_delayed_request.c @@ -144,7 +144,8 @@ static void simple_delayed_request_body(grpc_end2end_test_config config, GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f->server, &s, &call_details, &request_metadata_recv, - f->server_cq, tag(101))); + f->server_cq, f->server_cq, + tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/simple_request.c b/test/core/end2end/tests/simple_request.c index 0f046ae2d2..280bf98c16 100644 --- a/test/core/end2end/tests/simple_request.c +++ b/test/core/end2end/tests/simple_request.c @@ -150,7 +150,8 @@ static void simple_request_body(grpc_end2end_test_fixture f) { GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, tag(101))); + f.server_cq, f.server_cq, + tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); -- cgit v1.2.3 From 94f87588fadea5e5ee429ab0b0bff9143d366dba Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 11 Feb 2015 23:00:59 -0800 Subject: Fix up C tests --- test/core/end2end/tests/cancel_after_accept.c | 2 +- .../end2end/tests/request_response_with_binary_metadata_and_payload.c | 2 +- test/core/end2end/tests/request_response_with_metadata_and_payload.c | 2 +- test/core/end2end/tests/request_response_with_payload.c | 2 +- .../end2end/tests/request_response_with_trailing_metadata_and_payload.c | 2 +- test/core/end2end/tests/request_with_large_metadata.c | 2 +- test/core/end2end/tests/request_with_payload.c | 2 +- test/core/end2end/tests/simple_delayed_request.c | 2 +- test/core/end2end/tests/simple_request.c | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) (limited to 'test/core/end2end') diff --git a/test/core/end2end/tests/cancel_after_accept.c b/test/core/end2end/tests/cancel_after_accept.c index 6527fef846..17f37d6719 100644 --- a/test/core/end2end/tests/cancel_after_accept.c +++ b/test/core/end2end/tests/cancel_after_accept.c @@ -166,7 +166,7 @@ static void test_cancel_after_accept(grpc_end2end_test_config config, GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, f.server_cq, + f.server_cq, tag(2))); cq_expect_completion(v_server, tag(2), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/request_response_with_binary_metadata_and_payload.c b/test/core/end2end/tests/request_response_with_binary_metadata_and_payload.c index 695ddbca7d..a71e3a7736 100644 --- a/test/core/end2end/tests/request_response_with_binary_metadata_and_payload.c +++ b/test/core/end2end/tests/request_response_with_binary_metadata_and_payload.c @@ -175,7 +175,7 @@ static void test_request_response_with_metadata_and_payload( GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, f.server_cq, + f.server_cq, tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/request_response_with_metadata_and_payload.c b/test/core/end2end/tests/request_response_with_metadata_and_payload.c index d8624a3003..f7394a25b0 100644 --- a/test/core/end2end/tests/request_response_with_metadata_and_payload.c +++ b/test/core/end2end/tests/request_response_with_metadata_and_payload.c @@ -168,7 +168,7 @@ static void test_request_response_with_metadata_and_payload( GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, f.server_cq, + f.server_cq, tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/request_response_with_payload.c b/test/core/end2end/tests/request_response_with_payload.c index 6a6c792b26..be4beb537a 100644 --- a/test/core/end2end/tests/request_response_with_payload.c +++ b/test/core/end2end/tests/request_response_with_payload.c @@ -162,7 +162,7 @@ static void request_response_with_payload(grpc_end2end_test_fixture f) { GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, f.server_cq, + f.server_cq, tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/request_response_with_trailing_metadata_and_payload.c b/test/core/end2end/tests/request_response_with_trailing_metadata_and_payload.c index 76e298d802..637a9ffe1c 100644 --- a/test/core/end2end/tests/request_response_with_trailing_metadata_and_payload.c +++ b/test/core/end2end/tests/request_response_with_trailing_metadata_and_payload.c @@ -169,7 +169,7 @@ static void test_request_response_with_metadata_and_payload( GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, f.server_cq, + f.server_cq, tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/request_with_large_metadata.c b/test/core/end2end/tests/request_with_large_metadata.c index 693f81ef96..fff83dcbf0 100644 --- a/test/core/end2end/tests/request_with_large_metadata.c +++ b/test/core/end2end/tests/request_with_large_metadata.c @@ -166,7 +166,7 @@ static void test_request_with_large_metadata(grpc_end2end_test_config config) { GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, f.server_cq, + f.server_cq, tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/request_with_payload.c b/test/core/end2end/tests/request_with_payload.c index 3924991a78..2b520046bb 100644 --- a/test/core/end2end/tests/request_with_payload.c +++ b/test/core/end2end/tests/request_with_payload.c @@ -157,7 +157,7 @@ static void test_invoke_request_with_payload(grpc_end2end_test_config config) { GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, f.server_cq, + f.server_cq, tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/simple_delayed_request.c b/test/core/end2end/tests/simple_delayed_request.c index 851d0cb865..ac248f9be0 100644 --- a/test/core/end2end/tests/simple_delayed_request.c +++ b/test/core/end2end/tests/simple_delayed_request.c @@ -144,7 +144,7 @@ static void simple_delayed_request_body(grpc_end2end_test_config config, GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f->server, &s, &call_details, &request_metadata_recv, - f->server_cq, f->server_cq, + f->server_cq, tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); diff --git a/test/core/end2end/tests/simple_request.c b/test/core/end2end/tests/simple_request.c index 9b6230db15..ab03479586 100644 --- a/test/core/end2end/tests/simple_request.c +++ b/test/core/end2end/tests/simple_request.c @@ -150,7 +150,7 @@ static void simple_request_body(grpc_end2end_test_fixture f) { GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, - f.server_cq, f.server_cq, + f.server_cq, tag(101))); cq_expect_completion(v_server, tag(101), GRPC_OP_OK); cq_verify(v_server); -- cgit v1.2.3 From aea2fc053d415b2650753509f65855cb92552ca9 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 17 Feb 2015 16:54:53 -0800 Subject: Fix shutdown semantics. Document what they should be, ensure they're triggered, and fix what was broken. --- include/grpc/grpc.h | 10 ++++-- src/core/surface/server.c | 37 +++++++++++++++------- .../tests/early_server_shutdown_finishes_tags.c | 2 +- 3 files changed, 33 insertions(+), 16 deletions(-) (limited to 'test/core/end2end') diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index 9807de9f4b..cf84ac1c63 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -564,15 +564,19 @@ void grpc_server_start(grpc_server *server); /* Begin shutting down a server. After completion, no new calls or connections will be admitted. - Existing calls will be allowed to complete. */ + Existing calls will be allowed to complete. + Shutdown is idempotent. */ void grpc_server_shutdown(grpc_server *server); /* As per grpc_server_shutdown, but send a GRPC_SERVER_SHUTDOWN event when - there are no more calls being serviced. */ + there are no more calls being serviced. + Shutdown is idempotent, and all tags will be notified at once if multiple + grpc_server_shutdown_and_notify calls are made. */ void grpc_server_shutdown_and_notify(grpc_server *server, void *tag); /* Destroy a server. - Forcefully cancels all existing calls. */ + Forcefully cancels all existing calls. + Implies grpc_server_shutdown() if one was not previously performed. */ void grpc_server_destroy(grpc_server *server); #ifdef __cplusplus diff --git a/src/core/surface/server.c b/src/core/surface/server.c index ee0f96a580..456c7826a2 100644 --- a/src/core/surface/server.c +++ b/src/core/surface/server.c @@ -98,8 +98,8 @@ struct grpc_server { size_t requested_call_capacity; gpr_uint8 shutdown; - gpr_uint8 have_shutdown_tag; - void *shutdown_tag; + size_t num_shutdown_tags; + void **shutdown_tags; call_data *lists[CALL_LIST_COUNT]; channel_data root_channel_data; @@ -206,6 +206,7 @@ static void server_unref(grpc_server *server) { gpr_mu_destroy(&server->mu); gpr_free(server->channel_filters); gpr_free(server->requested_calls); + gpr_free(server->shutdown_tags); gpr_free(server); } } @@ -407,15 +408,17 @@ static void init_call_elem(grpc_call_element *elem, static void destroy_call_elem(grpc_call_element *elem) { channel_data *chand = elem->channel_data; call_data *calld = elem->call_data; - int i; + size_t i; gpr_mu_lock(&chand->server->mu); for (i = 0; i < CALL_LIST_COUNT; i++) { call_list_remove(chand->server, elem->call_data, i); } - if (chand->server->shutdown && chand->server->have_shutdown_tag && - chand->server->lists[ALL_CALLS] == NULL) { - grpc_cq_end_server_shutdown(chand->server->cq, chand->server->shutdown_tag); + if (chand->server->shutdown && chand->server->lists[ALL_CALLS] == NULL) { + for (i = 0; i < chand->server->num_shutdown_tags; i++) { + grpc_cq_end_server_shutdown(chand->server->cq, + chand->server->shutdown_tags[i]); + } } gpr_mu_unlock(&chand->server->mu); @@ -572,6 +575,13 @@ void shutdown_internal(grpc_server *server, gpr_uint8 have_shutdown_tag, /* lock, and gather up some stuff to do */ gpr_mu_lock(&server->mu); + if (have_shutdown_tag) { + grpc_cq_begin_op(server->cq, NULL, GRPC_SERVER_SHUTDOWN); + server->shutdown_tags = + gpr_realloc(server->shutdown_tags, + sizeof(void *) * (server->num_shutdown_tags + 1)); + server->shutdown_tags[server->num_shutdown_tags++] = shutdown_tag; + } if (server->shutdown) { gpr_mu_unlock(&server->mu); return; @@ -597,12 +607,9 @@ void shutdown_internal(grpc_server *server, gpr_uint8 have_shutdown_tag, server->requested_call_count = 0; server->shutdown = 1; - server->have_shutdown_tag = have_shutdown_tag; - server->shutdown_tag = shutdown_tag; - if (have_shutdown_tag) { - grpc_cq_begin_op(server->cq, NULL, GRPC_SERVER_SHUTDOWN); - if (server->lists[ALL_CALLS] == NULL) { - grpc_cq_end_server_shutdown(server->cq, shutdown_tag); + if (server->lists[ALL_CALLS] == NULL) { + for (i = 0; i < server->num_shutdown_tags; i++) { + grpc_cq_end_server_shutdown(server->cq, server->shutdown_tags[i]); } } gpr_mu_unlock(&server->mu); @@ -653,6 +660,12 @@ void grpc_server_shutdown_and_notify(grpc_server *server, void *tag) { void grpc_server_destroy(grpc_server *server) { channel_data *c; gpr_mu_lock(&server->mu); + if (!server->shutdown) { + gpr_mu_unlock(&server->mu); + grpc_server_shutdown(server); + gpr_mu_lock(&server->mu); + } + for (c = server->root_channel_data.next; c != &server->root_channel_data; c = c->next) { shutdown_channel(c); diff --git a/test/core/end2end/tests/early_server_shutdown_finishes_tags.c b/test/core/end2end/tests/early_server_shutdown_finishes_tags.c index 123c8bc415..51486cc169 100644 --- a/test/core/end2end/tests/early_server_shutdown_finishes_tags.c +++ b/test/core/end2end/tests/early_server_shutdown_finishes_tags.c @@ -79,7 +79,7 @@ static void drain_cq(grpc_completion_queue *cq) { static void shutdown_server(grpc_end2end_test_fixture *f) { if (!f->server) return; - grpc_server_shutdown(f->server); + /* don't shutdown, just destroy, to tickle this code edge */ grpc_server_destroy(f->server); f->server = NULL; } -- cgit v1.2.3