[init] Remove grpc_register_plugin (#30929)

* [init] Remove grpc_register_plugin

* Automated change: Fix sanity tests

* fix

* Automated change: Fix sanity tests

* fix

* fix

* fix

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

BUILD

@@ -512,6 +512,7 @@ grpc_cc_library(
         "src/core/plugin_registry/grpc_plugin_registry.cc",
         "src/core/plugin_registry/grpc_plugin_registry_noextra.cc",
     ],
+    defines = ["GRPC_NO_XDS"],
     external_deps = [
         "absl/base:core_headers",
     ],
@@ -4657,6 +4658,7 @@ grpc_cc_library(
         "absl/status:statusor",
         "absl/strings",
         "absl/strings:str_format",
+        "absl/synchronization",
         "absl/types:optional",
         "absl/types:variant",
         "upb_lib",

grpc.def

@@ -12,7 +12,6 @@ EXPORTS
     grpc_metadata_array_destroy
     grpc_call_details_init
     grpc_call_details_destroy
-    grpc_register_plugin
     grpc_init
     grpc_shutdown
     grpc_is_initialized

include/grpc/grpc.h

@@ -48,16 +48,6 @@ GRPCAPI void grpc_metadata_array_destroy(grpc_metadata_array* array);
 GRPCAPI void grpc_call_details_init(grpc_call_details* details);
 GRPCAPI void grpc_call_details_destroy(grpc_call_details* details);
 
-/** Registers a plugin to be initialized and destroyed with the library.
-
-    The \a init and \a destroy functions will be invoked as part of
-    \a grpc_init() and \a grpc_shutdown(), respectively.
-    Note that these functions can be invoked an arbitrary number of times
-    (and hence so will \a init and \a destroy).
-    It is safe to pass NULL to either argument. Plugins are destroyed in
-    the reverse order they were initialized. */
-GRPCAPI void grpc_register_plugin(void (*init)(void), void (*destroy)(void));
-
 /** Initialize the grpc library.
 
     After it's called, a matching invocation to grpc_shutdown() is expected.

src/core/ext/xds/xds_client_grpc.cc

@@ -26,6 +26,7 @@
 #include "absl/base/thread_annotations.h"
 #include "absl/status/status.h"
 #include "absl/strings/string_view.h"
+#include "absl/synchronization/mutex.h"
 #include "absl/types/optional.h"
 
 #include <grpc/grpc.h>
@@ -64,30 +65,17 @@ namespace grpc_core {
 
 namespace {
 
-Mutex* g_mu = nullptr;
+Mutex* g_mu = [] {
+  XdsHttpFilterRegistry::Init();
+  XdsClusterSpecifierPluginRegistry::Init();
+  return new Mutex;
+}();
 const grpc_channel_args* g_channel_args ABSL_GUARDED_BY(*g_mu) = nullptr;
 GrpcXdsClient* g_xds_client ABSL_GUARDED_BY(*g_mu) = nullptr;
 char* g_fallback_bootstrap_config ABSL_GUARDED_BY(*g_mu) = nullptr;
 
 }  // namespace
 
-void XdsClientGlobalInit() {
-  g_mu = new Mutex;
-  XdsHttpFilterRegistry::Init();
-  XdsClusterSpecifierPluginRegistry::Init();
-}
-
-// TODO(roth): Find a better way to clear the fallback config that does
-// not require using ABSL_NO_THREAD_SAFETY_ANALYSIS.
-void XdsClientGlobalShutdown() ABSL_NO_THREAD_SAFETY_ANALYSIS {
-  gpr_free(g_fallback_bootstrap_config);
-  g_fallback_bootstrap_config = nullptr;
-  delete g_mu;
-  g_mu = nullptr;
-  XdsHttpFilterRegistry::Shutdown();
-  XdsClusterSpecifierPluginRegistry::Shutdown();
-}
-
 namespace {
 
 absl::StatusOr<std::string> GetBootstrapContents(const char* fallback_config) {

src/core/lib/surface/init.cc

@@ -21,7 +21,6 @@
 #include "src/core/lib/surface/init.h"
 
 #include <limits.h>
-#include <stdint.h>
 
 #include "absl/base/thread_annotations.h"
 
@@ -57,8 +56,9 @@
 #include "src/core/lib/surface/channel_stack_type.h"
 #include "src/core/lib/surface/init_internally.h"
 
-/* (generated) built in registry of plugins */
+// Remnants of the old plugin system
-extern void grpc_register_built_in_plugins(void);
+void grpc_resolver_dns_ares_init(void);
+void grpc_resolver_dns_ares_shutdown(void);
 
 #define MAX_PLUGINS 128
 
@@ -123,7 +123,6 @@ static void do_basic_init(void) {
   gpr_log_verbosity_init();
   g_init_mu = new grpc_core::Mutex();
   g_shutting_down_cv = new grpc_core::CondVar();
-  grpc_register_built_in_plugins();
   gpr_time_init();
   grpc_core::PrintExperimentsList();
   grpc_core::Fork::GlobalInit();
@@ -133,23 +132,6 @@ static void do_basic_init(void) {
   grpc_client_channel_global_init_backup_polling();
 }
 
-typedef struct grpc_plugin {
-  void (*init)();
-  void (*destroy)();
-} grpc_plugin;
-
-static grpc_plugin g_all_of_the_plugins[MAX_PLUGINS];
-static int g_number_of_plugins = 0;
-
-void grpc_register_plugin(void (*init)(void), void (*destroy)(void)) {
-  GRPC_API_TRACE("grpc_register_plugin(init=%p, destroy=%p)", 2,
-                 ((void*)(intptr_t)init, (void*)(intptr_t)destroy));
-  GPR_ASSERT(g_number_of_plugins != MAX_PLUGINS);
-  g_all_of_the_plugins[g_number_of_plugins].init = init;
-  g_all_of_the_plugins[g_number_of_plugins].destroy = destroy;
-  g_number_of_plugins++;
-}
-
 void grpc_init(void) {
   gpr_once_init(&g_basic_init, do_basic_init);
 
@@ -160,11 +142,7 @@ void grpc_init(void) {
       g_shutting_down_cv->SignalAll();
     }
     grpc_iomgr_init();
-    for (int i = 0; i < g_number_of_plugins; i++) {
+    grpc_resolver_dns_ares_init();
-      if (g_all_of_the_plugins[i].init != nullptr) {
-        g_all_of_the_plugins[i].init();
-      }
-    }
     grpc_iomgr_start();
   }
 
@@ -173,18 +151,11 @@ void grpc_init(void) {
 
 void grpc_shutdown_internal_locked(void)
     ABSL_EXCLUSIVE_LOCKS_REQUIRED(g_init_mu) {
-  int i;
   {
     grpc_core::ExecCtx exec_ctx(0);
     grpc_iomgr_shutdown_background_closure();
-    {
     grpc_timer_manager_set_threading(false);  // shutdown timer_manager thread
-      for (i = g_number_of_plugins; i >= 0; i--) {
+    grpc_resolver_dns_ares_shutdown();
-        if (g_all_of_the_plugins[i].destroy != nullptr) {
-          g_all_of_the_plugins[i].destroy();
-        }
-      }
-    }
     grpc_event_engine::experimental::ResetDefaultEventEngine();
     grpc_iomgr_shutdown();
   }

src/core/plugin_registry/grpc_plugin_registry.cc

@@ -25,17 +25,6 @@
 #include "src/core/lib/transport/http_connect_handshaker.h"
 #include "src/core/lib/transport/tcp_connect_handshaker.h"
 
-extern void grpc_register_extra_plugins(void);
-
-void grpc_resolver_dns_ares_init(void);
-void grpc_resolver_dns_ares_shutdown(void);
-
-void grpc_register_built_in_plugins(void) {
-  grpc_register_plugin(grpc_resolver_dns_ares_init,
-                       grpc_resolver_dns_ares_shutdown);
-  grpc_register_extra_plugins();
-}
-
 namespace grpc_core {
 
 extern void BuildClientChannelConfiguration(

src/core/plugin_registry/grpc_plugin_registry_extra.cc

@@ -21,20 +21,6 @@
 #include "src/core/lib/config/core_configuration.h"
 #include "src/core/lib/surface/builtins.h"
 
-#ifndef GRPC_NO_XDS
-namespace grpc_core {
-void XdsClientGlobalInit();
-void XdsClientGlobalShutdown();
-}  // namespace grpc_core
-#endif
-
-void grpc_register_extra_plugins() {
-#ifndef GRPC_NO_XDS
-  grpc_register_plugin(grpc_core::XdsClientGlobalInit,
-                       grpc_core::XdsClientGlobalShutdown);
-#endif
-}
-
 namespace grpc_core {
 #ifndef GRPC_NO_XDS
 extern void RbacFilterRegister(CoreConfiguration::Builder* builder);

src/core/plugin_registry/grpc_plugin_registry_noextra.cc

@@ -21,8 +21,6 @@
 #include "src/core/lib/config/core_configuration.h"
 #include "src/core/lib/surface/builtins.h"
 
-void grpc_register_extra_plugins(void) {}
-
 namespace grpc_core {
-void RegisterExtraFilters(CoreConfiguration::Builder* /* builder */){}
+void RegisterExtraFilters(CoreConfiguration::Builder* /* builder */) {}
 }  // namespace grpc_core

src/ruby/ext/grpc/rb_grpc_imports.generated.c

@@ -35,7 +35,6 @@ grpc_metadata_array_init_type grpc_metadata_array_init_import;
 grpc_metadata_array_destroy_type grpc_metadata_array_destroy_import;
 grpc_call_details_init_type grpc_call_details_init_import;
 grpc_call_details_destroy_type grpc_call_details_destroy_import;
-grpc_register_plugin_type grpc_register_plugin_import;
 grpc_init_type grpc_init_import;
 grpc_shutdown_type grpc_shutdown_import;
 grpc_is_initialized_type grpc_is_initialized_import;
@@ -321,7 +320,6 @@ void grpc_rb_load_imports(HMODULE library) {
   grpc_metadata_array_destroy_import = (grpc_metadata_array_destroy_type) GetProcAddress(library, "grpc_metadata_array_destroy");
   grpc_call_details_init_import = (grpc_call_details_init_type) GetProcAddress(library, "grpc_call_details_init");
   grpc_call_details_destroy_import = (grpc_call_details_destroy_type) GetProcAddress(library, "grpc_call_details_destroy");
-  grpc_register_plugin_import = (grpc_register_plugin_type) GetProcAddress(library, "grpc_register_plugin");
   grpc_init_import = (grpc_init_type) GetProcAddress(library, "grpc_init");
   grpc_shutdown_import = (grpc_shutdown_type) GetProcAddress(library, "grpc_shutdown");
   grpc_is_initialized_import = (grpc_is_initialized_type) GetProcAddress(library, "grpc_is_initialized");

src/ruby/ext/grpc/rb_grpc_imports.generated.h

@@ -80,9 +80,6 @@ extern grpc_call_details_init_type grpc_call_details_init_import;
 typedef void(*grpc_call_details_destroy_type)(grpc_call_details* details);
 extern grpc_call_details_destroy_type grpc_call_details_destroy_import;
 #define grpc_call_details_destroy grpc_call_details_destroy_import
-typedef void(*grpc_register_plugin_type)(void (*init)(void), void (*destroy)(void));
-extern grpc_register_plugin_type grpc_register_plugin_import;
-#define grpc_register_plugin grpc_register_plugin_import
 typedef void(*grpc_init_type)(void);
 extern grpc_init_type grpc_init_import;
 #define grpc_init grpc_init_import

test/core/surface/init_test.cc

@@ -29,13 +29,6 @@
 #include "src/core/lib/iomgr/exec_ctx.h"
 #include "test/core/util/test_config.h"
 
-static int g_plugin_state;
-
-static void plugin_init(void) { g_plugin_state = 1; }
-static void plugin_destroy(void) { g_plugin_state = 2; }
-static bool plugin_is_intialized(void) { return g_plugin_state == 1; }
-static bool plugin_is_destroyed(void) { return g_plugin_state == 2; }
-
 static void test(int rounds) {
   int i;
   for (i = 0; i < rounds; i++) {
@@ -106,14 +99,6 @@ TEST(Init, mixed_with_thread) {
   EXPECT_FALSE(grpc_is_initialized());
 }
 
-TEST(Init, plugin) {
-  grpc_init();
-  EXPECT_TRUE(plugin_is_intialized());
-  grpc_shutdown_blocking();
-  EXPECT_TRUE(plugin_is_destroyed());
-  EXPECT_FALSE(grpc_is_initialized());
-}
-
 TEST(Init, repeatedly) {
   for (int i = 0; i < 10; i++) {
     grpc_init();
@@ -155,6 +140,5 @@ TEST(Init, TimerManagerHoldsLastInit) {
 int main(int argc, char** argv) {
   grpc::testing::TestEnvironment env(&argc, argv);
   ::testing::InitGoogleTest(&argc, argv);
-  grpc_register_plugin(plugin_init, plugin_destroy);
   return RUN_ALL_TESTS();
 }

test/core/surface/public_headers_must_be_c89.c

@@ -81,7 +81,6 @@ int main(int argc, char **argv) {
   printf("%lx", (unsigned long) grpc_metadata_array_destroy);
   printf("%lx", (unsigned long) grpc_call_details_init);
   printf("%lx", (unsigned long) grpc_call_details_destroy);
-  printf("%lx", (unsigned long) grpc_register_plugin);
   printf("%lx", (unsigned long) grpc_init);
   printf("%lx", (unsigned long) grpc_shutdown);
   printf("%lx", (unsigned long) grpc_is_initialized);