From 6bd7546ca985081fda11528909dbd2a0bebe77fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 18 Oct 2023 17:28:19 +0200 Subject: [PATCH 01/11] Clarify why QuarkusClassLoader doesn't pass its name to the super constructor --- .../io/quarkus/bootstrap/classloading/QuarkusClassLoader.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java index 8b653f334f3c5..bc2323fb2c056 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java @@ -131,6 +131,8 @@ public static boolean isResourcePresentAtRuntime(String resourcePath) { private volatile boolean driverLoaded; private QuarkusClassLoader(Builder builder) { + // Not passing the name to the parent constructor on purpose: + // stacktraces become very ugly if we do that. super(builder.parent); this.name = builder.name; this.elements = builder.elements; From 83a426a820c100bac992ade0ed3ab83b758799b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 17 Oct 2023 10:21:55 +0200 Subject: [PATCH 02/11] Enrich classloader names with the name of the application/QuarkusUnitTest --- .../deployment/dev/testing/TestSupport.java | 3 ++- .../quarkus/bootstrap/app/CuratedApplication.java | 14 ++++++++++---- .../java/io/quarkus/test/QuarkusDevModeTest.java | 1 + .../java/io/quarkus/test/QuarkusProdModeTest.java | 5 ++--- .../main/java/io/quarkus/test/QuarkusUnitTest.java | 4 +++- .../junit/AbstractJvmQuarkusTestExtension.java | 1 + 6 files changed, 19 insertions(+), 9 deletions(-) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/dev/testing/TestSupport.java b/core/deployment/src/main/java/io/quarkus/deployment/dev/testing/TestSupport.java index 50fa7935ddc2a..5d070d61d0fa4 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/dev/testing/TestSupport.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/dev/testing/TestSupport.java @@ -218,7 +218,8 @@ public void init() { for (ResolvedDependency d : testModel.getDependencies()) { if (d.isClassLoaderParentFirst() && !currentParentFirst.contains(d.getKey())) { if (clBuilder == null) { - clBuilder = QuarkusClassLoader.builder("Continuous Testing Parent-First", + clBuilder = QuarkusClassLoader.builder("Continuous Testing Parent-First" + + curatedApplication.getClassLoaderNameSuffix(), getClass().getClassLoader().getParent(), false); } clBuilder.addElement(ClassPathElement.fromDependency(d)); diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java index d51837e39e399..fce442143c5e8 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java @@ -193,7 +193,7 @@ public synchronized QuarkusClassLoader getAugmentClassLoader() { if (augmentClassLoader == null) { //first run, we need to build all the class loaders QuarkusClassLoader.Builder builder = QuarkusClassLoader.builder( - "Augmentation Class Loader: " + quarkusBootstrap.getMode(), + "Augmentation Class Loader: " + quarkusBootstrap.getMode() + getClassLoaderNameSuffix(), quarkusBootstrap.getBaseClassLoader(), !quarkusBootstrap.isIsolateDeployment()) .setAssertionsEnabled(quarkusBootstrap.isAssertionsEnabled()); builder.addClassLoaderEventListeners(quarkusBootstrap.getClassLoaderEventListeners()); @@ -238,7 +238,7 @@ public synchronized QuarkusClassLoader getAugmentClassLoader() { public synchronized QuarkusClassLoader getBaseRuntimeClassLoader() { if (baseRuntimeClassLoader == null) { QuarkusClassLoader.Builder builder = QuarkusClassLoader.builder( - "Quarkus Base Runtime ClassLoader: " + quarkusBootstrap.getMode(), + "Quarkus Base Runtime ClassLoader: " + quarkusBootstrap.getMode() + getClassLoaderNameSuffix(), quarkusBootstrap.getBaseClassLoader(), false) .setAssertionsEnabled(quarkusBootstrap.isAssertionsEnabled()); builder.addClassLoaderEventListeners(quarkusBootstrap.getClassLoaderEventListeners()); @@ -316,7 +316,7 @@ private static boolean isHotReloadable(ResolvedDependency a, Set hotReload public QuarkusClassLoader createDeploymentClassLoader() { //first run, we need to build all the class loaders QuarkusClassLoader.Builder builder = QuarkusClassLoader - .builder("Deployment Class Loader: " + quarkusBootstrap.getMode(), + .builder("Deployment Class Loader: " + quarkusBootstrap.getMode() + getClassLoaderNameSuffix(), getAugmentClassLoader(), false) .addClassLoaderEventListeners(quarkusBootstrap.getClassLoaderEventListeners()) .setAssertionsEnabled(quarkusBootstrap.isAssertionsEnabled()) @@ -350,6 +350,10 @@ public QuarkusClassLoader createDeploymentClassLoader() { return builder.build(); } + public String getClassLoaderNameSuffix() { + return quarkusBootstrap.getBaseName() != null ? " for " + quarkusBootstrap.getBaseName() : ""; + } + public QuarkusClassLoader createRuntimeClassLoader(Map resources, Map transformedClasses) { return createRuntimeClassLoader(getBaseRuntimeClassLoader(), resources, transformedClasses); } @@ -358,7 +362,9 @@ public QuarkusClassLoader createRuntimeClassLoader(ClassLoader base, Map transformedClasses) { QuarkusClassLoader.Builder builder = QuarkusClassLoader .builder( - "Quarkus Runtime ClassLoader: " + quarkusBootstrap.getMode() + " restart no:" + "Quarkus Runtime ClassLoader: " + quarkusBootstrap.getMode() + + getClassLoaderNameSuffix() + + " restart no:" + runtimeClassLoaderCount.getAndIncrement(), getBaseRuntimeClassLoader(), false) .setAssertionsEnabled(quarkusBootstrap.isAssertionsEnabled()) diff --git a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java index d202d862a2011..c80f987df6caf 100644 --- a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java +++ b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java @@ -292,6 +292,7 @@ public void close() throws Throwable { Path projectSourceParent = projectSourceRoot.getParent(); DevModeContext context = exportArchive(deploymentDir, projectSourceRoot, projectSourceParent); + context.setBaseName(extensionContext.getDisplayName() + " (QuarkusDevModeTest)"); context.setArgs(commandLineArgs); context.setTest(true); context.setAbortOnFailedStart(!allowFailedStart); diff --git a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusProdModeTest.java b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusProdModeTest.java index f090691b62a58..3e5b7c72ef43f 100644 --- a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusProdModeTest.java +++ b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusProdModeTest.java @@ -434,9 +434,8 @@ public void close() throws Throwable { .setProjectRoot(testLocation) .setTargetDirectory(buildDir) .setForcedDependencies(forcedDependencies); - if (applicationName != null) { - builder.setBaseName(applicationName); - } + builder.setBaseName(applicationName != null ? applicationName + : extensionContext.getDisplayName() + " (QuarkusProdModeTest)"); Map buildContext = new HashMap<>(); buildContext.put(BUILD_CONTEXT_CUSTOM_SOURCES_PATH_KEY, customSourcesDir); diff --git a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusUnitTest.java b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusUnitTest.java index 42d878b481e0a..f17fd2e19514c 100644 --- a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusUnitTest.java +++ b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusUnitTest.java @@ -617,6 +617,7 @@ public boolean test(String s) { final Path testLocation = PathTestHelper.getTestClassesLocation(testClass); final Path projectDir = Path.of("").normalize().toAbsolutePath(); QuarkusBootstrap.Builder builder = QuarkusBootstrap.builder() + .setBaseName(extensionContext.getDisplayName() + " (QuarkusUnitTest)") .setApplicationRoot(deploymentDir.resolve(APP_ROOT)) .setMode(QuarkusBootstrap.Mode.TEST) .addExcludedPath(testLocation) @@ -635,7 +636,8 @@ public boolean test(String s) { } if (!allowTestClassOutsideDeployment) { quarkusUnitTestClassLoader = QuarkusClassLoader - .builder("QuarkusUnitTest ClassLoader", getClass().getClassLoader(), false) + .builder("QuarkusUnitTest ClassLoader for " + extensionContext.getDisplayName(), + getClass().getClassLoader(), false) .addClassLoaderEventListeners(this.classLoadListeners) .addBannedElement(ClassPathElement.fromPath(testLocation, true)).build(); builder.setBaseClassLoader(quarkusUnitTestClassLoader); diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/AbstractJvmQuarkusTestExtension.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/AbstractJvmQuarkusTestExtension.java index 516e5ada77552..5623be978f8ef 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/AbstractJvmQuarkusTestExtension.java +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/AbstractJvmQuarkusTestExtension.java @@ -178,6 +178,7 @@ protected PrepareResult createAugmentor(ExtensionContext context, Class Date: Thu, 19 Oct 2023 17:24:30 +0200 Subject: [PATCH 03/11] Fix a suspicious H2 URL in Hibernate ORM test config It doesn't matter in practice since the only test using this config reproduces a startup failure that happens before we start H2; but still, let's fix this for correctness. --- ...ion-multiple-persistence-units-undefined-packages.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/hibernate-orm/deployment/src/test/resources/application-multiple-persistence-units-undefined-packages.properties b/extensions/hibernate-orm/deployment/src/test/resources/application-multiple-persistence-units-undefined-packages.properties index 78334cec77424..18a9e2b33b9c4 100644 --- a/extensions/hibernate-orm/deployment/src/test/resources/application-multiple-persistence-units-undefined-packages.properties +++ b/extensions/hibernate-orm/deployment/src/test/resources/application-multiple-persistence-units-undefined-packages.properties @@ -1,5 +1,5 @@ quarkus.datasource.users.db-kind=h2 -quarkus.datasource.users.jdbc.url=jdbc:h2:tcp://localhost/mem:users +quarkus.datasource.users.jdbc.url=jdbc:h2:mem:users quarkus.hibernate-orm."users".log.sql=true quarkus.hibernate-orm."users".database.generation=drop-and-create From 3374799f6d0523767d978ecbd35dbbe96f258ad5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 18 Oct 2023 11:39:16 +0200 Subject: [PATCH 04/11] Always reset the ForkJoinPool TCCL on startup --- .../java/io/quarkus/runner/bootstrap/StartupActionImpl.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/deployment/src/main/java/io/quarkus/runner/bootstrap/StartupActionImpl.java b/core/deployment/src/main/java/io/quarkus/runner/bootstrap/StartupActionImpl.java index b67e9d4b1852f..fc195c8eee40a 100644 --- a/core/deployment/src/main/java/io/quarkus/runner/bootstrap/StartupActionImpl.java +++ b/core/deployment/src/main/java/io/quarkus/runner/bootstrap/StartupActionImpl.java @@ -186,6 +186,9 @@ private void doClose() { @Override public int runMainClassBlocking(String... args) throws Exception { + //first we hack around class loading in the fork join pool + ForkJoinClassLoading.setForkJoinClassLoader(runtimeClassLoader); + //we have our class loaders ClassLoader old = Thread.currentThread().getContextClassLoader(); Thread.currentThread().setContextClassLoader(runtimeClassLoader); @@ -252,7 +255,7 @@ public void overrideConfig(Map config) { * Runs the application, and returns a handle that can be used to shut it down. */ public RunningQuarkusApplication run(String... args) throws Exception { - //first + //first we hack around class loading in the fork join pool ForkJoinClassLoading.setForkJoinClassLoader(runtimeClassLoader); //we have our class loaders From 133cbd90da15322fc82ddeddfee22970ebb77da2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Thu, 12 Oct 2023 15:27:48 +0200 Subject: [PATCH 05/11] Safer, slightly simpler register/release patterns for config 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. --- .../RunTimeConfigurationGenerator.java | 19 ------------ .../deployment/dev/IsolatedDevModeMain.java | 29 ++++++++----------- .../steps/ConfigGenerationBuildStep.java | 8 +++++ .../runtime/configuration/ConfigRecorder.java | 9 ++++++ .../configuration/QuarkusConfigFactory.java | 24 +++++++++++++-- 5 files changed, 50 insertions(+), 39 deletions(-) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/configuration/RunTimeConfigurationGenerator.java b/core/deployment/src/main/java/io/quarkus/deployment/configuration/RunTimeConfigurationGenerator.java index bbcdade143c99..b1e95250a5ddf 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/configuration/RunTimeConfigurationGenerator.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/configuration/RunTimeConfigurationGenerator.java @@ -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; @@ -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", @@ -595,17 +586,7 @@ private Set 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) { diff --git a/core/deployment/src/main/java/io/quarkus/deployment/dev/IsolatedDevModeMain.java b/core/deployment/src/main/java/io/quarkus/deployment/dev/IsolatedDevModeMain.java index 382af0089422d..424b7c271a070 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/dev/IsolatedDevModeMain.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/dev/IsolatedDevModeMain.java @@ -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; @@ -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() { diff --git a/core/deployment/src/main/java/io/quarkus/deployment/steps/ConfigGenerationBuildStep.java b/core/deployment/src/main/java/io/quarkus/deployment/steps/ConfigGenerationBuildStep.java index b1ae0b60f8459..d5f89be6eb98a 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/steps/ConfigGenerationBuildStep.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/steps/ConfigGenerationBuildStep.java @@ -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; @@ -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. */ diff --git a/core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigRecorder.java b/core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigRecorder.java index fd04bf02a8817..ff18b51bb6562 100644 --- a/core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigRecorder.java +++ b/core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigRecorder.java @@ -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; @@ -101,4 +102,12 @@ public void handleNativeProfileChange(List 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); + } } diff --git a/core/runtime/src/main/java/io/quarkus/runtime/configuration/QuarkusConfigFactory.java b/core/runtime/src/main/java/io/quarkus/runtime/configuration/QuarkusConfigFactory.java index ec8d0ff775fd9..2adc409fab816 100644 --- a/core/runtime/src/main/java/io/quarkus/runtime/configuration/QuarkusConfigFactory.java +++ b/core/runtime/src/main/java/io/quarkus/runtime/configuration/QuarkusConfigFactory.java @@ -19,10 +19,12 @@ 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; } @@ -30,10 +32,26 @@ public SmallRyeConfig getConfigFor(final SmallRyeConfigProviderResolver configPr 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; + } + // Also release the TCCL config, in case that config was not QuarkusConfigFactory.config + configProviderResolver.releaseConfig(Thread.currentThread().getContextClassLoader()); + // 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()); + } } From c25f1db3d6d26e14762faadbfc6f7dd9b2af93c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 18 Oct 2023 14:18:39 +0200 Subject: [PATCH 06/11] Avoid referencing ClassLoaders in ProtectionDomains This removes a whole class of metaspace leaks caused by Thread.inheritedAccessControlContext referencing ProtectionDomains which reference older classloaders. Of course this may have impacts on how the SecurityManager behaves, but as I understand it, absolutely no part of Quarkus is ready to run with a SecurityManager enabled anyway. --- .../java/io/quarkus/bootstrap/app/CuratedApplication.java | 4 ++-- .../io/quarkus/bootstrap/classloading/ClassPathElement.java | 4 ++-- .../bootstrap/classloading/DirectoryClassPathElement.java | 5 ++--- .../bootstrap/classloading/FilteredClassPathElement.java | 4 ++-- .../quarkus/bootstrap/classloading/JarClassPathElement.java | 4 ++-- .../bootstrap/classloading/MemoryClassPathElement.java | 5 ++--- .../bootstrap/classloading/PathTreeClassPathElement.java | 5 ++--- .../quarkus/bootstrap/classloading/QuarkusClassLoader.java | 2 +- .../io/quarkus/bootstrap/runner/ClassLoadingResource.java | 2 +- .../main/java/io/quarkus/bootstrap/runner/JarResource.java | 4 ++-- .../io/quarkus/bootstrap/runner/SerializedApplication.java | 2 +- 11 files changed, 19 insertions(+), 22 deletions(-) diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java index fce442143c5e8..da469231ba265 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java @@ -478,8 +478,8 @@ public Set getProvidedResources() { } @Override - public ProtectionDomain getProtectionDomain(ClassLoader classLoader) { - return delegate.getProtectionDomain(classLoader); + public ProtectionDomain getProtectionDomain() { + return delegate.getProtectionDomain(); } @Override diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/ClassPathElement.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/ClassPathElement.java index 26ade4b620f43..eefce68aa5f66 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/ClassPathElement.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/ClassPathElement.java @@ -65,7 +65,7 @@ default ArtifactKey getDependencyKey() { * * @return The protection domain that should be used to define classes from this element */ - ProtectionDomain getProtectionDomain(ClassLoader classLoader); + ProtectionDomain getProtectionDomain(); Manifest getManifest(); @@ -115,7 +115,7 @@ public Set getProvidedResources() { } @Override - public ProtectionDomain getProtectionDomain(ClassLoader classLoader) { + public ProtectionDomain getProtectionDomain() { return null; } diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/DirectoryClassPathElement.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/DirectoryClassPathElement.java index ad71f5d817fab..ea78be461995f 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/DirectoryClassPathElement.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/DirectoryClassPathElement.java @@ -160,7 +160,7 @@ public void accept(Path path) { } @Override - public ProtectionDomain getProtectionDomain(ClassLoader classLoader) { + public ProtectionDomain getProtectionDomain() { URL url = null; try { URI uri = root.toUri(); @@ -169,8 +169,7 @@ public ProtectionDomain getProtectionDomain(ClassLoader classLoader) { throw new RuntimeException("Unable to create protection domain for " + root, e); } CodeSource codesource = new CodeSource(url, (Certificate[]) null); - ProtectionDomain protectionDomain = new ProtectionDomain(codesource, null, classLoader, null); - return protectionDomain; + return new ProtectionDomain(codesource, null); } @Override diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/FilteredClassPathElement.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/FilteredClassPathElement.java index 5e362a7e27d33..a72fcee9259ff 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/FilteredClassPathElement.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/FilteredClassPathElement.java @@ -58,8 +58,8 @@ public Set getProvidedResources() { } @Override - public ProtectionDomain getProtectionDomain(ClassLoader classLoader) { - return delegate.getProtectionDomain(classLoader); + public ProtectionDomain getProtectionDomain() { + return delegate.getProtectionDomain(); } @Override diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/JarClassPathElement.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/JarClassPathElement.java index 4ffbabf05a7ae..1c2aad6399a03 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/JarClassPathElement.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/JarClassPathElement.java @@ -256,7 +256,7 @@ public Set apply(JarFile jarFile) { } @Override - public ProtectionDomain getProtectionDomain(ClassLoader classLoader) { + public ProtectionDomain getProtectionDomain() { final URL url; try { url = jarPath.toURI().toURL(); @@ -264,7 +264,7 @@ public ProtectionDomain getProtectionDomain(ClassLoader classLoader) { throw new RuntimeException("Unable to create protection domain for " + jarPath, e); } CodeSource codesource = new CodeSource(url, (Certificate[]) null); - return new ProtectionDomain(codesource, null, classLoader, null); + return new ProtectionDomain(codesource, null, null, null); } @Override diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/MemoryClassPathElement.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/MemoryClassPathElement.java index 4458602ae5730..b77b6dbf18168 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/MemoryClassPathElement.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/MemoryClassPathElement.java @@ -111,7 +111,7 @@ public Set getProvidedResources() { } @Override - public ProtectionDomain getProtectionDomain(ClassLoader classLoader) { + public ProtectionDomain getProtectionDomain() { URL url = null; try { url = new URL(null, "quarkus:/", new MemoryUrlStreamHandler("quarkus:/")); @@ -119,8 +119,7 @@ public ProtectionDomain getProtectionDomain(ClassLoader classLoader) { throw new RuntimeException("Unable to create protection domain for memory element", e); } CodeSource codesource = new CodeSource(url, (Certificate[]) null); - ProtectionDomain protectionDomain = new ProtectionDomain(codesource, null, classLoader, null); - return protectionDomain; + return new ProtectionDomain(codesource, null); } @Override diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/PathTreeClassPathElement.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/PathTreeClassPathElement.java index cba6b34e0c08b..6f05de1707e71 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/PathTreeClassPathElement.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/PathTreeClassPathElement.java @@ -159,7 +159,7 @@ protected Manifest readManifest() { } @Override - public ProtectionDomain getProtectionDomain(ClassLoader classLoader) { + public ProtectionDomain getProtectionDomain() { URL url = null; final Path root = getRoot(); try { @@ -168,8 +168,7 @@ public ProtectionDomain getProtectionDomain(ClassLoader classLoader) { throw new RuntimeException("Unable to create protection domain for " + root, e); } CodeSource codesource = new CodeSource(url, (Certificate[]) null); - ProtectionDomain protectionDomain = new ProtectionDomain(codesource, null, classLoader, null); - return protectionDomain; + return new ProtectionDomain(codesource, null); } @Override diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java index bc2323fb2c056..e1b20fe85d657 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java @@ -506,7 +506,7 @@ protected Class loadClass(String name, boolean resolve) throws ClassNotFoundE byte[] data = classPathElementResource.getData(); definePackage(name, classPathElement); Class cl = defineClass(name, data, 0, data.length, - protectionDomains.computeIfAbsent(classPathElement, (ce) -> ce.getProtectionDomain(this))); + protectionDomains.computeIfAbsent(classPathElement, ClassPathElement::getProtectionDomain)); if (Driver.class.isAssignableFrom(cl)) { driverLoaded = true; } diff --git a/independent-projects/bootstrap/runner/src/main/java/io/quarkus/bootstrap/runner/ClassLoadingResource.java b/independent-projects/bootstrap/runner/src/main/java/io/quarkus/bootstrap/runner/ClassLoadingResource.java index 58bae161452b2..a6a2540601a2d 100644 --- a/independent-projects/bootstrap/runner/src/main/java/io/quarkus/bootstrap/runner/ClassLoadingResource.java +++ b/independent-projects/bootstrap/runner/src/main/java/io/quarkus/bootstrap/runner/ClassLoadingResource.java @@ -9,7 +9,7 @@ public interface ClassLoadingResource { * A lifecycle hook that should be called when the ClassLoader to which this resource belongs to * is constructed */ - void init(ClassLoader runnerClassLoader); + void init(); byte[] getResourceData(String resource); diff --git a/independent-projects/bootstrap/runner/src/main/java/io/quarkus/bootstrap/runner/JarResource.java b/independent-projects/bootstrap/runner/src/main/java/io/quarkus/bootstrap/runner/JarResource.java index 0a8ff83203eb3..6b8a6117414f3 100644 --- a/independent-projects/bootstrap/runner/src/main/java/io/quarkus/bootstrap/runner/JarResource.java +++ b/independent-projects/bootstrap/runner/src/main/java/io/quarkus/bootstrap/runner/JarResource.java @@ -52,7 +52,7 @@ public JarResource(ManifestInfo manifestInfo, Path jarPath) { } @Override - public void init(ClassLoader runnerClassLoader) { + public void init() { final URL url; try { String path = jarPath.toAbsolutePath().toString(); @@ -64,7 +64,7 @@ public void init(ClassLoader runnerClassLoader) { } catch (URISyntaxException | MalformedURLException e) { throw new RuntimeException("Unable to create protection domain for " + jarPath, e); } - this.protectionDomain = new ProtectionDomain(new CodeSource(url, (Certificate[]) null), null, runnerClassLoader, null); + this.protectionDomain = new ProtectionDomain(new CodeSource(url, (Certificate[]) null), null); } @Override diff --git a/independent-projects/bootstrap/runner/src/main/java/io/quarkus/bootstrap/runner/SerializedApplication.java b/independent-projects/bootstrap/runner/src/main/java/io/quarkus/bootstrap/runner/SerializedApplication.java index fab316850bd73..1ba0a0fbbaf12 100644 --- a/independent-projects/bootstrap/runner/src/main/java/io/quarkus/bootstrap/runner/SerializedApplication.java +++ b/independent-projects/bootstrap/runner/src/main/java/io/quarkus/bootstrap/runner/SerializedApplication.java @@ -162,7 +162,7 @@ public static SerializedApplication read(InputStream inputStream, Path appRoot) resourceDirectoryTracker.getResult(), parentFirstPackages, nonExistentResources, FULLY_INDEXED_PATHS, directlyIndexedResourcesIndexMap); for (ClassLoadingResource classLoadingResource : allClassLoadingResources) { - classLoadingResource.init(runnerClassLoader); + classLoadingResource.init(); } return new SerializedApplication(runnerClassLoader, mainClass); } From 2e9bfde8cc9fa91941c39300660c1735e0cdf228 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Fri, 20 Oct 2023 12:46:58 +0200 Subject: [PATCH 07/11] Improve error messages on KafkaDevServicesDevModeTestCase failures --- .../dev/KafkaDevServicesDevModeTestCase.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/extensions/smallrye-reactive-messaging-kafka/deployment/src/test/java/io/quarkus/smallrye/reactivemessaging/kafka/deployment/dev/KafkaDevServicesDevModeTestCase.java b/extensions/smallrye-reactive-messaging-kafka/deployment/src/test/java/io/quarkus/smallrye/reactivemessaging/kafka/deployment/dev/KafkaDevServicesDevModeTestCase.java index 34ea6085e013b..0f8c763678b1f 100644 --- a/extensions/smallrye-reactive-messaging-kafka/deployment/src/test/java/io/quarkus/smallrye/reactivemessaging/kafka/deployment/dev/KafkaDevServicesDevModeTestCase.java +++ b/extensions/smallrye-reactive-messaging-kafka/deployment/src/test/java/io/quarkus/smallrye/reactivemessaging/kafka/deployment/dev/KafkaDevServicesDevModeTestCase.java @@ -1,5 +1,7 @@ package io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev; +import static org.assertj.core.api.Assertions.assertThat; + import java.net.URI; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -10,7 +12,6 @@ import jakarta.ws.rs.client.WebTarget; import jakarta.ws.rs.sse.SseEventSource; -import org.assertj.core.api.Assertions; import org.awaitility.Awaitility; import org.jboss.shrinkwrap.api.ShrinkWrap; import org.jboss.shrinkwrap.api.asset.StringAsset; @@ -57,10 +58,10 @@ public void sseStream() { source.open(); Awaitility.await() - .until(() -> received.size() >= 2); + .untilAsserted(() -> assertThat(received).hasSizeGreaterThanOrEqualTo(2)); } - Assertions.assertThat(received) + assertThat(received) .hasSizeGreaterThanOrEqualTo(2) .allMatch(value -> (value >= 0) && (value < 100)); @@ -75,10 +76,10 @@ public void sseStream() { source.open(); Awaitility.await() - .until(() -> received.size() >= 2); + .untilAsserted(() -> assertThat(received).hasSizeGreaterThanOrEqualTo(2)); } - Assertions.assertThat(received) + assertThat(received) .hasSizeGreaterThanOrEqualTo(2) .allMatch(value -> (value >= 0) && (value < 100)); } From b1a4ab83d399c8a2de842c225c1b741c06080974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 25 Oct 2023 10:54:26 +0200 Subject: [PATCH 08/11] Plug leak in tests relying on lazily-initialized config Since a previous commit, we now remember the lazily-created config in QuarkusConfigFactory, which is good because in dev mode it allows us to release it upon restart (when we call setConfig(null)). However, in *tests* that rely on this lazily initialized config, just calling releaseConfig(ConfigProvider.getConfig()) is no longer enough, because of that remembered config in QuarkusConfigFactory: we need to reset that reference too. If we forget to reset it, then when KafkaDevServicesDevModeTestCase will execute, TestHTTPResourceManager#getUri will retrieve the leaked config from DefaultSerdeConfigTest or DefaultSchemaConfigTest, the injected URI in KafkaDevServicesDevModeTestCase will be wrong, and the test will fail (with very unhelpful error messages). --- .../kafka/deployment/DefaultSerdeConfigTest.java | 6 +++--- .../pulsar/deployment/DefaultSchemaConfigTest.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/extensions/smallrye-reactive-messaging-kafka/deployment/src/test/java/io/quarkus/smallrye/reactivemessaging/kafka/deployment/DefaultSerdeConfigTest.java b/extensions/smallrye-reactive-messaging-kafka/deployment/src/test/java/io/quarkus/smallrye/reactivemessaging/kafka/deployment/DefaultSerdeConfigTest.java index 08d3fa4245d92..e6448af4e145c 100644 --- a/extensions/smallrye-reactive-messaging-kafka/deployment/src/test/java/io/quarkus/smallrye/reactivemessaging/kafka/deployment/DefaultSerdeConfigTest.java +++ b/extensions/smallrye-reactive-messaging-kafka/deployment/src/test/java/io/quarkus/smallrye/reactivemessaging/kafka/deployment/DefaultSerdeConfigTest.java @@ -26,7 +26,6 @@ import org.assertj.core.api.Assert; import org.assertj.core.groups.Tuple; import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import org.eclipse.microprofile.reactive.messaging.Channel; import org.eclipse.microprofile.reactive.messaging.Emitter; import org.eclipse.microprofile.reactive.messaging.Incoming; @@ -47,6 +46,7 @@ import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem; import io.quarkus.kafka.client.serialization.JsonbSerializer; import io.quarkus.kafka.client.serialization.ObjectMapperDeserializer; +import io.quarkus.runtime.configuration.QuarkusConfigFactory; import io.quarkus.smallrye.reactivemessaging.deployment.items.ConnectorManagedChannelBuildItem; import io.smallrye.config.SmallRyeConfigBuilder; import io.smallrye.config.common.MapBackedConfigSource; @@ -126,9 +126,9 @@ boolean isKafkaConnector(List list, boolean in .flatExtracting(ReflectiveClassBuildItem::getClassNames) .allSatisfy(s -> assertThat(reflectiveNames).satisfiesOnlyOnce(c -> c.apply(s))); } finally { - // must not leak the Config instance associated to the system classloader + // must not leak the lazily-initialized Config instance associated to the system classloader if (customConfig == null) { - ConfigProviderResolver.instance().releaseConfig(discovery.getConfig()); + QuarkusConfigFactory.setConfig(null); } } } diff --git a/extensions/smallrye-reactive-messaging-pulsar/deployment/src/test/java/io/quarkus/smallrye/reactivemessaging/pulsar/deployment/DefaultSchemaConfigTest.java b/extensions/smallrye-reactive-messaging-pulsar/deployment/src/test/java/io/quarkus/smallrye/reactivemessaging/pulsar/deployment/DefaultSchemaConfigTest.java index 32ed032c356ed..5f8fea4d404e5 100644 --- a/extensions/smallrye-reactive-messaging-pulsar/deployment/src/test/java/io/quarkus/smallrye/reactivemessaging/pulsar/deployment/DefaultSchemaConfigTest.java +++ b/extensions/smallrye-reactive-messaging-pulsar/deployment/src/test/java/io/quarkus/smallrye/reactivemessaging/pulsar/deployment/DefaultSchemaConfigTest.java @@ -23,7 +23,6 @@ import org.apache.pulsar.client.api.Schema; import org.assertj.core.groups.Tuple; import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import org.eclipse.microprofile.reactive.messaging.Channel; import org.eclipse.microprofile.reactive.messaging.Emitter; import org.eclipse.microprofile.reactive.messaging.Incoming; @@ -47,6 +46,7 @@ import io.quarkus.deployment.recording.RecorderContext; import io.quarkus.pulsar.SchemaProviderRecorder; import io.quarkus.runtime.RuntimeValue; +import io.quarkus.runtime.configuration.QuarkusConfigFactory; import io.quarkus.smallrye.reactivemessaging.deployment.items.ConnectorManagedChannelBuildItem; import io.smallrye.common.annotation.Identifier; import io.smallrye.config.SmallRyeConfigBuilder; @@ -125,9 +125,9 @@ String generateId(Type type, String targetType) { assertThat(syntheticBean.alreadyGeneratedSchema).containsExactlyInAnyOrderEntriesOf(generatedSchemas); } finally { - // must not leak the Config instance associated to the system classloader + // must not leak the lazily-initialized Config instance associated to the system classloader if (customConfig == null) { - ConfigProviderResolver.instance().releaseConfig(discovery.getConfig()); + QuarkusConfigFactory.setConfig(null); } } } From 711cb7abca1660cdd8037e8bc4bf8eb5cfc1d9b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 25 Oct 2023 13:22:56 +0200 Subject: [PATCH 09/11] Stronger safeguards against leaked config in internal Quarkus*Test extensions If someone calls ConfigProvider.getConfig() out of the blue in a test that doesn't use any Quarkus*Test extension, this will call QuarkusConfigFactory and leak config in two ways: 1. In QuarkusConfigFactory#config 2. In SmallRyeConfigProviderResolver, registering config for the TCCL, which in such a case is most likely the system CL. A well-behaved test would call QuarkusConfigFactory.setConfig(null) to clean up all that, but it's easy to miss and there is potential for ConfigProvider.getConfig() being called indirectly, so there's no way we can guarantee all tests are well-behaved. This should at least guarantee that after a badly behaving test executes, the next test using a Quarkus*Test extension will clean up the mess. --- .../main/java/io/quarkus/test/ConfigUtil.java | 30 +++++++++++++++++++ .../io/quarkus/test/QuarkusDevModeTest.java | 2 ++ .../io/quarkus/test/QuarkusProdModeTest.java | 3 ++ .../java/io/quarkus/test/QuarkusUnitTest.java | 2 ++ 4 files changed, 37 insertions(+) create mode 100644 test-framework/junit5-internal/src/main/java/io/quarkus/test/ConfigUtil.java diff --git a/test-framework/junit5-internal/src/main/java/io/quarkus/test/ConfigUtil.java b/test-framework/junit5-internal/src/main/java/io/quarkus/test/ConfigUtil.java new file mode 100644 index 0000000000000..c7c0c5ebc1488 --- /dev/null +++ b/test-framework/junit5-internal/src/main/java/io/quarkus/test/ConfigUtil.java @@ -0,0 +1,30 @@ +package io.quarkus.test; + +import io.quarkus.runtime.configuration.QuarkusConfigFactory; + +public final class ConfigUtil { + private ConfigUtil() { + } + + /** + * Clean up config left over from badly-behaving previous tests. + *

+ * Tests may call ConfigProvider.getConfig() directly or indirectly, + * triggering lazy initialization in QuarkusConfigFactory#getConfigFor, + * and if they don't call QuarkusConfigFactory.setConfig(null) afterward, + * they may leak that config. + * As it's quite hard to guarantee that all tests clean up after them, + * especially if they don't use any Quarkus*Test extensions, + * we call this cleanup here in Quarkus*Test extensions as an additional safeguard. + *

+ * Note this must be called quite early in extensions in order to be effective: + * things like TestHTTPResourceManager#getUri make use of config and may be called very early, + * upon test instantiation, so cleaning up in beforeEach() won't cut it for example. + */ + public static void cleanUp() { + try { + QuarkusConfigFactory.setConfig(null); + } catch (Throwable ignored) { + } + } +} diff --git a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java index c80f987df6caf..79cc8f6b2c1b7 100644 --- a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java +++ b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java @@ -228,6 +228,7 @@ public Object createTestInstance(TestInstanceFactoryContext factoryContext, Exte @Override public void beforeAll(ExtensionContext context) throws Exception { + ConfigUtil.cleanUp(); GroovyClassValue.disable(); //set the right launch mode in the outer CL, used by the HTTP host config source ProfileManager.setLaunchMode(LaunchMode.DEVELOPMENT); @@ -324,6 +325,7 @@ public void afterAll(ExtensionContext context) throws Exception { inMemoryLogHandler.clearRecords(); inMemoryLogHandler.setFilter(null); ClearCache.clearAnnotationCache(); + ConfigUtil.cleanUp(); } @Override diff --git a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusProdModeTest.java b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusProdModeTest.java index 3e5b7c72ef43f..936ce48d4cb6e 100644 --- a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusProdModeTest.java +++ b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusProdModeTest.java @@ -365,6 +365,7 @@ private JavaArchive getArchiveProducerOrDefault() { @Override public void beforeAll(ExtensionContext extensionContext) throws Exception { + ConfigUtil.cleanUp(); ensureNoInjectAnnotationIsUsed(extensionContext.getRequiredTestClass()); ExclusivityChecker.checkTestType(extensionContext, QuarkusProdModeTest.class); @@ -757,6 +758,8 @@ public void afterAll(ExtensionContext extensionContext) throws Exception { if ((outputDir != null) && !preventOutputDirCleanup) { FileUtil.deleteDirectory(outputDir); } + + ConfigUtil.cleanUp(); } } diff --git a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusUnitTest.java b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusUnitTest.java index f17fd2e19514c..53c82ed45d735 100644 --- a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusUnitTest.java +++ b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusUnitTest.java @@ -507,6 +507,7 @@ private void runExtensionMethod(ReflectiveInvocationContext invocationCo @Override public void beforeAll(ExtensionContext extensionContext) throws Exception { + ConfigUtil.cleanUp(); GroovyClassValue.disable(); //set the right launch mode in the outer CL, used by the HTTP host config source ProfileManager.setLaunchMode(LaunchMode.TEST); @@ -766,6 +767,7 @@ public void afterAll(ExtensionContext extensionContext) throws Exception { afterAllCustomizer.run(); } ClearCache.clearAnnotationCache(); + ConfigUtil.cleanUp(); } if (records != null) { assertLogRecords.accept(records); From e76a0e537f4588fa268cf58058ff4dc1a764560e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 25 Oct 2023 11:40:48 +0200 Subject: [PATCH 10/11] More comprehensive registration/release of config in some tests If someone calls ConfigProvider.getConfig() out of the blue in a test that doesn't use any Quarkus*Test extension, this will indirectly call QuarkusConfigFactory and leak config in two ways: 1. In QuarkusConfigFactory#config 2. In SmallRyeConfigProviderResolver, registering config for the TCCL, which in such a case is most likely the system CL. Thus, a well-behaved test should call QuarkusConfigFactory.setConfig(null) to clean up all that, no just SmallRyeConfigProviderResolver.releaseConfig(). Similarly, tests that register configuration explicitly can just call QuarkusConfigFactory.setConfig(config) at the beginning and QuarkusConfigFactory.setConfig(null) at the end, which will properly simulate how a real Quarkus application behaves, and should cover all edge cases involving multiple classloaders, properly cleaning up everything at the end of the test. --- .../yaml/runtime/ApplicationYamlTest.java | 24 ++++---------- .../config/RestClientConfigTest.java | 31 ++++--------------- .../runtime/RestClientBaseTest.java | 12 ------- .../RestClientCDIDelegateBuilderTest.java | 12 ------- .../ConfigMappingStartupValidatorTest.java | 12 +------ .../io/quarkus/test/common/LauncherUtil.java | 9 ------ .../test/common/TestResourceManager.java | 7 +++++ 7 files changed, 20 insertions(+), 87 deletions(-) diff --git a/extensions/config-yaml/runtime/src/test/java/io/quarkus/config/yaml/runtime/ApplicationYamlTest.java b/extensions/config-yaml/runtime/src/test/java/io/quarkus/config/yaml/runtime/ApplicationYamlTest.java index ec9621c9d49d8..77794401d0d05 100644 --- a/extensions/config-yaml/runtime/src/test/java/io/quarkus/config/yaml/runtime/ApplicationYamlTest.java +++ b/extensions/config-yaml/runtime/src/test/java/io/quarkus/config/yaml/runtime/ApplicationYamlTest.java @@ -3,14 +3,10 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; -import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.ConfigProvider; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import io.quarkus.runtime.configuration.QuarkusConfigFactory; import io.smallrye.config.SmallRyeConfig; import io.smallrye.config.SmallRyeConfigBuilder; @@ -21,34 +17,26 @@ */ public class ApplicationYamlTest { - static volatile SmallRyeConfig config; + private static SmallRyeConfig config; @BeforeAll public static void doBefore() { SmallRyeConfigBuilder builder = new SmallRyeConfigBuilder(); builder.addDefaultSources().addDiscoveredConverters().addDiscoveredSources(); - QuarkusConfigFactory.setConfig(config = builder.build()); - Config conf = ConfigProvider.getConfig(); - if (conf != config) { - ConfigProviderResolver cpr = ConfigProviderResolver.instance(); - cpr.releaseConfig(conf); - ConfigProvider.getConfig(); - } + config = builder.build(); System.out.println(System.getProperty("user.dir")); } @Test public void testBasicApplicationYaml() { - assertEquals("something", ConfigProvider.getConfig().getValue("foo.bar", String.class)); - assertEquals("somethingElse", ConfigProvider.getConfig().getValue("foo2.bar", String.class)); - assertEquals("other", ConfigProvider.getConfig().getValue("foo.baz", String.class)); - assertTrue(ConfigProvider.getConfig().getValue("file.system", Boolean.class).booleanValue()); + assertEquals("something", config.getValue("foo.bar", String.class)); + assertEquals("somethingElse", config.getValue("foo2.bar", String.class)); + assertEquals("other", config.getValue("foo.baz", String.class)); + assertTrue(config.getValue("file.system", Boolean.class).booleanValue()); } @AfterAll public static void doAfter() { - ConfigProviderResolver cpr = ConfigProviderResolver.instance(); - cpr.releaseConfig(config); config = null; } } diff --git a/extensions/resteasy-classic/rest-client-config/runtime/src/test/java/io/quarkus/restclient/config/RestClientConfigTest.java b/extensions/resteasy-classic/rest-client-config/runtime/src/test/java/io/quarkus/restclient/config/RestClientConfigTest.java index 8ce5ca469dd37..051d4db37e3a1 100644 --- a/extensions/resteasy-classic/rest-client-config/runtime/src/test/java/io/quarkus/restclient/config/RestClientConfigTest.java +++ b/extensions/resteasy-classic/rest-client-config/runtime/src/test/java/io/quarkus/restclient/config/RestClientConfigTest.java @@ -6,38 +6,19 @@ import java.net.URL; import java.util.Optional; -import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.ConfigProvider; -import org.eclipse.microprofile.config.spi.ConfigBuilder; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import org.eclipse.microprofile.rest.client.ext.QueryParamStyle; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import io.smallrye.config.PropertiesConfigSource; +import io.smallrye.config.SmallRyeConfig; +import io.smallrye.config.SmallRyeConfigBuilder; public class RestClientConfigTest { - private static ClassLoader classLoader; - private static ConfigProviderResolver configProviderResolver; - - @BeforeAll - public static void initConfig() { - classLoader = Thread.currentThread().getContextClassLoader(); - configProviderResolver = ConfigProviderResolver.instance(); - } - - @AfterEach - public void releaseConfig() { - configProviderResolver.releaseConfig(configProviderResolver.getConfig()); - } - @Test public void testLoadRestClientConfig() throws IOException { - setupMPConfig(); + SmallRyeConfig config = createMPConfig(); - Config config = ConfigProvider.getConfig(); Optional optionalValue = config.getOptionalValue("quarkus.rest-client.test-client.url", String.class); assertThat(optionalValue).isPresent(); @@ -75,11 +56,11 @@ private void verifyConfig(RestClientConfig config) { assertThat(config.multipart.maxChunkSize.get()).isEqualTo(1024); } - private static void setupMPConfig() throws IOException { - ConfigBuilder configBuilder = configProviderResolver.getBuilder(); + private static SmallRyeConfig createMPConfig() throws IOException { + SmallRyeConfigBuilder configBuilder = new SmallRyeConfigBuilder().addDefaultInterceptors(); URL propertyFile = RestClientConfigTest.class.getClassLoader().getResource("application.properties"); assertThat(propertyFile).isNotNull(); configBuilder.withSources(new PropertiesConfigSource(propertyFile)); - configProviderResolver.registerConfig(configBuilder.build(), classLoader); + return configBuilder.build(); } } diff --git a/extensions/resteasy-classic/rest-client/runtime/src/test/java/io/quarkus/restclient/runtime/RestClientBaseTest.java b/extensions/resteasy-classic/rest-client/runtime/src/test/java/io/quarkus/restclient/runtime/RestClientBaseTest.java index 858fcb8bd346f..52f2b42a2aa3f 100644 --- a/extensions/resteasy-classic/rest-client/runtime/src/test/java/io/quarkus/restclient/runtime/RestClientBaseTest.java +++ b/extensions/resteasy-classic/rest-client/runtime/src/test/java/io/quarkus/restclient/runtime/RestClientBaseTest.java @@ -21,13 +21,10 @@ import jakarta.ws.rs.client.ClientResponseContext; import jakarta.ws.rs.client.ClientResponseFilter; -import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import org.eclipse.microprofile.rest.client.RestClientBuilder; import org.eclipse.microprofile.rest.client.ext.QueryParamStyle; import org.eclipse.microprofile.rest.client.inject.RegisterRestClient; import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -42,7 +39,6 @@ public class RestClientBaseTest { private static Path truststorePath; private static Path keystorePath; - private static Config createdConfig; @BeforeAll public static void beforeAll() throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException { @@ -65,14 +61,6 @@ public static void beforeAll() throws IOException, KeyStoreException, Certificat } } - @AfterEach - public void afterEach() { - if (createdConfig != null) { - ConfigProviderResolver.instance().releaseConfig(createdConfig); - createdConfig = null; - } - } - @AfterAll public static void afterAll() { if (truststorePath != null) { diff --git a/extensions/resteasy-reactive/rest-client-reactive/runtime/src/test/java/io/quarkus/rest/client/reactive/runtime/RestClientCDIDelegateBuilderTest.java b/extensions/resteasy-reactive/rest-client-reactive/runtime/src/test/java/io/quarkus/rest/client/reactive/runtime/RestClientCDIDelegateBuilderTest.java index 36556d81add90..f028d6841b9df 100644 --- a/extensions/resteasy-reactive/rest-client-reactive/runtime/src/test/java/io/quarkus/rest/client/reactive/runtime/RestClientCDIDelegateBuilderTest.java +++ b/extensions/resteasy-reactive/rest-client-reactive/runtime/src/test/java/io/quarkus/rest/client/reactive/runtime/RestClientCDIDelegateBuilderTest.java @@ -16,12 +16,9 @@ import jakarta.ws.rs.client.ClientResponseContext; import jakarta.ws.rs.client.ClientResponseFilter; -import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import org.eclipse.microprofile.rest.client.ext.QueryParamStyle; import org.eclipse.microprofile.rest.client.inject.RegisterRestClient; import org.jboss.resteasy.reactive.client.api.QuarkusRestClientProperties; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -42,7 +39,6 @@ public class RestClientCDIDelegateBuilderTest { static File tempDir; private static File truststoreFile; private static File keystoreFile; - private static Config createdConfig; @BeforeAll public static void beforeAll() throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException { @@ -60,14 +56,6 @@ public static void beforeAll() throws IOException, KeyStoreException, Certificat keystore.store(new FileOutputStream(keystoreFile), KEYSTORE_PASSWORD.toCharArray()); } - @AfterEach - public void afterEach() { - if (createdConfig != null) { - ConfigProviderResolver.instance().releaseConfig(createdConfig); - createdConfig = null; - } - } - @Test public void testClientSpecificConfigs() { // given diff --git a/integration-tests/hibernate-validator-resteasy-reactive/src/test/java/io/quarkus/it/hibernate/validator/ConfigMappingStartupValidatorTest.java b/integration-tests/hibernate-validator-resteasy-reactive/src/test/java/io/quarkus/it/hibernate/validator/ConfigMappingStartupValidatorTest.java index 3abcdfc7058e2..16652a5a9c213 100644 --- a/integration-tests/hibernate-validator-resteasy-reactive/src/test/java/io/quarkus/it/hibernate/validator/ConfigMappingStartupValidatorTest.java +++ b/integration-tests/hibernate-validator-resteasy-reactive/src/test/java/io/quarkus/it/hibernate/validator/ConfigMappingStartupValidatorTest.java @@ -11,9 +11,6 @@ import jakarta.inject.Inject; import jakarta.validation.constraints.Pattern; -import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.ConfigProvider; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -88,19 +85,12 @@ public void validateMapping(Class mappingClass, String prefix, Object mapping } }); QuarkusConfigFactory.setConfig(smallryeConfig = builder.build()); - Config conf = ConfigProvider.getConfig(); - if (conf != smallryeConfig) { - ConfigProviderResolver cpr = ConfigProviderResolver.instance(); - cpr.releaseConfig(conf); - ConfigProvider.getConfig(); - } suppressedConfigValidatorExceptions = new HashMap<>(); } @AfterAll public static void doAfter() { - ConfigProviderResolver cpr = ConfigProviderResolver.instance(); - cpr.releaseConfig(smallryeConfig); + QuarkusConfigFactory.setConfig(null); smallryeConfig = null; suppressedConfigValidatorExceptions = null; } diff --git a/test-framework/common/src/main/java/io/quarkus/test/common/LauncherUtil.java b/test-framework/common/src/main/java/io/quarkus/test/common/LauncherUtil.java index 3742af25af83f..544514617d488 100644 --- a/test-framework/common/src/main/java/io/quarkus/test/common/LauncherUtil.java +++ b/test-framework/common/src/main/java/io/quarkus/test/common/LauncherUtil.java @@ -22,7 +22,6 @@ import java.util.regex.Pattern; import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.spi.ConfigProviderResolver; import io.quarkus.runtime.LaunchMode; import io.quarkus.runtime.configuration.ConfigUtils; @@ -41,14 +40,6 @@ private LauncherUtil() { public static Config installAndGetSomeConfig() { final SmallRyeConfig config = ConfigUtils.configBuilder(false, LaunchMode.NORMAL).build(); QuarkusConfigFactory.setConfig(config); - final ConfigProviderResolver cpr = ConfigProviderResolver.instance(); - try { - final Config installed = cpr.getConfig(); - if (installed != config) { - cpr.releaseConfig(installed); - } - } catch (IllegalStateException ignored) { - } return config; } diff --git a/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java b/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java index 9e088371f3fde..dfe8865ee850b 100644 --- a/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java +++ b/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java @@ -179,6 +179,13 @@ public void close() { throw new RuntimeException("Unable to stop Quarkus test resource " + entry.getTestResource(), e); } } + // TODO using QuarkusConfigFactory.setConfig(null) here makes continuous testing fail, + // e.g. in io.quarkus.hibernate.orm.HibernateHotReloadTestCase + // or io.quarkus.opentelemetry.deployment.OpenTelemetryContinuousTestingTest; + // maybe this cleanup is not really necessary and just "doesn't hurt" because + // the released config is still cached in QuarkusConfigFactory#config + // and will be restored soon after when QuarkusConfigFactory#getConfigFor is called? + // In that case we should remove this cleanup. try { ((SmallRyeConfigProviderResolver) SmallRyeConfigProviderResolver.instance()) .releaseConfig(Thread.currentThread().getContextClassLoader()); From 683741bf3d696b061cb45ce21d0fb3937058bfa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Thu, 26 Oct 2023 08:51:35 +0200 Subject: [PATCH 11/11] Merge test-related ConfigUtils into a single class --- .../quarkus/test/common/TestConfigUtil.java} | 33 ++++++++++++++++--- .../main/java/io/quarkus/test/ConfigUtil.java | 30 ----------------- .../io/quarkus/test/QuarkusDevModeTest.java | 5 +-- .../io/quarkus/test/QuarkusProdModeTest.java | 5 +-- .../java/io/quarkus/test/QuarkusUnitTest.java | 5 +-- .../test/junit/IntegrationTestUtil.java | 1 - .../QuarkusIntegrationTestExtension.java | 6 ++-- .../DockerContainerLauncherProvider.java | 9 ++--- .../junit/launcher/JarLauncherProvider.java | 9 ++--- .../launcher/NativeImageLauncherProvider.java | 9 ++--- 10 files changed, 55 insertions(+), 57 deletions(-) rename test-framework/{junit5/src/main/java/io/quarkus/test/junit/launcher/ConfigUtil.java => common/src/main/java/io/quarkus/test/common/TestConfigUtil.java} (61%) delete mode 100644 test-framework/junit5-internal/src/main/java/io/quarkus/test/ConfigUtil.java diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/ConfigUtil.java b/test-framework/common/src/main/java/io/quarkus/test/common/TestConfigUtil.java similarity index 61% rename from test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/ConfigUtil.java rename to test-framework/common/src/main/java/io/quarkus/test/common/TestConfigUtil.java index a3d99b23f1b85..8c6e128b28ad7 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/ConfigUtil.java +++ b/test-framework/common/src/main/java/io/quarkus/test/common/TestConfigUtil.java @@ -1,6 +1,4 @@ -package io.quarkus.test.junit.launcher; - -import static io.quarkus.test.junit.IntegrationTestUtil.DEFAULT_WAIT_TIME_SECONDS; +package io.quarkus.test.common; import java.time.Duration; import java.util.ArrayList; @@ -10,11 +8,14 @@ import org.eclipse.microprofile.config.Config; +import io.quarkus.runtime.configuration.QuarkusConfigFactory; import io.smallrye.config.SmallRyeConfig; -public final class ConfigUtil { +public final class TestConfigUtil { + + public static final long DEFAULT_WAIT_TIME_SECONDS = 60; - private ConfigUtil() { + private TestConfigUtil() { } public static List argLineValue(Config config) { @@ -57,4 +58,26 @@ public static String runTarget(Config config) { return config.getOptionalValue("quarkus.run.target", String.class) .orElse(null); } + + /** + * Clean up config left over from badly-behaving previous tests. + *

+ * Tests may call ConfigProvider.getConfig() directly or indirectly, + * triggering lazy initialization in QuarkusConfigFactory#getConfigFor, + * and if they don't call QuarkusConfigFactory.setConfig(null) afterward, + * they may leak that config. + * As it's quite hard to guarantee that all tests clean up after them, + * especially if they don't use any Quarkus*Test extensions, + * we call this cleanup here in Quarkus*Test extensions as an additional safeguard. + *

+ * Note this must be called quite early in extensions in order to be effective: + * things like TestHTTPResourceManager#getUri make use of config and may be called very early, + * upon test instantiation, so cleaning up in beforeEach() won't cut it for example. + */ + public static void cleanUp() { + try { + QuarkusConfigFactory.setConfig(null); + } catch (Throwable ignored) { + } + } } diff --git a/test-framework/junit5-internal/src/main/java/io/quarkus/test/ConfigUtil.java b/test-framework/junit5-internal/src/main/java/io/quarkus/test/ConfigUtil.java deleted file mode 100644 index c7c0c5ebc1488..0000000000000 --- a/test-framework/junit5-internal/src/main/java/io/quarkus/test/ConfigUtil.java +++ /dev/null @@ -1,30 +0,0 @@ -package io.quarkus.test; - -import io.quarkus.runtime.configuration.QuarkusConfigFactory; - -public final class ConfigUtil { - private ConfigUtil() { - } - - /** - * Clean up config left over from badly-behaving previous tests. - *

- * Tests may call ConfigProvider.getConfig() directly or indirectly, - * triggering lazy initialization in QuarkusConfigFactory#getConfigFor, - * and if they don't call QuarkusConfigFactory.setConfig(null) afterward, - * they may leak that config. - * As it's quite hard to guarantee that all tests clean up after them, - * especially if they don't use any Quarkus*Test extensions, - * we call this cleanup here in Quarkus*Test extensions as an additional safeguard. - *

- * Note this must be called quite early in extensions in order to be effective: - * things like TestHTTPResourceManager#getUri make use of config and may be called very early, - * upon test instantiation, so cleaning up in beforeEach() won't cut it for example. - */ - public static void cleanUp() { - try { - QuarkusConfigFactory.setConfig(null); - } catch (Throwable ignored) { - } - } -} diff --git a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java index 79cc8f6b2c1b7..d12b7e583b734 100644 --- a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java +++ b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java @@ -54,6 +54,7 @@ import io.quarkus.test.common.GroovyClassValue; import io.quarkus.test.common.PathTestHelper; import io.quarkus.test.common.PropertyTestUtil; +import io.quarkus.test.common.TestConfigUtil; import io.quarkus.test.common.TestResourceManager; import io.quarkus.test.common.http.TestHTTPResourceManager; @@ -228,7 +229,7 @@ public Object createTestInstance(TestInstanceFactoryContext factoryContext, Exte @Override public void beforeAll(ExtensionContext context) throws Exception { - ConfigUtil.cleanUp(); + TestConfigUtil.cleanUp(); GroovyClassValue.disable(); //set the right launch mode in the outer CL, used by the HTTP host config source ProfileManager.setLaunchMode(LaunchMode.DEVELOPMENT); @@ -325,7 +326,7 @@ public void afterAll(ExtensionContext context) throws Exception { inMemoryLogHandler.clearRecords(); inMemoryLogHandler.setFilter(null); ClearCache.clearAnnotationCache(); - ConfigUtil.cleanUp(); + TestConfigUtil.cleanUp(); } @Override diff --git a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusProdModeTest.java b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusProdModeTest.java index 936ce48d4cb6e..f77201d07bf6b 100644 --- a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusProdModeTest.java +++ b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusProdModeTest.java @@ -63,6 +63,7 @@ import io.quarkus.maven.dependency.Dependency; import io.quarkus.test.common.PathTestHelper; import io.quarkus.test.common.RestAssuredURLManager; +import io.quarkus.test.common.TestConfigUtil; import io.quarkus.test.common.TestResourceManager; import io.quarkus.utilities.JavaBinFinder; @@ -365,7 +366,7 @@ private JavaArchive getArchiveProducerOrDefault() { @Override public void beforeAll(ExtensionContext extensionContext) throws Exception { - ConfigUtil.cleanUp(); + TestConfigUtil.cleanUp(); ensureNoInjectAnnotationIsUsed(extensionContext.getRequiredTestClass()); ExclusivityChecker.checkTestType(extensionContext, QuarkusProdModeTest.class); @@ -759,7 +760,7 @@ public void afterAll(ExtensionContext extensionContext) throws Exception { FileUtil.deleteDirectory(outputDir); } - ConfigUtil.cleanUp(); + TestConfigUtil.cleanUp(); } } diff --git a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusUnitTest.java b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusUnitTest.java index 53c82ed45d735..39cfc3e9aa36d 100644 --- a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusUnitTest.java +++ b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusUnitTest.java @@ -75,6 +75,7 @@ import io.quarkus.test.common.PathTestHelper; import io.quarkus.test.common.PropertyTestUtil; import io.quarkus.test.common.RestAssuredURLManager; +import io.quarkus.test.common.TestConfigUtil; import io.quarkus.test.common.TestResourceManager; import io.quarkus.test.common.http.TestHTTPResourceManager; @@ -507,7 +508,7 @@ private void runExtensionMethod(ReflectiveInvocationContext invocationCo @Override public void beforeAll(ExtensionContext extensionContext) throws Exception { - ConfigUtil.cleanUp(); + TestConfigUtil.cleanUp(); GroovyClassValue.disable(); //set the right launch mode in the outer CL, used by the HTTP host config source ProfileManager.setLaunchMode(LaunchMode.TEST); @@ -767,7 +768,7 @@ public void afterAll(ExtensionContext extensionContext) throws Exception { afterAllCustomizer.run(); } ClearCache.clearAnnotationCache(); - ConfigUtil.cleanUp(); + TestConfigUtil.cleanUp(); } if (records != null) { assertLogRecords.accept(records); diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/IntegrationTestUtil.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/IntegrationTestUtil.java index f7f1a4cf31650..47ff74106e7dc 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/IntegrationTestUtil.java +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/IntegrationTestUtil.java @@ -64,7 +64,6 @@ public final class IntegrationTestUtil { public static final int DEFAULT_PORT = 8081; public static final int DEFAULT_HTTPS_PORT = 8444; - public static final long DEFAULT_WAIT_TIME_SECONDS = 60; private IntegrationTestUtil() { } diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusIntegrationTestExtension.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusIntegrationTestExtension.java index 3957326e54067..6192437575331 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusIntegrationTestExtension.java +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusIntegrationTestExtension.java @@ -52,12 +52,12 @@ import io.quarkus.test.common.PropertyTestUtil; import io.quarkus.test.common.RestAssuredURLManager; import io.quarkus.test.common.RunCommandLauncher; +import io.quarkus.test.common.TestConfigUtil; import io.quarkus.test.common.TestHostLauncher; import io.quarkus.test.common.TestResourceManager; import io.quarkus.test.common.TestScopeManager; import io.quarkus.test.junit.callback.QuarkusTestMethodContext; import io.quarkus.test.junit.launcher.ArtifactLauncherProvider; -import io.quarkus.test.junit.launcher.ConfigUtil; public class QuarkusIntegrationTestExtension extends AbstractQuarkusTestWithContextExtension implements BeforeTestExecutionCallback, AfterTestExecutionCallback, BeforeEachCallback, AfterEachCallback, @@ -267,8 +267,8 @@ public void close() throws Throwable { launcher = new TestHostLauncher(); } else { Config config = LauncherUtil.installAndGetSomeConfig(); - Duration waitDuration = ConfigUtil.waitTimeValue(config); - String target = ConfigUtil.runTarget(config); + Duration waitDuration = TestConfigUtil.waitTimeValue(config); + String target = TestConfigUtil.runTarget(config); // try to execute a run command published by an extension if it exists. We do this so that extensions that have a custom run don't have to create any special artifact type launcher = RunCommandLauncher.tryLauncher(devServicesLaunchResult.getCuratedApplication().getQuarkusBootstrap(), target, waitDuration); diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/DockerContainerLauncherProvider.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/DockerContainerLauncherProvider.java index 118e9f7323124..68e5d02992776 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/DockerContainerLauncherProvider.java +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/DockerContainerLauncherProvider.java @@ -17,6 +17,7 @@ import io.quarkus.test.common.DefaultDockerContainerLauncher; import io.quarkus.test.common.DockerContainerArtifactLauncher; import io.quarkus.test.common.LauncherUtil; +import io.quarkus.test.common.TestConfigUtil; import io.smallrye.config.SmallRyeConfig; public class DockerContainerLauncherProvider implements ArtifactLauncherProvider { @@ -44,10 +45,10 @@ public DockerContainerArtifactLauncher create(CreateContext context) { launcher.init(new DefaultDockerInitContext( config.getValue("quarkus.http.test-port", OptionalInt.class).orElse(DEFAULT_PORT), config.getValue("quarkus.http.test-ssl-port", OptionalInt.class).orElse(DEFAULT_HTTPS_PORT), - ConfigUtil.waitTimeValue(config), - ConfigUtil.integrationTestProfile(config), - ConfigUtil.argLineValue(config), - ConfigUtil.env(config), + TestConfigUtil.waitTimeValue(config), + TestConfigUtil.integrationTestProfile(config), + TestConfigUtil.argLineValue(config), + TestConfigUtil.env(config), context.devServicesLaunchResult(), containerImage, pullRequired, diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/JarLauncherProvider.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/JarLauncherProvider.java index 75a1f7ed4d89e..ce2efc03d62a9 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/JarLauncherProvider.java +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/JarLauncherProvider.java @@ -18,6 +18,7 @@ import io.quarkus.test.common.DefaultJarLauncher; import io.quarkus.test.common.JarArtifactLauncher; import io.quarkus.test.common.LauncherUtil; +import io.quarkus.test.common.TestConfigUtil; public class JarLauncherProvider implements ArtifactLauncherProvider { @@ -43,10 +44,10 @@ public JarArtifactLauncher create(CreateContext context) { launcher.init(new DefaultJarInitContext( config.getValue("quarkus.http.test-port", OptionalInt.class).orElse(DEFAULT_PORT), config.getValue("quarkus.http.test-ssl-port", OptionalInt.class).orElse(DEFAULT_HTTPS_PORT), - ConfigUtil.waitTimeValue(config), - ConfigUtil.integrationTestProfile(config), - ConfigUtil.argLineValue(config), - ConfigUtil.env(config), + TestConfigUtil.waitTimeValue(config), + TestConfigUtil.integrationTestProfile(config), + TestConfigUtil.argLineValue(config), + TestConfigUtil.env(config), context.devServicesLaunchResult(), context.buildOutputDirectory().resolve(pathStr))); return launcher; diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/NativeImageLauncherProvider.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/NativeImageLauncherProvider.java index f4ea8ece1921c..1ade26e674a59 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/NativeImageLauncherProvider.java +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/launcher/NativeImageLauncherProvider.java @@ -17,6 +17,7 @@ import io.quarkus.test.common.DefaultNativeImageLauncher; import io.quarkus.test.common.LauncherUtil; import io.quarkus.test.common.NativeImageLauncher; +import io.quarkus.test.common.TestConfigUtil; public class NativeImageLauncherProvider implements ArtifactLauncherProvider { @Override @@ -41,10 +42,10 @@ public NativeImageLauncher create(CreateContext context) { launcher.init(new NativeImageLauncherProvider.DefaultNativeImageInitContext( config.getValue("quarkus.http.test-port", OptionalInt.class).orElse(DEFAULT_PORT), config.getValue("quarkus.http.test-ssl-port", OptionalInt.class).orElse(DEFAULT_HTTPS_PORT), - ConfigUtil.waitTimeValue(config), - ConfigUtil.integrationTestProfile(config), - ConfigUtil.argLineValue(config), - ConfigUtil.env(config), + TestConfigUtil.waitTimeValue(config), + TestConfigUtil.integrationTestProfile(config), + TestConfigUtil.argLineValue(config), + TestConfigUtil.env(config), context.devServicesLaunchResult(), System.getProperty("native.image.path"), config.getOptionalValue("quarkus.package.output-directory", String.class).orElse(null),