diff --git a/src/core/ext/filters/client_channel/client_channel_channelz.cc b/src/core/ext/filters/client_channel/client_channel_channelz.cc index e9534f22071..08ceb2dd05a 100644 --- a/src/core/ext/filters/client_channel/client_channel_channelz.cc +++ b/src/core/ext/filters/client_channel/client_channel_channelz.cc @@ -26,14 +26,15 @@ namespace grpc_core { namespace channelz { +namespace { -static void* client_channel_channelz_copy(void* p) { return p; } +void* client_channel_channelz_copy(void* p) { return p; } -static void client_channel_channelz_destroy(void* p) {} +void client_channel_channelz_destroy(void* p) {} -static int client_channel_channelz_cmp(void* a, void* b) { - return GPR_ICMP(a, b); -} +int client_channel_channelz_cmp(void* a, void* b) { return GPR_ICMP(a, b); } + +} // namespace static const grpc_arg_pointer_vtable client_channel_channelz_vtable = { client_channel_channelz_copy, client_channel_channelz_destroy, @@ -62,17 +63,17 @@ void ClientChannelNode::PopulateConnectivityState(grpc_json* json) { false); } -grpc_arg ClientChannelNode::CreateArg() { +grpc_arg ClientChannelNode::CreateChannelArg() { return grpc_channel_arg_pointer_create( const_cast(GRPC_ARG_CHANNELZ_CHANNEL_NODE_CREATION_FUNC), reinterpret_cast(MakeClientChannelNode), &client_channel_channelz_vtable); } -RefCountedPtr MakeClientChannelNode( +RefCountedPtr ClientChannelNode::MakeClientChannelNode( grpc_channel* channel, size_t channel_tracer_max_nodes) { - return RefCountedPtr( - New(channel, channel_tracer_max_nodes)); + return MakePolymorphicRefCounted( + channel, channel_tracer_max_nodes); } } // namespace channelz diff --git a/src/core/ext/filters/client_channel/client_channel_channelz.h b/src/core/ext/filters/client_channel/client_channel_channelz.h index a3f8b07fd6a..cf3ef7b6f28 100644 --- a/src/core/ext/filters/client_channel/client_channel_channelz.h +++ b/src/core/ext/filters/client_channel/client_channel_channelz.h @@ -31,8 +31,8 @@ namespace channelz { // functionality like querying for connectivity_state and subchannel data. class ClientChannelNode : public ChannelNode { public: - ClientChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes); - virtual ~ClientChannelNode() {} + static RefCountedPtr MakeClientChannelNode( + grpc_channel* channel, size_t channel_tracer_max_nodes); // Override this functionality since client_channels have a notion of // channel connectivity. @@ -40,17 +40,18 @@ class ClientChannelNode : public ChannelNode { // Helper to create a channel arg to ensure this type of ChannelNode is // created. - static grpc_arg CreateArg(); + static grpc_arg CreateChannelArg(); + + protected: + GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE + GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW + ClientChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes); + virtual ~ClientChannelNode() {} private: grpc_channel_element* client_channel_; }; -RefCountedPtr MakeClientChannelNode( - grpc_channel* channel, size_t channel_tracer_max_nodes); - -grpc_arg BlahBlah(); - } // namespace channelz } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/client_channel_plugin.cc b/src/core/ext/filters/client_channel/client_channel_plugin.cc index 8385852d1b1..e0784b7e5c1 100644 --- a/src/core/ext/filters/client_channel/client_channel_plugin.cc +++ b/src/core/ext/filters/client_channel/client_channel_plugin.cc @@ -25,6 +25,7 @@ #include #include "src/core/ext/filters/client_channel/client_channel.h" +#include "src/core/ext/filters/client_channel/client_channel_channelz.h" #include "src/core/ext/filters/client_channel/http_connect_handshaker.h" #include "src/core/ext/filters/client_channel/http_proxy.h" #include "src/core/ext/filters/client_channel/lb_policy_registry.h" @@ -35,6 +36,14 @@ #include "src/core/lib/surface/channel_init.h" static bool append_filter(grpc_channel_stack_builder* builder, void* arg) { + const grpc_channel_args* args = + grpc_channel_stack_builder_get_channel_arguments(builder); + grpc_arg args_to_add[] = { + grpc_core::channelz::ClientChannelNode::CreateChannelArg()}; + grpc_channel_args* new_args = grpc_channel_args_copy_and_add( + args, args_to_add, GPR_ARRAY_SIZE(args_to_add)); + grpc_channel_stack_builder_set_channel_arguments(builder, new_args); + grpc_channel_args_destroy(new_args); return grpc_channel_stack_builder_append_filter( builder, static_cast(arg), nullptr, nullptr); } 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 420629f6b79..e6c8c382607 100644 --- a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc +++ b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc @@ -26,7 +26,6 @@ #include #include "src/core/ext/filters/client_channel/client_channel.h" -#include "src/core/ext/filters/client_channel/client_channel_channelz.h" #include "src/core/ext/filters/client_channel/resolver_registry.h" #include "src/core/ext/transport/chttp2/client/authority.h" #include "src/core/ext/transport/chttp2/client/chttp2_connector.h" @@ -93,11 +92,10 @@ grpc_channel* grpc_insecure_channel_create(const char* target, "grpc_insecure_channel_create(target=%s, args=%p, reserved=%p)", 3, (target, args, reserved)); GPR_ASSERT(reserved == nullptr); - grpc_arg args_to_add[] = { - grpc_client_channel_factory_create_channel_arg(&client_channel_factory), - grpc_core::channelz::ClientChannelNode::CreateArg()}; - grpc_channel_args* new_args = grpc_channel_args_copy_and_add( - args, args_to_add, GPR_ARRAY_SIZE(args_to_add)); + // Add channel arg containing the client channel factory. + grpc_arg arg = + grpc_client_channel_factory_create_channel_arg(&client_channel_factory); + grpc_channel_args* new_args = grpc_channel_args_copy_and_add(args, &arg, 1); // Create channel. grpc_channel* channel = client_channel_factory_create_channel( &client_channel_factory, target, GRPC_CLIENT_CHANNEL_TYPE_REGULAR, 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 82691d4e251..5ce73a95d76 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 @@ -26,7 +26,6 @@ #include #include "src/core/ext/filters/client_channel/client_channel.h" -#include "src/core/ext/filters/client_channel/client_channel_channelz.h" #include "src/core/ext/filters/client_channel/resolver_registry.h" #include "src/core/ext/filters/client_channel/uri_parser.h" #include "src/core/ext/transport/chttp2/client/chttp2_connector.h" @@ -214,8 +213,7 @@ grpc_channel* grpc_secure_channel_create(grpc_channel_credentials* creds, // credentials. grpc_arg args_to_add[] = { grpc_client_channel_factory_create_channel_arg(&client_channel_factory), - grpc_channel_credentials_to_arg(creds), - grpc_core::channelz::ClientChannelNode::CreateArg()}; + grpc_channel_credentials_to_arg(creds)}; grpc_channel_args* new_args = grpc_channel_args_copy_and_add( args, args_to_add, GPR_ARRAY_SIZE(args_to_add)); // Create channel. diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index 0f13551c2a0..2074cb0cc53 100644 --- a/src/core/lib/channel/channelz.cc +++ b/src/core/lib/channel/channelz.cc @@ -129,7 +129,6 @@ char* ChannelNode::RenderJSON() { GRPC_JSON_OBJECT, false); json = data; json_iterator = nullptr; - PopulateConnectivityState(json); json_iterator = grpc_json_create_child( json_iterator, json, "target", target_.get(), GRPC_JSON_STRING, false); @@ -166,8 +165,8 @@ char* ChannelNode::RenderJSON() { return json_str; } -RefCountedPtr MakeChannelNode(grpc_channel* channel, - size_t channel_tracer_max_nodes) { +RefCountedPtr ChannelNode::MakeChannelNode( + grpc_channel* channel, size_t channel_tracer_max_nodes) { return MakeRefCounted( channel, channel_tracer_max_nodes); } diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h index e14e1b73cb7..9bd01ece502 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -44,8 +44,8 @@ class ChannelNodePeer; class ChannelNode : public RefCounted { public: - ChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes); - virtual ~ChannelNode(); + static RefCountedPtr MakeChannelNode( + grpc_channel* channel, size_t channel_tracer_max_nodes); void RecordCallStarted(); void RecordCallFailed() { @@ -73,6 +73,12 @@ class ChannelNode : public RefCounted { intptr_t channel_uuid() { return channel_uuid_; } + protected: + GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE + GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW + ChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes); + virtual ~ChannelNode(); + private: // testing peer friend. friend class testing::ChannelNodePeer; @@ -92,9 +98,6 @@ class ChannelNode : public RefCounted { typedef RefCountedPtr (*ChannelNodeCreationFunc)(grpc_channel*, size_t); -RefCountedPtr MakeChannelNode(grpc_channel* channel, - size_t channel_tracer_max_nodes); - } // namespace channelz } // namespace grpc_core diff --git a/src/core/lib/gprpp/memory.h b/src/core/lib/gprpp/memory.h index 28fcdf1779e..e90bedcd9b4 100644 --- a/src/core/lib/gprpp/memory.h +++ b/src/core/lib/gprpp/memory.h @@ -31,12 +31,12 @@ // protected destructor. #define GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE \ template \ - friend void Delete(T*); + friend void grpc_core::Delete(T*); // Add this to a class that want to use New(), but has a private or // protected constructor. #define GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW \ template \ - friend T* New(Args&&...); + friend T* grpc_core::New(Args&&...); namespace grpc_core { diff --git a/src/core/lib/gprpp/ref_counted_ptr.h b/src/core/lib/gprpp/ref_counted_ptr.h index 388e2ec4107..534d3d03cb4 100644 --- a/src/core/lib/gprpp/ref_counted_ptr.h +++ b/src/core/lib/gprpp/ref_counted_ptr.h @@ -107,6 +107,11 @@ inline RefCountedPtr MakeRefCounted(Args&&... args) { return RefCountedPtr(New(std::forward(args)...)); } +template +inline RefCountedPtr MakePolymorphicRefCounted(Args&&... args) { + return RefCountedPtr(New(std::forward(args)...)); +} + } // namespace grpc_core #endif /* GRPC_CORE_LIB_GPRPP_REF_COUNTED_PTR_H */ diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index 71bd24ce952..3e4e434a63b 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -108,7 +108,7 @@ grpc_channel* grpc_channel_create_with_builder( // this creates the default ChannelNode. Different types of channels may // override this to ensure a correct ChannelNode is created. grpc_core::channelz::ChannelNodeCreationFunc channel_node_create_func = - grpc_core::channelz::MakeChannelNode; + grpc_core::channelz::ChannelNode::MakeChannelNode; gpr_mu_init(&channel->registered_call_mu); channel->registered_calls = nullptr;