From 5e9404e41a06d102b41483f5523fcd8faa3e8e84 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Tue, 1 Oct 2019 09:34:05 -0700 Subject: [PATCH] Fix polymorphism for UniquePtr<> and OrphanablePtr<>. --- .../filters/client_channel/client_channel.cc | 9 ++--- .../client_channel/http_connect_handshaker.cc | 2 +- .../client_channel/lb_policy/grpclb/grpclb.cc | 10 +++--- .../lb_policy/pick_first/pick_first.cc | 31 ++++++++--------- .../lb_policy/round_robin/round_robin.cc | 17 +++++----- .../client_channel/lb_policy/xds/xds.cc | 18 +++++----- .../resolver/dns/c_ares/dns_resolver_ares.cc | 5 ++- .../dns/c_ares/grpc_ares_ev_driver_libuv.cc | 2 +- .../dns/c_ares/grpc_ares_ev_driver_posix.cc | 2 +- .../dns/c_ares/grpc_ares_ev_driver_windows.cc | 3 +- .../resolver/dns/native/dns_resolver.cc | 8 ++--- .../resolver/fake/fake_resolver.cc | 5 ++- .../resolver/sockaddr/sockaddr_resolver.cc | 13 +++----- .../resolver/xds/xds_resolver.cc | 5 ++- .../client_channel/resolver_result_parsing.cc | 17 ++++------ .../client_channel/resolving_lb_policy.cc | 9 +++-- .../ext/filters/client_channel/subchannel.cc | 3 +- .../message_size/message_size_filter.cc | 8 ++--- src/core/lib/gprpp/memory.h | 33 ++++++++----------- src/core/lib/gprpp/orphanable.h | 8 +++-- .../security/transport/security_handshaker.cc | 4 +-- .../session_cache/ssl_session_boringssl.cc | 3 +- .../ssl/session_cache/ssl_session_openssl.cc | 3 +- .../resolvers/dns_resolver_test.cc | 10 ++---- .../resolvers/sockaddr_resolver_test.cc | 8 ++--- .../client_channel/service_config_test.cc | 29 +++++++--------- .../readahead_handshaker_server_ssl.cc | 2 +- test/core/surface/lame_client_test.cc | 4 +-- .../core/transport/connectivity_state_test.cc | 12 +++---- test/core/util/test_lb_policies.cc | 5 ++- 30 files changed, 115 insertions(+), 173 deletions(-) diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 4768e46449e..af2425a6faa 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -1515,9 +1515,7 @@ void ChannelData::CreateResolvingLoadBalancingPolicyLocked() { // Instantiate resolving LB policy. LoadBalancingPolicy::Args lb_args; lb_args.combiner = combiner_; - lb_args.channel_control_helper = - UniquePtr( - New(this)); + lb_args.channel_control_helper = MakeUnique(this); lb_args.args = channel_args_; UniquePtr target_uri(gpr_strdup(target_uri_.get())); resolving_lb_policy_.reset(New( @@ -1791,9 +1789,8 @@ void ChannelData::StartTransportOpLocked(void* arg, grpc_error* ignored) { MemoryOrder::RELEASE); chand->UpdateStateAndPickerLocked( GRPC_CHANNEL_SHUTDOWN, "shutdown from API", - UniquePtr( - New( - GRPC_ERROR_REF(op->disconnect_with_error)))); + MakeUnique( + GRPC_ERROR_REF(op->disconnect_with_error))); } } GRPC_CHANNEL_STACK_UNREF(chand->owning_stack_, "start_transport_op"); diff --git a/src/core/ext/filters/client_channel/http_connect_handshaker.cc b/src/core/ext/filters/client_channel/http_connect_handshaker.cc index 95366b57386..660f6b85c00 100644 --- a/src/core/ext/filters/client_channel/http_connect_handshaker.cc +++ b/src/core/ext/filters/client_channel/http_connect_handshaker.cc @@ -356,5 +356,5 @@ void grpc_http_connect_register_handshaker_factory() { using namespace grpc_core; HandshakerRegistry::RegisterHandshakerFactory( true /* at_start */, HANDSHAKER_CLIENT, - UniquePtr(New())); + MakeUnique()); } diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index af7632c1728..20c6c1af601 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -716,9 +716,8 @@ void GrpcLb::Helper::UpdateState(grpc_connectivity_state state, client_stats = parent_->lb_calld_->client_stats()->Ref(); } parent_->channel_control_helper()->UpdateState( - state, UniquePtr( - New(parent_.get(), parent_->serverlist_, - std::move(picker), std::move(client_stats)))); + state, MakeUnique(parent_.get(), parent_->serverlist_, + std::move(picker), std::move(client_stats))); } void GrpcLb::Helper::RequestReresolution() { @@ -1794,7 +1793,7 @@ class GrpcLbFactory : public LoadBalancingPolicyFactory { public: OrphanablePtr CreateLoadBalancingPolicy( LoadBalancingPolicy::Args args) const override { - return OrphanablePtr(New(std::move(args))); + return MakeOrphanable(std::move(args)); } const char* name() const override { return kGrpclb; } @@ -1869,8 +1868,7 @@ bool maybe_add_client_load_reporting_filter(grpc_channel_stack_builder* builder, void grpc_lb_policy_grpclb_init() { grpc_core::LoadBalancingPolicyRegistry::Builder:: RegisterLoadBalancingPolicyFactory( - grpc_core::UniquePtr( - grpc_core::New())); + grpc_core::MakeUnique()); grpc_channel_init_register_stage(GRPC_CLIENT_SUBCHANNEL, GRPC_CHANNEL_INIT_BUILTIN_PRIORITY, maybe_add_client_load_reporting_filter, diff --git a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc index 000462092ac..1b14f96748c 100644 --- a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc +++ b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc @@ -201,7 +201,7 @@ void PickFirst::AttemptToConnectUsingLatestUpdateArgsLocked() { GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE); channel_control_helper()->UpdateState( GRPC_CHANNEL_TRANSIENT_FAILURE, - UniquePtr(New(error))); + MakeUnique(error)); return; } // If one of the subchannels in the new list is already in state @@ -319,12 +319,11 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked( GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE); p->channel_control_helper()->UpdateState( GRPC_CHANNEL_TRANSIENT_FAILURE, - UniquePtr(New(error))); + MakeUnique(error)); } else { p->channel_control_helper()->UpdateState( GRPC_CHANNEL_CONNECTING, - UniquePtr( - New(p->Ref(DEBUG_LOCATION, "QueuePicker")))); + MakeUnique(p->Ref(DEBUG_LOCATION, "QueuePicker"))); } } else { if (connectivity_state == GRPC_CHANNEL_TRANSIENT_FAILURE) { @@ -339,20 +338,19 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked( p->selected_ = nullptr; p->subchannel_list_.reset(); p->channel_control_helper()->UpdateState( - GRPC_CHANNEL_IDLE, UniquePtr(New( - p->Ref(DEBUG_LOCATION, "QueuePicker")))); + GRPC_CHANNEL_IDLE, + MakeUnique(p->Ref(DEBUG_LOCATION, "QueuePicker"))); } else { // This is unlikely but can happen when a subchannel has been asked // to reconnect by a different channel and this channel has dropped // some connectivity state notifications. if (connectivity_state == GRPC_CHANNEL_READY) { p->channel_control_helper()->UpdateState( - GRPC_CHANNEL_READY, - UniquePtr(New(subchannel()->Ref()))); + GRPC_CHANNEL_READY, MakeUnique(subchannel()->Ref())); } else { // CONNECTING p->channel_control_helper()->UpdateState( - connectivity_state, UniquePtr(New( - p->Ref(DEBUG_LOCATION, "QueuePicker")))); + connectivity_state, + MakeUnique(p->Ref(DEBUG_LOCATION, "QueuePicker"))); } } } @@ -396,7 +394,7 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked( GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE); p->channel_control_helper()->UpdateState( GRPC_CHANNEL_TRANSIENT_FAILURE, - UniquePtr(New(error))); + MakeUnique(error)); } } sd->CheckConnectivityStateAndStartWatchingLocked(); @@ -408,8 +406,7 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked( if (subchannel_list() == p->subchannel_list_.get()) { p->channel_control_helper()->UpdateState( GRPC_CHANNEL_CONNECTING, - UniquePtr( - New(p->Ref(DEBUG_LOCATION, "QueuePicker")))); + MakeUnique(p->Ref(DEBUG_LOCATION, "QueuePicker"))); } break; } @@ -448,8 +445,7 @@ void PickFirst::PickFirstSubchannelData::ProcessUnselectedReadyLocked() { } p->selected_ = this; p->channel_control_helper()->UpdateState( - GRPC_CHANNEL_READY, - UniquePtr(New(subchannel()->Ref()))); + GRPC_CHANNEL_READY, MakeUnique(subchannel()->Ref())); for (size_t i = 0; i < subchannel_list()->num_subchannels(); ++i) { if (i != Index()) { subchannel_list()->subchannel(i)->ShutdownLocked(); @@ -488,7 +484,7 @@ class PickFirstFactory : public LoadBalancingPolicyFactory { public: OrphanablePtr CreateLoadBalancingPolicy( LoadBalancingPolicy::Args args) const override { - return OrphanablePtr(New(std::move(args))); + return MakeOrphanable(std::move(args)); } const char* name() const override { return kPickFirst; } @@ -510,8 +506,7 @@ class PickFirstFactory : public LoadBalancingPolicyFactory { void grpc_lb_policy_pick_first_init() { grpc_core::LoadBalancingPolicyRegistry::Builder:: RegisterLoadBalancingPolicyFactory( - grpc_core::UniquePtr( - grpc_core::New())); + grpc_core::MakeUnique()); } void grpc_lb_policy_pick_first_shutdown() {} diff --git a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc index 69b3d6746e0..66fa66f4eff 100644 --- a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc +++ b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc @@ -321,13 +321,13 @@ void RoundRobin::RoundRobinSubchannelList:: */ if (num_ready_ > 0) { /* 1) READY */ - p->channel_control_helper()->UpdateState( - GRPC_CHANNEL_READY, UniquePtr(New(p, this))); + p->channel_control_helper()->UpdateState(GRPC_CHANNEL_READY, + MakeUnique(p, this)); } else if (num_connecting_ > 0) { /* 2) CONNECTING */ p->channel_control_helper()->UpdateState( - GRPC_CHANNEL_CONNECTING, UniquePtr(New( - p->Ref(DEBUG_LOCATION, "QueuePicker")))); + GRPC_CHANNEL_CONNECTING, + MakeUnique(p->Ref(DEBUG_LOCATION, "QueuePicker"))); } else if (num_transient_failure_ == num_subchannels()) { /* 3) TRANSIENT_FAILURE */ grpc_error* error = @@ -336,7 +336,7 @@ void RoundRobin::RoundRobinSubchannelList:: GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE); p->channel_control_helper()->UpdateState( GRPC_CHANNEL_TRANSIENT_FAILURE, - UniquePtr(New(error))); + MakeUnique(error)); } } @@ -453,7 +453,7 @@ void RoundRobin::UpdateLocked(UpdateArgs args) { GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE); channel_control_helper()->UpdateState( GRPC_CHANNEL_TRANSIENT_FAILURE, - UniquePtr(New(error))); + MakeUnique(error)); subchannel_list_ = std::move(latest_pending_subchannel_list_); } else if (subchannel_list_ == nullptr) { // If there is no current list, immediately promote the new list to @@ -480,7 +480,7 @@ class RoundRobinFactory : public LoadBalancingPolicyFactory { public: OrphanablePtr CreateLoadBalancingPolicy( LoadBalancingPolicy::Args args) const override { - return OrphanablePtr(New(std::move(args))); + return MakeOrphanable(std::move(args)); } const char* name() const override { return kRoundRobin; } @@ -502,8 +502,7 @@ class RoundRobinFactory : public LoadBalancingPolicyFactory { void grpc_lb_policy_round_robin_init() { grpc_core::LoadBalancingPolicyRegistry::Builder:: RegisterLoadBalancingPolicyFactory( - grpc_core::UniquePtr( - grpc_core::New())); + grpc_core::MakeUnique()); } void grpc_lb_policy_round_robin_shutdown() {} diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc index c5b9b3dc437..c148023bf5b 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc @@ -2179,7 +2179,7 @@ void XdsLb::PriorityList::UpdateXdsPickerLocked() { GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE); xds_policy_->channel_control_helper()->UpdateState( GRPC_CHANNEL_TRANSIENT_FAILURE, - UniquePtr(New(error))); + MakeUnique(error)); return; } priorities_[current_priority_]->UpdateXdsPickerLocked(); @@ -2272,9 +2272,8 @@ XdsLb::PriorityList::LocalityMap::LocalityMap(RefCountedPtr xds_policy, // This is the first locality map ever created, report CONNECTING. if (priority_ == 0) { xds_policy_->channel_control_helper()->UpdateState( - GRPC_CHANNEL_CONNECTING, - UniquePtr( - New(xds_policy_->Ref(DEBUG_LOCATION, "QueuePicker")))); + GRPC_CHANNEL_CONNECTING, MakeUnique(xds_policy_->Ref( + DEBUG_LOCATION, "QueuePicker"))); } } @@ -2347,9 +2346,9 @@ void XdsLb::PriorityList::LocalityMap::UpdateXdsPickerLocked() { picker_list.push_back(MakePair(end, locality->picker_wrapper())); } xds_policy()->channel_control_helper()->UpdateState( - GRPC_CHANNEL_READY, UniquePtr(New( - xds_policy_->Ref(DEBUG_LOCATION, "XdsLb+Picker"), - std::move(picker_list)))); + GRPC_CHANNEL_READY, + MakeUnique(xds_policy_->Ref(DEBUG_LOCATION, "XdsLb+Picker"), + std::move(picker_list))); } OrphanablePtr @@ -2898,7 +2897,7 @@ class XdsFactory : public LoadBalancingPolicyFactory { public: OrphanablePtr CreateLoadBalancingPolicy( LoadBalancingPolicy::Args args) const override { - return OrphanablePtr(New(std::move(args))); + return MakeOrphanable(std::move(args)); } const char* name() const override { return kXds; } @@ -2982,8 +2981,7 @@ class XdsFactory : public LoadBalancingPolicyFactory { void grpc_lb_policy_xds_init() { grpc_core::LoadBalancingPolicyRegistry::Builder:: RegisterLoadBalancingPolicyFactory( - grpc_core::UniquePtr( - grpc_core::New())); + grpc_core::MakeUnique()); } void grpc_lb_policy_xds_shutdown() {} diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc index 65f80525e54..8924fdf4172 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc @@ -436,7 +436,7 @@ class AresDnsResolverFactory : public ResolverFactory { bool IsValidUri(const grpc_uri* uri) const override { return true; } OrphanablePtr CreateResolver(ResolverArgs args) const override { - return OrphanablePtr(New(std::move(args))); + return MakeOrphanable(std::move(args)); } const char* scheme() const override { return "dns"; } @@ -494,8 +494,7 @@ void grpc_resolver_dns_ares_init() { } grpc_set_resolver_impl(&ares_resolver); grpc_core::ResolverRegistry::Builder::RegisterResolverFactory( - grpc_core::UniquePtr( - grpc_core::New())); + grpc_core::MakeUnique()); } else { g_use_ares_dns_resolver = false; } diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_libuv.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_libuv.cc index 04e36fbcee7..81d2f7becd4 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_libuv.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_libuv.cc @@ -171,7 +171,7 @@ class GrpcPolledFdFactoryLibuv : public GrpcPolledFdFactory { }; UniquePtr NewGrpcPolledFdFactory(grpc_combiner* combiner) { - return UniquePtr(New()); + return MakeUnique(); } } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_posix.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_posix.cc index aa58e1aaf50..f167047ebc9 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_posix.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_posix.cc @@ -98,7 +98,7 @@ class GrpcPolledFdFactoryPosix : public GrpcPolledFdFactory { }; UniquePtr NewGrpcPolledFdFactory(grpc_combiner* combiner) { - return UniquePtr(New()); + return MakeUnique(); } } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_windows.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_windows.cc index d72c453d748..bc79b7c3648 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_windows.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_windows.cc @@ -904,8 +904,7 @@ class GrpcPolledFdFactoryWindows : public GrpcPolledFdFactory { }; UniquePtr NewGrpcPolledFdFactory(grpc_combiner* combiner) { - return UniquePtr( - New(combiner)); + return MakeUnique(combiner); } } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc index 629b7360a88..5f674992e64 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc @@ -268,7 +268,7 @@ class NativeDnsResolverFactory : public ResolverFactory { OrphanablePtr CreateResolver(ResolverArgs args) const override { if (!IsValidUri(args.uri)) return nullptr; - return OrphanablePtr(New(std::move(args))); + return MakeOrphanable(std::move(args)); } const char* scheme() const override { return "dns"; } @@ -284,8 +284,7 @@ void grpc_resolver_dns_native_init() { if (gpr_stricmp(resolver.get(), "native") == 0) { gpr_log(GPR_DEBUG, "Using native dns resolver"); grpc_core::ResolverRegistry::Builder::RegisterResolverFactory( - grpc_core::UniquePtr( - grpc_core::New())); + grpc_core::MakeUnique()); } else { grpc_core::ResolverRegistry::Builder::InitRegistry(); grpc_core::ResolverFactory* existing_factory = @@ -293,8 +292,7 @@ void grpc_resolver_dns_native_init() { if (existing_factory == nullptr) { gpr_log(GPR_DEBUG, "Using native dns resolver"); grpc_core::ResolverRegistry::Builder::RegisterResolverFactory( - grpc_core::UniquePtr( - grpc_core::New())); + grpc_core::MakeUnique()); } } } diff --git a/src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc b/src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc index 443a709ea6c..98beef37de3 100644 --- a/src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc @@ -382,7 +382,7 @@ class FakeResolverFactory : public ResolverFactory { bool IsValidUri(const grpc_uri* uri) const override { return true; } OrphanablePtr CreateResolver(ResolverArgs args) const override { - return OrphanablePtr(New(std::move(args))); + return MakeOrphanable(std::move(args)); } const char* scheme() const override { return "fake"; } @@ -394,8 +394,7 @@ class FakeResolverFactory : public ResolverFactory { void grpc_resolver_fake_init() { grpc_core::ResolverRegistry::Builder::RegisterResolverFactory( - grpc_core::UniquePtr( - grpc_core::New())); + grpc_core::MakeUnique()); } void grpc_resolver_fake_shutdown() {} diff --git a/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc b/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc index 532fd6df9b7..ece5ffe86d4 100644 --- a/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc @@ -119,8 +119,8 @@ OrphanablePtr CreateSockaddrResolver( ServerAddressList addresses; if (!ParseUri(args.uri, parse, &addresses)) return nullptr; // Instantiate resolver. - return OrphanablePtr( - New(std::move(addresses), std::move(args))); + return MakeOrphanable(std::move(addresses), + std::move(args)); } class IPv4ResolverFactory : public ResolverFactory { @@ -174,15 +174,12 @@ class UnixResolverFactory : public ResolverFactory { void grpc_resolver_sockaddr_init() { grpc_core::ResolverRegistry::Builder::RegisterResolverFactory( - grpc_core::UniquePtr( - grpc_core::New())); + grpc_core::MakeUnique()); grpc_core::ResolverRegistry::Builder::RegisterResolverFactory( - grpc_core::UniquePtr( - grpc_core::New())); + grpc_core::MakeUnique()); #ifdef GRPC_HAVE_UNIX_SOCKET grpc_core::ResolverRegistry::Builder::RegisterResolverFactory( - grpc_core::UniquePtr( - grpc_core::New())); + grpc_core::MakeUnique()); #endif } diff --git a/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc b/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc index 8be1b89eee0..eac64b44a98 100644 --- a/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc @@ -70,7 +70,7 @@ class XdsResolverFactory : public ResolverFactory { OrphanablePtr CreateResolver(ResolverArgs args) const override { if (!IsValidUri(args.uri)) return nullptr; - return OrphanablePtr(New(std::move(args))); + return MakeOrphanable(std::move(args)); } const char* scheme() const override { return "xds-experimental"; } @@ -82,8 +82,7 @@ class XdsResolverFactory : public ResolverFactory { void grpc_resolver_xds_init() { grpc_core::ResolverRegistry::Builder::RegisterResolverFactory( - grpc_core::UniquePtr( - grpc_core::New())); + grpc_core::MakeUnique()); } void grpc_resolver_xds_shutdown() {} diff --git a/src/core/ext/filters/client_channel/resolver_result_parsing.cc b/src/core/ext/filters/client_channel/resolver_result_parsing.cc index 3a1960bfded..221e0f4feb3 100644 --- a/src/core/ext/filters/client_channel/resolver_result_parsing.cc +++ b/src/core/ext/filters/client_channel/resolver_result_parsing.cc @@ -53,9 +53,8 @@ size_t ClientChannelServiceConfigParser::ParserIndex() { } void ClientChannelServiceConfigParser::Register() { - g_client_channel_service_config_parser_index = - ServiceConfig::RegisterParser(UniquePtr( - New())); + g_client_channel_service_config_parser_index = ServiceConfig::RegisterParser( + MakeUnique()); } namespace { @@ -439,10 +438,9 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const grpc_json* json, *error = GRPC_ERROR_CREATE_FROM_VECTOR("Client channel global parser", &error_list); if (*error == GRPC_ERROR_NONE) { - return UniquePtr( - New( - std::move(parsed_lb_config), std::move(lb_policy_name), - retry_throttling, health_check_service_name)); + return MakeUnique( + std::move(parsed_lb_config), std::move(lb_policy_name), + retry_throttling, health_check_service_name); } return nullptr; } @@ -493,9 +491,8 @@ ClientChannelServiceConfigParser::ParsePerMethodParams(const grpc_json* json, } *error = GRPC_ERROR_CREATE_FROM_VECTOR("Client channel parser", &error_list); if (*error == GRPC_ERROR_NONE) { - return UniquePtr( - New(timeout, wait_for_ready, - std::move(retry_policy))); + return MakeUnique(timeout, wait_for_ready, + std::move(retry_policy)); } return nullptr; } diff --git a/src/core/ext/filters/client_channel/resolving_lb_policy.cc b/src/core/ext/filters/client_channel/resolving_lb_policy.cc index d997e26f979..a2b946bb5c6 100644 --- a/src/core/ext/filters/client_channel/resolving_lb_policy.cc +++ b/src/core/ext/filters/client_channel/resolving_lb_policy.cc @@ -187,16 +187,15 @@ ResolvingLoadBalancingPolicy::ResolvingLoadBalancingPolicy( GPR_ASSERT(process_resolver_result != nullptr); resolver_ = ResolverRegistry::CreateResolver( target_uri_.get(), args.args, interested_parties(), combiner(), - UniquePtr(New(Ref()))); + MakeUnique(Ref())); // Since the validity of args has been checked when create the channel, // CreateResolver() must return a non-null result. GPR_ASSERT(resolver_ != nullptr); if (GRPC_TRACE_FLAG_ENABLED(*tracer_)) { gpr_log(GPR_INFO, "resolving_lb=%p: starting name resolution", this); } - channel_control_helper()->UpdateState( - GRPC_CHANNEL_CONNECTING, - UniquePtr(New(Ref()))); + channel_control_helper()->UpdateState(GRPC_CHANNEL_CONNECTING, + MakeUnique(Ref())); resolver_->StartLocked(); } @@ -262,7 +261,7 @@ void ResolvingLoadBalancingPolicy::OnResolverError(grpc_error* error) { "Resolver transient failure", &error, 1); channel_control_helper()->UpdateState( GRPC_CHANNEL_TRANSIENT_FAILURE, - UniquePtr(New(state_error))); + MakeUnique(state_error)); } GRPC_ERROR_UNREF(error); } diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 2c3f899d2a7..df8473d160a 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -1071,8 +1071,7 @@ bool Subchannel::PublishTransportLocked() { } // Start watching connected subchannel. connected_subchannel_->StartWatch( - pollset_set_, OrphanablePtr( - New(this))); + pollset_set_, MakeOrphanable(this)); // Report initial state. SetConnectivityStateLocked(GRPC_CHANNEL_READY); return true; diff --git a/src/core/ext/filters/message_size/message_size_filter.cc b/src/core/ext/filters/message_size/message_size_filter.cc index df447c226b1..7e6c71de77e 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -88,13 +88,13 @@ UniquePtr MessageSizeParser::ParsePerMethodParams( *error = GRPC_ERROR_CREATE_FROM_VECTOR("Message size parser", &error_list); return nullptr; } - return UniquePtr(New( - max_request_message_bytes, max_response_message_bytes)); + return MakeUnique(max_request_message_bytes, + max_response_message_bytes); } void MessageSizeParser::Register() { - g_message_size_parser_index = ServiceConfig::RegisterParser( - UniquePtr(New())); + g_message_size_parser_index = + ServiceConfig::RegisterParser(MakeUnique()); } size_t MessageSizeParser::ParserIndex() { return g_message_size_parser_index; } diff --git a/src/core/lib/gprpp/memory.h b/src/core/lib/gprpp/memory.h index be158d118a1..53b89507c6f 100644 --- a/src/core/lib/gprpp/memory.h +++ b/src/core/lib/gprpp/memory.h @@ -33,10 +33,8 @@ // Should not be used in new code. // TODO(juanlishen): Remove this macro, and instead comment that the public dtor // should not be used directly. -#define GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE \ - template \ - friend void ::grpc_core::Delete(_Delete_T*); \ - template \ +#define GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE \ + template \ friend void ::grpc_core::Delete(_Delete_T*); // Add this to a class that want to use New(), but has a private or @@ -58,33 +56,28 @@ inline T* New(Args&&... args) { } // Alternative to delete, since we cannot use it (for fear of libstdc++) -// We cannot add a default value for can_be_null, because they are used as -// as friend template methods where we cannot define a default value. -// Instead we simply define two variants, one with and one without the boolean -// argument. -template +template inline void Delete(T* p) { - GPR_DEBUG_ASSERT(can_be_null || p != nullptr); - if (can_be_null && p == nullptr) return; + if (p == nullptr) return; p->~T(); gpr_free(p); } -template -inline void Delete(T* p) { - Delete(p); -} -template class DefaultDelete { public: + template void operator()(T* p) { - // std::unique_ptr is gauranteed not to call the deleter - // if the pointer is nullptr. - Delete(p); + // Delete() checks whether the value is null, but std::unique_ptr<> is + // gauranteed not to call the deleter if the pointer is nullptr + // (i.e., it already does this check for us), and we don't want to + // do the check twice. So, instead of calling Delete() here, we + // manually call the object's dtor and free it. + p->~T(); + gpr_free(p); } }; -template > +template using UniquePtr = std::unique_ptr; template diff --git a/src/core/lib/gprpp/orphanable.h b/src/core/lib/gprpp/orphanable.h index 82350a19a2c..ab02d80827b 100644 --- a/src/core/lib/gprpp/orphanable.h +++ b/src/core/lib/gprpp/orphanable.h @@ -59,13 +59,15 @@ class Orphanable { virtual ~Orphanable() {} }; -template class OrphanableDelete { public: - void operator()(T* p) { p->Orphan(); } + template + void operator()(T* p) { + p->Orphan(); + } }; -template > +template using OrphanablePtr = std::unique_ptr; template diff --git a/src/core/lib/security/transport/security_handshaker.cc b/src/core/lib/security/transport/security_handshaker.cc index 1d0b5ec0c6e..0128a344e1f 100644 --- a/src/core/lib/security/transport/security_handshaker.cc +++ b/src/core/lib/security/transport/security_handshaker.cc @@ -515,10 +515,10 @@ RefCountedPtr SecurityHandshakerCreate( void SecurityRegisterHandshakerFactories() { HandshakerRegistry::RegisterHandshakerFactory( false /* at_start */, HANDSHAKER_CLIENT, - UniquePtr(New())); + MakeUnique()); HandshakerRegistry::RegisterHandshakerFactory( false /* at_start */, HANDSHAKER_SERVER, - UniquePtr(New())); + MakeUnique()); } } // namespace grpc_core diff --git a/src/core/tsi/ssl/session_cache/ssl_session_boringssl.cc b/src/core/tsi/ssl/session_cache/ssl_session_boringssl.cc index 0da5a96164e..a3ba5fedee9 100644 --- a/src/core/tsi/ssl/session_cache/ssl_session_boringssl.cc +++ b/src/core/tsi/ssl/session_cache/ssl_session_boringssl.cc @@ -49,8 +49,7 @@ class BoringSslCachedSession : public SslCachedSession { grpc_core::UniquePtr SslCachedSession::Create( SslSessionPtr session) { - return grpc_core::UniquePtr( - grpc_core::New(std::move(session))); + return grpc_core::MakeUnique(std::move(session)); } } // namespace tsi diff --git a/src/core/tsi/ssl/session_cache/ssl_session_openssl.cc b/src/core/tsi/ssl/session_cache/ssl_session_openssl.cc index 61c036c7eb5..b03d005574e 100644 --- a/src/core/tsi/ssl/session_cache/ssl_session_openssl.cc +++ b/src/core/tsi/ssl/session_cache/ssl_session_openssl.cc @@ -67,8 +67,7 @@ class OpenSslCachedSession : public SslCachedSession { grpc_core::UniquePtr SslCachedSession::Create( SslSessionPtr session) { - return grpc_core::UniquePtr( - grpc_core::New(std::move(session))); + return grpc_core::MakeUnique(std::move(session)); } } // namespace tsi diff --git a/test/core/client_channel/resolvers/dns_resolver_test.cc b/test/core/client_channel/resolvers/dns_resolver_test.cc index ce44fd51b82..d03782ef159 100644 --- a/test/core/client_channel/resolvers/dns_resolver_test.cc +++ b/test/core/client_channel/resolvers/dns_resolver_test.cc @@ -35,12 +35,6 @@ class TestResultHandler : public grpc_core::Resolver::ResultHandler { void ReturnError(grpc_error* error) override {} }; -static grpc_core::UniquePtr -create_test_result_handler() { - return grpc_core::UniquePtr( - grpc_core::New()); -} - static void test_succeeds(grpc_core::ResolverFactory* factory, const char* string) { gpr_log(GPR_DEBUG, "test: '%s' should be valid for '%s'", string, @@ -51,7 +45,7 @@ static void test_succeeds(grpc_core::ResolverFactory* factory, grpc_core::ResolverArgs args; args.uri = uri; args.combiner = g_combiner; - args.result_handler = create_test_result_handler(); + args.result_handler = grpc_core::MakeUnique(); grpc_core::OrphanablePtr resolver = factory->CreateResolver(std::move(args)); GPR_ASSERT(resolver != nullptr); @@ -68,7 +62,7 @@ static void test_fails(grpc_core::ResolverFactory* factory, grpc_core::ResolverArgs args; args.uri = uri; args.combiner = g_combiner; - args.result_handler = create_test_result_handler(); + args.result_handler = grpc_core::MakeUnique(); grpc_core::OrphanablePtr resolver = factory->CreateResolver(std::move(args)); GPR_ASSERT(resolver == nullptr); diff --git a/test/core/client_channel/resolvers/sockaddr_resolver_test.cc b/test/core/client_channel/resolvers/sockaddr_resolver_test.cc index ac3d31b8ff8..8cbacaca999 100644 --- a/test/core/client_channel/resolvers/sockaddr_resolver_test.cc +++ b/test/core/client_channel/resolvers/sockaddr_resolver_test.cc @@ -47,9 +47,7 @@ static void test_succeeds(grpc_core::ResolverFactory* factory, grpc_core::ResolverArgs args; args.uri = uri; args.combiner = g_combiner; - args.result_handler = - grpc_core::UniquePtr( - grpc_core::New()); + args.result_handler = grpc_core::MakeUnique(); grpc_core::OrphanablePtr resolver = factory->CreateResolver(std::move(args)); GPR_ASSERT(resolver != nullptr); @@ -70,9 +68,7 @@ static void test_fails(grpc_core::ResolverFactory* factory, grpc_core::ResolverArgs args; args.uri = uri; args.combiner = g_combiner; - args.result_handler = - grpc_core::UniquePtr( - grpc_core::New()); + args.result_handler = grpc_core::MakeUnique(); grpc_core::OrphanablePtr resolver = factory->CreateResolver(std::move(args)); GPR_ASSERT(resolver == nullptr); diff --git a/test/core/client_channel/service_config_test.cc b/test/core/client_channel/service_config_test.cc index b1fe6453d62..491b43dfb50 100644 --- a/test/core/client_channel/service_config_test.cc +++ b/test/core/client_channel/service_config_test.cc @@ -60,8 +60,7 @@ class TestParser1 : public ServiceConfig::Parser { GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidValueErrorMessage()); return nullptr; } - return UniquePtr( - New(value)); + return MakeUnique(value); } } return nullptr; @@ -98,8 +97,7 @@ class TestParser2 : public ServiceConfig::Parser { GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidValueErrorMessage()); return nullptr; } - return UniquePtr( - New(value)); + return MakeUnique(value); } } return nullptr; @@ -148,10 +146,8 @@ class ServiceConfigTest : public ::testing::Test { void SetUp() override { ServiceConfig::Shutdown(); ServiceConfig::Init(); - EXPECT_TRUE(ServiceConfig::RegisterParser( - UniquePtr(New())) == 0); - EXPECT_TRUE(ServiceConfig::RegisterParser( - UniquePtr(New())) == 1); + EXPECT_TRUE(ServiceConfig::RegisterParser(MakeUnique()) == 0); + EXPECT_TRUE(ServiceConfig::RegisterParser(MakeUnique()) == 1); } }; @@ -312,10 +308,8 @@ class ErroredParsersScopingTest : public ::testing::Test { void SetUp() override { ServiceConfig::Shutdown(); ServiceConfig::Init(); - EXPECT_TRUE(ServiceConfig::RegisterParser( - UniquePtr(New())) == 0); - EXPECT_TRUE(ServiceConfig::RegisterParser( - UniquePtr(New())) == 1); + EXPECT_TRUE(ServiceConfig::RegisterParser(MakeUnique()) == 0); + EXPECT_TRUE(ServiceConfig::RegisterParser(MakeUnique()) == 1); } }; @@ -359,10 +353,9 @@ class ClientChannelParserTest : public ::testing::Test { void SetUp() override { ServiceConfig::Shutdown(); ServiceConfig::Init(); - EXPECT_TRUE( - ServiceConfig::RegisterParser(UniquePtr( - New())) == - 0); + EXPECT_TRUE(ServiceConfig::RegisterParser( + MakeUnique()) == + 0); } }; @@ -929,8 +922,8 @@ class MessageSizeParserTest : public ::testing::Test { void SetUp() override { ServiceConfig::Shutdown(); ServiceConfig::Init(); - EXPECT_TRUE(ServiceConfig::RegisterParser(UniquePtr( - New())) == 0); + EXPECT_TRUE( + ServiceConfig::RegisterParser(MakeUnique()) == 0); } }; diff --git a/test/core/handshake/readahead_handshaker_server_ssl.cc b/test/core/handshake/readahead_handshaker_server_ssl.cc index c0ab61136cb..36d489cc815 100644 --- a/test/core/handshake/readahead_handshaker_server_ssl.cc +++ b/test/core/handshake/readahead_handshaker_server_ssl.cc @@ -81,7 +81,7 @@ int main(int argc, char* argv[]) { grpc_init(); HandshakerRegistry::RegisterHandshakerFactory( true /* at_start */, HANDSHAKER_SERVER, - UniquePtr(New())); + MakeUnique()); const char* full_alpn_list[] = {"grpc-exp", "h2"}; GPR_ASSERT(server_ssl_test(full_alpn_list, 2, "grpc-exp")); grpc_shutdown_blocking(); diff --git a/test/core/surface/lame_client_test.cc b/test/core/surface/lame_client_test.cc index 34cafbbd5b7..dc961b5b458 100644 --- a/test/core/surface/lame_client_test.cc +++ b/test/core/surface/lame_client_test.cc @@ -44,9 +44,7 @@ static void do_nothing(void* arg, grpc_error* error) {} void test_transport_op(grpc_channel* channel) { grpc_core::ExecCtx exec_ctx; grpc_transport_op* op = grpc_make_transport_op(nullptr); - op->start_connectivity_watch = - grpc_core::OrphanablePtr( - grpc_core::New()); + op->start_connectivity_watch = grpc_core::MakeOrphanable(); grpc_channel_element* elem = grpc_channel_stack_element(grpc_channel_get_channel_stack(channel), 0); elem->filter->start_transport_op(elem, op); diff --git a/test/core/transport/connectivity_state_test.cc b/test/core/transport/connectivity_state_test.cc index 6493a62ff3c..5a82bb5979a 100644 --- a/test/core/transport/connectivity_state_test.cc +++ b/test/core/transport/connectivity_state_test.cc @@ -73,8 +73,7 @@ TEST(StateTracker, NotificationUponAddingWatcher) { grpc_connectivity_state state = GRPC_CHANNEL_IDLE; ConnectivityStateTracker tracker("xxx", GRPC_CHANNEL_CONNECTING); tracker.AddWatcher(GRPC_CHANNEL_IDLE, - OrphanablePtr( - New(&count, &state))); + MakeOrphanable(&count, &state)); EXPECT_EQ(count, 1); EXPECT_EQ(state, GRPC_CHANNEL_CONNECTING); } @@ -84,8 +83,7 @@ TEST(StateTracker, NotificationUponStateChange) { grpc_connectivity_state state = GRPC_CHANNEL_IDLE; ConnectivityStateTracker tracker("xxx", GRPC_CHANNEL_IDLE); tracker.AddWatcher(GRPC_CHANNEL_IDLE, - OrphanablePtr( - New(&count, &state))); + MakeOrphanable(&count, &state)); EXPECT_EQ(count, 0); EXPECT_EQ(state, GRPC_CHANNEL_IDLE); tracker.SetState(GRPC_CHANNEL_CONNECTING, "whee"); @@ -153,8 +151,7 @@ TEST(StateTracker, NotifyShutdownAtDestruction) { { ConnectivityStateTracker tracker("xxx", GRPC_CHANNEL_IDLE); tracker.AddWatcher(GRPC_CHANNEL_IDLE, - OrphanablePtr( - New(&count, &state))); + MakeOrphanable(&count, &state)); // No initial notification, since we started the watch from the // current state. EXPECT_EQ(count, 0); @@ -171,8 +168,7 @@ TEST(StateTracker, DoNotNotifyShutdownAtDestructionIfAlreadyInShutdown) { { ConnectivityStateTracker tracker("xxx", GRPC_CHANNEL_SHUTDOWN); tracker.AddWatcher(GRPC_CHANNEL_SHUTDOWN, - OrphanablePtr( - New(&count, &state))); + MakeOrphanable(&count, &state)); // No initial notification, since we started the watch from the // current state. EXPECT_EQ(count, 0); diff --git a/test/core/util/test_lb_policies.cc b/test/core/util/test_lb_policies.cc index d4ce3a27e50..8330c47ac46 100644 --- a/test/core/util/test_lb_policies.cc +++ b/test/core/util/test_lb_policies.cc @@ -219,9 +219,8 @@ class InterceptTrailingFactory : public LoadBalancingPolicyFactory { OrphanablePtr CreateLoadBalancingPolicy( LoadBalancingPolicy::Args args) const override { - return OrphanablePtr( - New(std::move(args), - cb_, user_data_)); + return MakeOrphanable( + std::move(args), cb_, user_data_); } const char* name() const override {