From 7b6e07db0cc31a1597bf06128542d66d8b611d86 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Tue, 15 May 2018 11:19:37 -0700 Subject: [PATCH 1/2] Call on_handshake_done with an error when handshaker is shut down. --- src/core/lib/channel/handshaker.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/core/lib/channel/handshaker.cc b/src/core/lib/channel/handshaker.cc index 2faeb64cb60..f0b1fe7369b 100644 --- a/src/core/lib/channel/handshaker.cc +++ b/src/core/lib/channel/handshaker.cc @@ -220,8 +220,14 @@ static bool call_next_handshaker_locked(grpc_handshake_manager* mgr, // callback. Otherwise, call the next handshaker. if (error != GRPC_ERROR_NONE || mgr->shutdown || mgr->args.exit_early || mgr->index == mgr->count) { + if (error == GRPC_ERROR_NONE && mgr->shutdown) { + error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("handshaker shutdown"); + } if (grpc_handshaker_trace.enabled()) { - gpr_log(GPR_INFO, "handshake_manager %p: handshaking complete", mgr); + gpr_log(GPR_INFO, + "handshake_manager %p: handshaking complete -- scheduling " + "on_handshake_done with error=%s", + mgr, grpc_error_string(error)); } // Cancel deadline timer, since we're invoking the on_handshake_done // callback now. From 979888b32688f16020e7b05f1838a67408a1e53f Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 17 May 2018 09:47:09 -0700 Subject: [PATCH 2/2] Clean up endpoint, args, and read buffer upon handshake timeout. --- src/core/lib/channel/handshaker.cc | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/core/lib/channel/handshaker.cc b/src/core/lib/channel/handshaker.cc index f0b1fe7369b..86f8699e044 100644 --- a/src/core/lib/channel/handshaker.cc +++ b/src/core/lib/channel/handshaker.cc @@ -28,6 +28,7 @@ #include "src/core/lib/channel/handshaker.h" #include "src/core/lib/debug/trace.h" #include "src/core/lib/iomgr/timer.h" +#include "src/core/lib/slice/slice_internal.h" grpc_core::TraceFlag grpc_handshaker_trace(false, "handshaker"); @@ -222,6 +223,18 @@ static bool call_next_handshaker_locked(grpc_handshake_manager* mgr, mgr->index == mgr->count) { if (error == GRPC_ERROR_NONE && mgr->shutdown) { error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("handshaker shutdown"); + // TODO(roth): It is currently necessary to shutdown endpoints + // before destroying then, even when we know that there are no + // pending read/write callbacks. This should be fixed, at which + // point this can be removed. + grpc_endpoint_shutdown(mgr->args.endpoint, GRPC_ERROR_REF(error)); + grpc_endpoint_destroy(mgr->args.endpoint); + mgr->args.endpoint = nullptr; + grpc_channel_args_destroy(mgr->args.args); + mgr->args.args = nullptr; + grpc_slice_buffer_destroy_internal(mgr->args.read_buffer); + gpr_free(mgr->args.read_buffer); + mgr->args.read_buffer = nullptr; } if (grpc_handshaker_trace.enabled()) { gpr_log(GPR_INFO,