diff --git a/src/core/ext/filters/client_channel/connector.h b/src/core/ext/filters/client_channel/connector.h index f7adb46995a..c1c60925141 100644 --- a/src/core/ext/filters/client_channel/connector.h +++ b/src/core/ext/filters/client_channel/connector.h @@ -28,6 +28,7 @@ #include "src/core/lib/iomgr/error.h" #include "src/core/lib/iomgr/iomgr_fwd.h" #include "src/core/lib/iomgr/resolved_address.h" +#include "src/core/lib/transport/transport.h" #include "src/core/lib/transport/transport_fwd.h" namespace grpc_core { @@ -57,7 +58,10 @@ class SubchannelConnector : public InternallyRefCounted { RefCountedPtr socket_node; void Reset() { - transport = nullptr; + if (transport != nullptr) { + grpc_transport_destroy(transport); + transport = nullptr; + } channel_args = ChannelArgs(); socket_node.reset(); } diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 77f97309486..83cfd37fe87 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -879,6 +879,7 @@ void Subchannel::OnConnectingFinished(void* arg, grpc_error_handle error) { void Subchannel::OnConnectingFinishedLocked(grpc_error_handle error) { if (shutdown_) { + connecting_result_.Reset(); return; } // If we didn't get a transport or we fail to publish it, report @@ -926,12 +927,15 @@ bool Subchannel::PublishTransportLocked() { absl::StatusOr> stk = builder.Build(); if (!stk.ok()) { auto error = absl_status_to_grpc_error(stk.status()); - grpc_transport_destroy(connecting_result_.transport); + connecting_result_.Reset(); gpr_log(GPR_ERROR, "subchannel %p %s: error initializing subchannel stack: %s", this, key_.ToString().c_str(), StatusToString(error).c_str()); return false; } + // Release the ownership since it is now owned by the connected filter in the + // channel stack (published). + connecting_result_.transport = nullptr; RefCountedPtr socket = std::move(connecting_result_.socket_node); connecting_result_.Reset(); diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.cc b/src/core/ext/transport/chttp2/client/chttp2_connector.cc index fa4c16a46c3..d5fb27a9f6e 100644 --- a/src/core/ext/transport/chttp2/client/chttp2_connector.cc +++ b/src/core/ext/transport/chttp2/client/chttp2_connector.cc @@ -167,10 +167,10 @@ void Chttp2Connector::OnHandshakeDone(void* arg, grpc_error_handle error) { } else if (args->endpoint != nullptr) { self->result_->transport = grpc_create_chttp2_transport(args->args, args->endpoint, true); + GPR_ASSERT(self->result_->transport != nullptr); self->result_->socket_node = grpc_chttp2_transport_get_socket_node(self->result_->transport); self->result_->channel_args = args->args; - GPR_ASSERT(self->result_->transport != nullptr); self->endpoint_ = args->endpoint; self->Ref().release(); // Ref held by OnReceiveSettings() GRPC_CLOSURE_INIT(&self->on_receive_settings_, OnReceiveSettings, self, @@ -206,9 +206,6 @@ void Chttp2Connector::OnReceiveSettings(void* arg, grpc_error_handle error) { self->args_.interested_parties); if (!error.ok()) { // Transport got an error while waiting on SETTINGS frame. - // TODO(yashykt): The following two lines should be moved to - // SubchannelConnector::Result::Reset() - grpc_transport_destroy(self->result_->transport); self->result_->Reset(); } self->MaybeNotify(error); @@ -236,9 +233,6 @@ void Chttp2Connector::OnTimeout() { // The transport did not receive the settings frame in time. Destroy the // transport. grpc_endpoint_delete_from_pollset_set(endpoint_, args_.interested_parties); - // TODO(yashykt): The following two lines should be moved to - // SubchannelConnector::Result::Reset() - grpc_transport_destroy(result_->transport); result_->Reset(); MaybeNotify(GRPC_ERROR_CREATE( "connection attempt timed out before receiving SETTINGS frame")); diff --git a/test/core/end2end/fuzzers/api_fuzzer_corpus/testcase-6256752625319936 b/test/core/end2end/fuzzers/api_fuzzer_corpus/testcase-6256752625319936 new file mode 100644 index 00000000000..d666cbec3ba --- /dev/null +++ b/test/core/end2end/fuzzers/api_fuzzer_corpus/testcase-6256752625319936 @@ -0,0 +1,86 @@ +actions { + create_server { + channel_args { + key: "grpc.resource_quota" + str: "unix::67440737046,,Sac,,t49,Sa\n\000c,,t" + } + } +} +actions { + create_channel { + target: "unix::67440737046,,Sac,,t49,Sa\n\000c,,t" + channel_args { + key: "M" + } + channel_args { + key: "grpc.resource_quota" + resource_quota { + } + } + channel_actions { + wait_ms: 4096 + } + channel_actions { + add_n_bytes_writable: 1024 + add_n_bytes_readable: 1 + } + channel_actions { + wait_ms: 1 + } + } +} +actions { + create_call { + method { + value: "2" + } + host { + value: "1" + } + timeout: 1000000127 + } +} +actions { + resize_resource_quota: 3881880 +} +actions { + queue_batch { + operations { + send_initial_metadata { + metadata { + key { + value: "2" + } + } + } + } + } +} +actions { + resize_resource_quota: 4294967294 +} +actions { +} +actions { +} +actions { + resize_resource_quota: 3881880 +} +actions { + request_call { + } +} +actions { +} +actions { + resize_resource_quota: 3881880 +} +actions { +} +event_engine_actions { + tick_lengths { + } + tick_lengths { + delay_us: 5692549928996306944 + } +}