diff --git a/examples/cpp/helloworld/CMakeLists.txt b/examples/cpp/helloworld/CMakeLists.txt index b972e270fb5..384e88a5aac 100644 --- a/examples/cpp/helloworld/CMakeLists.txt +++ b/examples/cpp/helloworld/CMakeLists.txt @@ -146,7 +146,7 @@ include_directories("${CMAKE_CURRENT_BINARY_DIR}") # Targets greeter_[async_](client|server) foreach(_target greeter_client greeter_server - greeter_async_client greeter_async_server) + greeter_async_client greeter_async_client2 greeter_async_server) add_executable(${_target} "${_target}.cc" ${hw_proto_srcs} ${hw_grpc_srcs}) diff --git a/include/grpcpp/impl/codegen/client_callback_impl.h b/include/grpcpp/impl/codegen/client_callback_impl.h index 1a2bae1b5db..fac67c5f49b 100644 --- a/include/grpcpp/impl/codegen/client_callback_impl.h +++ b/include/grpcpp/impl/codegen/client_callback_impl.h @@ -209,18 +209,18 @@ class ClientBidiReactor { /// Initiate a write operation (or post it for later initiation if StartCall /// has not yet been invoked). /// - /// \param[in] req The message to be written. The library takes temporary - /// ownership until OnWriteDone, at which point the application - /// regains ownership of msg. + /// \param[in] req The message to be written. The library does not take + /// ownership but the caller must ensure that the message is + /// not deleted or modified until OnWriteDone is called. void StartWrite(const Request* req) { StartWrite(req, ::grpc::WriteOptions()); } /// Initiate/post a write operation with specified options. /// - /// \param[in] req The message to be written. The library takes temporary - /// ownership until OnWriteDone, at which point the application - /// regains ownership of msg. + /// \param[in] req The message to be written. The library does not take + /// ownership but the caller must ensure that the message is + /// not deleted or modified until OnWriteDone is called. /// \param[in] options The WriteOptions to use for writing this message void StartWrite(const Request* req, ::grpc::WriteOptions options) { stream_->Write(req, std::move(options)); @@ -231,9 +231,9 @@ class ClientBidiReactor { /// Note that calling this means that no more calls to StartWrite, /// StartWriteLast, or StartWritesDone are allowed. /// - /// \param[in] req The message to be written. The library takes temporary - /// ownership until OnWriteDone, at which point the application - /// regains ownership of msg. + /// \param[in] req The message to be written. The library does not take + /// ownership but the caller must ensure that the message is + /// not deleted or modified until OnWriteDone is called. /// \param[in] options The WriteOptions to use for writing this message void StartWriteLast(const Request* req, ::grpc::WriteOptions options) { StartWrite(req, std::move(options.set_last_message())); diff --git a/include/grpcpp/impl/codegen/completion_queue_impl.h b/include/grpcpp/impl/codegen/completion_queue_impl.h index 4549aa37194..41622f60900 100644 --- a/include/grpcpp/impl/codegen/completion_queue_impl.h +++ b/include/grpcpp/impl/codegen/completion_queue_impl.h @@ -32,11 +32,14 @@ #ifndef GRPCPP_IMPL_CODEGEN_COMPLETION_QUEUE_IMPL_H #define GRPCPP_IMPL_CODEGEN_COMPLETION_QUEUE_IMPL_H +#include + #include #include #include #include #include +#include #include struct grpc_completion_queue; @@ -250,6 +253,11 @@ class CompletionQueue : private ::grpc::GrpcLibraryCodegen { } private: + // Friends for access to server registration lists that enable checking and + // logging on shutdown + friend class ::grpc_impl::ServerBuilder; + friend class ::grpc_impl::Server; + // Friend synchronous wrappers so that they can access Pluck(), which is // a semi-private API geared towards the synchronous implementation. template @@ -274,7 +282,6 @@ class CompletionQueue : private ::grpc::GrpcLibraryCodegen { friend class ::grpc_impl::internal::TemplatedBidiStreamingHandler; template <::grpc::StatusCode code> friend class ::grpc_impl::internal::ErrorMethodHandler; - friend class ::grpc_impl::Server; friend class ::grpc_impl::ServerContextBase; friend class ::grpc::ServerInterface; template @@ -379,13 +386,38 @@ class CompletionQueue : private ::grpc::GrpcLibraryCodegen { } } + void RegisterServer(const Server* server) { +#ifndef NDEBUG + grpc::internal::MutexLock l(&server_list_mutex_); + server_list_.push_back(server); +#endif + } + void UnregisterServer(const Server* server) { +#ifndef NDEBUG + grpc::internal::MutexLock l(&server_list_mutex_); + server_list_.remove(server); +#endif + } + bool ServerListEmpty() const { +#ifndef NDEBUG + grpc::internal::MutexLock l(&server_list_mutex_); + return server_list_.empty(); +#endif + return true; + } + +#ifndef NDEBUG + mutable grpc::internal::Mutex server_list_mutex_; + std::list server_list_ /* GUARDED_BY(server_list_mutex_) */; +#endif + grpc_completion_queue* cq_; // owned gpr_atm avalanches_in_flight_; }; /// A specific type of completion queue used by the processing of notifications -/// by servers. Instantiated by \a ServerBuilder. +/// by servers. Instantiated by \a ServerBuilder or Server (for health checker). class ServerCompletionQueue : public CompletionQueue { public: bool IsFrequentlyPolled() { return polling_type_ != GRPC_CQ_NON_LISTENING; } diff --git a/include/grpcpp/impl/codegen/server_callback_impl.h b/include/grpcpp/impl/codegen/server_callback_impl.h index 781e96f974f..6edf6e4dddd 100644 --- a/include/grpcpp/impl/codegen/server_callback_impl.h +++ b/include/grpcpp/impl/codegen/server_callback_impl.h @@ -277,18 +277,18 @@ class ServerBidiReactor : public internal::ServerReactor { /// Initiate a write operation. /// - /// \param[in] resp The message to be written. The library takes temporary - /// ownership until OnWriteDone, at which point the - /// application regains ownership of resp. + /// \param[in] resp The message to be written. The library does not take + /// ownership but the caller must ensure that the message is + /// not deleted or modified until OnWriteDone is called. void StartWrite(const Response* resp) { StartWrite(resp, ::grpc::WriteOptions()); } /// Initiate a write operation with specified options. /// - /// \param[in] resp The message to be written. The library takes temporary - /// ownership until OnWriteDone, at which point the - /// application regains ownership of resp. + /// \param[in] resp The message to be written. The library does not take + /// ownership but the caller must ensure that the message is + /// not deleted or modified until OnWriteDone is called. /// \param[in] options The WriteOptions to use for writing this message void StartWrite(const Response* resp, ::grpc::WriteOptions options) { ServerCallbackReaderWriter* stream = @@ -313,9 +313,9 @@ class ServerBidiReactor : public internal::ServerReactor { /// available. An RPC can either have StartWriteAndFinish or Finish, but not /// both. /// - /// \param[in] resp The message to be written. The library takes temporary - /// ownership until OnWriteDone, at which point the - /// application regains ownership of resp. + /// \param[in] resp The message to be written. The library does not take + /// ownership but the caller must ensure that the message is + /// not deleted or modified until OnDone is called. /// \param[in] options The WriteOptions to use for writing this message /// \param[in] s The status outcome of this RPC void StartWriteAndFinish(const Response* resp, ::grpc::WriteOptions options, @@ -340,9 +340,9 @@ class ServerBidiReactor : public internal::ServerReactor { /// allow the library to schedule the actual write coalesced with the writing /// of trailing metadata (which takes place on a Finish call). /// - /// \param[in] resp The message to be written. The library takes temporary - /// ownership until OnWriteDone, at which point the - /// application regains ownership of resp. + /// \param[in] resp The message to be written. The library does not take + /// ownership but the caller must ensure that the message is + /// not deleted or modified until OnWriteDone is called. /// \param[in] options The WriteOptions to use for writing this message void StartWriteLast(const Response* resp, ::grpc::WriteOptions options) { StartWrite(resp, std::move(options.set_last_message())); diff --git a/include/grpcpp/security/tls_credentials_options.h b/include/grpcpp/security/tls_credentials_options.h index d83226865d8..b5bb7c78b7f 100644 --- a/include/grpcpp/security/tls_credentials_options.h +++ b/include/grpcpp/security/tls_credentials_options.h @@ -55,13 +55,12 @@ class TlsKeyMaterialsConfig { } int version() const { return version_; } - /** Setter for key materials that will be called by the user. Ownership of the - * arguments will not be transferred. **/ - void set_pem_root_certs(const grpc::string& pem_root_certs); + /** Setter for key materials that will be called by the user. The setter + * transfers ownership of the arguments to the config. **/ + void set_pem_root_certs(grpc::string pem_root_certs); void add_pem_key_cert_pair(const PemKeyCertPair& pem_key_cert_pair); - void set_key_materials( - const grpc::string& pem_root_certs, - const std::vector& pem_key_cert_pair_list); + void set_key_materials(grpc::string pem_root_certs, + std::vector pem_key_cert_pair_list); void set_version(int version) { version_ = version; }; private: @@ -71,36 +70,40 @@ class TlsKeyMaterialsConfig { }; /** TLS credential reload arguments, wraps grpc_tls_credential_reload_arg. It is - * used for experimental purposes for now and it is subject to change. + * used for experimental purposes for now and it is subject to change. * - * The credential reload arg contains all the info necessary to schedule/cancel - * a credential reload request. The callback function must be called after - * finishing the schedule operation. See the description of the - * grpc_tls_credential_reload_arg struct in grpc_security.h for more details. + * The credential reload arg contains all the info necessary to schedule/cancel + * a credential reload request. The callback function must be called after + * finishing the schedule operation. See the description of the + * grpc_tls_credential_reload_arg struct in grpc_security.h for more details. * **/ class TlsCredentialReloadArg { public: /** TlsCredentialReloadArg does not take ownership of the C arg that is passed - * to the constructor. One must remember to free any memory allocated to the - * C arg after using the setter functions below. **/ + * to the constructor. One must remember to free any memory allocated to the C + * arg after using the setter functions below. **/ TlsCredentialReloadArg(grpc_tls_credential_reload_arg* arg); ~TlsCredentialReloadArg(); - /** Getters for member fields. **/ + /** Getters for member fields. The callback function is not exposed. + * They return the corresponding fields of the underlying C arg. In the case + * of the key materials config, it creates a new instance of the C++ key + * materials config from the underlying C grpc_tls_key_materials_config. **/ void* cb_user_data() const; bool is_pem_key_cert_pair_list_empty() const; grpc_ssl_certificate_config_reload_status status() const; grpc::string error_details() const; - /** Setters for member fields. Ownership of the arguments will not be - * transferred. **/ + /** Setters for member fields. They modify the fields of the underlying C arg. + * The setters for the key_materials_config and the error_details allocate + * memory when modifying c_arg_, so one must remember to free c_arg_'s + * original key_materials_config or error_details after using the appropriate + * setter function. + * **/ void set_cb_user_data(void* cb_user_data); void set_pem_root_certs(const grpc::string& pem_root_certs); void add_pem_key_cert_pair( - const TlsKeyMaterialsConfig::PemKeyCertPair& pem_key_cert_pair); - void set_key_materials(const grpc::string& pem_root_certs, - std::vector - pem_key_cert_pair_list); + TlsKeyMaterialsConfig::PemKeyCertPair pem_key_cert_pair); void set_key_materials_config( const std::shared_ptr& key_materials_config); void set_status(grpc_ssl_certificate_config_reload_status status); @@ -184,7 +187,8 @@ class TlsServerAuthorizationCheckArg { TlsServerAuthorizationCheckArg(grpc_tls_server_authorization_check_arg* arg); ~TlsServerAuthorizationCheckArg(); - /** Getters for member fields. **/ + /** Getters for member fields. They return the corresponding fields of the + * underlying C arg.**/ void* cb_user_data() const; int success() const; grpc::string target_name() const; @@ -193,7 +197,12 @@ class TlsServerAuthorizationCheckArg { grpc_status_code status() const; grpc::string error_details() const; - /** Setters for member fields. **/ + /** Setters for member fields. They modify the fields of the underlying C arg. + * The setters for target_name, peer_cert, and error_details allocate memory + * when modifying c_arg_, so one must remember to free c_arg_'s original + * target_name, peer_cert, or error_details after using the appropriate setter + * function. + * **/ void set_cb_user_data(void* cb_user_data); void set_success(int success); void set_target_name(const grpc::string& target_name); diff --git a/include/grpcpp/server_impl.h b/include/grpcpp/server_impl.h index 9506c419018..bcc4e3b45c9 100644 --- a/include/grpcpp/server_impl.h +++ b/include/grpcpp/server_impl.h @@ -385,6 +385,12 @@ class Server : public grpc::ServerInterface, private grpc::GrpcLibraryCodegen { // shutdown callback tag (invoked when the CQ is fully shutdown). // It is protected by mu_ CompletionQueue* callback_cq_ = nullptr; + +#ifndef NDEBUG + // List of CQs passed in by user that must be Shutdown only after Server is + // Shutdown. + std::vector cq_list_; +#endif }; } // namespace grpc_impl diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 7e3fe1b3be9..958be7250a0 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -1594,7 +1594,8 @@ void ChannelData::CreateResolvingLoadBalancingPolicyLocked() { // Instantiate resolving LB policy. LoadBalancingPolicy::Args lb_args; lb_args.combiner = combiner_; - lb_args.channel_control_helper = MakeUnique(this); + lb_args.channel_control_helper = + grpc_core::MakeUnique(this); lb_args.args = channel_args_; grpc_core::UniquePtr target_uri(gpr_strdup(target_uri_.get())); resolving_lb_policy_.reset(new ResolvingLoadBalancingPolicy( @@ -1870,7 +1871,7 @@ void ChannelData::StartTransportOpLocked(void* arg, grpc_error* /*ignored*/) { MemoryOrder::RELEASE); chand->UpdateStateAndPickerLocked( GRPC_CHANNEL_SHUTDOWN, "shutdown from API", - MakeUnique( + grpc_core::MakeUnique( GRPC_ERROR_REF(op->disconnect_with_error))); } } 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 f3087e23db8..cc8c5b02b1c 100644 --- a/src/core/ext/filters/client_channel/http_connect_handshaker.cc +++ b/src/core/ext/filters/client_channel/http_connect_handshaker.cc @@ -385,5 +385,5 @@ void grpc_http_connect_register_handshaker_factory() { using namespace grpc_core; HandshakerRegistry::RegisterHandshakerFactory( true /* at_start */, HANDSHAKER_CLIENT, - MakeUnique()); + grpc_core::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 cdcf997e62b..00bc77d7e5d 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 @@ -715,8 +715,9 @@ void GrpcLb::Helper::UpdateState(grpc_connectivity_state state, client_stats = parent_->lb_calld_->client_stats()->Ref(); } parent_->channel_control_helper()->UpdateState( - state, MakeUnique(parent_.get(), parent_->serverlist_, - std::move(picker), std::move(client_stats))); + state, grpc_core::MakeUnique(parent_.get(), parent_->serverlist_, + std::move(picker), + std::move(client_stats))); } void GrpcLb::Helper::RequestReresolution() { 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 bc00d31f442..7627092ad0d 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, - MakeUnique(error)); + grpc_core::MakeUnique(error)); return; } // If one of the subchannels in the new list is already in state @@ -319,11 +319,11 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked( GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE); p->channel_control_helper()->UpdateState( GRPC_CHANNEL_TRANSIENT_FAILURE, - MakeUnique(error)); + grpc_core::MakeUnique(error)); } else { p->channel_control_helper()->UpdateState( - GRPC_CHANNEL_CONNECTING, - MakeUnique(p->Ref(DEBUG_LOCATION, "QueuePicker"))); + GRPC_CHANNEL_CONNECTING, grpc_core::MakeUnique(p->Ref( + DEBUG_LOCATION, "QueuePicker"))); } } else { if (connectivity_state == GRPC_CHANNEL_TRANSIENT_FAILURE) { @@ -338,19 +338,20 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked( p->selected_ = nullptr; p->subchannel_list_.reset(); p->channel_control_helper()->UpdateState( - GRPC_CHANNEL_IDLE, - MakeUnique(p->Ref(DEBUG_LOCATION, "QueuePicker"))); + GRPC_CHANNEL_IDLE, grpc_core::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, MakeUnique(subchannel()->Ref())); + GRPC_CHANNEL_READY, + grpc_core::MakeUnique(subchannel()->Ref())); } else { // CONNECTING p->channel_control_helper()->UpdateState( - connectivity_state, - MakeUnique(p->Ref(DEBUG_LOCATION, "QueuePicker"))); + connectivity_state, grpc_core::MakeUnique( + p->Ref(DEBUG_LOCATION, "QueuePicker"))); } } } @@ -394,7 +395,7 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked( GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE); p->channel_control_helper()->UpdateState( GRPC_CHANNEL_TRANSIENT_FAILURE, - MakeUnique(error)); + grpc_core::MakeUnique(error)); } } sd->CheckConnectivityStateAndStartWatchingLocked(); @@ -405,8 +406,8 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked( // Only update connectivity state in case 1. if (subchannel_list() == p->subchannel_list_.get()) { p->channel_control_helper()->UpdateState( - GRPC_CHANNEL_CONNECTING, - MakeUnique(p->Ref(DEBUG_LOCATION, "QueuePicker"))); + GRPC_CHANNEL_CONNECTING, grpc_core::MakeUnique(p->Ref( + DEBUG_LOCATION, "QueuePicker"))); } break; } @@ -445,7 +446,7 @@ void PickFirst::PickFirstSubchannelData::ProcessUnselectedReadyLocked() { } p->selected_ = this; p->channel_control_helper()->UpdateState( - GRPC_CHANNEL_READY, MakeUnique(subchannel()->Ref())); + GRPC_CHANNEL_READY, grpc_core::MakeUnique(subchannel()->Ref())); for (size_t i = 0; i < subchannel_list()->num_subchannels(); ++i) { if (i != Index()) { subchannel_list()->subchannel(i)->ShutdownLocked(); 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 67fbf17fda2..e7933980d2e 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, - MakeUnique(p, this)); + p->channel_control_helper()->UpdateState( + GRPC_CHANNEL_READY, grpc_core::MakeUnique(p, this)); } else if (num_connecting_ > 0) { /* 2) CONNECTING */ p->channel_control_helper()->UpdateState( - GRPC_CHANNEL_CONNECTING, - MakeUnique(p->Ref(DEBUG_LOCATION, "QueuePicker"))); + GRPC_CHANNEL_CONNECTING, grpc_core::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, - MakeUnique(error)); + grpc_core::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, - MakeUnique(error)); + grpc_core::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 diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc index 341d2903891..d76dbab6269 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc @@ -162,7 +162,7 @@ void CdsLb::ClusterWatcher::OnClusterChanged(CdsUpdate cluster_data) { LoadBalancingPolicy::Args args; args.combiner = parent_->combiner(); args.args = parent_->args_; - args.channel_control_helper = MakeUnique(parent_->Ref()); + args.channel_control_helper = grpc_core::MakeUnique(parent_->Ref()); parent_->child_policy_ = LoadBalancingPolicyRegistry::CreateLoadBalancingPolicy( "xds_experimental", std::move(args)); @@ -186,7 +186,7 @@ void CdsLb::ClusterWatcher::OnError(grpc_error* error) { if (parent_->child_policy_ == nullptr) { parent_->channel_control_helper()->UpdateState( GRPC_CHANNEL_TRANSIENT_FAILURE, - MakeUnique(error)); + grpc_core::MakeUnique(error)); } else { GRPC_ERROR_UNREF(error); } @@ -287,7 +287,7 @@ void CdsLb::UpdateLocked(UpdateArgs args) { xds_client_->CancelClusterDataWatch(StringView(old_config->cluster()), cluster_watcher_); } - auto watcher = MakeUnique(Ref()); + auto watcher = grpc_core::MakeUnique(Ref()); cluster_watcher_ = watcher.get(); xds_client_->WatchClusterData(StringView(config_->cluster()), std::move(watcher)); 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 9fa273d4b55..1a7a27a00ae 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 @@ -834,8 +834,8 @@ void XdsLb::UpdateLocked(UpdateArgs args) { xds_client()->CancelEndpointDataWatch(StringView(old_eds_service_name), endpoint_watcher_); } - auto watcher = - MakeUnique(Ref(DEBUG_LOCATION, "EndpointWatcher")); + auto watcher = grpc_core::MakeUnique( + Ref(DEBUG_LOCATION, "EndpointWatcher")); endpoint_watcher_ = watcher.get(); xds_client()->WatchEndpointData(StringView(eds_service_name()), std::move(watcher)); @@ -1092,7 +1092,7 @@ void XdsLb::PriorityList::UpdateXdsPickerLocked() { GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE); xds_policy_->channel_control_helper()->UpdateState( GRPC_CHANNEL_TRANSIENT_FAILURE, - MakeUnique(error)); + grpc_core::MakeUnique(error)); return; } priorities_[current_priority_]->UpdateXdsPickerLocked(); @@ -1183,8 +1183,9 @@ 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, MakeUnique(xds_policy_->Ref( - DEBUG_LOCATION, "QueuePicker"))); + GRPC_CHANNEL_CONNECTING, + grpc_core::MakeUnique( + xds_policy_->Ref(DEBUG_LOCATION, "QueuePicker"))); } } 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 5d26f5bb88c..62576c706ba 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 @@ -173,7 +173,7 @@ class GrpcPolledFdFactoryLibuv : public GrpcPolledFdFactory { std::unique_ptr NewGrpcPolledFdFactory( Combiner* combiner) { - return MakeUnique(); + return grpc_core::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 5ae7d3c1adb..8a51475d4ec 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 @@ -99,7 +99,7 @@ class GrpcPolledFdFactoryPosix : public GrpcPolledFdFactory { std::unique_ptr NewGrpcPolledFdFactory( Combiner* /*combiner*/) { - return MakeUnique(); + return grpc_core::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 0b8d767630d..ed3daf3af77 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 @@ -934,7 +934,7 @@ class GrpcPolledFdFactoryWindows : public GrpcPolledFdFactory { std::unique_ptr NewGrpcPolledFdFactory( Combiner* combiner) { - return MakeUnique(combiner); + return grpc_core::MakeUnique(combiner); } } // namespace grpc_core 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 e3ef191016b..32f5e5c3523 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 @@ -89,7 +89,7 @@ void XdsResolver::StartLocked() { grpc_error* error = GRPC_ERROR_NONE; xds_client_ = MakeOrphanable( combiner(), interested_parties_, StringView(server_name_.get()), - MakeUnique(Ref()), *args_, &error); + grpc_core::MakeUnique(Ref()), *args_, &error); if (error != GRPC_ERROR_NONE) { gpr_log(GPR_ERROR, "Failed to create xds client -- channel will remain in " 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 df201fd310a..92b60a2e1f9 100644 --- a/src/core/ext/filters/client_channel/resolver_result_parsing.cc +++ b/src/core/ext/filters/client_channel/resolver_result_parsing.cc @@ -54,7 +54,7 @@ size_t ClientChannelServiceConfigParser::ParserIndex() { void ClientChannelServiceConfigParser::Register() { g_client_channel_service_config_parser_index = ServiceConfig::RegisterParser( - MakeUnique()); + grpc_core::MakeUnique()); } namespace { @@ -95,7 +95,7 @@ std::unique_ptr ParseRetryPolicy( grpc_json* field, grpc_error** error) { GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE); auto retry_policy = - MakeUnique(); + grpc_core::MakeUnique(); if (field->type != GRPC_JSON_OBJECT) { *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:retryPolicy error:should be of type object"); @@ -438,7 +438,7 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const grpc_json* json, *error = GRPC_ERROR_CREATE_FROM_VECTOR("Client channel global parser", &error_list); if (*error == GRPC_ERROR_NONE) { - return MakeUnique( + return grpc_core::MakeUnique( std::move(parsed_lb_config), std::move(lb_policy_name), retry_throttling, health_check_service_name); } @@ -491,8 +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 MakeUnique(timeout, wait_for_ready, - std::move(retry_policy)); + return grpc_core::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 fc5a398da84..26766801243 100644 --- a/src/core/ext/filters/client_channel/resolving_lb_policy.cc +++ b/src/core/ext/filters/client_channel/resolving_lb_policy.cc @@ -188,15 +188,15 @@ ResolvingLoadBalancingPolicy::ResolvingLoadBalancingPolicy( GPR_ASSERT(process_resolver_result != nullptr); resolver_ = ResolverRegistry::CreateResolver( target_uri_.get(), args.args, interested_parties(), combiner(), - MakeUnique(Ref())); + grpc_core::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, - MakeUnique(Ref())); + channel_control_helper()->UpdateState( + GRPC_CHANNEL_CONNECTING, grpc_core::MakeUnique(Ref())); resolver_->StartLocked(); } @@ -262,7 +262,7 @@ void ResolvingLoadBalancingPolicy::OnResolverError(grpc_error* error) { "Resolver transient failure", &error, 1); channel_control_helper()->UpdateState( GRPC_CHANNEL_TRANSIENT_FAILURE, - MakeUnique(state_error)); + grpc_core::MakeUnique(state_error)); } GRPC_ERROR_UNREF(error); } diff --git a/src/core/ext/filters/client_channel/service_config.cc b/src/core/ext/filters/client_channel/service_config.cc index 0b38559cd45..69e9d0a1870 100644 --- a/src/core/ext/filters/client_channel/service_config.cc +++ b/src/core/ext/filters/client_channel/service_config.cc @@ -104,7 +104,7 @@ grpc_error* ServiceConfig::ParseGlobalParams(const grpc_json* json_tree) { grpc_error* ServiceConfig::ParseJsonMethodConfigToServiceConfigVectorTable( const grpc_json* json, SliceHashTable::Entry* entries, size_t* idx) { - auto objs_vector = MakeUnique(); + auto objs_vector = grpc_core::MakeUnique(); InlinedVector error_list; for (size_t i = 0; i < g_registered_parsers->size(); i++) { grpc_error* parser_error = GRPC_ERROR_NONE; diff --git a/src/core/ext/filters/client_channel/xds/xds_bootstrap.cc b/src/core/ext/filters/client_channel/xds/xds_bootstrap.cc index 5824391cf1e..1be01a0dca0 100644 --- a/src/core/ext/filters/client_channel/xds/xds_bootstrap.cc +++ b/src/core/ext/filters/client_channel/xds/xds_bootstrap.cc @@ -39,7 +39,7 @@ std::unique_ptr XdsBootstrap::ReadFromFile(grpc_error** error) { grpc_slice contents; *error = grpc_load_file(path.get(), /*add_null_terminator=*/true, &contents); if (*error != GRPC_ERROR_NONE) return nullptr; - return MakeUnique(contents, error); + return grpc_core::MakeUnique(contents, error); } XdsBootstrap::XdsBootstrap(grpc_slice contents, grpc_error** error) @@ -248,7 +248,7 @@ grpc_error* XdsBootstrap::ParseChannelCreds(grpc_json* json, size_t idx, grpc_error* XdsBootstrap::ParseNode(grpc_json* json) { InlinedVector error_list; - node_ = MakeUnique(); + node_ = grpc_core::MakeUnique(); bool seen_metadata = false; bool seen_locality = false; for (grpc_json* child = json->child; child != nullptr; child = child->next) { diff --git a/src/core/ext/filters/client_channel/xds/xds_client.cc b/src/core/ext/filters/client_channel/xds/xds_client.cc index c9f07690814..e533bd490a2 100644 --- a/src/core/ext/filters/client_channel/xds/xds_client.cc +++ b/src/core/ext/filters/client_channel/xds/xds_client.cc @@ -341,9 +341,14 @@ grpc_channel_args* BuildXdsChannelArgs(const grpc_channel_args& args) { // Don't want to pass down channelz node from parent; the balancer // channel will get its own. GRPC_ARG_CHANNELZ_CHANNEL_NODE, + // Keepalive interval. We are explicitly setting our own value below. + GRPC_ARG_KEEPALIVE_TIME_MS, }; // Channel args to add. - InlinedVector args_to_add; + InlinedVector args_to_add; + // Keepalive interval. + args_to_add.emplace_back(grpc_channel_arg_integer_create( + const_cast(GRPC_ARG_KEEPALIVE_TIME_MS), 5000)); // A channel arg indicating that the target is an xds server. // TODO(roth): Once we figure out our fallback and credentials story, decide // whether this is actually needed. Note that it's currently used by the 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 4a5f1391bf3..1ec34e24f66 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -89,13 +89,13 @@ MessageSizeParser::ParsePerMethodParams(const grpc_json* json, *error = GRPC_ERROR_CREATE_FROM_VECTOR("Message size parser", &error_list); return nullptr; } - return MakeUnique(max_request_message_bytes, - max_response_message_bytes); + return grpc_core::MakeUnique( + max_request_message_bytes, max_response_message_bytes); } void MessageSizeParser::Register() { g_message_size_parser_index = - ServiceConfig::RegisterParser(MakeUnique()); + ServiceConfig::RegisterParser(grpc_core::MakeUnique()); } size_t MessageSizeParser::ParserIndex() { return g_message_size_parser_index; } diff --git a/src/core/lib/security/transport/security_handshaker.cc b/src/core/lib/security/transport/security_handshaker.cc index 1ffdd7a3bf9..1f37b7f7763 100644 --- a/src/core/lib/security/transport/security_handshaker.cc +++ b/src/core/lib/security/transport/security_handshaker.cc @@ -559,10 +559,10 @@ RefCountedPtr SecurityHandshakerCreate( void SecurityRegisterHandshakerFactories() { HandshakerRegistry::RegisterHandshakerFactory( false /* at_start */, HANDSHAKER_CLIENT, - MakeUnique()); + grpc_core::MakeUnique()); HandshakerRegistry::RegisterHandshakerFactory( false /* at_start */, HANDSHAKER_SERVER, - MakeUnique()); + grpc_core::MakeUnique()); } } // namespace grpc_core diff --git a/src/cpp/common/completion_queue_cc.cc b/src/cpp/common/completion_queue_cc.cc index 43c2eee96f8..93f5f4c4c0f 100644 --- a/src/cpp/common/completion_queue_cc.cc +++ b/src/cpp/common/completion_queue_cc.cc @@ -39,6 +39,12 @@ CompletionQueue::CompletionQueue(grpc_completion_queue* take) void CompletionQueue::Shutdown() { g_gli_initializer.summon(); +#ifndef NDEBUG + if (!ServerListEmpty()) { + gpr_log(GPR_ERROR, + "CompletionQueue shutdown being shutdown before its server."); + } +#endif CompleteAvalanching(); } diff --git a/src/cpp/common/tls_credentials_options.cc b/src/cpp/common/tls_credentials_options.cc index 1c65ea66d89..d5692c6effd 100644 --- a/src/cpp/common/tls_credentials_options.cc +++ b/src/cpp/common/tls_credentials_options.cc @@ -16,18 +16,19 @@ * */ -#include #include #include "src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h" + +#include + #include "src/cpp/common/tls_credentials_options_util.h" namespace grpc_impl { namespace experimental { /** TLS key materials config API implementation **/ -void TlsKeyMaterialsConfig::set_pem_root_certs( - const grpc::string& pem_root_certs) { - pem_root_certs_ = pem_root_certs; +void TlsKeyMaterialsConfig::set_pem_root_certs(grpc::string pem_root_certs) { + pem_root_certs_ = std::move(pem_root_certs); } void TlsKeyMaterialsConfig::add_pem_key_cert_pair( @@ -36,10 +37,10 @@ void TlsKeyMaterialsConfig::add_pem_key_cert_pair( } void TlsKeyMaterialsConfig::set_key_materials( - const grpc::string& pem_root_certs, - const std::vector& pem_key_cert_pair_list) { - pem_key_cert_pair_list_ = pem_key_cert_pair_list; - pem_root_certs_ = pem_root_certs; + grpc::string pem_root_certs, + std::vector pem_key_cert_pair_list) { + pem_key_cert_pair_list_ = std::move(pem_key_cert_pair_list); + pem_root_certs_ = std::move(pem_root_certs); } /** TLS credential reload arg API implementation **/ @@ -58,6 +59,7 @@ TlsCredentialReloadArg::~TlsCredentialReloadArg() {} void* TlsCredentialReloadArg::cb_user_data() const { return c_arg_->cb_user_data; } + bool TlsCredentialReloadArg::is_pem_key_cert_pair_list_empty() const { return c_arg_->key_materials_config->pem_key_cert_pair_list().empty(); } @@ -83,46 +85,17 @@ void TlsCredentialReloadArg::set_pem_root_certs( c_arg_->key_materials_config->set_pem_root_certs(std::move(c_pem_root_certs)); } -namespace { - -::grpc_core::PemKeyCertPair ConvertToCorePemKeyCertPair( - const TlsKeyMaterialsConfig::PemKeyCertPair& pem_key_cert_pair) { +void TlsCredentialReloadArg::add_pem_key_cert_pair( + TlsKeyMaterialsConfig::PemKeyCertPair pem_key_cert_pair) { grpc_ssl_pem_key_cert_pair* ssl_pair = (grpc_ssl_pem_key_cert_pair*)gpr_malloc( sizeof(grpc_ssl_pem_key_cert_pair)); ssl_pair->private_key = gpr_strdup(pem_key_cert_pair.private_key.c_str()); ssl_pair->cert_chain = gpr_strdup(pem_key_cert_pair.cert_chain.c_str()); - return ::grpc_core::PemKeyCertPair(ssl_pair); -} - -} // namespace - -void TlsCredentialReloadArg::add_pem_key_cert_pair( - const TlsKeyMaterialsConfig::PemKeyCertPair& pem_key_cert_pair) { + ::grpc_core::PemKeyCertPair c_pem_key_cert_pair = + ::grpc_core::PemKeyCertPair(ssl_pair); c_arg_->key_materials_config->add_pem_key_cert_pair( - ConvertToCorePemKeyCertPair(pem_key_cert_pair)); -} - -void TlsCredentialReloadArg::set_key_materials( - const grpc::string& pem_root_certs, - std::vector pem_key_cert_pair_list) { - /** Initialize the |key_materials_config| field of |c_arg_|, if it has not - * already been done. **/ - if (c_arg_->key_materials_config == nullptr) { - c_arg_->key_materials_config = grpc_tls_key_materials_config_create(); - } - /** Convert |pem_key_cert_pair_list| to an inlined vector of ssl pairs. **/ - ::grpc_core::InlinedVector<::grpc_core::PemKeyCertPair, 1> - c_pem_key_cert_pair_list; - for (const auto& key_cert_pair : pem_key_cert_pair_list) { - c_pem_key_cert_pair_list.emplace_back( - ConvertToCorePemKeyCertPair(key_cert_pair)); - } - /** Populate the key materials config field of |c_arg_|. **/ - ::grpc_core::UniquePtr c_pem_root_certs( - gpr_strdup(pem_root_certs.c_str())); - c_arg_->key_materials_config->set_key_materials(std::move(c_pem_root_certs), - c_pem_key_cert_pair_list); + std::move(c_pem_key_cert_pair)); } void TlsCredentialReloadArg::set_key_materials_config( @@ -315,11 +288,6 @@ TlsCredentialsOptions::TlsCredentialsOptions( c_credentials_options_, server_verification_option); } -/** Whenever a TlsCredentialsOptions instance is created, the caller takes - * ownership of the c_credentials_options_ pointer (see e.g. the implementation - * of the TlsCredentials API in secure_credentials.cc). For this reason, the - * TlsCredentialsOptions destructor is not responsible for freeing - * c_credentials_options_. **/ TlsCredentialsOptions::~TlsCredentialsOptions() {} } // namespace experimental diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index 71f17da0a4b..d5e93476532 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -354,9 +354,8 @@ std::unique_ptr ServerBuilder::BuildAndStart() { // server // 2. cqs_: Completion queues added via AddCompletionQueue() call - for (const auto& value : *sync_server_cqs) { - grpc_server_register_completion_queue(server->server_, value->cq(), - nullptr); + for (const auto& cq : *sync_server_cqs) { + grpc_server_register_completion_queue(server->server_, cq->cq(), nullptr); has_frequently_polled_cqs = true; } @@ -369,10 +368,12 @@ std::unique_ptr ServerBuilder::BuildAndStart() { // AddCompletionQueue() API. Some of them may not be frequently polled (i.e by // calling Next() or AsyncNext()) and hence are not safe to be used for // listening to incoming channels. Such completion queues must be registered - // as non-listening queues - for (const auto& value : cqs_) { - grpc_server_register_completion_queue(server->server_, value->cq(), - nullptr); + // as non-listening queues. In debug mode, these should have their server list + // tracked since these are provided the user and must be Shutdown by the user + // after the server is shutdown. + for (const auto& cq : cqs_) { + grpc_server_register_completion_queue(server->server_, cq->cq(), nullptr); + cq->RegisterServer(server.get()); } if (!has_frequently_polled_cqs) { diff --git a/src/cpp/server/server_cc.cc b/src/cpp/server/server_cc.cc index 5367fb25ebb..34ffd59489f 100644 --- a/src/cpp/server/server_cc.cc +++ b/src/cpp/server/server_cc.cc @@ -1249,6 +1249,9 @@ void Server::Start(grpc::ServerCompletionQueue** cqs, size_t num_cqs) { } for (size_t i = 0; i < num_cqs; i++) { +#ifndef NDEBUG + cq_list_.push_back(cqs[i]); +#endif if (cqs[i]->IsFrequentlyPolled()) { new UnimplementedAsyncRequest(this, cqs[i]); } @@ -1360,6 +1363,15 @@ void Server::ShutdownInternal(gpr_timespec deadline) { shutdown_notified_ = true; shutdown_cv_.Broadcast(); + +#ifndef NDEBUG + // Unregister this server with the CQs passed into it by the user so that + // those can be checked for properly-ordered shutdown. + for (auto* cq : cq_list_) { + cq->UnregisterServer(this); + } + cq_list_.clear(); +#endif } void Server::Wait() { diff --git a/test/core/channel/channelz_test.cc b/test/core/channel/channelz_test.cc index 34fa53d9ca5..3f42d9fc81a 100644 --- a/test/core/channel/channelz_test.cc +++ b/test/core/channel/channelz_test.cc @@ -464,8 +464,8 @@ TEST_F(ChannelzRegistryBasedTest, GetTopChannelsUuidAfterCompaction) { // these will delete and unregister themselves after this block. std::vector> odd_channels; for (int i = 0; i < kLoopIterations; i++) { - odd_channels.push_back(MakeUnique()); - even_channels.push_back(MakeUnique()); + odd_channels.push_back(grpc_core::MakeUnique()); + even_channels.push_back(grpc_core::MakeUnique()); } } char* json_str = ChannelzRegistry::GetTopChannels(0); diff --git a/test/core/client_channel/service_config_test.cc b/test/core/client_channel/service_config_test.cc index cca72709fbf..0cb23bda44f 100644 --- a/test/core/client_channel/service_config_test.cc +++ b/test/core/client_channel/service_config_test.cc @@ -60,7 +60,7 @@ class TestParser1 : public ServiceConfig::Parser { GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidValueErrorMessage()); return nullptr; } - return MakeUnique(value); + return grpc_core::MakeUnique(value); } } return nullptr; @@ -97,7 +97,7 @@ class TestParser2 : public ServiceConfig::Parser { GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidValueErrorMessage()); return nullptr; } - return MakeUnique(value); + return grpc_core::MakeUnique(value); } } return nullptr; @@ -146,8 +146,10 @@ class ServiceConfigTest : public ::testing::Test { void SetUp() override { ServiceConfig::Shutdown(); ServiceConfig::Init(); - EXPECT_TRUE(ServiceConfig::RegisterParser(MakeUnique()) == 0); - EXPECT_TRUE(ServiceConfig::RegisterParser(MakeUnique()) == 1); + EXPECT_TRUE(ServiceConfig::RegisterParser( + grpc_core::MakeUnique()) == 0); + EXPECT_TRUE(ServiceConfig::RegisterParser( + grpc_core::MakeUnique()) == 1); } }; @@ -308,8 +310,10 @@ class ErroredParsersScopingTest : public ::testing::Test { void SetUp() override { ServiceConfig::Shutdown(); ServiceConfig::Init(); - EXPECT_TRUE(ServiceConfig::RegisterParser(MakeUnique()) == 0); - EXPECT_TRUE(ServiceConfig::RegisterParser(MakeUnique()) == 1); + EXPECT_TRUE(ServiceConfig::RegisterParser( + grpc_core::MakeUnique()) == 0); + EXPECT_TRUE(ServiceConfig::RegisterParser( + grpc_core::MakeUnique()) == 1); } }; @@ -354,8 +358,8 @@ class ClientChannelParserTest : public ::testing::Test { ServiceConfig::Shutdown(); ServiceConfig::Init(); EXPECT_TRUE(ServiceConfig::RegisterParser( - MakeUnique()) == - 0); + grpc_core::MakeUnique< + internal::ClientChannelServiceConfigParser>()) == 0); } }; @@ -922,8 +926,8 @@ class MessageSizeParserTest : public ::testing::Test { void SetUp() override { ServiceConfig::Shutdown(); ServiceConfig::Init(); - EXPECT_TRUE( - ServiceConfig::RegisterParser(MakeUnique()) == 0); + EXPECT_TRUE(ServiceConfig::RegisterParser( + grpc_core::MakeUnique()) == 0); } }; diff --git a/test/core/gprpp/inlined_vector_test.cc b/test/core/gprpp/inlined_vector_test.cc index 34e7d1443eb..9ad2bfad49a 100644 --- a/test/core/gprpp/inlined_vector_test.cc +++ b/test/core/gprpp/inlined_vector_test.cc @@ -63,7 +63,7 @@ TEST(InlinedVectorTest, ValuesAreInlined) { TEST(InlinedVectorTest, PushBackWithMove) { InlinedVector, 1> v; - std::unique_ptr i = MakeUnique(3); + std::unique_ptr i = grpc_core::MakeUnique(3); v.push_back(std::move(i)); EXPECT_EQ(nullptr, i.get()); EXPECT_EQ(1UL, v.size()); @@ -304,15 +304,15 @@ TEST(InlinedVectorTest, MoveAssignmentAllocatedAllocated) { // and move methods are called correctly. class Value { public: - explicit Value(int v) : value_(MakeUnique(v)) {} + explicit Value(int v) : value_(grpc_core::MakeUnique(v)) {} // copyable Value(const Value& v) { - value_ = MakeUnique(*v.value_); + value_ = grpc_core::MakeUnique(*v.value_); copied_ = true; } Value& operator=(const Value& v) { - value_ = MakeUnique(*v.value_); + value_ = grpc_core::MakeUnique(*v.value_); copied_ = true; return *this; } @@ -463,10 +463,10 @@ TEST(InlinedVectorTest, MoveAssignmentMovesElementsAllocated) { TEST(InlinedVectorTest, PopBackInlined) { InlinedVector, 2> v; // Add two elements, pop one out - v.push_back(MakeUnique(3)); + v.push_back(grpc_core::MakeUnique(3)); EXPECT_EQ(1UL, v.size()); EXPECT_EQ(3, *v[0]); - v.push_back(MakeUnique(5)); + v.push_back(grpc_core::MakeUnique(5)); EXPECT_EQ(2UL, v.size()); EXPECT_EQ(5, *v[1]); v.pop_back(); @@ -478,7 +478,7 @@ TEST(InlinedVectorTest, PopBackAllocated) { InlinedVector, kInlinedSize> v; // Add elements to ensure allocated backing. for (size_t i = 0; i < kInlinedSize + 1; ++i) { - v.push_back(MakeUnique(3)); + v.push_back(grpc_core::MakeUnique(3)); EXPECT_EQ(i + 1, v.size()); } size_t sz = v.size(); @@ -494,7 +494,7 @@ TEST(InlinedVectorTest, Resize) { EXPECT_EQ(5UL, v.size()); EXPECT_EQ(nullptr, v[4]); // Size down. - v[4] = MakeUnique(5); + v[4] = grpc_core::MakeUnique(5); v.resize(1); EXPECT_EQ(1UL, v.size()); } diff --git a/test/core/handshake/readahead_handshaker_server_ssl.cc b/test/core/handshake/readahead_handshaker_server_ssl.cc index af9cdade173..7b3b8393442 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, - MakeUnique()); + grpc_core::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/cpp/client/credentials_test.cc b/test/cpp/client/credentials_test.cc index 43bc83636b6..c182e1c52b9 100644 --- a/test/cpp/client/credentials_test.cc +++ b/test/cpp/client/credentials_test.cc @@ -17,7 +17,6 @@ */ #include -#include #include #include @@ -54,10 +53,10 @@ static void tls_credential_reload_callback( class TestTlsCredentialReload : public TlsCredentialReloadInterface { int Schedule(TlsCredentialReloadArg* arg) override { GPR_ASSERT(arg != nullptr); - TlsKeyMaterialsConfig::PemKeyCertPair pair = {"private_key3", - "cert_chain3"}; + struct TlsKeyMaterialsConfig::PemKeyCertPair pair3 = {"private_key3", + "cert_chain3"}; arg->set_pem_root_certs("new_pem_root_certs"); - arg->add_pem_key_cert_pair(pair); + arg->add_pem_key_cert_pair(pair3); arg->set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); return 0; } @@ -101,6 +100,7 @@ class TestTlsServerAuthorizationCheck arg->set_error_details("cancelled"); } }; + } // namespace namespace grpc { @@ -293,7 +293,8 @@ TEST_F(CredentialsTest, TlsKeyMaterialsConfigCppToC) { TEST_F(CredentialsTest, TlsKeyMaterialsModifiers) { std::shared_ptr config(new TlsKeyMaterialsConfig()); - TlsKeyMaterialsConfig::PemKeyCertPair pair = {"private_key", "cert_chain"}; + struct TlsKeyMaterialsConfig::PemKeyCertPair pair = {"private_key", + "cert_chain"}; config->add_pem_key_cert_pair(pair); config->set_pem_root_certs("pem_root_certs"); EXPECT_STREQ(config->pem_root_certs().c_str(), "pem_root_certs"); @@ -311,28 +312,15 @@ typedef class ::grpc_impl::experimental::TlsCredentialReloadConfig TEST_F(CredentialsTest, TlsCredentialReloadArgCallback) { grpc_tls_credential_reload_arg* c_arg = new grpc_tls_credential_reload_arg; - c_arg->key_materials_config = grpc_tls_key_materials_config_create(); c_arg->cb = tls_credential_reload_callback; c_arg->context = nullptr; TlsCredentialReloadArg* arg = new TlsCredentialReloadArg(c_arg); - arg->set_pem_root_certs("pem_root_certs"); - TlsKeyMaterialsConfig::PemKeyCertPair pair = {"private_key", "cert_chain"}; - arg->add_pem_key_cert_pair(pair); arg->set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); arg->OnCredentialReloadDoneCallback(); EXPECT_EQ(arg->status(), GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED); - EXPECT_STREQ(c_arg->key_materials_config->pem_root_certs(), "pem_root_certs"); - EXPECT_EQ(c_arg->key_materials_config->pem_key_cert_pair_list().size(), 1); - EXPECT_STREQ( - c_arg->key_materials_config->pem_key_cert_pair_list()[0].private_key(), - "private_key"); - EXPECT_STREQ( - c_arg->key_materials_config->pem_key_cert_pair_list()[0].cert_chain(), - "cert_chain"); // Cleanup. delete arg; - delete c_arg->key_materials_config; delete c_arg; } @@ -344,12 +332,15 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) { grpc_tls_credential_reload_arg* c_arg = new grpc_tls_credential_reload_arg(); c_arg->context = nullptr; TlsCredentialReloadArg* arg = new TlsCredentialReloadArg(c_arg); + std::shared_ptr key_materials_config( + new TlsKeyMaterialsConfig()); struct TlsKeyMaterialsConfig::PemKeyCertPair pair1 = {"private_key1", "cert_chain1"}; struct TlsKeyMaterialsConfig::PemKeyCertPair pair2 = {"private_key2", "cert_chain2"}; std::vector pair_list = {pair1, pair2}; - arg->set_key_materials("pem_root_certs", pair_list); + key_materials_config->set_key_materials("pem_root_certs", pair_list); + arg->set_key_materials_config(key_materials_config); arg->set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); arg->set_error_details("error_details"); const char* error_details_before_schedule = c_arg->error_details; @@ -657,7 +648,7 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { delete c_options; } -// This test demonstrates how the TLS credentials will be used. +// This test demonstrates how the SPIFFE credentials will be used. TEST_F(CredentialsTest, LoadTlsChannelCredentials) { std::shared_ptr test_credential_reload( new TestTlsCredentialReload()); diff --git a/test/cpp/microbenchmarks/fullstack_fixtures.h b/test/cpp/microbenchmarks/fullstack_fixtures.h index fb56569e0cc..deee5909ca0 100644 --- a/test/cpp/microbenchmarks/fullstack_fixtures.h +++ b/test/cpp/microbenchmarks/fullstack_fixtures.h @@ -62,15 +62,6 @@ class FixtureConfiguration { class BaseFixture : public TrackCounters {}; -// Special tag to be used in Server shutdown. This tag is *NEVER* returned when -// Cq->Next() API is called (This is because FinalizeResult() function in this -// class always returns 'false'). This is intentional and makes writing shutdown -// code easier. -class ShutdownTag : public internal::CompletionQueueTag { - public: - bool FinalizeResult(void** /*tag*/, bool* /*status*/) { return false; } -}; - class FullstackFixture : public BaseFixture { public: FullstackFixture(Service* service, const FixtureConfiguration& config, @@ -94,11 +85,7 @@ class FullstackFixture : public BaseFixture { } virtual ~FullstackFixture() { - // Dummy shutdown tag (this tag is swallowed by cq->Next() and is not - // returned to the user) see ShutdownTag definition for more details - ShutdownTag shutdown_tag; - grpc_server_shutdown_and_notify(server_->c_server(), cq_->cq(), - &shutdown_tag); + server_->Shutdown(); cq_->Shutdown(); void* tag; bool ok; @@ -226,11 +213,7 @@ class EndpointPairFixture : public BaseFixture { } virtual ~EndpointPairFixture() { - // Dummy shutdown tag (this tag is swallowed by cq->Next() and is not - // returned to the user) see ShutdownTag definition for more details - ShutdownTag shutdown_tag; - grpc_server_shutdown_and_notify(server_->c_server(), cq_->cq(), - &shutdown_tag); + server_->Shutdown(); cq_->Shutdown(); void* tag; bool ok; diff --git a/tools/gce/linux_kokoro_performance_worker_init.sh b/tools/gce/linux_kokoro_performance_worker_init.sh index b0c7263fc2d..9c9ce39ba5a 100755 --- a/tools/gce/linux_kokoro_performance_worker_init.sh +++ b/tools/gce/linux_kokoro_performance_worker_init.sh @@ -140,7 +140,7 @@ tar zxf dotnet_old.tar.gz -C dotnet_old sudo cp -r dotnet_old/shared/Microsoft.NETCore.App/1.1.10/ /usr/share/dotnet/shared/Microsoft.NETCore.App/ # Ruby dependencies -gpg --keyserver hkp://keys.gnupg.net --recv-keys 409B6B1796C275462A1703113804BB82D39DC0E3 +gpg --keyserver hkp://pgp.mit.edu --recv-keys 409B6B1796C275462A1703113804BB82D39DC0E3 7D2BAF1CF37B13E2069D6956105BD0E739499BDB curl -sSL https://get.rvm.io | bash -s stable --ruby # silence shellcheck as it cannot follow the following `source` path statically: # shellcheck disable=SC1090