Merge pull request quarkusio#36560 from yrodiere/metaspace-leaks-3

Metaspace improvements in QuarkusUnitTest (and dev mode!) - round 3
mswatosh · Oct 30, 2023 · 8d6d112 · 8d6d112
2 parents 7b0f197 + 683741b
commit 8d6d112
Show file tree

Hide file tree

Showing 39 changed files with 183 additions and 191 deletions.
diff --git a/...ment/src/main/java/io/quarkus/deployment/configuration/RunTimeConfigurationGenerator.java b/...ment/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<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) {

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
@@ -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/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/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/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<String, String> 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

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
@@ -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<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);
+    }
 }
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
@@ -19,21 +19,39 @@ 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;
+        }
+        // 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());
+    }
 }
diff --git a/...config-yaml/runtime/src/test/java/io/quarkus/config/yaml/runtime/ApplicationYamlTest.java b/...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/...t/src/test/resources/application-multiple-persistence-units-undefined-packages.properties b/...t/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

diff --git a/...lient-config/runtime/src/test/java/io/quarkus/restclient/config/RestClientConfigTest.java b/...lient-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<String> 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/...c/rest-client/runtime/src/test/java/io/quarkus/restclient/runtime/RestClientBaseTest.java b/...c/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/...c/test/java/io/quarkus/rest/client/reactive/runtime/RestClientCDIDelegateBuilderTest.java b/...c/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