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

This commit is contained in:
Abhishek Agrawal 2025-04-04 05:53:08 +00:00 committed by GitHub
parent c8d1e6e39c
commit d4c46a7f1f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 30 additions and 26 deletions

View File

@ -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,11 +56,10 @@ final class ConfiguratorRegistry {
* @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;
}
@ -68,10 +67,20 @@ final class ConfiguratorRegistry {
* 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;
}

View File

@ -48,4 +48,8 @@ public final class InternalConfiguratorRegistry {
public static boolean wasSetConfiguratorsCalled() {
return ConfiguratorRegistry.getDefaultRegistry().wasSetConfiguratorsCalled();
}
public static int getConfiguratorsCallCountBeforeSet() {
return ConfiguratorRegistry.getDefaultRegistry().getConfiguratorsCallCountBeforeSet();
}
}

View File

@ -85,14 +85,12 @@ public class ConfiguratorRegistryTest {
@Override
public void run() {
assertThat(ConfiguratorRegistry.getDefaultRegistry().getConfigurators()).isEmpty();
try {
NoopConfigurator noopConfigurator = new NoopConfigurator();
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");
}
.setConfigurators(Arrays.asList(noopConfigurator));
assertThat(ConfiguratorRegistry.getDefaultRegistry().getConfigurators())
.containsExactly(noopConfigurator);
assertThat(InternalConfiguratorRegistry.getConfiguratorsCallCountBeforeSet()).isEqualTo(1);
}
}

View File

@ -529,12 +529,9 @@ public class ManagedChannelImplBuilderTest {
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");
}
assertThat(InternalConfiguratorRegistry.getConfigurators()).isEmpty();
assertThat(InternalConfiguratorRegistry.getConfiguratorsCallCountBeforeSet()).isEqualTo(1);
}
}

View File

@ -18,7 +18,6 @@ package io.grpc.internal;
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 class ServerImplBuilderTest {
});
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");
}
assertThat(InternalConfiguratorRegistry.getConfigurators()).isEmpty();
assertThat(InternalConfiguratorRegistry.getConfiguratorsCallCountBeforeSet()).isEqualTo(1);
}
}