From fbc3f04eabe2c5b98553679b1b5b1f48367fe7fe Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 9 Nov 2017 07:58:40 -0800 Subject: Add channel arg for server handshake timeout. --- src/core/ext/transport/chttp2/server/chttp2_server.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.cc b/src/core/ext/transport/chttp2/server/chttp2_server.cc index 98683acc59..208c9ec437 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.cc +++ b/src/core/ext/transport/chttp2/server/chttp2_server.cc @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -132,10 +133,12 @@ static void on_accept(grpc_exec_ctx* exec_ctx, void* arg, grpc_endpoint* tcp, connection_state->handshake_mgr = handshake_mgr; grpc_handshakers_add(exec_ctx, HANDSHAKER_SERVER, state->args, connection_state->handshake_mgr); - // TODO(roth): We should really get this timeout value from channel - // args instead of hard-coding it. + const grpc_arg* timeout_arg = + grpc_channel_args_find(state->args, GRPC_ARG_SERVER_HANDSHAKE_TIMEOUT_MS); const grpc_millis deadline = - grpc_exec_ctx_now(exec_ctx) + 120 * GPR_MS_PER_SEC; + grpc_exec_ctx_now(exec_ctx) + + grpc_channel_arg_get_integer(timeout_arg, + {120 * GPR_MS_PER_SEC, 1, INT_MAX}); grpc_handshake_manager_do_handshake(exec_ctx, connection_state->handshake_mgr, tcp, state->args, deadline, acceptor, on_handshake_done, connection_state); -- cgit v1.2.3 From bcfd0f38fcdcee62d5187ab2a0d24a06f37241b5 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 9 Nov 2017 08:04:47 -0800 Subject: Bool-ify is_client param. --- src/core/ext/transport/chttp2/client/chttp2_connector.cc | 4 ++-- .../ext/transport/chttp2/client/insecure/channel_create_posix.cc | 2 +- src/core/ext/transport/chttp2/server/chttp2_server.cc | 4 ++-- .../ext/transport/chttp2/server/insecure/server_chttp2_posix.cc | 2 +- src/core/ext/transport/chttp2/transport/chttp2_transport.cc | 4 ++-- src/core/ext/transport/chttp2/transport/chttp2_transport.h | 2 +- test/core/bad_client/bad_client.cc | 2 +- test/core/end2end/fixtures/h2_sockpair+trace.cc | 4 ++-- test/core/end2end/fixtures/h2_sockpair.cc | 4 ++-- test/core/end2end/fixtures/h2_sockpair_1byte.cc | 4 ++-- test/core/end2end/fuzzers/api_fuzzer.cc | 2 +- test/core/end2end/fuzzers/client_fuzzer.cc | 2 +- test/core/end2end/fuzzers/server_fuzzer.cc | 2 +- test/cpp/microbenchmarks/fullstack_fixtures.h | 6 +++--- test/cpp/performance/writes_per_rpc_test.cc | 6 +++--- 15 files changed, 25 insertions(+), 25 deletions(-) (limited to 'src') diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.cc b/src/core/ext/transport/chttp2/client/chttp2_connector.cc index 6cd476f4ca..4efd129384 100644 --- a/src/core/ext/transport/chttp2/client/chttp2_connector.cc +++ b/src/core/ext/transport/chttp2/client/chttp2_connector.cc @@ -117,8 +117,8 @@ static void on_handshake_done(grpc_exec_ctx* exec_ctx, void* arg, } else { grpc_endpoint_delete_from_pollset_set(exec_ctx, args->endpoint, c->args.interested_parties); - c->result->transport = - grpc_create_chttp2_transport(exec_ctx, args->args, args->endpoint, 1); + c->result->transport = grpc_create_chttp2_transport( + exec_ctx, args->args, args->endpoint, true); GPR_ASSERT(c->result->transport); grpc_chttp2_transport_start_reading(exec_ctx, c->result->transport, args->read_buffer); diff --git a/src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc b/src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc index 0974a7c393..fcc2f4249a 100644 --- a/src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc +++ b/src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc @@ -53,7 +53,7 @@ grpc_channel* grpc_insecure_channel_create_from_fd( &exec_ctx, grpc_fd_create(fd, "client"), args, "fd-client"); grpc_transport* transport = - grpc_create_chttp2_transport(&exec_ctx, final_args, client, 1); + grpc_create_chttp2_transport(&exec_ctx, final_args, client, true); GPR_ASSERT(transport); grpc_channel* channel = grpc_channel_create( &exec_ctx, target, final_args, GRPC_CLIENT_DIRECT_CHANNEL, transport); diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.cc b/src/core/ext/transport/chttp2/server/chttp2_server.cc index 208c9ec437..1b4d89b5ee 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.cc +++ b/src/core/ext/transport/chttp2/server/chttp2_server.cc @@ -88,8 +88,8 @@ static void on_handshake_done(grpc_exec_ctx* exec_ctx, void* arg, // handshaker may have handed off the connection to some external // code, so we can just clean up here without creating a transport. if (args->endpoint != NULL) { - grpc_transport* transport = - grpc_create_chttp2_transport(exec_ctx, args->args, args->endpoint, 0); + grpc_transport* transport = grpc_create_chttp2_transport( + exec_ctx, args->args, args->endpoint, false); grpc_server_setup_transport( exec_ctx, connection_state->svr_state->server, transport, connection_state->accepting_pollset, args->args); diff --git a/src/core/ext/transport/chttp2/server/insecure/server_chttp2_posix.cc b/src/core/ext/transport/chttp2/server/insecure/server_chttp2_posix.cc index e37d69e5e9..70d4864710 100644 --- a/src/core/ext/transport/chttp2/server/insecure/server_chttp2_posix.cc +++ b/src/core/ext/transport/chttp2/server/insecure/server_chttp2_posix.cc @@ -50,7 +50,7 @@ void grpc_server_add_insecure_channel_from_fd(grpc_server* server, const grpc_channel_args* server_args = grpc_server_get_channel_args(server); grpc_transport* transport = grpc_create_chttp2_transport( - &exec_ctx, server_args, server_endpoint, 0 /* is_client */); + &exec_ctx, server_args, server_endpoint, false /* is_client */); grpc_pollset** pollsets; size_t num_pollsets = 0; diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 034e6ed8ca..b4edb17cde 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -3224,10 +3224,10 @@ static const grpc_transport_vtable* get_vtable(void) { return &vtable; } grpc_transport* grpc_create_chttp2_transport( grpc_exec_ctx* exec_ctx, const grpc_channel_args* channel_args, - grpc_endpoint* ep, int is_client) { + grpc_endpoint* ep, bool is_client) { grpc_chttp2_transport* t = (grpc_chttp2_transport*)gpr_zalloc(sizeof(grpc_chttp2_transport)); - init_transport(exec_ctx, t, channel_args, ep, is_client != 0); + init_transport(exec_ctx, t, channel_args, ep, is_client); return &t->base; } diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.h b/src/core/ext/transport/chttp2/transport/chttp2_transport.h index 972104f62c..4fe12d42e9 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.h +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.h @@ -37,7 +37,7 @@ extern grpc_tracer_flag grpc_trace_chttp2_refcount; grpc_transport* grpc_create_chttp2_transport( grpc_exec_ctx* exec_ctx, const grpc_channel_args* channel_args, - grpc_endpoint* ep, int is_client); + grpc_endpoint* ep, bool is_client); /// Takes ownership of \a read_buffer, which (if non-NULL) contains /// leftover bytes previously read from the endpoint (e.g., by handshakers). diff --git a/test/core/bad_client/bad_client.cc b/test/core/bad_client/bad_client.cc index b1944425ba..5ab5436d2f 100644 --- a/test/core/bad_client/bad_client.cc +++ b/test/core/bad_client/bad_client.cc @@ -115,7 +115,7 @@ void grpc_run_bad_client_test( GRPC_BAD_CLIENT_REGISTERED_HOST, GRPC_SRM_PAYLOAD_READ_INITIAL_BYTE_BUFFER, 0); grpc_server_start(a.server); - transport = grpc_create_chttp2_transport(&exec_ctx, NULL, sfd.server, 0); + transport = grpc_create_chttp2_transport(&exec_ctx, NULL, sfd.server, false); server_setup_transport(&a, transport); grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); grpc_exec_ctx_finish(&exec_ctx); diff --git a/test/core/end2end/fixtures/h2_sockpair+trace.cc b/test/core/end2end/fixtures/h2_sockpair+trace.cc index c5dfe40391..8914af499b 100644 --- a/test/core/end2end/fixtures/h2_sockpair+trace.cc +++ b/test/core/end2end/fixtures/h2_sockpair+trace.cc @@ -97,7 +97,7 @@ static void chttp2_init_client_socketpair(grpc_end2end_test_fixture* f, cs.client_args = client_args; cs.f = f; transport = - grpc_create_chttp2_transport(&exec_ctx, client_args, sfd->client, 1); + grpc_create_chttp2_transport(&exec_ctx, client_args, sfd->client, true); client_setup_transport(&exec_ctx, &cs, transport); GPR_ASSERT(f->client); grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); @@ -114,7 +114,7 @@ static void chttp2_init_server_socketpair(grpc_end2end_test_fixture* f, grpc_server_register_completion_queue(f->server, f->cq, NULL); grpc_server_start(f->server); transport = - grpc_create_chttp2_transport(&exec_ctx, server_args, sfd->server, 0); + grpc_create_chttp2_transport(&exec_ctx, server_args, sfd->server, false); server_setup_transport(f, transport); grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); grpc_exec_ctx_finish(&exec_ctx); diff --git a/test/core/end2end/fixtures/h2_sockpair.cc b/test/core/end2end/fixtures/h2_sockpair.cc index f07722e52d..b79c8e7a25 100644 --- a/test/core/end2end/fixtures/h2_sockpair.cc +++ b/test/core/end2end/fixtures/h2_sockpair.cc @@ -91,7 +91,7 @@ static void chttp2_init_client_socketpair(grpc_end2end_test_fixture* f, cs.client_args = client_args; cs.f = f; transport = - grpc_create_chttp2_transport(&exec_ctx, client_args, sfd->client, 1); + grpc_create_chttp2_transport(&exec_ctx, client_args, sfd->client, true); client_setup_transport(&exec_ctx, &cs, transport); GPR_ASSERT(f->client); grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); @@ -108,7 +108,7 @@ static void chttp2_init_server_socketpair(grpc_end2end_test_fixture* f, grpc_server_register_completion_queue(f->server, f->cq, NULL); grpc_server_start(f->server); transport = - grpc_create_chttp2_transport(&exec_ctx, server_args, sfd->server, 0); + grpc_create_chttp2_transport(&exec_ctx, server_args, sfd->server, false); server_setup_transport(f, transport); grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); grpc_exec_ctx_finish(&exec_ctx); diff --git a/test/core/end2end/fixtures/h2_sockpair_1byte.cc b/test/core/end2end/fixtures/h2_sockpair_1byte.cc index 7d00c69a18..529866b3a7 100644 --- a/test/core/end2end/fixtures/h2_sockpair_1byte.cc +++ b/test/core/end2end/fixtures/h2_sockpair_1byte.cc @@ -102,7 +102,7 @@ static void chttp2_init_client_socketpair(grpc_end2end_test_fixture* f, cs.client_args = client_args; cs.f = f; transport = - grpc_create_chttp2_transport(&exec_ctx, client_args, sfd->client, 1); + grpc_create_chttp2_transport(&exec_ctx, client_args, sfd->client, true); client_setup_transport(&exec_ctx, &cs, transport); GPR_ASSERT(f->client); grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); @@ -119,7 +119,7 @@ static void chttp2_init_server_socketpair(grpc_end2end_test_fixture* f, grpc_server_register_completion_queue(f->server, f->cq, NULL); grpc_server_start(f->server); transport = - grpc_create_chttp2_transport(&exec_ctx, server_args, sfd->server, 0); + grpc_create_chttp2_transport(&exec_ctx, server_args, sfd->server, false); server_setup_transport(f, transport); grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); grpc_exec_ctx_finish(&exec_ctx); diff --git a/test/core/end2end/fuzzers/api_fuzzer.cc b/test/core/end2end/fuzzers/api_fuzzer.cc index 69a5670d22..d625ec8b0d 100644 --- a/test/core/end2end/fuzzers/api_fuzzer.cc +++ b/test/core/end2end/fuzzers/api_fuzzer.cc @@ -464,7 +464,7 @@ static void do_connect(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) { *fc->ep = client; grpc_transport* transport = - grpc_create_chttp2_transport(exec_ctx, NULL, server, 0); + grpc_create_chttp2_transport(exec_ctx, NULL, server, false); grpc_server_setup_transport(exec_ctx, g_server, transport, NULL, NULL); grpc_chttp2_transport_start_reading(exec_ctx, transport, NULL); diff --git a/test/core/end2end/fuzzers/client_fuzzer.cc b/test/core/end2end/fuzzers/client_fuzzer.cc index d90ec40a27..f61067b277 100644 --- a/test/core/end2end/fuzzers/client_fuzzer.cc +++ b/test/core/end2end/fuzzers/client_fuzzer.cc @@ -54,7 +54,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { grpc_completion_queue* cq = grpc_completion_queue_create_for_next(NULL); grpc_transport* transport = - grpc_create_chttp2_transport(&exec_ctx, NULL, mock_endpoint, 1); + grpc_create_chttp2_transport(&exec_ctx, NULL, mock_endpoint, true); grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); grpc_channel* channel = grpc_channel_create( diff --git a/test/core/end2end/fuzzers/server_fuzzer.cc b/test/core/end2end/fuzzers/server_fuzzer.cc index 87bccc70cd..4754712ad0 100644 --- a/test/core/end2end/fuzzers/server_fuzzer.cc +++ b/test/core/end2end/fuzzers/server_fuzzer.cc @@ -61,7 +61,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { // grpc_server_register_method(server, "/reg", NULL, 0); grpc_server_start(server); grpc_transport* transport = - grpc_create_chttp2_transport(&exec_ctx, NULL, mock_endpoint, 0); + grpc_create_chttp2_transport(&exec_ctx, NULL, mock_endpoint, false); grpc_server_setup_transport(&exec_ctx, server, transport, NULL, NULL); grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); diff --git a/test/cpp/microbenchmarks/fullstack_fixtures.h b/test/cpp/microbenchmarks/fullstack_fixtures.h index 71bbb393db..7db23234b6 100644 --- a/test/cpp/microbenchmarks/fullstack_fixtures.h +++ b/test/cpp/microbenchmarks/fullstack_fixtures.h @@ -174,7 +174,7 @@ class EndpointPairFixture : public BaseFixture { const grpc_channel_args* server_args = grpc_server_get_channel_args(server_->c_server()); server_transport_ = grpc_create_chttp2_transport( - &exec_ctx, server_args, endpoints.server, 0 /* is_client */); + &exec_ctx, server_args, endpoints.server, false /* is_client */); grpc_pollset** pollsets; size_t num_pollsets = 0; @@ -196,8 +196,8 @@ class EndpointPairFixture : public BaseFixture { fixture_configuration.ApplyCommonChannelArguments(&args); grpc_channel_args c_args = args.c_channel_args(); - client_transport_ = - grpc_create_chttp2_transport(&exec_ctx, &c_args, endpoints.client, 1); + client_transport_ = grpc_create_chttp2_transport( + &exec_ctx, &c_args, endpoints.client, true); GPR_ASSERT(client_transport_); grpc_channel* channel = grpc_channel_create(&exec_ctx, "target", &c_args, diff --git a/test/cpp/performance/writes_per_rpc_test.cc b/test/cpp/performance/writes_per_rpc_test.cc index 6c23245021..ecf67a2c27 100644 --- a/test/cpp/performance/writes_per_rpc_test.cc +++ b/test/cpp/performance/writes_per_rpc_test.cc @@ -89,7 +89,7 @@ class EndpointPairFixture { const grpc_channel_args* server_args = grpc_server_get_channel_args(server_->c_server()); grpc_transport* transport = grpc_create_chttp2_transport( - &exec_ctx, server_args, endpoints.server, 0 /* is_client */); + &exec_ctx, server_args, endpoints.server, false /* is_client */); grpc_pollset** pollsets; size_t num_pollsets = 0; @@ -111,8 +111,8 @@ class EndpointPairFixture { ApplyCommonChannelArguments(&args); grpc_channel_args c_args = args.c_channel_args(); - grpc_transport* transport = - grpc_create_chttp2_transport(&exec_ctx, &c_args, endpoints.client, 1); + grpc_transport* transport = grpc_create_chttp2_transport( + &exec_ctx, &c_args, endpoints.client, true); GPR_ASSERT(transport); grpc_channel* channel = grpc_channel_create( &exec_ctx, "target", &c_args, GRPC_CLIENT_DIRECT_CHANNEL, transport); -- cgit v1.2.3 From 04c97d0e0daec7c47d0ee5fcb8038270dd2d3328 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 9 Nov 2017 09:14:14 -0800 Subject: Add notify_on_receive_settings closure to chttp2 transport. --- .../transport/chttp2/client/chttp2_connector.cc | 26 +++++++++++++++++++++- .../chttp2/client/insecure/channel_create_posix.cc | 2 +- .../ext/transport/chttp2/server/chttp2_server.cc | 4 +++- .../chttp2/server/insecure/server_chttp2_posix.cc | 2 +- .../transport/chttp2/transport/chttp2_transport.cc | 19 ++++++++++------ .../transport/chttp2/transport/chttp2_transport.h | 8 ++++--- .../transport/chttp2/transport/frame_settings.cc | 5 +++++ src/core/ext/transport/chttp2/transport/internal.h | 2 ++ test/core/bad_client/bad_client.cc | 2 +- test/core/end2end/fixtures/h2_sockpair+trace.cc | 4 ++-- test/core/end2end/fixtures/h2_sockpair.cc | 4 ++-- test/core/end2end/fixtures/h2_sockpair_1byte.cc | 4 ++-- test/core/end2end/fuzzers/api_fuzzer.cc | 2 +- test/core/end2end/fuzzers/client_fuzzer.cc | 2 +- test/core/end2end/fuzzers/server_fuzzer.cc | 2 +- test/cpp/microbenchmarks/bm_chttp2_transport.cc | 2 +- test/cpp/microbenchmarks/fullstack_fixtures.h | 6 +++-- test/cpp/performance/writes_per_rpc_test.cc | 6 +++-- 18 files changed, 73 insertions(+), 29 deletions(-) (limited to 'src') diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.cc b/src/core/ext/transport/chttp2/client/chttp2_connector.cc index 4efd129384..5870a3e6d5 100644 --- a/src/core/ext/transport/chttp2/client/chttp2_connector.cc +++ b/src/core/ext/transport/chttp2/client/chttp2_connector.cc @@ -120,8 +120,32 @@ static void on_handshake_done(grpc_exec_ctx* exec_ctx, void* arg, c->result->transport = grpc_create_chttp2_transport( exec_ctx, args->args, args->endpoint, true); GPR_ASSERT(c->result->transport); + // TODO(roth): We ideally want to wait until we receive HTTP/2 + // settings from the server before we consider the connection + // established. If that doesn't happen before the connection + // timeout expires, then we should consider the connection attempt a + // failure and feed that information back into the backoff code. + // We could pass a notify_on_receive_settings callback to + // grpc_chttp2_transport_start_reading() to let us know when + // settings are received, but we would need to figure out how to use + // that information here. + // + // Unfortunately, we don't currently have a way to split apart the two + // effects of scheduling c->notify: we start sending RPCs immediately + // (which we want to do) and we consider the connection attempt successful + // (which we don't want to do until we get the notify_on_receive_settings + // callback from the transport). If we could split those things + // apart, then we could start sending RPCs but then wait for our + // timeout before deciding if the connection attempt is successful. + // If the attempt is not successful, then we would tear down the + // transport and feed the failure back into the backoff code. + // + // In addition, even if we did that, we would probably not want to do + // so until after transparent retries is implemented. Otherwise, any + // RPC that we attempt to send on the connection before the timeout + // would fail instead of being retried on a subsequent attempt. grpc_chttp2_transport_start_reading(exec_ctx, c->result->transport, - args->read_buffer); + args->read_buffer, nullptr); c->result->channel_args = args->args; } grpc_closure* notify = c->notify; diff --git a/src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc b/src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc index fcc2f4249a..ad64f740b8 100644 --- a/src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc +++ b/src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc @@ -58,7 +58,7 @@ grpc_channel* grpc_insecure_channel_create_from_fd( grpc_channel* channel = grpc_channel_create( &exec_ctx, target, final_args, GRPC_CLIENT_DIRECT_CHANNEL, transport); grpc_channel_args_destroy(&exec_ctx, final_args); - grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, transport, nullptr, nullptr); grpc_exec_ctx_finish(&exec_ctx); diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.cc b/src/core/ext/transport/chttp2/server/chttp2_server.cc index 1b4d89b5ee..39e8dfd684 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.cc +++ b/src/core/ext/transport/chttp2/server/chttp2_server.cc @@ -93,8 +93,10 @@ static void on_handshake_done(grpc_exec_ctx* exec_ctx, void* arg, grpc_server_setup_transport( exec_ctx, connection_state->svr_state->server, transport, connection_state->accepting_pollset, args->args); +// FIXME: set notify_on_receive_settings callback and use it to enforce +// handshaking deadline grpc_chttp2_transport_start_reading(exec_ctx, transport, - args->read_buffer); + args->read_buffer, nullptr); grpc_channel_args_destroy(exec_ctx, args->args); } } diff --git a/src/core/ext/transport/chttp2/server/insecure/server_chttp2_posix.cc b/src/core/ext/transport/chttp2/server/insecure/server_chttp2_posix.cc index 70d4864710..09ee14c022 100644 --- a/src/core/ext/transport/chttp2/server/insecure/server_chttp2_posix.cc +++ b/src/core/ext/transport/chttp2/server/insecure/server_chttp2_posix.cc @@ -61,7 +61,7 @@ void grpc_server_add_insecure_channel_from_fd(grpc_server* server, } grpc_server_setup_transport(&exec_ctx, server, transport, NULL, server_args); - grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, transport, nullptr, nullptr); grpc_exec_ctx_finish(&exec_ctx); } diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index b4edb17cde..21808115d7 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -1788,7 +1788,6 @@ static void perform_transport_op_locked(grpc_exec_ctx* exec_ctx, grpc_transport_op* op = (grpc_transport_op*)stream_op; grpc_chttp2_transport* t = (grpc_chttp2_transport*)op->handler_private.extra_arg; - grpc_error* close_transport = op->disconnect_with_error; if (op->goaway_error) { send_goaway(exec_ctx, t, op->goaway_error); @@ -1820,8 +1819,13 @@ static void perform_transport_op_locked(grpc_exec_ctx* exec_ctx, op->on_connectivity_state_change); } - if (close_transport != GRPC_ERROR_NONE) { - close_transport_locked(exec_ctx, t, close_transport); + if (op->disconnect_with_error != GRPC_ERROR_NONE) { + close_transport_locked(exec_ctx, t, op->disconnect_with_error); + if (t->notify_on_receive_settings != nullptr) { + GRPC_CLOSURE_SCHED(exec_ctx, t->notify_on_receive_settings, + GRPC_ERROR_CANCELLED); + t->notify_on_receive_settings = nullptr; + } } GRPC_CLOSURE_RUN(exec_ctx, op->on_consumed, GRPC_ERROR_NONE); @@ -3231,15 +3235,16 @@ grpc_transport* grpc_create_chttp2_transport( return &t->base; } -void grpc_chttp2_transport_start_reading(grpc_exec_ctx* exec_ctx, - grpc_transport* transport, - grpc_slice_buffer* read_buffer) { +void grpc_chttp2_transport_start_reading( + grpc_exec_ctx* exec_ctx, grpc_transport* transport, + grpc_slice_buffer* read_buffer, grpc_closure* notify_on_receive_settings) { grpc_chttp2_transport* t = (grpc_chttp2_transport*)transport; GRPC_CHTTP2_REF_TRANSPORT( t, "reading_action"); /* matches unref inside reading_action */ - if (read_buffer != NULL) { + if (read_buffer != nullptr) { grpc_slice_buffer_move_into(read_buffer, &t->read_buffer); gpr_free(read_buffer); } + t->notify_on_receive_settings = notify_on_receive_settings; GRPC_CLOSURE_SCHED(exec_ctx, &t->read_action_locked, GRPC_ERROR_NONE); } diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.h b/src/core/ext/transport/chttp2/transport/chttp2_transport.h index 4fe12d42e9..a349e00498 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.h +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.h @@ -41,9 +41,11 @@ grpc_transport* grpc_create_chttp2_transport( /// Takes ownership of \a read_buffer, which (if non-NULL) contains /// leftover bytes previously read from the endpoint (e.g., by handshakers). -void grpc_chttp2_transport_start_reading(grpc_exec_ctx* exec_ctx, - grpc_transport* transport, - grpc_slice_buffer* read_buffer); +/// If non-null, \a notify_on_receive_settings will be scheduled when +/// HTTP/2 settings are received from the peer. +void grpc_chttp2_transport_start_reading( + grpc_exec_ctx* exec_ctx, grpc_transport* transport, + grpc_slice_buffer* read_buffer, grpc_closure* notify_on_receive_settings); #ifdef __cplusplus } diff --git a/src/core/ext/transport/chttp2/transport/frame_settings.cc b/src/core/ext/transport/chttp2/transport/frame_settings.cc index d33da721a5..a1da607516 100644 --- a/src/core/ext/transport/chttp2/transport/frame_settings.cc +++ b/src/core/ext/transport/chttp2/transport/frame_settings.cc @@ -131,6 +131,11 @@ grpc_error* grpc_chttp2_settings_parser_parse(grpc_exec_ctx* exec_ctx, void* p, memcpy(parser->target_settings, parser->incoming_settings, GRPC_CHTTP2_NUM_SETTINGS * sizeof(uint32_t)); grpc_slice_buffer_add(&t->qbuf, grpc_chttp2_settings_ack_create()); + if (t->notify_on_receive_settings != nullptr) { + GRPC_CLOSURE_SCHED(exec_ctx, t->notify_on_receive_settings, + GRPC_ERROR_NONE); + t->notify_on_receive_settings = nullptr; + } } return GRPC_ERROR_NONE; } diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index a5a0a804a2..b4fe6fdcbe 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -245,6 +245,8 @@ struct grpc_chttp2_transport { grpc_combiner* combiner; + grpc_closure* notify_on_receive_settings; + /** write execution state of the transport */ grpc_chttp2_write_state write_state; /** is this the first write in a series of writes? diff --git a/test/core/bad_client/bad_client.cc b/test/core/bad_client/bad_client.cc index 5ab5436d2f..4d594d7951 100644 --- a/test/core/bad_client/bad_client.cc +++ b/test/core/bad_client/bad_client.cc @@ -117,7 +117,7 @@ void grpc_run_bad_client_test( grpc_server_start(a.server); transport = grpc_create_chttp2_transport(&exec_ctx, NULL, sfd.server, false); server_setup_transport(&a, transport); - grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, transport, nullptr, nullptr); grpc_exec_ctx_finish(&exec_ctx); /* Bind everything into the same pollset */ diff --git a/test/core/end2end/fixtures/h2_sockpair+trace.cc b/test/core/end2end/fixtures/h2_sockpair+trace.cc index 8914af499b..a1dea10225 100644 --- a/test/core/end2end/fixtures/h2_sockpair+trace.cc +++ b/test/core/end2end/fixtures/h2_sockpair+trace.cc @@ -100,7 +100,7 @@ static void chttp2_init_client_socketpair(grpc_end2end_test_fixture* f, grpc_create_chttp2_transport(&exec_ctx, client_args, sfd->client, true); client_setup_transport(&exec_ctx, &cs, transport); GPR_ASSERT(f->client); - grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, transport, nullptr, nullptr); grpc_exec_ctx_finish(&exec_ctx); } @@ -116,7 +116,7 @@ static void chttp2_init_server_socketpair(grpc_end2end_test_fixture* f, transport = grpc_create_chttp2_transport(&exec_ctx, server_args, sfd->server, false); server_setup_transport(f, transport); - grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, transport, nullptr. nullptr); grpc_exec_ctx_finish(&exec_ctx); } diff --git a/test/core/end2end/fixtures/h2_sockpair.cc b/test/core/end2end/fixtures/h2_sockpair.cc index b79c8e7a25..b2d946c88a 100644 --- a/test/core/end2end/fixtures/h2_sockpair.cc +++ b/test/core/end2end/fixtures/h2_sockpair.cc @@ -94,7 +94,7 @@ static void chttp2_init_client_socketpair(grpc_end2end_test_fixture* f, grpc_create_chttp2_transport(&exec_ctx, client_args, sfd->client, true); client_setup_transport(&exec_ctx, &cs, transport); GPR_ASSERT(f->client); - grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, transport, nullptr, nullptr); grpc_exec_ctx_finish(&exec_ctx); } @@ -110,7 +110,7 @@ static void chttp2_init_server_socketpair(grpc_end2end_test_fixture* f, transport = grpc_create_chttp2_transport(&exec_ctx, server_args, sfd->server, false); server_setup_transport(f, transport); - grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, transport, nullptr, nullptr); grpc_exec_ctx_finish(&exec_ctx); } diff --git a/test/core/end2end/fixtures/h2_sockpair_1byte.cc b/test/core/end2end/fixtures/h2_sockpair_1byte.cc index 529866b3a7..4e847a1d16 100644 --- a/test/core/end2end/fixtures/h2_sockpair_1byte.cc +++ b/test/core/end2end/fixtures/h2_sockpair_1byte.cc @@ -105,7 +105,7 @@ static void chttp2_init_client_socketpair(grpc_end2end_test_fixture* f, grpc_create_chttp2_transport(&exec_ctx, client_args, sfd->client, true); client_setup_transport(&exec_ctx, &cs, transport); GPR_ASSERT(f->client); - grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, transport, nullptr, nullptr); grpc_exec_ctx_finish(&exec_ctx); } @@ -121,7 +121,7 @@ static void chttp2_init_server_socketpair(grpc_end2end_test_fixture* f, transport = grpc_create_chttp2_transport(&exec_ctx, server_args, sfd->server, false); server_setup_transport(f, transport); - grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, transport, nullptr, nullptr); grpc_exec_ctx_finish(&exec_ctx); } diff --git a/test/core/end2end/fuzzers/api_fuzzer.cc b/test/core/end2end/fuzzers/api_fuzzer.cc index d625ec8b0d..296b0a335f 100644 --- a/test/core/end2end/fuzzers/api_fuzzer.cc +++ b/test/core/end2end/fuzzers/api_fuzzer.cc @@ -466,7 +466,7 @@ static void do_connect(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) { grpc_transport* transport = grpc_create_chttp2_transport(exec_ctx, NULL, server, false); grpc_server_setup_transport(exec_ctx, g_server, transport, NULL, NULL); - grpc_chttp2_transport_start_reading(exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(exec_ctx, transport, nullptr, nullptr); GRPC_CLOSURE_SCHED(exec_ctx, fc->closure, GRPC_ERROR_NONE); } else { diff --git a/test/core/end2end/fuzzers/client_fuzzer.cc b/test/core/end2end/fuzzers/client_fuzzer.cc index f61067b277..1046d08139 100644 --- a/test/core/end2end/fuzzers/client_fuzzer.cc +++ b/test/core/end2end/fuzzers/client_fuzzer.cc @@ -55,7 +55,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { grpc_completion_queue* cq = grpc_completion_queue_create_for_next(NULL); grpc_transport* transport = grpc_create_chttp2_transport(&exec_ctx, NULL, mock_endpoint, true); - grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, transport, nullptr. nullptr); grpc_channel* channel = grpc_channel_create( &exec_ctx, "test-target", NULL, GRPC_CLIENT_DIRECT_CHANNEL, transport); diff --git a/test/core/end2end/fuzzers/server_fuzzer.cc b/test/core/end2end/fuzzers/server_fuzzer.cc index 4754712ad0..fad5df1246 100644 --- a/test/core/end2end/fuzzers/server_fuzzer.cc +++ b/test/core/end2end/fuzzers/server_fuzzer.cc @@ -63,7 +63,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { grpc_transport* transport = grpc_create_chttp2_transport(&exec_ctx, NULL, mock_endpoint, false); grpc_server_setup_transport(&exec_ctx, server, transport, NULL, NULL); - grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, transport, nullptr, nullptr); grpc_call* call1 = NULL; grpc_call_details call_details1; diff --git a/test/cpp/microbenchmarks/bm_chttp2_transport.cc b/test/cpp/microbenchmarks/bm_chttp2_transport.cc index 154cc91778..d5d423c8e8 100644 --- a/test/cpp/microbenchmarks/bm_chttp2_transport.cc +++ b/test/cpp/microbenchmarks/bm_chttp2_transport.cc @@ -137,7 +137,7 @@ class Fixture { grpc_channel_args c_args = args.c_channel_args(); ep_ = new DummyEndpoint; t_ = grpc_create_chttp2_transport(exec_ctx(), &c_args, ep_, client); - grpc_chttp2_transport_start_reading(exec_ctx(), t_, NULL); + grpc_chttp2_transport_start_reading(exec_ctx(), t_, nullptr, nullptr); FlushExecCtx(); } diff --git a/test/cpp/microbenchmarks/fullstack_fixtures.h b/test/cpp/microbenchmarks/fullstack_fixtures.h index 7db23234b6..d59b00b048 100644 --- a/test/cpp/microbenchmarks/fullstack_fixtures.h +++ b/test/cpp/microbenchmarks/fullstack_fixtures.h @@ -186,7 +186,8 @@ class EndpointPairFixture : public BaseFixture { grpc_server_setup_transport(&exec_ctx, server_->c_server(), server_transport_, NULL, server_args); - grpc_chttp2_transport_start_reading(&exec_ctx, server_transport_, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, server_transport_, + nullptr, nullptr); } /* create channel */ @@ -202,7 +203,8 @@ class EndpointPairFixture : public BaseFixture { grpc_channel* channel = grpc_channel_create(&exec_ctx, "target", &c_args, GRPC_CLIENT_DIRECT_CHANNEL, client_transport_); - grpc_chttp2_transport_start_reading(&exec_ctx, client_transport_, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, client_transport_, + nullptr, nullptr); channel_ = CreateChannelInternal("", channel); } diff --git a/test/cpp/performance/writes_per_rpc_test.cc b/test/cpp/performance/writes_per_rpc_test.cc index ecf67a2c27..1d8b30f0ad 100644 --- a/test/cpp/performance/writes_per_rpc_test.cc +++ b/test/cpp/performance/writes_per_rpc_test.cc @@ -101,7 +101,8 @@ class EndpointPairFixture { grpc_server_setup_transport(&exec_ctx, server_->c_server(), transport, NULL, server_args); - grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, transport, nullptr, + nullptr); } /* create channel */ @@ -116,7 +117,8 @@ class EndpointPairFixture { GPR_ASSERT(transport); grpc_channel* channel = grpc_channel_create( &exec_ctx, "target", &c_args, GRPC_CLIENT_DIRECT_CHANNEL, transport); - grpc_chttp2_transport_start_reading(&exec_ctx, transport, NULL); + grpc_chttp2_transport_start_reading(&exec_ctx, transport, nullptr, + nullptr); channel_ = CreateChannelInternal("", channel); } -- cgit v1.2.3 From 8eeedab365743cf704ab544a7bff97b52cdddff6 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 9 Nov 2017 10:03:23 -0800 Subject: Remove declaration of non-existant function. --- src/core/lib/transport/transport.h | 3 --- 1 file changed, 3 deletions(-) (limited to 'src') diff --git a/src/core/lib/transport/transport.h b/src/core/lib/transport/transport.h index 973018e5a5..920ad60db8 100644 --- a/src/core/lib/transport/transport.h +++ b/src/core/lib/transport/transport.h @@ -333,9 +333,6 @@ void grpc_transport_ping(grpc_transport* transport, grpc_closure* cb); void grpc_transport_goaway(grpc_transport* transport, grpc_status_code status, grpc_slice debug_data); -/* Close a transport. Aborts all open streams. */ -void grpc_transport_close(grpc_transport* transport); - /* Destroy the transport */ void grpc_transport_destroy(grpc_exec_ctx* exec_ctx, grpc_transport* transport); -- cgit v1.2.3 From faeabfff5130a75f6f457384fcf6f6f1e092a611 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 9 Nov 2017 10:47:35 -0800 Subject: Use notify_on_receive_settings to enforce server handshake deadline. --- .../ext/transport/chttp2/server/chttp2_server.cc | 74 +++++++++++++++++++--- 1 file changed, 64 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.cc b/src/core/ext/transport/chttp2/server/chttp2_server.cc index 39e8dfd684..b28224f6bb 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.cc +++ b/src/core/ext/transport/chttp2/server/chttp2_server.cc @@ -32,6 +32,7 @@ #include "src/core/ext/filters/http/server/http_server_filter.h" #include "src/core/ext/transport/chttp2/transport/chttp2_transport.h" +#include "src/core/ext/transport/chttp2/transport/internal.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/handshaker.h" #include "src/core/lib/channel/handshaker_registry.h" @@ -54,12 +55,50 @@ typedef struct { } server_state; typedef struct { + gpr_refcount refs; server_state* svr_state; grpc_pollset* accepting_pollset; grpc_tcp_server_acceptor* acceptor; grpc_handshake_manager* handshake_mgr; + // State for enforcing handshake timeout on receiving HTTP/2 settings. + grpc_chttp2_transport* transport; + grpc_millis deadline; + grpc_timer timer; + grpc_closure on_timeout; + grpc_closure on_receive_settings; } server_connection_state; +static void server_connection_state_unref( + grpc_exec_ctx* exec_ctx, server_connection_state* connection_state) { + if (gpr_unref(&connection_state->refs)) { + if (connection_state->transport != nullptr) { + GRPC_CHTTP2_UNREF_TRANSPORT(exec_ctx, connection_state->transport, + "receive settings timeout"); + } + gpr_free(connection_state); + } +} + +static void on_timeout(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) { + server_connection_state* connection_state = (server_connection_state*)arg; + if (error == GRPC_ERROR_NONE) { + grpc_transport_op* op = grpc_make_transport_op(nullptr); + op->disconnect_with_error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "Did not receive HTTP/2 settings before handshake timeout"); + grpc_transport_perform_op(exec_ctx, &connection_state->transport->base, op); + } + server_connection_state_unref(exec_ctx, connection_state); +} + +static void on_receive_settings(grpc_exec_ctx* exec_ctx, void* arg, + grpc_error* error) { + server_connection_state* connection_state = (server_connection_state*)arg; + if (error == GRPC_ERROR_NONE) { + grpc_timer_cancel(exec_ctx, &connection_state->timer); + } + server_connection_state_unref(exec_ctx, connection_state); +} + static void on_handshake_done(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) { grpc_handshaker_args* args = (grpc_handshaker_args*)arg; @@ -69,7 +108,6 @@ static void on_handshake_done(grpc_exec_ctx* exec_ctx, void* arg, if (error != GRPC_ERROR_NONE || connection_state->svr_state->shutdown) { const char* error_str = grpc_error_string(error); gpr_log(GPR_DEBUG, "Handshaking failed: %s", error_str); - if (error == GRPC_ERROR_NONE && args->endpoint != NULL) { // We were shut down after handshaking completed successfully, so // destroy the endpoint here. @@ -93,11 +131,25 @@ static void on_handshake_done(grpc_exec_ctx* exec_ctx, void* arg, grpc_server_setup_transport( exec_ctx, connection_state->svr_state->server, transport, connection_state->accepting_pollset, args->args); -// FIXME: set notify_on_receive_settings callback and use it to enforce -// handshaking deadline - grpc_chttp2_transport_start_reading(exec_ctx, transport, - args->read_buffer, nullptr); + // Use notify_on_receive_settings callback to enforce the + // handshake deadline. + connection_state->transport = (grpc_chttp2_transport*)transport; + gpr_ref(&connection_state->refs); + GRPC_CLOSURE_INIT(&connection_state->on_receive_settings, + on_receive_settings, connection_state, + grpc_schedule_on_exec_ctx); + grpc_chttp2_transport_start_reading( + exec_ctx, transport, args->read_buffer, + &connection_state->on_receive_settings); grpc_channel_args_destroy(exec_ctx, args->args); + gpr_ref(&connection_state->refs); + GRPC_CHTTP2_REF_TRANSPORT((grpc_chttp2_transport*)transport, + "receive settings timeout"); + GRPC_CLOSURE_INIT(&connection_state->on_timeout, on_timeout, + connection_state, grpc_schedule_on_exec_ctx); + grpc_timer_init(exec_ctx, &connection_state->timer, + connection_state->deadline, + &connection_state->on_timeout); } } grpc_handshake_manager_pending_list_remove( @@ -105,9 +157,9 @@ static void on_handshake_done(grpc_exec_ctx* exec_ctx, void* arg, connection_state->handshake_mgr); gpr_mu_unlock(&connection_state->svr_state->mu); grpc_handshake_manager_destroy(exec_ctx, connection_state->handshake_mgr); - grpc_tcp_server_unref(exec_ctx, connection_state->svr_state->tcp_server); gpr_free(connection_state->acceptor); - gpr_free(connection_state); + grpc_tcp_server_unref(exec_ctx, connection_state->svr_state->tcp_server); + server_connection_state_unref(exec_ctx, connection_state); } static void on_accept(grpc_exec_ctx* exec_ctx, void* arg, grpc_endpoint* tcp, @@ -128,7 +180,8 @@ static void on_accept(grpc_exec_ctx* exec_ctx, void* arg, grpc_endpoint* tcp, gpr_mu_unlock(&state->mu); grpc_tcp_server_ref(state->tcp_server); server_connection_state* connection_state = - (server_connection_state*)gpr_malloc(sizeof(*connection_state)); + (server_connection_state*)gpr_zalloc(sizeof(*connection_state)); + gpr_ref_init(&connection_state->refs, 1); connection_state->svr_state = state; connection_state->accepting_pollset = accepting_pollset; connection_state->acceptor = acceptor; @@ -137,12 +190,13 @@ static void on_accept(grpc_exec_ctx* exec_ctx, void* arg, grpc_endpoint* tcp, connection_state->handshake_mgr); const grpc_arg* timeout_arg = grpc_channel_args_find(state->args, GRPC_ARG_SERVER_HANDSHAKE_TIMEOUT_MS); - const grpc_millis deadline = + connection_state->deadline = grpc_exec_ctx_now(exec_ctx) + grpc_channel_arg_get_integer(timeout_arg, {120 * GPR_MS_PER_SEC, 1, INT_MAX}); grpc_handshake_manager_do_handshake(exec_ctx, connection_state->handshake_mgr, - tcp, state->args, deadline, acceptor, + tcp, state->args, + connection_state->deadline, acceptor, on_handshake_done, connection_state); } -- cgit v1.2.3 From aaad0c2e5a301d27a3e077e89325eb91cc24da22 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Mon, 27 Nov 2017 08:31:50 -0800 Subject: clang-format --- .../transport/chttp2/client/chttp2_connector.cc | 4 ++-- .../core/transport/chttp2/settings_timeout_test.cc | 26 +++++++++++----------- test/cpp/microbenchmarks/fullstack_fixtures.h | 12 +++++----- 3 files changed, 21 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.cc b/src/core/ext/transport/chttp2/client/chttp2_connector.cc index 78fa30be62..7b2bb7d2be 100644 --- a/src/core/ext/transport/chttp2/client/chttp2_connector.cc +++ b/src/core/ext/transport/chttp2/client/chttp2_connector.cc @@ -117,8 +117,8 @@ static void on_handshake_done(grpc_exec_ctx* exec_ctx, void* arg, } else { grpc_endpoint_delete_from_pollset_set(exec_ctx, args->endpoint, c->args.interested_parties); - c->result->transport = grpc_create_chttp2_transport( - exec_ctx, args->args, args->endpoint, true); + c->result->transport = grpc_create_chttp2_transport(exec_ctx, args->args, + args->endpoint, true); GPR_ASSERT(c->result->transport); // TODO(roth): We ideally want to wait until we receive HTTP/2 // settings from the server before we consider the connection diff --git a/test/core/transport/chttp2/settings_timeout_test.cc b/test/core/transport/chttp2/settings_timeout_test.cc index c40dad471b..aac8e15b31 100644 --- a/test/core/transport/chttp2/settings_timeout_test.cc +++ b/test/core/transport/chttp2/settings_timeout_test.cc @@ -67,7 +67,8 @@ class ServerThread { grpc_server_shutdown_and_notify(server_, shutdown_cq, nullptr); GPR_ASSERT(grpc_completion_queue_pluck(shutdown_cq, nullptr, grpc_timeout_seconds_to_deadline(1), - nullptr).type == GRPC_OP_COMPLETE); + nullptr) + .type == GRPC_OP_COMPLETE); grpc_completion_queue_destroy(shutdown_cq); grpc_server_destroy(server_); grpc_completion_queue_destroy(cq_); @@ -98,8 +99,8 @@ class Client { void Connect() { grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; grpc_resolved_addresses* server_addresses = nullptr; - grpc_error* error = grpc_blocking_resolve_address(server_address_, - "80", &server_addresses); + grpc_error* error = + grpc_blocking_resolve_address(server_address_, "80", &server_addresses); ASSERT_EQ(GRPC_ERROR_NONE, error) << grpc_error_string(error); ASSERT_GE(server_addresses->naddrs, 1UL); pollset_ = (grpc_pollset*)gpr_zalloc(grpc_pollset_size()); @@ -107,12 +108,12 @@ class Client { grpc_pollset_set* pollset_set = grpc_pollset_set_create(); grpc_pollset_set_add_pollset(&exec_ctx, pollset_set, pollset_); EventState state; - grpc_tcp_client_connect(&exec_ctx, state.closure(), &endpoint_, - pollset_set, nullptr /* channel_args */, - server_addresses->addrs, 1000); - ASSERT_TRUE(PollUntilDone(&exec_ctx, &state, - grpc_timespec_to_millis_round_up( - gpr_inf_future(GPR_CLOCK_MONOTONIC)))); + grpc_tcp_client_connect(&exec_ctx, state.closure(), &endpoint_, pollset_set, + nullptr /* channel_args */, server_addresses->addrs, + 1000); + ASSERT_TRUE(PollUntilDone( + &exec_ctx, &state, + grpc_timespec_to_millis_round_up(gpr_inf_future(GPR_CLOCK_MONOTONIC)))); ASSERT_EQ(GRPC_ERROR_NONE, state.error()); grpc_pollset_set_destroy(&exec_ctx, pollset_set); grpc_endpoint_add_to_pollset(&exec_ctx, endpoint_, pollset_); @@ -195,10 +196,9 @@ class Client { while (true) { grpc_pollset_worker* worker = nullptr; gpr_mu_lock(mu_); - GRPC_LOG_IF_ERROR( - "grpc_pollset_work", - grpc_pollset_work(exec_ctx, pollset_, &worker, - grpc_exec_ctx_now(exec_ctx) + 1000)); + GRPC_LOG_IF_ERROR("grpc_pollset_work", + grpc_pollset_work(exec_ctx, pollset_, &worker, + grpc_exec_ctx_now(exec_ctx) + 1000)); gpr_mu_unlock(mu_); if (state != nullptr && state->done()) return true; if (grpc_exec_ctx_now(exec_ctx) >= deadline) return false; diff --git a/test/cpp/microbenchmarks/fullstack_fixtures.h b/test/cpp/microbenchmarks/fullstack_fixtures.h index 9a91190049..7e20843875 100644 --- a/test/cpp/microbenchmarks/fullstack_fixtures.h +++ b/test/cpp/microbenchmarks/fullstack_fixtures.h @@ -186,8 +186,8 @@ class EndpointPairFixture : public BaseFixture { grpc_server_setup_transport(&exec_ctx, server_->c_server(), server_transport_, nullptr, server_args); - grpc_chttp2_transport_start_reading(&exec_ctx, server_transport_, - nullptr, nullptr); + grpc_chttp2_transport_start_reading(&exec_ctx, server_transport_, nullptr, + nullptr); } /* create channel */ @@ -197,14 +197,14 @@ class EndpointPairFixture : public BaseFixture { fixture_configuration.ApplyCommonChannelArguments(&args); grpc_channel_args c_args = args.c_channel_args(); - client_transport_ = grpc_create_chttp2_transport( - &exec_ctx, &c_args, endpoints.client, true); + client_transport_ = grpc_create_chttp2_transport(&exec_ctx, &c_args, + endpoints.client, true); GPR_ASSERT(client_transport_); grpc_channel* channel = grpc_channel_create(&exec_ctx, "target", &c_args, GRPC_CLIENT_DIRECT_CHANNEL, client_transport_); - grpc_chttp2_transport_start_reading(&exec_ctx, client_transport_, - nullptr, nullptr); + grpc_chttp2_transport_start_reading(&exec_ctx, client_transport_, nullptr, + nullptr); channel_ = CreateChannelInternal("", channel); } -- cgit v1.2.3 From dc8be882f7489d38c9fe1ea61cf70a349b4a9357 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Mon, 27 Nov 2017 10:16:55 -0800 Subject: Fix handling of grpc shutdown in timer callback. --- src/core/ext/transport/chttp2/server/chttp2_server.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.cc b/src/core/ext/transport/chttp2/server/chttp2_server.cc index 88d848095b..1f4517ac28 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.cc +++ b/src/core/ext/transport/chttp2/server/chttp2_server.cc @@ -81,7 +81,9 @@ static void server_connection_state_unref( static void on_timeout(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) { server_connection_state* connection_state = (server_connection_state*)arg; - if (error == GRPC_ERROR_NONE) { + // Note that we may be called with GRPC_ERROR_NONE when the timer fires + // or with an error indicating that the timer system is being shut down. + if (error != GRPC_ERROR_CANCELLED) { grpc_transport_op* op = grpc_make_transport_op(nullptr); op->disconnect_with_error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( "Did not receive HTTP/2 settings before handshake timeout"); -- cgit v1.2.3 From a92428bc85381bac5767688813fcaa9a6dbf586b Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Mon, 4 Dec 2017 08:45:22 -0800 Subject: Move chttp2 cleanup code into close_transport_locked(). --- src/core/ext/transport/chttp2/transport/chttp2_transport.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 607fbf306a..3a2c4b6d1b 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -651,6 +651,11 @@ static void close_transport_locked(grpc_exec_ctx* exec_ctx, GPR_ASSERT(t->write_state == GRPC_CHTTP2_WRITE_STATE_IDLE); grpc_endpoint_shutdown(exec_ctx, t->ep, GRPC_ERROR_REF(error)); } + if (t->notify_on_receive_settings != nullptr) { + GRPC_CLOSURE_SCHED(exec_ctx, t->notify_on_receive_settings, + GRPC_ERROR_CANCELLED); + t->notify_on_receive_settings = nullptr; + } GRPC_ERROR_UNREF(error); } @@ -1823,11 +1828,6 @@ static void perform_transport_op_locked(grpc_exec_ctx* exec_ctx, if (op->disconnect_with_error != GRPC_ERROR_NONE) { close_transport_locked(exec_ctx, t, op->disconnect_with_error); - if (t->notify_on_receive_settings != nullptr) { - GRPC_CLOSURE_SCHED(exec_ctx, t->notify_on_receive_settings, - GRPC_ERROR_CANCELLED); - t->notify_on_receive_settings = nullptr; - } } GRPC_CLOSURE_RUN(exec_ctx, op->on_consumed, GRPC_ERROR_NONE); -- cgit v1.2.3