refactor: prevents global stats config freeze in ConfiguratorRegistry.getConfigurators() (#11991)

Author: AgraVator

api/src/main/java/io/grpc/ConfiguratorRegistry.java

@@ -33,9 +33,9 @@ final class ConfiguratorRegistry {
   @GuardedBy("this")
   private boolean wasConfiguratorsSet;
   @GuardedBy("this")
-  private boolean configFrozen;
-  @GuardedBy("this")
   private List<Configurator> configurators = Collections.emptyList();
+  @GuardedBy("this")
+  private int configuratorsCallCountBeforeSet = 0;
 
   ConfiguratorRegistry() {}
 
@@ -56,22 +56,31 @@ public static synchronized ConfiguratorRegistry getDefaultRegistry() {
    * @throws IllegalStateException if this method is called more than once
    */
   public synchronized void setConfigurators(List<? extends Configurator> configurators) {
-    if (configFrozen) {
+    if (wasConfiguratorsSet) {
       throw new IllegalStateException("Configurators are already set");
     }
     this.configurators = Collections.unmodifiableList(new ArrayList<>(configurators));
-    configFrozen = true;
     wasConfiguratorsSet = true;
   }
 
   /**
    * Returns a list of the configurators in this registry.
    */
   public synchronized List<Configurator> getConfigurators() {
-    configFrozen = true;
+    if (!wasConfiguratorsSet) {
+      configuratorsCallCountBeforeSet++;
+    }
     return configurators;
   }
 
+  /**
+   * Returns the number of times getConfigurators() was called before
+   * setConfigurators() was successfully invoked.
+   */
+  public synchronized int getConfiguratorsCallCountBeforeSet() {
+    return configuratorsCallCountBeforeSet;
+  }
+
   public synchronized boolean wasSetConfiguratorsCalled() {
     return wasConfiguratorsSet;
   }

api/src/main/java/io/grpc/InternalConfiguratorRegistry.java

@@ -48,4 +48,8 @@ public static void configureServerBuilder(ServerBuilder<?> serverBuilder) {
   public static boolean wasSetConfiguratorsCalled() {
     return ConfiguratorRegistry.getDefaultRegistry().wasSetConfiguratorsCalled();
   }
+
+  public static int getConfiguratorsCallCountBeforeSet() {
+    return ConfiguratorRegistry.getDefaultRegistry().getConfiguratorsCallCountBeforeSet();
+  }
 }

api/src/test/java/io/grpc/ConfiguratorRegistryTest.java

@@ -85,14 +85,12 @@ public static final class StaticTestingClassLoaderGetBeforeSet implements Runnab
     @Override
     public void run() {
       assertThat(ConfiguratorRegistry.getDefaultRegistry().getConfigurators()).isEmpty();
-
-      try {
-        ConfiguratorRegistry.getDefaultRegistry()
-            .setConfigurators(Arrays.asList(new NoopConfigurator()));
-        fail("should have failed for invoking set call after get is already called");
-      } catch (IllegalStateException e) {
-        assertThat(e).hasMessageThat().isEqualTo("Configurators are already set");
-      }
+      NoopConfigurator noopConfigurator = new NoopConfigurator();
+      ConfiguratorRegistry.getDefaultRegistry()
+              .setConfigurators(Arrays.asList(noopConfigurator));
+      assertThat(ConfiguratorRegistry.getDefaultRegistry().getConfigurators())
+              .containsExactly(noopConfigurator);
+      assertThat(InternalConfiguratorRegistry.getConfiguratorsCallCountBeforeSet()).isEqualTo(1);
     }
   }
 

core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java

@@ -529,12 +529,9 @@ public void run() {
       List<ClientInterceptor> effectiveInterceptors =
           builder.getEffectiveInterceptors("unused:///");
       assertThat(effectiveInterceptors).hasSize(2);
-      try {
-        InternalConfiguratorRegistry.setConfigurators(Collections.emptyList());
-        fail("exception expected");
-      } catch (IllegalStateException e) {
-        assertThat(e).hasMessageThat().contains("Configurators are already set");
-      }
+      InternalConfiguratorRegistry.setConfigurators(Collections.emptyList());
+      assertThat(InternalConfiguratorRegistry.getConfigurators()).isEmpty();
+      assertThat(InternalConfiguratorRegistry.getConfiguratorsCallCountBeforeSet()).isEqualTo(1);
     }
   }
 

core/src/test/java/io/grpc/internal/ServerImplBuilderTest.java

@@ -18,7 +18,6 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.fail;
 
 import io.grpc.InternalConfigurator;
 import io.grpc.InternalConfiguratorRegistry;
@@ -145,12 +144,9 @@ public void run() {
               });
       assertThat(builder.getTracerFactories()).hasSize(2);
       assertThat(builder.interceptors).hasSize(0);
-      try {
-        InternalConfiguratorRegistry.setConfigurators(Collections.emptyList());
-        fail("exception expected");
-      } catch (IllegalStateException e) {
-        assertThat(e).hasMessageThat().contains("Configurators are already set");
-      }
+      InternalConfiguratorRegistry.setConfigurators(Collections.emptyList());
+      assertThat(InternalConfiguratorRegistry.getConfigurators()).isEmpty();
+      assertThat(InternalConfiguratorRegistry.getConfiguratorsCallCountBeforeSet()).isEqualTo(1);
     }
   }