Skip to content

Commit

Permalink
Safer, slightly simpler register/release patterns for config
Browse files Browse the repository at this point in the history 
Some config was lazily created and bound to the dev class loader
through the configsForClassLoader map
io.smallrye.config.SmallRyeConfigProviderResolver#getConfig(java.lang.ClassLoader),
and the corresponding map entry was never cleared,
resulting in the corresponding classloader never being garbage
collected.
  • Loading branch information
@yrodiere
yrodiere committed Oct 18, 2023
1 parent c4ffaeb commit d1b33b3
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 39 deletions.
19 changes: 0 additions & 19 deletions ...ment/src/main/java/io/quarkus/deployment/configuration/RunTimeConfigurationGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@
import java.util.function.IntFunction;
import java.util.regex.Pattern;

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.spi.ConfigBuilder;
import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import org.eclipse.microprofile.config.spi.Converter;
import org.objectweb.asm.Opcodes;
import org.wildfly.common.Assert;
Expand Down Expand Up @@ -123,13 +121,6 @@ public final class RunTimeConfigurationGenerator {
static final MethodDescriptor CONVS_PATTERN_CONVERTER = MethodDescriptor.ofMethod(Converters.class,
"patternConverter", Converter.class, Converter.class, Pattern.class);

static final MethodDescriptor CPR_GET_CONFIG = MethodDescriptor.ofMethod(ConfigProviderResolver.class, "getConfig",
Config.class);
static final MethodDescriptor CPR_INSTANCE = MethodDescriptor.ofMethod(ConfigProviderResolver.class, "instance",
ConfigProviderResolver.class);
static final MethodDescriptor CPR_RELEASE_CONFIG = MethodDescriptor.ofMethod(ConfigProviderResolver.class, "releaseConfig",
void.class, Config.class);

static final MethodDescriptor CU_LIST_FACTORY = MethodDescriptor.ofMethod(ConfigUtils.class, "listFactory",
IntFunction.class);
static final MethodDescriptor CU_SET_FACTORY = MethodDescriptor.ofMethod(ConfigUtils.class, "setFactory",
Expand Down Expand Up @@ -595,17 +586,7 @@ private Set<String> getRegisteredRoots(ConfigPhase configPhase) {
}

private void installConfiguration(ResultHandle config, MethodCreator methodCreator) {
// install config
methodCreator.invokeStaticMethod(QCF_SET_CONFIG, config);
// now invalidate the cached config, so the next one to load the config gets the new one
final ResultHandle configProviderResolver = methodCreator.invokeStaticMethod(CPR_INSTANCE);
try (TryBlock getConfigTry = methodCreator.tryBlock()) {
final ResultHandle initialConfigHandle = getConfigTry.invokeVirtualMethod(CPR_GET_CONFIG,
configProviderResolver);
getConfigTry.invokeVirtualMethod(CPR_RELEASE_CONFIG, configProviderResolver, initialConfigHandle);
// ignore
getConfigTry.addCatch(IllegalStateException.class);
}
}

private MethodDescriptor generateInitGroup(ClassDefinition definition) {
Expand Down
29 changes: 12 additions & 17 deletions core/deployment/src/main/java/io/quarkus/deployment/dev/IsolatedDevModeMain.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.function.Consumer;
import java.util.function.Predicate;

import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import org.jboss.logging.Logger;
import org.jboss.logmanager.formatters.ColorPatternFormatter;
import org.jboss.logmanager.handlers.ConsoleHandler;
Expand Down Expand Up @@ -284,26 +283,22 @@ public void stop() {
ClassLoader old = Thread.currentThread().getContextClassLoader();
Thread.currentThread().setContextClassLoader(runner.getClassLoader());
try {
runner.close();
} catch (Exception e) {
e.printStackTrace();
try {
runner.close();
} catch (Exception e) {
e.printStackTrace();
}
try {
// TODO is this even useful?
// It's not using the config factory from the right classloader...
QuarkusConfigFactory.setConfig(null);
} catch (Exception e) {
e.printStackTrace();
}
} finally {
Thread.currentThread().setContextClassLoader(old);
}
}
QuarkusConfigFactory.setConfig(null);

ClassLoader old = Thread.currentThread().getContextClassLoader();
Thread.currentThread().setContextClassLoader(getClass().getClassLoader());
try {
final ConfigProviderResolver cpr = ConfigProviderResolver.instance();
cpr.releaseConfig(cpr.getConfig());
} catch (Throwable ignored) {
// just means no config was installed, which is fine
} finally {
Thread.currentThread().setContextClassLoader(old);
}
runner = null;
}

public void close() {
Expand Down
8 changes: 8 additions & 0 deletions core/deployment/src/main/java/io/quarkus/deployment/steps/ConfigGenerationBuildStep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import io.quarkus.deployment.builditem.QuarkusBuildCloseablesBuildItem;
import io.quarkus.deployment.builditem.RunTimeConfigBuilderBuildItem;
import io.quarkus.deployment.builditem.RunTimeConfigurationDefaultBuildItem;
import io.quarkus.deployment.builditem.ShutdownContextBuildItem;
import io.quarkus.deployment.builditem.StaticInitConfigBuilderBuildItem;
import io.quarkus.deployment.builditem.SuppressNonRuntimeConfigChangedWarningBuildItem;
import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
Expand Down Expand Up @@ -305,6 +306,13 @@ public void suppressNonRuntimeConfigChanged(
suppressNonRuntimeConfigChanged.produce(new SuppressNonRuntimeConfigChangedWarningBuildItem("quarkus.test.arg-line"));
}

@BuildStep
@Record(ExecutionTime.RUNTIME_INIT)
public void releaseConfigOnShutdown(ShutdownContextBuildItem shutdownContext,
ConfigRecorder recorder) {
recorder.releaseConfig(shutdownContext);
}

/**
* Warns if build time config properties have been changed at runtime.
*/
Expand Down
9 changes: 9 additions & 0 deletions core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigRecorder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.eclipse.microprofile.config.spi.ConfigSource;
import org.jboss.logging.Logger;

import io.quarkus.runtime.ShutdownContext;
import io.quarkus.runtime.annotations.Recorder;
import io.quarkus.runtime.configuration.ConfigurationRuntimeConfig.BuildTimeMismatchAtRuntime;
import io.smallrye.config.SmallRyeConfig;
Expand Down Expand Up @@ -101,4 +102,12 @@ public void handleNativeProfileChange(List<String> buildProfiles) {
public void unknownConfigFiles() throws Exception {
ConfigDiagnostic.unknownConfigFiles(ConfigDiagnostic.configFilesFromLocations());
}

public void releaseConfig(ShutdownContext shutdownContext) {
// This is mostly useful to handle restarts in Dev/Test mode.
// While this may seem to duplicate code in IsolatedDevModeMain,
// it actually does not because it operates on a different instance
// of QuarkusConfigFactory from a different classloader.
shutdownContext.addLastShutdownTask(QuarkusConfigFactory::releaseTCCLConfig);
}
}
22 changes: 19 additions & 3 deletions core/runtime/src/main/java/io/quarkus/runtime/configuration/QuarkusConfigFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,37 @@ public QuarkusConfigFactory() {
// todo: replace with {@code provider()} post-Java 11
}

@Override
public SmallRyeConfig getConfigFor(final SmallRyeConfigProviderResolver configProviderResolver,
final ClassLoader classLoader) {
if (config == null) {
return ConfigUtils.configBuilder(true, true, LaunchMode.NORMAL).build();
// Remember the config so that we can uninstall it when "setConfig" is next called
config = ConfigUtils.configBuilder(true, true, LaunchMode.NORMAL).build();
}
return config;
}

public static void setConfig(SmallRyeConfig config) {
SmallRyeConfigProviderResolver configProviderResolver = (SmallRyeConfigProviderResolver) SmallRyeConfigProviderResolver
.instance();
configProviderResolver.releaseConfig(Thread.currentThread().getContextClassLoader());
QuarkusConfigFactory.config = config;
// Uninstall previous config
if (QuarkusConfigFactory.config != null) {
configProviderResolver.releaseConfig(QuarkusConfigFactory.config);
QuarkusConfigFactory.config = null;
}
// Install new config
if (config != null) {
QuarkusConfigFactory.config = config;
// Register the new config for the TCCL,
// just in case the TCCL was using a different config
// than the one we uninstalled above.
configProviderResolver.registerConfig(config, Thread.currentThread().getContextClassLoader());
}
}

public static void releaseTCCLConfig() {
SmallRyeConfigProviderResolver configProviderResolver = (SmallRyeConfigProviderResolver) SmallRyeConfigProviderResolver
.instance();
configProviderResolver.releaseConfig(Thread.currentThread().getContextClassLoader());
}
}

0 comments on commit d1b33b3

Please sign in to comment.