From dbade1f5e7692cad1f1bbdfbb1868d6f02051cb0 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Tue, 28 Sep 2021 09:54:07 -0700 Subject: [PATCH] Revert "Revert "pass subchannel address separately instead of as a channel arg (#27443)" (#27489)" (#27491) This reverts commit 20cc6e7414bc8a7ea5c4b1166ea8376a4dd0bb11. --- .../filters/client_channel/client_channel.cc | 5 +- .../client_channel/client_channel_factory.h | 36 +++-- .../ext/filters/client_channel/connector.h | 36 ++--- .../client_channel/global_subchannel_pool.h | 1 - .../ext/filters/client_channel/subchannel.cc | 134 ++++++------------ .../ext/filters/client_channel/subchannel.h | 65 +++------ .../subchannel_pool_interface.cc | 19 ++- .../subchannel_pool_interface.h | 10 +- .../chttp2/client/chttp2_connector.cc | 7 +- .../chttp2/client/insecure/channel_create.cc | 37 +++-- .../client/secure/secure_channel_create.cc | 37 +++-- src/core/lib/address_utils/parse_address.cc | 2 + test/core/end2end/goaway_server_test.cc | 1 + test/cpp/microbenchmarks/bm_call_create.cc | 1 + 14 files changed, 164 insertions(+), 227 deletions(-) diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index c12a1d98128..4ffd76a5d00 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -954,7 +954,6 @@ class ClientChannel::ClientChannelControlHelper }; // Add channel args needed for the subchannel. absl::InlinedVector args_to_add = { - Subchannel::CreateSubchannelAddressArg(&address.address()), SubchannelPoolInterface::CreateChannelArg( chand_->subchannel_pool_.get()), }; @@ -966,10 +965,10 @@ class ClientChannel::ClientChannelControlHelper grpc_channel_args* new_args = grpc_channel_args_copy_and_add_and_remove( &args, args_to_remove, GPR_ARRAY_SIZE(args_to_remove), args_to_add.data(), args_to_add.size()); - gpr_free(args_to_add[0].value.string); // Create subchannel. RefCountedPtr subchannel = - chand_->client_channel_factory_->CreateSubchannel(new_args); + chand_->client_channel_factory_->CreateSubchannel(address.address(), + new_args); grpc_channel_args_destroy(new_args); if (subchannel == nullptr) return nullptr; // Make sure the subchannel has updated keepalive time. diff --git a/src/core/ext/filters/client_channel/client_channel_factory.h b/src/core/ext/filters/client_channel/client_channel_factory.h index 75d74d676b2..0e00edc5afe 100644 --- a/src/core/ext/filters/client_channel/client_channel_factory.h +++ b/src/core/ext/filters/client_channel/client_channel_factory.h @@ -1,20 +1,18 @@ -/* - * - * Copyright 2015 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ +// +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// #ifndef GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CLIENT_CHANNEL_FACTORY_H #define GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CLIENT_CHANNEL_FACTORY_H @@ -33,7 +31,7 @@ class ClientChannelFactory { // Creates a subchannel with the specified args. virtual RefCountedPtr CreateSubchannel( - const grpc_channel_args* args) = 0; + const grpc_resolved_address& address, const grpc_channel_args* args) = 0; // Returns a channel arg containing the specified factory. static grpc_arg CreateChannelArg(ClientChannelFactory* factory); @@ -45,4 +43,4 @@ class ClientChannelFactory { } // namespace grpc_core -#endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CLIENT_CHANNEL_FACTORY_H */ +#endif // GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CLIENT_CHANNEL_FACTORY_H diff --git a/src/core/ext/filters/client_channel/connector.h b/src/core/ext/filters/client_channel/connector.h index 20e25b5bc19..3e193be4cd2 100644 --- a/src/core/ext/filters/client_channel/connector.h +++ b/src/core/ext/filters/client_channel/connector.h @@ -1,20 +1,18 @@ -/* - * - * Copyright 2015 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ +// +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// #ifndef GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CONNECTOR_H #define GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CONNECTOR_H @@ -35,6 +33,8 @@ namespace grpc_core { class SubchannelConnector : public InternallyRefCounted { public: struct Args { + // Address to connect to. + grpc_resolved_address* address; // Set of pollsets interested in this connection. grpc_pollset_set* interested_parties; // Deadline for connection. @@ -76,4 +76,4 @@ class SubchannelConnector : public InternallyRefCounted { } // namespace grpc_core -#endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CONNECTOR_H */ +#endif // GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CONNECTOR_H diff --git a/src/core/ext/filters/client_channel/global_subchannel_pool.h b/src/core/ext/filters/client_channel/global_subchannel_pool.h index 7ff9b070023..5b29a89a39d 100644 --- a/src/core/ext/filters/client_channel/global_subchannel_pool.h +++ b/src/core/ext/filters/client_channel/global_subchannel_pool.h @@ -32,7 +32,6 @@ namespace grpc_core { // should be only one instance of this class. Init() should be called once at // the filter initialization time; Shutdown() should be called once at the // filter shutdown time. -// TODO(juanlishen): Enable subchannel retention. class GlobalSubchannelPool final : public SubchannelPoolInterface { public: // The ctor and dtor are not intended to use directly. diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index a9e38dc5d87..7c37ba15d39 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -1,20 +1,18 @@ -/* - * - * Copyright 2015 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ +// +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// #include @@ -655,42 +653,39 @@ Subchannel::Subchannel(SubchannelKey key, GRPC_TRACE_FLAG_ENABLED(grpc_trace_subchannel_refcount) ? "Subchannel" : nullptr), key_(std::move(key)), + pollset_set_(grpc_pollset_set_create()), connector_(std::move(connector)), backoff_(ParseArgsForBackoffValues(args, &min_connect_timeout_ms_)) { GRPC_STATS_INC_CLIENT_SUBCHANNELS_CREATED(); - pollset_set_ = grpc_pollset_set_create(); - grpc_resolved_address* addr = - static_cast(gpr_malloc(sizeof(*addr))); - GetAddressFromSubchannelAddressArg(args, addr); + GRPC_CLOSURE_INIT(&on_connecting_finished_, OnConnectingFinished, this, + grpc_schedule_on_exec_ctx); + // Check proxy mapper to determine address to connect to and channel + // args to use. + address_for_connect_ = key_.address(); grpc_resolved_address* new_address = nullptr; grpc_channel_args* new_args = nullptr; - if (ProxyMapperRegistry::MapAddress(*addr, args, &new_address, &new_args)) { + if (ProxyMapperRegistry::MapAddress(address_for_connect_, args, &new_address, + &new_args)) { GPR_ASSERT(new_address != nullptr); - gpr_free(addr); - addr = new_address; + address_for_connect_ = *new_address; + gpr_free(new_address); } - static const char* keys_to_remove[] = {GRPC_ARG_SUBCHANNEL_ADDRESS}; - grpc_arg new_arg = CreateSubchannelAddressArg(addr); - gpr_free(addr); - args_ = grpc_channel_args_copy_and_add_and_remove( - new_args != nullptr ? new_args : args, keys_to_remove, - GPR_ARRAY_SIZE(keys_to_remove), &new_arg, 1); - gpr_free(new_arg.value.string); - if (new_args != nullptr) grpc_channel_args_destroy(new_args); - GRPC_CLOSURE_INIT(&on_connecting_finished_, OnConnectingFinished, this, - grpc_schedule_on_exec_ctx); - const grpc_arg* arg = grpc_channel_args_find(args_, GRPC_ARG_ENABLE_CHANNELZ); - const bool channelz_enabled = - grpc_channel_arg_get_bool(arg, GRPC_ENABLE_CHANNELZ_DEFAULT); - arg = grpc_channel_args_find( - args_, GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE); - const grpc_integer_options options = { - GRPC_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE_DEFAULT, 0, INT_MAX}; - size_t channel_tracer_max_memory = - static_cast(grpc_channel_arg_get_integer(arg, options)); + if (new_args != nullptr) { + args_ = new_args; + } else { + args_ = grpc_channel_args_copy(args); + } + // Initialize channelz. + const bool channelz_enabled = grpc_channel_args_find_bool( + args_, GRPC_ARG_ENABLE_CHANNELZ, GRPC_ENABLE_CHANNELZ_DEFAULT); if (channelz_enabled) { + const size_t channel_tracer_max_memory = + static_cast(grpc_channel_args_find_integer( + args_, GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE, + {GRPC_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE_DEFAULT, 0, + INT_MAX})); channelz_node_ = MakeRefCounted( - GetTargetAddress(), channel_tracer_max_memory); + grpc_sockaddr_to_uri(&key_.address()), channel_tracer_max_memory); channelz_node_->AddTraceEvent( channelz::ChannelTrace::Severity::Info, grpc_slice_from_static_string("subchannel created")); @@ -711,8 +706,8 @@ Subchannel::~Subchannel() { RefCountedPtr Subchannel::Create( OrphanablePtr connector, - const grpc_channel_args* args) { - SubchannelKey key(args); + const grpc_resolved_address& address, const grpc_channel_args* args) { + SubchannelKey key(address, args); SubchannelPoolInterface* subchannel_pool = SubchannelPoolInterface::GetSubchannelPoolFromChannelArgs(args); GPR_ASSERT(subchannel_pool != nullptr); @@ -750,14 +745,6 @@ void Subchannel::ThrottleKeepaliveTime(int new_keepalive_time) { } } -const char* Subchannel::GetTargetAddress() { - const grpc_arg* addr_arg = - grpc_channel_args_find(args_, GRPC_ARG_SUBCHANNEL_ADDRESS); - const char* addr_str = grpc_channel_arg_get_string(addr_arg); - GPR_ASSERT(addr_str != nullptr); // Should have been set by LB policy. - return addr_str; -} - channelz::SubchannelNode* Subchannel::channelz_node() { return channelz_node_.get(); } @@ -848,44 +835,6 @@ void Subchannel::Orphan() { health_watcher_map_.ShutdownLocked(); } -grpc_arg Subchannel::CreateSubchannelAddressArg( - const grpc_resolved_address* addr) { - return grpc_channel_arg_string_create( - const_cast(GRPC_ARG_SUBCHANNEL_ADDRESS), - gpr_strdup(addr->len > 0 ? grpc_sockaddr_to_uri(addr).c_str() : "")); -} - -const char* Subchannel::GetUriFromSubchannelAddressArg( - const grpc_channel_args* args) { - const grpc_arg* addr_arg = - grpc_channel_args_find(args, GRPC_ARG_SUBCHANNEL_ADDRESS); - const char* addr_str = grpc_channel_arg_get_string(addr_arg); - GPR_ASSERT(addr_str != nullptr); // Should have been set by LB policy. - return addr_str; -} - -namespace { - -void UriToSockaddr(const char* uri_str, grpc_resolved_address* addr) { - absl::StatusOr uri = URI::Parse(uri_str); - if (!uri.ok()) { - gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); - GPR_ASSERT(uri.ok()); - } - if (!grpc_parse_uri(*uri, addr)) memset(addr, 0, sizeof(*addr)); -} - -} // namespace - -void Subchannel::GetAddressFromSubchannelAddressArg( - const grpc_channel_args* args, grpc_resolved_address* addr) { - const char* addr_uri_str = GetUriFromSubchannelAddressArg(args); - memset(addr, 0, sizeof(*addr)); - if (*addr_uri_str != '\0') { - UriToSockaddr(addr_uri_str, addr); - } -} - namespace { // Returns a string indicating the subchannel's connectivity state change to @@ -988,6 +937,7 @@ void Subchannel::OnRetryAlarm(void* arg, grpc_error_handle error) { void Subchannel::ContinueConnectingLocked() { SubchannelConnector::Args args; + args.address = &address_for_connect_; args.interested_parties = pollset_set_; const grpc_millis min_deadline = min_connect_timeout_ms_ + ExecCtx::Get()->Now(); diff --git a/src/core/ext/filters/client_channel/subchannel.h b/src/core/ext/filters/client_channel/subchannel.h index 9667fe7fd72..60b8261e27e 100644 --- a/src/core/ext/filters/client_channel/subchannel.h +++ b/src/core/ext/filters/client_channel/subchannel.h @@ -1,20 +1,18 @@ -/* - * - * Copyright 2015 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ +// +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// #ifndef GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_SUBCHANNEL_H #define GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_SUBCHANNEL_H @@ -39,9 +37,6 @@ #include "src/core/lib/transport/connectivity_state.h" #include "src/core/lib/transport/metadata.h" -// Channel arg containing a URI indicating the address to connect to. -#define GRPC_ARG_SUBCHANNEL_ADDRESS "grpc.subchannel_address" - namespace grpc_core { class SubchannelCall; @@ -190,10 +185,10 @@ class Subchannel : public DualRefCounted { ABSL_GUARDED_BY(&mu_); }; - // Creates a subchannel given \a connector and \a args. + // Creates a subchannel. static RefCountedPtr Create( OrphanablePtr connector, - const grpc_channel_args* args); + const grpc_resolved_address& address, const grpc_channel_args* args); // The ctor and dtor are not intended to use directly. Subchannel(SubchannelKey key, OrphanablePtr connector, @@ -205,10 +200,6 @@ class Subchannel : public DualRefCounted { // will have an affect when the subchannel creates a new ConnectedSubchannel. void ThrottleKeepaliveTime(int new_keepalive_time) ABSL_LOCKS_EXCLUDED(mu_); - // Gets the string representing the subchannel address. - // Caller doesn't take ownership. - const char* GetTargetAddress(); - const grpc_channel_args* channel_args() const { return args_; } channelz::SubchannelNode* channelz_node(); @@ -247,27 +238,11 @@ class Subchannel : public DualRefCounted { void AttemptToConnect() ABSL_LOCKS_EXCLUDED(mu_); // Resets the connection backoff of the subchannel. - // TODO(roth): Move connection backoff out of subchannels and up into LB - // policy code (probably by adding a SubchannelGroup between - // SubchannelList and SubchannelData), at which point this method can - // go away. void ResetBackoff() ABSL_LOCKS_EXCLUDED(mu_); // Tears down any existing connection, and arranges for destruction void Orphan() override ABSL_LOCKS_EXCLUDED(mu_); - // Returns a new channel arg encoding the subchannel address as a URI - // string. Caller is responsible for freeing the string. - static grpc_arg CreateSubchannelAddressArg(const grpc_resolved_address* addr); - - // Returns the URI string from the subchannel address arg in \a args. - static const char* GetUriFromSubchannelAddressArg( - const grpc_channel_args* args); - - // Sets \a addr from the subchannel address arg in \a args. - static void GetAddressFromSubchannelAddressArg(const grpc_channel_args* args, - grpc_resolved_address* addr); - private: // A linked list of ConnectivityStateWatcherInterfaces that are monitoring // the subchannel's state. @@ -350,9 +325,11 @@ class Subchannel : public DualRefCounted { // The subchannel pool this subchannel is in. RefCountedPtr subchannel_pool_; - // TODO(juanlishen): Consider using args_ as key_ directly. // Subchannel key that identifies this subchannel in the subchannel pool. const SubchannelKey key_; + // Actual address to connect to. May be different than the address in + // key_ if overridden by proxy mapper. + grpc_resolved_address address_for_connect_; // Channel args. grpc_channel_args* args_; // pollset_set tracking who's interested in a connection being setup. @@ -399,4 +376,4 @@ class Subchannel : public DualRefCounted { } // namespace grpc_core -#endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_SUBCHANNEL_H */ +#endif // GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_SUBCHANNEL_H diff --git a/src/core/ext/filters/client_channel/subchannel_pool_interface.cc b/src/core/ext/filters/client_channel/subchannel_pool_interface.cc index 283b70d00a9..61b977dc236 100644 --- a/src/core/ext/filters/client_channel/subchannel_pool_interface.cc +++ b/src/core/ext/filters/client_channel/subchannel_pool_interface.cc @@ -31,8 +31,9 @@ namespace grpc_core { TraceFlag grpc_subchannel_pool_trace(false, "subchannel_pool"); -SubchannelKey::SubchannelKey(const grpc_channel_args* args) { - Init(args, grpc_channel_args_normalize); +SubchannelKey::SubchannelKey(const grpc_resolved_address& address, + const grpc_channel_args* args) { + Init(address, args, grpc_channel_args_normalize); } SubchannelKey::~SubchannelKey() { @@ -40,7 +41,7 @@ SubchannelKey::~SubchannelKey() { } SubchannelKey::SubchannelKey(const SubchannelKey& other) { - Init(other.args_, grpc_channel_args_copy); + Init(other.address_, other.args_, grpc_channel_args_copy); } SubchannelKey& SubchannelKey::operator=(const SubchannelKey& other) { @@ -48,28 +49,36 @@ SubchannelKey& SubchannelKey::operator=(const SubchannelKey& other) { return *this; } grpc_channel_args_destroy(const_cast(args_)); - Init(other.args_, grpc_channel_args_copy); + Init(other.address_, other.args_, grpc_channel_args_copy); return *this; } SubchannelKey::SubchannelKey(SubchannelKey&& other) noexcept { + address_ = other.address_; args_ = other.args_; other.args_ = nullptr; } SubchannelKey& SubchannelKey::operator=(SubchannelKey&& other) noexcept { + address_ = other.address_; args_ = other.args_; other.args_ = nullptr; return *this; } bool SubchannelKey::operator<(const SubchannelKey& other) const { + if (address_.len < other.address_.len) return true; + if (address_.len > other.address_.len) return false; + int r = memcmp(address_.addr, other.address_.addr, address_.len); + if (r < 0) return true; + if (r > 0) return false; return grpc_channel_args_compare(args_, other.args_) < 0; } void SubchannelKey::Init( - const grpc_channel_args* args, + const grpc_resolved_address& address, const grpc_channel_args* args, grpc_channel_args* (*copy_channel_args)(const grpc_channel_args* args)) { + address_ = address; args_ = copy_channel_args(args); } diff --git a/src/core/ext/filters/client_channel/subchannel_pool_interface.h b/src/core/ext/filters/client_channel/subchannel_pool_interface.h index 8a8a0cb6934..7042f15574d 100644 --- a/src/core/ext/filters/client_channel/subchannel_pool_interface.h +++ b/src/core/ext/filters/client_channel/subchannel_pool_interface.h @@ -25,6 +25,7 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/debug/trace.h" #include "src/core/lib/gprpp/ref_counted.h" +#include "src/core/lib/iomgr/resolve_address.h" namespace grpc_core { @@ -35,7 +36,8 @@ extern TraceFlag grpc_subchannel_pool_trace; // A key that can uniquely identify a subchannel. class SubchannelKey { public: - explicit SubchannelKey(const grpc_channel_args* args); + SubchannelKey(const grpc_resolved_address& address, + const grpc_channel_args* args); ~SubchannelKey(); // Copyable. @@ -47,13 +49,17 @@ class SubchannelKey { bool operator<(const SubchannelKey& other) const; + const grpc_resolved_address& address() const { return address_; } + const grpc_channel_args* args() const { return args_; } + private: // Initializes the subchannel key with the given \a args and the function to // copy channel args. void Init( - const grpc_channel_args* args, + const grpc_resolved_address& address, const grpc_channel_args* args, grpc_channel_args* (*copy_channel_args)(const grpc_channel_args* args)); + grpc_resolved_address address_; const grpc_channel_args* args_; }; diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.cc b/src/core/ext/transport/chttp2/client/chttp2_connector.cc index 96dd4eb7bf2..03e95a13c5c 100644 --- a/src/core/ext/transport/chttp2/client/chttp2_connector.cc +++ b/src/core/ext/transport/chttp2/client/chttp2_connector.cc @@ -28,7 +28,6 @@ #include #include "src/core/ext/filters/client_channel/connector.h" -#include "src/core/ext/filters/client_channel/subchannel.h" #include "src/core/ext/transport/chttp2/transport/chttp2_transport.h" #include "src/core/lib/address_utils/sockaddr_utils.h" #include "src/core/lib/channel/channel_args.h" @@ -54,8 +53,6 @@ Chttp2Connector::~Chttp2Connector() { void Chttp2Connector::Connect(const Args& args, Result* result, grpc_closure* notify) { - grpc_resolved_address addr; - Subchannel::GetAddressFromSubchannelAddressArg(args.channel_args, &addr); grpc_endpoint** ep; { MutexLock lock(&mu_); @@ -83,9 +80,9 @@ void Chttp2Connector::Connect(const Args& args, Result* result, grpc_tcp_client_connect( &connected_, ep, grpc_slice_allocator_create(resource_quota_, - grpc_sockaddr_to_string(&addr, false), + grpc_sockaddr_to_string(args.address, false), args.channel_args), - args.interested_parties, args.channel_args, &addr, args.deadline); + args.interested_parties, args.channel_args, args.address, args.deadline); } void Chttp2Connector::Shutdown(grpc_error_handle error) { diff --git a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc index 40b3f6de220..9680a74e7d3 100644 --- a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc +++ b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc @@ -1,20 +1,18 @@ -/* - * - * Copyright 2015 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ +// +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// #include @@ -37,11 +35,12 @@ namespace grpc_core { class Chttp2InsecureClientChannelFactory : public ClientChannelFactory { public: RefCountedPtr CreateSubchannel( + const grpc_resolved_address& address, const grpc_channel_args* args) override { grpc_channel_args* new_args = grpc_default_authority_add_if_not_present(args); - RefCountedPtr s = - Subchannel::Create(MakeOrphanable(), new_args); + RefCountedPtr s = Subchannel::Create( + MakeOrphanable(), address, new_args); grpc_channel_args_destroy(new_args); return s; } diff --git a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc index e6fccf66964..c76b6e96d37 100644 --- a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc +++ b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc @@ -1,20 +1,18 @@ -/* - * - * Copyright 2015 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ +// +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// #include @@ -43,6 +41,7 @@ namespace grpc_core { class Chttp2SecureClientChannelFactory : public ClientChannelFactory { public: RefCountedPtr CreateSubchannel( + const grpc_resolved_address& address, const grpc_channel_args* args) override { grpc_channel_args* new_args = GetSecureNamingChannelArgs(args); if (new_args == nullptr) { @@ -50,8 +49,8 @@ class Chttp2SecureClientChannelFactory : public ClientChannelFactory { "Failed to create channel args during subchannel creation."); return nullptr; } - RefCountedPtr s = - Subchannel::Create(MakeOrphanable(), new_args); + RefCountedPtr s = Subchannel::Create( + MakeOrphanable(), address, new_args); grpc_channel_args_destroy(new_args); return s; } diff --git a/src/core/lib/address_utils/parse_address.cc b/src/core/lib/address_utils/parse_address.cc index a621514414e..c254d2eb194 100644 --- a/src/core/lib/address_utils/parse_address.cc +++ b/src/core/lib/address_utils/parse_address.cc @@ -83,6 +83,7 @@ namespace grpc_core { grpc_error_handle UnixSockaddrPopulate(absl::string_view path, grpc_resolved_address* resolved_addr) { + memset(resolved_addr, 0, sizeof(*resolved_addr)); struct sockaddr_un* un = reinterpret_cast(resolved_addr->addr); const size_t maxlen = sizeof(un->sun_path) - 1; @@ -99,6 +100,7 @@ grpc_error_handle UnixSockaddrPopulate(absl::string_view path, grpc_error_handle UnixAbstractSockaddrPopulate( absl::string_view path, grpc_resolved_address* resolved_addr) { + memset(resolved_addr, 0, sizeof(*resolved_addr)); struct sockaddr_un* un = reinterpret_cast(resolved_addr->addr); const size_t maxlen = sizeof(un->sun_path) - 1; diff --git a/test/core/end2end/goaway_server_test.cc b/test/core/end2end/goaway_server_test.cc index e0aaef622f1..4d4509bc53a 100644 --- a/test/core/end2end/goaway_server_test.cc +++ b/test/core/end2end/goaway_server_test.cc @@ -123,6 +123,7 @@ static grpc_ares_request* my_dns_lookup_ares_locked( } else { *addresses = absl::make_unique(); grpc_sockaddr_in sa; + memset(&sa, 0, sizeof(sa)); sa.sin_family = GRPC_AF_INET; sa.sin_addr.s_addr = 0x100007f; sa.sin_port = grpc_htons(static_cast(g_resolve_port)); diff --git a/test/cpp/microbenchmarks/bm_call_create.cc b/test/cpp/microbenchmarks/bm_call_create.cc index 73d31678eae..ae27fd34d9b 100644 --- a/test/cpp/microbenchmarks/bm_call_create.cc +++ b/test/cpp/microbenchmarks/bm_call_create.cc @@ -324,6 +324,7 @@ static void DoNothing(void* /*arg*/, grpc_error_handle /*error*/) {} class FakeClientChannelFactory : public grpc_core::ClientChannelFactory { public: grpc_core::RefCountedPtr CreateSubchannel( + const grpc_resolved_address& /*address*/, const grpc_channel_args* /*args*/) override { return nullptr; }