Move handshakers to core configuration system (#27195)

* Move handshakers to core configuration

* Automated change: Fix sanity tests

* fix memory ordering

* Update http_connect_handshaker.h

* slightly simpler code

* clangfmt

* review feedback

* review feedback

Co-authored-by: ctiller <ctiller@users.noreply.github.com>

BUILD

@@ -801,6 +801,7 @@ grpc_cc_library(
     ],
     deps = [
         "gpr_platform",
+        "handshaker_registry",
     ],
 )
 
@@ -1209,6 +1210,32 @@ grpc_cc_library(
     ],
 )
 
+grpc_cc_library(
+    name = "handshaker_factory",
+    language = "c++",
+    public_hdrs = [
+        "src/core/lib/channel/handshaker_factory.h",
+    ],
+    deps = [
+        "gpr_base",
+    ],
+)
+
+grpc_cc_library(
+    name = "handshaker_registry",
+    srcs = [
+        "src/core/lib/channel/handshaker_registry.cc",
+    ],
+    language = "c++",
+    public_hdrs = [
+        "src/core/lib/channel/handshaker_registry.h",
+    ],
+    deps = [
+        "gpr_base",
+        "handshaker_factory",
+    ],
+)
+
 grpc_cc_library(
     name = "grpc_base_c",
     srcs = [
@@ -1224,7 +1251,6 @@ grpc_cc_library(
         "src/core/lib/channel/channelz_registry.cc",
         "src/core/lib/channel/connected_channel.cc",
         "src/core/lib/channel/handshaker.cc",
-        "src/core/lib/channel/handshaker_registry.cc",
         "src/core/lib/channel/status_util.cc",
         "src/core/lib/compression/compression.cc",
         "src/core/lib/compression/compression_args.cc",
@@ -1394,8 +1420,6 @@ grpc_cc_library(
         "src/core/lib/channel/connected_channel.h",
         "src/core/lib/channel/context.h",
         "src/core/lib/channel/handshaker.h",
-        "src/core/lib/channel/handshaker_factory.h",
-        "src/core/lib/channel/handshaker_registry.h",
         "src/core/lib/channel/status_util.h",
         "src/core/lib/compression/algorithm_metadata.h",
         "src/core/lib/compression/compression_args.h",
@@ -1678,6 +1702,7 @@ grpc_cc_library(
     language = "c++",
     visibility = ["@grpc:client_channel"],
     deps = [
+        "config",
         "debug_location",
         "gpr_base",
         "grpc_base_c",
@@ -1685,6 +1710,7 @@ grpc_cc_library(
         "grpc_deadline_filter",
         "grpc_health_upb",
         "grpc_trace",
+        "handshaker_registry",
         "orphanable",
         "ref_counted",
         "ref_counted_ptr",
@@ -2668,6 +2694,7 @@ grpc_cc_library(
     visibility = ["@grpc:public"],
     deps = [
         "alts_util",
+        "config",
         "gpr_base",
         "grpc_base",
         "grpc_base_c",
@@ -2972,6 +2999,7 @@ grpc_cc_library(
     ],
     language = "c++",
     deps = [
+        "config",
         "gpr_base",
         "grpc_base_c",
         "grpc_client_channel",
@@ -3025,6 +3053,7 @@ grpc_cc_library(
     ],
     language = "c++",
     deps = [
+        "config",
         "gpr_base",
         "grpc_base_c",
         "grpc_codegen",

CMakeLists.txt

@@ -10419,7 +10419,64 @@ endif()
 if(gRPC_BUILD_TESTS)
 
 add_executable(core_configuration_test
+  src/core/ext/upb-generated/google/api/annotations.upb.c
+  src/core/ext/upb-generated/google/api/expr/v1alpha1/checked.upb.c
+  src/core/ext/upb-generated/google/api/expr/v1alpha1/syntax.upb.c
+  src/core/ext/upb-generated/google/api/http.upb.c
+  src/core/ext/upb-generated/google/protobuf/any.upb.c
+  src/core/ext/upb-generated/google/protobuf/duration.upb.c
+  src/core/ext/upb-generated/google/protobuf/empty.upb.c
+  src/core/ext/upb-generated/google/protobuf/struct.upb.c
+  src/core/ext/upb-generated/google/protobuf/timestamp.upb.c
+  src/core/ext/upb-generated/google/protobuf/wrappers.upb.c
+  src/core/ext/upb-generated/google/rpc/status.upb.c
+  src/core/lib/channel/handshaker_registry.cc
   src/core/lib/config/core_configuration.cc
+  src/core/lib/gpr/alloc.cc
+  src/core/lib/gpr/atm.cc
+  src/core/lib/gpr/cpu_iphone.cc
+  src/core/lib/gpr/cpu_linux.cc
+  src/core/lib/gpr/cpu_posix.cc
+  src/core/lib/gpr/cpu_windows.cc
+  src/core/lib/gpr/env_linux.cc
+  src/core/lib/gpr/env_posix.cc
+  src/core/lib/gpr/env_windows.cc
+  src/core/lib/gpr/log.cc
+  src/core/lib/gpr/log_android.cc
+  src/core/lib/gpr/log_linux.cc
+  src/core/lib/gpr/log_posix.cc
+  src/core/lib/gpr/log_windows.cc
+  src/core/lib/gpr/murmur_hash.cc
+  src/core/lib/gpr/string.cc
+  src/core/lib/gpr/string_posix.cc
+  src/core/lib/gpr/string_util_windows.cc
+  src/core/lib/gpr/string_windows.cc
+  src/core/lib/gpr/sync.cc
+  src/core/lib/gpr/sync_abseil.cc
+  src/core/lib/gpr/sync_posix.cc
+  src/core/lib/gpr/sync_windows.cc
+  src/core/lib/gpr/time.cc
+  src/core/lib/gpr/time_posix.cc
+  src/core/lib/gpr/time_precise.cc
+  src/core/lib/gpr/time_windows.cc
+  src/core/lib/gpr/tmpfile_msys.cc
+  src/core/lib/gpr/tmpfile_posix.cc
+  src/core/lib/gpr/tmpfile_windows.cc
+  src/core/lib/gpr/wrap_memcpy.cc
+  src/core/lib/gprpp/arena.cc
+  src/core/lib/gprpp/examine_stack.cc
+  src/core/lib/gprpp/fork.cc
+  src/core/lib/gprpp/global_config_env.cc
+  src/core/lib/gprpp/host_port.cc
+  src/core/lib/gprpp/mpscq.cc
+  src/core/lib/gprpp/stat_posix.cc
+  src/core/lib/gprpp/stat_windows.cc
+  src/core/lib/gprpp/status_helper.cc
+  src/core/lib/gprpp/thd_posix.cc
+  src/core/lib/gprpp/thd_windows.cc
+  src/core/lib/gprpp/time_util.cc
+  src/core/lib/profiling/basic_timers.cc
+  src/core/lib/profiling/stap_timers.cc
   test/core/config/core_configuration_test.cc
   third_party/googletest/googletest/src/gtest-all.cc
   third_party/googletest/googlemock/src/gmock-all.cc
@@ -10447,6 +10504,17 @@ target_include_directories(core_configuration_test
 target_link_libraries(core_configuration_test
   ${_gRPC_PROTOBUF_LIBRARIES}
   ${_gRPC_ALLTARGETS_LIBRARIES}
+  absl::base
+  absl::core_headers
+  absl::memory
+  absl::status
+  absl::cord
+  absl::str_format
+  absl::strings
+  absl::synchronization
+  absl::time
+  absl::optional
+  upb
 )
 
 

build_autogenerated.yaml

@@ -5333,11 +5333,121 @@ targets:
   build: test
   language: c++
   headers:
+  - src/core/ext/upb-generated/google/api/annotations.upb.h
+  - src/core/ext/upb-generated/google/api/expr/v1alpha1/checked.upb.h
+  - src/core/ext/upb-generated/google/api/expr/v1alpha1/syntax.upb.h
+  - src/core/ext/upb-generated/google/api/http.upb.h
+  - src/core/ext/upb-generated/google/protobuf/any.upb.h
+  - src/core/ext/upb-generated/google/protobuf/duration.upb.h
+  - src/core/ext/upb-generated/google/protobuf/empty.upb.h
+  - src/core/ext/upb-generated/google/protobuf/struct.upb.h
+  - src/core/ext/upb-generated/google/protobuf/timestamp.upb.h
+  - src/core/ext/upb-generated/google/protobuf/wrappers.upb.h
+  - src/core/ext/upb-generated/google/rpc/status.upb.h
+  - src/core/lib/channel/handshaker_factory.h
+  - src/core/lib/channel/handshaker_registry.h
   - src/core/lib/config/core_configuration.h
+  - src/core/lib/gpr/alloc.h
+  - src/core/lib/gpr/env.h
+  - src/core/lib/gpr/murmur_hash.h
+  - src/core/lib/gpr/spinlock.h
+  - src/core/lib/gpr/string.h
+  - src/core/lib/gpr/string_windows.h
+  - src/core/lib/gpr/time_precise.h
+  - src/core/lib/gpr/tls.h
+  - src/core/lib/gpr/tmpfile.h
+  - src/core/lib/gpr/useful.h
+  - src/core/lib/gprpp/arena.h
+  - src/core/lib/gprpp/construct_destruct.h
+  - src/core/lib/gprpp/debug_location.h
+  - src/core/lib/gprpp/examine_stack.h
+  - src/core/lib/gprpp/fork.h
+  - src/core/lib/gprpp/global_config.h
+  - src/core/lib/gprpp/global_config_custom.h
+  - src/core/lib/gprpp/global_config_env.h
+  - src/core/lib/gprpp/global_config_generic.h
+  - src/core/lib/gprpp/host_port.h
+  - src/core/lib/gprpp/manual_constructor.h
+  - src/core/lib/gprpp/memory.h
+  - src/core/lib/gprpp/mpscq.h
+  - src/core/lib/gprpp/stat.h
+  - src/core/lib/gprpp/status_helper.h
+  - src/core/lib/gprpp/sync.h
+  - src/core/lib/gprpp/thd.h
+  - src/core/lib/gprpp/time_util.h
+  - src/core/lib/profiling/timers.h
   src:
+  - src/core/ext/upb-generated/google/api/annotations.upb.c
+  - src/core/ext/upb-generated/google/api/expr/v1alpha1/checked.upb.c
+  - src/core/ext/upb-generated/google/api/expr/v1alpha1/syntax.upb.c
+  - src/core/ext/upb-generated/google/api/http.upb.c
+  - src/core/ext/upb-generated/google/protobuf/any.upb.c
+  - src/core/ext/upb-generated/google/protobuf/duration.upb.c
+  - src/core/ext/upb-generated/google/protobuf/empty.upb.c
+  - src/core/ext/upb-generated/google/protobuf/struct.upb.c
+  - src/core/ext/upb-generated/google/protobuf/timestamp.upb.c
+  - src/core/ext/upb-generated/google/protobuf/wrappers.upb.c
+  - src/core/ext/upb-generated/google/rpc/status.upb.c
+  - src/core/lib/channel/handshaker_registry.cc
   - src/core/lib/config/core_configuration.cc
+  - src/core/lib/gpr/alloc.cc
+  - src/core/lib/gpr/atm.cc
+  - src/core/lib/gpr/cpu_iphone.cc
+  - src/core/lib/gpr/cpu_linux.cc
+  - src/core/lib/gpr/cpu_posix.cc
+  - src/core/lib/gpr/cpu_windows.cc
+  - src/core/lib/gpr/env_linux.cc
+  - src/core/lib/gpr/env_posix.cc
+  - src/core/lib/gpr/env_windows.cc
+  - src/core/lib/gpr/log.cc
+  - src/core/lib/gpr/log_android.cc
+  - src/core/lib/gpr/log_linux.cc
+  - src/core/lib/gpr/log_posix.cc
+  - src/core/lib/gpr/log_windows.cc
+  - src/core/lib/gpr/murmur_hash.cc
+  - src/core/lib/gpr/string.cc
+  - src/core/lib/gpr/string_posix.cc
+  - src/core/lib/gpr/string_util_windows.cc
+  - src/core/lib/gpr/string_windows.cc
+  - src/core/lib/gpr/sync.cc
+  - src/core/lib/gpr/sync_abseil.cc
+  - src/core/lib/gpr/sync_posix.cc
+  - src/core/lib/gpr/sync_windows.cc
+  - src/core/lib/gpr/time.cc
+  - src/core/lib/gpr/time_posix.cc
+  - src/core/lib/gpr/time_precise.cc
+  - src/core/lib/gpr/time_windows.cc
+  - src/core/lib/gpr/tmpfile_msys.cc
+  - src/core/lib/gpr/tmpfile_posix.cc
+  - src/core/lib/gpr/tmpfile_windows.cc
+  - src/core/lib/gpr/wrap_memcpy.cc
+  - src/core/lib/gprpp/arena.cc
+  - src/core/lib/gprpp/examine_stack.cc
+  - src/core/lib/gprpp/fork.cc
+  - src/core/lib/gprpp/global_config_env.cc
+  - src/core/lib/gprpp/host_port.cc
+  - src/core/lib/gprpp/mpscq.cc
+  - src/core/lib/gprpp/stat_posix.cc
+  - src/core/lib/gprpp/stat_windows.cc
+  - src/core/lib/gprpp/status_helper.cc
+  - src/core/lib/gprpp/thd_posix.cc
+  - src/core/lib/gprpp/thd_windows.cc
+  - src/core/lib/gprpp/time_util.cc
+  - src/core/lib/profiling/basic_timers.cc
+  - src/core/lib/profiling/stap_timers.cc
   - test/core/config/core_configuration_test.cc
-  deps: []
+  deps:
+  - absl/base:base
+  - absl/base:core_headers
+  - absl/memory:memory
+  - absl/status:status
+  - absl/strings:cord
+  - absl/strings:str_format
+  - absl/strings:strings
+  - absl/synchronization:synchronization
+  - absl/time:time
+  - absl/types:optional
+  - upb
   uses_polling: false
 - name: delegating_channel_test
   gtest: true

src/core/ext/filters/client_channel/client_channel_plugin.cc

@@ -58,7 +58,6 @@ void grpc_client_channel_init(void) {
       GRPC_CLIENT_CHANNEL, GRPC_CHANNEL_INIT_BUILTIN_PRIORITY, append_filter,
       const_cast<grpc_channel_filter*>(
           &grpc_core::ClientChannel::kFilterVtable));
-  grpc_http_connect_register_handshaker_factory();
   grpc_client_channel_global_init_backup_polling();
 }
 
@@ -71,3 +70,11 @@ void grpc_client_channel_shutdown(void) {
   grpc_core::LoadBalancingPolicyRegistry::Builder::ShutdownRegistry();
   grpc_core::ServiceConfigParser::Shutdown();
 }
+
+namespace grpc_core {
+
+void BuildClientChannelConfiguration(CoreConfiguration::Builder* builder) {
+  RegisterHttpConnectHandshaker(builder);
+}
+
+}  // namespace grpc_core

src/core/ext/filters/client_channel/http_connect_handshaker.cc

@@ -32,7 +32,9 @@
 #include "src/core/ext/filters/client_channel/client_channel.h"
 #include "src/core/ext/filters/client_channel/resolver_registry.h"
 #include "src/core/lib/channel/channel_args.h"
+#include "src/core/lib/channel/handshaker.h"
 #include "src/core/lib/channel/handshaker_registry.h"
+#include "src/core/lib/config/core_configuration.h"
 #include "src/core/lib/gpr/string.h"
 #include "src/core/lib/gprpp/sync.h"
 #include "src/core/lib/http/format_request.h"
@@ -382,10 +384,10 @@ class HttpConnectHandshakerFactory : public HandshakerFactory {
 
 }  // namespace
 
-}  // namespace grpc_core
+void RegisterHttpConnectHandshaker(CoreConfiguration::Builder* builder) {
-
+  builder->handshaker_registry()->RegisterHandshakerFactory(
-void grpc_http_connect_register_handshaker_factory() {
-  grpc_core::HandshakerRegistry::RegisterHandshakerFactory(
       true /* at_start */, grpc_core::HANDSHAKER_CLIENT,
       absl::make_unique<grpc_core::HttpConnectHandshakerFactory>());
 }
+
+}  // namespace grpc_core

src/core/ext/filters/client_channel/http_connect_handshaker.h

@@ -19,6 +19,10 @@
 #ifndef GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_HTTP_CONNECT_HANDSHAKER_H
 #define GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_HTTP_CONNECT_HANDSHAKER_H
 
+#include <grpc/impl/codegen/port_platform.h>
+
+#include "src/core/lib/config/core_configuration.h"
+
 /// Channel arg indicating the server in HTTP CONNECT request (string).
 /// The presence of this arg triggers the use of HTTP CONNECT.
 #define GRPC_ARG_HTTP_CONNECT_SERVER "grpc.http_connect_server"
@@ -28,7 +32,11 @@
 /// separated by colons.
 #define GRPC_ARG_HTTP_CONNECT_HEADERS "grpc.http_connect_headers"
 
-/// Registers handshaker factory.
+namespace grpc_core {
-void grpc_http_connect_register_handshaker_factory();
+
+// Register the HTTP Connect handshaker into the configuration builder.
+void RegisterHttpConnectHandshaker(CoreConfiguration::Builder* builder);
+
+}  // namespace grpc_core
 
 #endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_HTTP_CONNECT_HANDSHAKER_H */

src/core/ext/transport/chttp2/client/chttp2_connector.cc

@@ -28,13 +28,12 @@
 #include <grpc/support/string_util.h>
 
 #include "src/core/ext/filters/client_channel/connector.h"
-#include "src/core/ext/filters/client_channel/http_connect_handshaker.h"
 #include "src/core/ext/filters/client_channel/subchannel.h"
 #include "src/core/ext/transport/chttp2/transport/chttp2_transport.h"
 #include "src/core/lib/address_utils/sockaddr_utils.h"
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/channel/handshaker.h"
-#include "src/core/lib/channel/handshaker_registry.h"
+#include "src/core/lib/config/core_configuration.h"
 #include "src/core/lib/iomgr/tcp_client.h"
 #include "src/core/lib/slice/slice_internal.h"
 
@@ -134,8 +133,8 @@ void Chttp2Connector::Connected(void* arg, grpc_error_handle error) {
 
 void Chttp2Connector::StartHandshakeLocked() {
   handshake_mgr_ = MakeRefCounted<HandshakeManager>();
-  HandshakerRegistry::AddHandshakers(HANDSHAKER_CLIENT, args_.channel_args,
+  CoreConfiguration::Get().handshaker_registry().AddHandshakers(
-                                     args_.interested_parties,
+      HANDSHAKER_CLIENT, args_.channel_args, args_.interested_parties,
       handshake_mgr_.get());
   grpc_endpoint_add_to_pollset_set(endpoint_, args_.interested_parties);
   handshake_mgr_->DoHandshake(endpoint_, args_.channel_args, args_.deadline,

src/core/ext/transport/chttp2/server/chttp2_server.cc

@@ -42,7 +42,7 @@
 #include "src/core/lib/address_utils/sockaddr_utils.h"
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/channel/handshaker.h"
-#include "src/core/lib/channel/handshaker_registry.h"
+#include "src/core/lib/config/core_configuration.h"
 #include "src/core/lib/gprpp/ref_counted.h"
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/iomgr/endpoint.h"
@@ -320,8 +320,8 @@ Chttp2ServerListener::ActiveConnection::HandshakingState::HandshakingState(
       interested_parties_(grpc_pollset_set_create()),
       channel_resource_user_(channel_resource_user) {
   grpc_pollset_set_add_pollset(interested_parties_, accepting_pollset_);
-  HandshakerRegistry::AddHandshakers(HANDSHAKER_SERVER, args,
+  CoreConfiguration::Get().handshaker_registry().AddHandshakers(
-                                     interested_parties_, handshake_mgr_.get());
+      HANDSHAKER_SERVER, args, interested_parties_, handshake_mgr_.get());
 }
 
 Chttp2ServerListener::ActiveConnection::HandshakingState::~HandshakingState() {

src/core/lib/channel/handshaker_factory.h

@@ -23,12 +23,20 @@
 
 #include <grpc/impl/codegen/grpc_types.h>
 
-#include "src/core/lib/channel/handshaker.h"
-
 // A handshaker factory is used to create handshakers.
 
+// TODO(ctiller): grpc_pollset_set and HandshakeManager are forward declared in
+// this file. grpc_pollset_set ought to be eliminated when EventEngine lands IO
+// support. At the same time, we ought to be able to include handshake_manager.h
+// here and eliminate the HandshakeManager dependency - we cannot right now
+// because HandshakeManager names too many iomgr types.
+
+typedef struct grpc_pollset_set grpc_pollset_set;
+
 namespace grpc_core {
 
+class HandshakeManager;
+
 class HandshakerFactory {
  public:
   virtual void AddHandshakers(const grpc_channel_args* args,

src/core/lib/channel/handshaker_registry.cc

@@ -20,88 +20,31 @@
 
 #include "src/core/lib/channel/handshaker_registry.h"
 
-#include <string.h>
-
-#include <algorithm>
-
-#include "absl/container/inlined_vector.h"
-
-#include <grpc/support/alloc.h>
-
-#include "src/core/lib/gpr/alloc.h"
-#include "src/core/lib/gprpp/memory.h"
-
-//
-// grpc_handshaker_factory_list
-//
-
 namespace grpc_core {
 
-namespace {
+void HandshakerRegistry::Builder::RegisterHandshakerFactory(
-
+    bool at_start, HandshakerType handshaker_type,
-class HandshakerFactoryList {
+    std::unique_ptr<HandshakerFactory> factory) {
- public:
+  auto& vec = factories_[handshaker_type];
-  void Register(bool at_start, std::unique_ptr<HandshakerFactory> factory);
+  auto where = at_start ? vec.begin() : vec.end();
-  void AddHandshakers(const grpc_channel_args* args,
+  vec.insert(where, std::move(factory));
-                      grpc_pollset_set* interested_parties,
-                      HandshakeManager* handshake_mgr);
-
- private:
-  absl::InlinedVector<std::unique_ptr<HandshakerFactory>, 2> factories_;
-};
-
-HandshakerFactoryList* g_handshaker_factory_lists = nullptr;
-
-}  // namespace
-
-void HandshakerFactoryList::Register(
-    bool at_start, std::unique_ptr<HandshakerFactory> factory) {
-  factories_.push_back(std::move(factory));
-  if (at_start) {
-    auto* end = &factories_[factories_.size() - 1];
-    std::rotate(&factories_[0], end, end + 1);
-  }
-}
-
-void HandshakerFactoryList::AddHandshakers(const grpc_channel_args* args,
-                                           grpc_pollset_set* interested_parties,
-                                           HandshakeManager* handshake_mgr) {
-  for (size_t idx = 0; idx < factories_.size(); ++idx) {
-    auto& handshaker_factory = factories_[idx];
-    handshaker_factory->AddHandshakers(args, interested_parties, handshake_mgr);
-  }
-}
-
-//
-// plugin
-//
-
-void HandshakerRegistry::Init() {
-  GPR_ASSERT(g_handshaker_factory_lists == nullptr);
-  g_handshaker_factory_lists = new HandshakerFactoryList[NUM_HANDSHAKER_TYPES];
 }
 
-void HandshakerRegistry::Shutdown() {
+HandshakerRegistry HandshakerRegistry::Builder::Build() {
-  GPR_ASSERT(g_handshaker_factory_lists != nullptr);
+  HandshakerRegistry out;
-  delete[] g_handshaker_factory_lists;
+  for (size_t i = 0; i < NUM_HANDSHAKER_TYPES; i++) {
-  g_handshaker_factory_lists = nullptr;
+    out.factories_[i] = std::move(factories_[i]);
   }
-
+  return out;
-void HandshakerRegistry::RegisterHandshakerFactory(
-    bool at_start, HandshakerType handshaker_type,
-    std::unique_ptr<HandshakerFactory> factory) {
-  GPR_ASSERT(g_handshaker_factory_lists != nullptr);
-  auto& factory_list = g_handshaker_factory_lists[handshaker_type];
-  factory_list.Register(at_start, std::move(factory));
 }
 
 void HandshakerRegistry::AddHandshakers(HandshakerType handshaker_type,
                                         const grpc_channel_args* args,
                                         grpc_pollset_set* interested_parties,
-                                        HandshakeManager* handshake_mgr) {
+                                        HandshakeManager* handshake_mgr) const {
-  GPR_ASSERT(g_handshaker_factory_lists != nullptr);
+  for (const auto& factory : factories_[handshaker_type]) {
-  auto& factory_list = g_handshaker_factory_lists[handshaker_type];
+    factory->AddHandshakers(args, interested_parties, handshake_mgr);
-  factory_list.AddHandshakers(args, interested_parties, handshake_mgr);
+  }
 }
 
 }  // namespace grpc_core

src/core/lib/channel/handshaker_registry.h

@@ -21,6 +21,9 @@
 
 #include <grpc/support/port_platform.h>
 
+#include <memory>
+#include <vector>
+
 #include <grpc/impl/codegen/grpc_types.h>
 
 #include "src/core/lib/channel/handshaker_factory.h"
@@ -34,19 +37,33 @@ typedef enum {
 } HandshakerType;
 
 class HandshakerRegistry {
+ public:
+  class Builder {
    public:
     /// Registers a new handshaker factory.  Takes ownership.
     /// If \a at_start is true, the new handshaker will be at the beginning of
     /// the list.  Otherwise, it will be added to the end.
-  static void RegisterHandshakerFactory(
+    void RegisterHandshakerFactory(bool at_start,
-      bool at_start, HandshakerType handshaker_type,
+                                   HandshakerType handshaker_type,
                                    std::unique_ptr<HandshakerFactory> factory);
-  static void AddHandshakers(HandshakerType handshaker_type,
+
+    HandshakerRegistry Build();
+
+   private:
+    std::vector<std::unique_ptr<HandshakerFactory>>
+        factories_[NUM_HANDSHAKER_TYPES];
+  };
+
+  void AddHandshakers(HandshakerType handshaker_type,
                       const grpc_channel_args* args,
                       grpc_pollset_set* interested_parties,
-                             HandshakeManager* handshake_mgr);
+                      HandshakeManager* handshake_mgr) const;
-  static void Init();
+
-  static void Shutdown();
+ private:
+  HandshakerRegistry() = default;
+
+  std::vector<std::unique_ptr<HandshakerFactory>>
+      factories_[NUM_HANDSHAKER_TYPES];
 };
 
 }  // namespace grpc_core

src/core/lib/config/core_configuration.cc

@@ -23,10 +23,11 @@ std::atomic<CoreConfiguration*> CoreConfiguration::config_{nullptr};
 CoreConfiguration::Builder::Builder() = default;
 
 CoreConfiguration* CoreConfiguration::Builder::Build() {
-  return new CoreConfiguration;
+  return new CoreConfiguration(this);
 }
 
-CoreConfiguration::CoreConfiguration() = default;
+CoreConfiguration::CoreConfiguration(Builder* builder)
+    : handshaker_registry_(builder->handshaker_registry_.Build()) {}
 
 const CoreConfiguration& CoreConfiguration::BuildNewAndMaybeSet() {
   // Construct builder, pass it up to code that knows about build configuration
@@ -38,8 +39,8 @@ const CoreConfiguration& CoreConfiguration::BuildNewAndMaybeSet() {
   // here, in which case we drop the work we did and use the one that got set
   // first
   CoreConfiguration* expected = nullptr;
-  if (!config_.compare_exchange_strong(expected, p,
+  if (!config_.compare_exchange_strong(expected, p, std::memory_order_acq_rel,
-                                       std::memory_order_release)) {
+                                       std::memory_order_acquire)) {
     delete p;
     return *expected;
   }

src/core/lib/config/core_configuration.h

@@ -19,6 +19,8 @@
 
 #include <atomic>
 
+#include "src/core/lib/channel/handshaker_registry.h"
+
 namespace grpc_core {
 
 // Global singleton that stores library configuration - factories, etc...
@@ -32,9 +34,15 @@ class CoreConfiguration {
   // their configuration and assemble the published CoreConfiguration.
   class Builder {
    public:
+    HandshakerRegistry::Builder* handshaker_registry() {
+      return &handshaker_registry_;
+    }
+
    private:
     friend class CoreConfiguration;
 
+    HandshakerRegistry::Builder handshaker_registry_;
+
     Builder();
     CoreConfiguration* Build();
   };
@@ -50,14 +58,38 @@ class CoreConfiguration {
     return BuildNewAndMaybeSet();
   }
 
+  // Build a special core configuration.
+  // Requires no concurrent Get() be called.
+  // Doesn't call the regular BuildCoreConfiguration function, instead calls
+  // `build`.
+  // BuildFunc is a callable that takes a Builder* and returns void.
+  // We use a template instead of std::function<void(Builder*)> to avoid
+  // including std::function in this widely used header, and to ensure no code
+  // is generated in programs that do not use this function.
+  // This is sometimes useful for testing.
+  template <typename BuildFunc>
+  static void BuildSpecialConfiguration(BuildFunc build) {
+    // Build bespoke configuration
+    Builder builder;
+    build(&builder);
+    CoreConfiguration* p = builder.Build();
+    // Swap in final configuration, deleting anything that was already present.
+    delete config_.exchange(p, std::memory_order_release);
+  }
+
   // Drop the core configuration. Users must ensure no other threads are
   // accessing the configuration.
+  // Clears any dynamically registered builders.
   static void Reset();
 
   // Accessors
 
+  const HandshakerRegistry& handshaker_registry() const {
+    return handshaker_registry_;
+  }
+
  private:
-  CoreConfiguration();
+  explicit CoreConfiguration(Builder* builder);
 
   // Create a new CoreConfiguration, and either set it or throw it away.
   // We allow multiple CoreConfiguration's to be created in parallel.
@@ -65,6 +97,8 @@ class CoreConfiguration {
 
   // The configuration
   static std::atomic<CoreConfiguration*> config_;
+
+  HandshakerRegistry handshaker_registry_;
 };
 
 extern void BuildCoreConfiguration(CoreConfiguration::Builder* builder);

src/core/lib/http/httpcli_security_connector.cc

@@ -28,7 +28,7 @@
 #include <grpc/support/string_util.h>
 
 #include "src/core/lib/channel/channel_args.h"
-#include "src/core/lib/channel/handshaker_registry.h"
+#include "src/core/lib/config/core_configuration.h"
 #include "src/core/lib/gpr/string.h"
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/http/httpcli.h"
@@ -205,7 +205,7 @@ static void ssl_handshake(void* arg, grpc_endpoint* tcp, const char* host,
   grpc_arg channel_arg = grpc_security_connector_to_arg(sc.get());
   grpc_channel_args args = {1, &channel_arg};
   c->handshake_mgr = grpc_core::MakeRefCounted<grpc_core::HandshakeManager>();
-  grpc_core::HandshakerRegistry::AddHandshakers(
+  grpc_core::CoreConfiguration::Get().handshaker_registry().AddHandshakers(
       grpc_core::HANDSHAKER_CLIENT, &args,
       /*interested_parties=*/nullptr, c->handshake_mgr.get());
   c->handshake_mgr->DoHandshake(tcp, /*channel_args=*/nullptr, deadline,

src/core/lib/security/transport/security_handshaker.cc

@@ -32,7 +32,7 @@
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/channel/channelz.h"
 #include "src/core/lib/channel/handshaker.h"
-#include "src/core/lib/channel/handshaker_registry.h"
+#include "src/core/lib/config/core_configuration.h"
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/security/context/security_context.h"
 #include "src/core/lib/security/transport/secure_endpoint.h"
@@ -594,11 +594,11 @@ RefCountedPtr<Handshaker> SecurityHandshakerCreate(
   }
 }
 
-void SecurityRegisterHandshakerFactories() {
+void SecurityRegisterHandshakerFactories(CoreConfiguration::Builder* builder) {
-  HandshakerRegistry::RegisterHandshakerFactory(
+  builder->handshaker_registry()->RegisterHandshakerFactory(
       false /* at_start */, HANDSHAKER_CLIENT,
       absl::make_unique<ClientSecurityHandshakerFactory>());
-  HandshakerRegistry::RegisterHandshakerFactory(
+  builder->handshaker_registry()->RegisterHandshakerFactory(
       false /* at_start */, HANDSHAKER_SERVER,
       absl::make_unique<ServerSecurityHandshakerFactory>());
 }

src/core/lib/security/transport/security_handshaker.h

@@ -22,6 +22,7 @@
 #include <grpc/support/port_platform.h>
 
 #include "src/core/lib/channel/handshaker.h"
+#include "src/core/lib/config/core_configuration.h"
 #include "src/core/lib/security/security_connector/security_connector.h"
 
 namespace grpc_core {
@@ -32,7 +33,7 @@ RefCountedPtr<Handshaker> SecurityHandshakerCreate(
     const grpc_channel_args* args);
 
 /// Registers security handshaker factories.
-void SecurityRegisterHandshakerFactories();
+void SecurityRegisterHandshakerFactories(CoreConfiguration::Builder*);
 
 }  // namespace grpc_core
 

src/core/lib/surface/init.cc

@@ -32,7 +32,6 @@
 #include "src/core/lib/channel/channel_stack.h"
 #include "src/core/lib/channel/channelz_registry.h"
 #include "src/core/lib/channel/connected_channel.h"
-#include "src/core/lib/channel/handshaker_registry.h"
 #include "src/core/lib/debug/stats.h"
 #include "src/core/lib/debug/trace.h"
 #include "src/core/lib/gprpp/fork.h"
@@ -145,8 +144,6 @@ void grpc_init(void) {
     grpc_core::ExecCtx::GlobalInit();
     grpc_iomgr_init();
     gpr_timers_global_init();
-    grpc_core::HandshakerRegistry::Init();
-    grpc_security_init();
     for (int i = 0; i < g_number_of_plugins; i++) {
       if (g_all_of_the_plugins[i].init != nullptr) {
         g_all_of_the_plugins[i].init();
@@ -183,7 +180,6 @@ void grpc_shutdown_internal_locked(void)
     gpr_timers_global_destroy();
     grpc_tracer_shutdown();
     grpc_mdctx_global_shutdown();
-    grpc_core::HandshakerRegistry::Shutdown();
     grpc_slice_intern_shutdown();
     grpc_core::channelz::ChannelzRegistry::Shutdown();
     grpc_stats_shutdown();

src/core/lib/surface/init.h

@@ -30,7 +30,6 @@ class EventEngine;
 
 void grpc_register_security_filters(void);
 void grpc_security_pre_init(void);
-void grpc_security_init(void);
 void grpc_maybe_wait_for_async_shutdown(void);
 
 #endif /* GRPC_CORE_LIB_SURFACE_INIT_H */

src/core/lib/surface/init_secure.cc

@@ -98,5 +98,3 @@ void grpc_register_security_filters(void) {
                                    maybe_prepend_sdk_server_authz_filter,
                                    nullptr);
 }
-
-void grpc_security_init() { grpc_core::SecurityRegisterHandshakerFactories(); }

src/core/lib/surface/init_unsecure.cc

@@ -25,9 +25,3 @@
 void grpc_security_pre_init(void) {}
 
 void grpc_register_security_filters(void) {}
-
-void grpc_security_init(void) {
-  gpr_log(GPR_DEBUG,
-          "Using insecure gRPC build. Security handshakers will not be invoked "
-          "even if secure credentials are used.");
-}

src/core/plugin_registry/grpc_plugin_registry.cc

@@ -165,8 +165,12 @@ void grpc_register_built_in_plugins(void) {
 
 namespace grpc_core {
 
-void BuildCoreConfiguration(CoreConfiguration::Builder*) {
+extern void BuildClientChannelConfiguration(CoreConfiguration::Builder* builder);
-  // TODO(ctiller): Incrementally call out to plugins as we require them to register things with builder.
+extern void SecurityRegisterHandshakerFactories(CoreConfiguration::Builder* builder);
-}
 
+void BuildCoreConfiguration(CoreConfiguration::Builder* builder) {
+  BuildClientChannelConfiguration(builder);
+  SecurityRegisterHandshakerFactories(builder);
 }
+
+}  // namespace grpc_core

src/core/plugin_registry/grpc_unsecure_plugin_registry.cc

@@ -118,8 +118,10 @@ void grpc_register_built_in_plugins(void) {
 
 namespace grpc_core {
 
-void BuildCoreConfiguration(CoreConfiguration::Builder*) {
+extern void BuildClientChannelConfiguration(CoreConfiguration::Builder* builder);
-  // TODO(ctiller): Incrementally call out to plugins as we require them to register things with builder.
+
+void BuildCoreConfiguration(CoreConfiguration::Builder* builder) {
+  BuildClientChannelConfiguration(builder);
 }
 
 }

test/core/handshake/readahead_handshaker_server_ssl.cc

@@ -33,6 +33,7 @@
 
 #include "src/core/lib/channel/handshaker_factory.h"
 #include "src/core/lib/channel/handshaker_registry.h"
+#include "src/core/lib/config/core_configuration.h"
 #include "src/core/lib/iomgr/load_file.h"
 #include "src/core/lib/security/transport/security_handshaker.h"
 #include "test/core/handshake/server_ssl_common.h"
@@ -76,10 +77,15 @@ class ReadAheadHandshakerFactory : public HandshakerFactory {
 }  // namespace grpc_core
 
 int main(int /*argc*/, char* /*argv*/[]) {
-  grpc_init();
+  grpc_core::CoreConfiguration::BuildSpecialConfiguration(
-  grpc_core::HandshakerRegistry::RegisterHandshakerFactory(
+      [](grpc_core::CoreConfiguration::Builder* builder) {
+        BuildCoreConfiguration(builder);
+        builder->handshaker_registry()->RegisterHandshakerFactory(
             true /* at_start */, grpc_core::HANDSHAKER_SERVER,
             absl::make_unique<grpc_core::ReadAheadHandshakerFactory>());
+      });
+
+  grpc_init();
   const char* full_alpn_list[] = {"grpc-exp", "h2"};
   GPR_ASSERT(server_ssl_test(full_alpn_list, 2, "grpc-exp"));
   CleanupSslLibrary();