From 44e1cfeee55271fe6ddbd8f022c2c66f89ef9eb1 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Sun, 16 Jun 2024 13:25:08 +0200 Subject: [PATCH] Close the deployment CL in the creator when augmenting Currently, we pass the CL to something that will close it, rather than controlling the lifecycle in the creator method. This is an issue in native ITs: java.lang.RuntimeException: java.lang.IllegalStateException: This class loader has been closed at io.quarkus.test.junit.QuarkusIntegrationTestExtension.throwBootFailureException(QuarkusIntegrationTestExtension.java:373) at io.quarkus.test.junit.QuarkusIntegrationTestExtension.beforeEach(QuarkusIntegrationTestExtension.java:117) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) Caused by: java.lang.IllegalStateException: This class loader has been closed at io.quarkus.bootstrap.classloading.QuarkusClassLoader.ensureOpen(QuarkusClassLoader.java:716) at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:495) at java.base/java.lang.Class.forName0(Native Method) at java.base/java.lang.Class.forName(Class.java:467) at io.quarkus.runner.bootstrap.AugmentActionImpl.performCustomBuild(AugmentActionImpl.java:158) at io.quarkus.test.junit.IntegrationTestUtil.handleDevServices(IntegrationTestUtil.java:297) at io.quarkus.test.junit.QuarkusIntegrationTestExtension.doProcessStart(QuarkusIntegrationTestExtension.java:199) at io.quarkus.test.junit.QuarkusIntegrationTestExtension.ensureStarted(QuarkusIntegrationTestExtension.java:169) at io.quarkus.test.junit.QuarkusIntegrationTestExtension.beforeAll(QuarkusIntegrationTestExtension.java:130) Related to #41233 --- .../quarkus/deployment/QuarkusAugmentor.java | 4 - .../deployment/jbang/JBangAugmentorImpl.java | 180 +++++++++--------- .../runner/bootstrap/AugmentActionImpl.java | 106 ++++++----- 3 files changed, 147 insertions(+), 143 deletions(-) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/QuarkusAugmentor.java b/core/deployment/src/main/java/io/quarkus/deployment/QuarkusAugmentor.java index c5380c77d5664..028d481c2d91b 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/QuarkusAugmentor.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/QuarkusAugmentor.java @@ -1,6 +1,5 @@ package io.quarkus.deployment; -import java.io.Closeable; import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; @@ -181,9 +180,6 @@ public BuildResult run() throws Exception { .releaseConfig(deploymentClassLoader); } catch (Exception ignore) { - } - if (deploymentClassLoader instanceof Closeable) { - ((Closeable) deploymentClassLoader).close(); } Thread.currentThread().setContextClassLoader(originalClassLoader); buildCloseables.close(); diff --git a/core/deployment/src/main/java/io/quarkus/deployment/jbang/JBangAugmentorImpl.java b/core/deployment/src/main/java/io/quarkus/deployment/jbang/JBangAugmentorImpl.java index f0289037eea19..40b2b0f3f8a23 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/jbang/JBangAugmentorImpl.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/jbang/JBangAugmentorImpl.java @@ -42,102 +42,106 @@ public void accept(CuratedApplication curatedApplication, Map re QuarkusClassLoader classLoader = curatedApplication.getAugmentClassLoader(); - QuarkusBootstrap quarkusBootstrap = curatedApplication.getQuarkusBootstrap(); - QuarkusAugmentor.Builder builder = QuarkusAugmentor.builder() - .setRoot(quarkusBootstrap.getApplicationRoot()) - .setClassLoader(classLoader) - .addFinal(ApplicationClassNameBuildItem.class) - .setTargetDir(quarkusBootstrap.getTargetDirectory()) - .setDeploymentClassLoader(curatedApplication.createDeploymentClassLoader()) - .setBuildSystemProperties(quarkusBootstrap.getBuildSystemProperties()) - .setEffectiveModel(curatedApplication.getApplicationModel()); - if (quarkusBootstrap.getBaseName() != null) { - builder.setBaseName(quarkusBootstrap.getBaseName()); - } - if (quarkusBootstrap.getOriginalBaseName() != null) { - builder.setOriginalBaseName(quarkusBootstrap.getOriginalBaseName()); - } - - boolean auxiliaryApplication = curatedApplication.getQuarkusBootstrap().isAuxiliaryApplication(); - builder.setAuxiliaryApplication(auxiliaryApplication); - builder.setAuxiliaryDevModeType( - curatedApplication.getQuarkusBootstrap().isHostApplicationIsTestOnly() ? DevModeType.TEST_ONLY - : (auxiliaryApplication ? DevModeType.LOCAL : null)); - builder.setLaunchMode(LaunchMode.NORMAL); - builder.setRebuild(quarkusBootstrap.isRebuild()); - builder.setLiveReloadState( - new LiveReloadBuildItem(false, Collections.emptySet(), new HashMap<>(), null)); - for (AdditionalDependency i : quarkusBootstrap.getAdditionalApplicationArchives()) { - //this gets added to the class path either way - //but we only need to add it to the additional app archives - //if it is forced as an app archive - if (i.isForceApplicationArchive()) { - builder.addAdditionalApplicationArchive(i.getResolvedPaths()); + try (QuarkusClassLoader deploymentClassLoader = curatedApplication.createDeploymentClassLoader()) { + QuarkusBootstrap quarkusBootstrap = curatedApplication.getQuarkusBootstrap(); + QuarkusAugmentor.Builder builder = QuarkusAugmentor.builder() + .setRoot(quarkusBootstrap.getApplicationRoot()) + .setClassLoader(classLoader) + .addFinal(ApplicationClassNameBuildItem.class) + .setTargetDir(quarkusBootstrap.getTargetDirectory()) + .setDeploymentClassLoader(curatedApplication.createDeploymentClassLoader()) + .setBuildSystemProperties(quarkusBootstrap.getBuildSystemProperties()) + .setEffectiveModel(curatedApplication.getApplicationModel()); + if (quarkusBootstrap.getBaseName() != null) { + builder.setBaseName(quarkusBootstrap.getBaseName()); } - } - builder.addBuildChainCustomizer(new Consumer() { - @Override - public void accept(BuildChainBuilder builder) { - final BuildStepBuilder stepBuilder = builder.addBuildStep((ctx) -> { - ctx.produce(new ProcessInheritIODisabledBuildItem()); - }); - stepBuilder.produces(ProcessInheritIODisabledBuildItem.class).build(); + if (quarkusBootstrap.getOriginalBaseName() != null) { + builder.setOriginalBaseName(quarkusBootstrap.getOriginalBaseName()); } - }); - builder.excludeFromIndexing(quarkusBootstrap.getExcludeFromClassPath()); - builder.addFinal(GeneratedClassBuildItem.class); - builder.addFinal(MainClassBuildItem.class); - builder.addFinal(GeneratedResourceBuildItem.class); - builder.addFinal(TransformedClassesBuildItem.class); - builder.addFinal(DeploymentResultBuildItem.class); - // note: quarkus.package.type is deprecated - boolean nativeRequested = "native".equals(System.getProperty("quarkus.package.type")) - || "true".equals(System.getProperty("quarkus.native.enabled")); - boolean containerBuildRequested = Boolean.getBoolean("quarkus.container-image.build"); - if (nativeRequested) { - builder.addFinal(NativeImageBuildItem.class); - } - if (containerBuildRequested) { - //TODO: this is a bit ugly - //we don't necessarily need these artifacts - //but if we include them it does mean that you can auto create docker images - //and deploy to kube etc - //for an ordinary build with no native and no docker this is a waste - builder.addFinal(ArtifactResultBuildItem.class); - } - try { - BuildResult buildResult = builder.build().run(); - Map result = new HashMap<>(); - for (GeneratedClassBuildItem i : buildResult.consumeMulti(GeneratedClassBuildItem.class)) { - result.put(i.getName().replace(".", "/") + ".class", i.getClassData()); + boolean auxiliaryApplication = curatedApplication.getQuarkusBootstrap().isAuxiliaryApplication(); + builder.setAuxiliaryApplication(auxiliaryApplication); + builder.setAuxiliaryDevModeType( + curatedApplication.getQuarkusBootstrap().isHostApplicationIsTestOnly() ? DevModeType.TEST_ONLY + : (auxiliaryApplication ? DevModeType.LOCAL : null)); + builder.setLaunchMode(LaunchMode.NORMAL); + builder.setRebuild(quarkusBootstrap.isRebuild()); + builder.setLiveReloadState( + new LiveReloadBuildItem(false, Collections.emptySet(), new HashMap<>(), null)); + for (AdditionalDependency i : quarkusBootstrap.getAdditionalApplicationArchives()) { + //this gets added to the class path either way + //but we only need to add it to the additional app archives + //if it is forced as an app archive + if (i.isForceApplicationArchive()) { + builder.addAdditionalApplicationArchive(i.getResolvedPaths()); + } + } + builder.addBuildChainCustomizer(new Consumer() { + @Override + public void accept(BuildChainBuilder builder) { + final BuildStepBuilder stepBuilder = builder.addBuildStep((ctx) -> { + ctx.produce(new ProcessInheritIODisabledBuildItem()); + }); + stepBuilder.produces(ProcessInheritIODisabledBuildItem.class).build(); + } + }); + builder.excludeFromIndexing(quarkusBootstrap.getExcludeFromClassPath()); + builder.addFinal(GeneratedClassBuildItem.class); + builder.addFinal(MainClassBuildItem.class); + builder.addFinal(GeneratedResourceBuildItem.class); + builder.addFinal(TransformedClassesBuildItem.class); + builder.addFinal(DeploymentResultBuildItem.class); + // note: quarkus.package.type is deprecated + boolean nativeRequested = "native".equals(System.getProperty("quarkus.package.type")) + || "true".equals(System.getProperty("quarkus.native.enabled")); + boolean containerBuildRequested = Boolean.getBoolean("quarkus.container-image.build"); + if (nativeRequested) { + builder.addFinal(NativeImageBuildItem.class); } - for (GeneratedResourceBuildItem i : buildResult.consumeMulti(GeneratedResourceBuildItem.class)) { - result.put(i.getName(), i.getData()); + if (containerBuildRequested) { + //TODO: this is a bit ugly + //we don't necessarily need these artifacts + //but if we include them it does mean that you can auto create docker images + //and deploy to kube etc + //for an ordinary build with no native and no docker this is a waste + builder.addFinal(ArtifactResultBuildItem.class); } - for (Map.Entry> entry : buildResult - .consume(TransformedClassesBuildItem.class).getTransformedClassesByJar().entrySet()) { - for (TransformedClassesBuildItem.TransformedClass transformed : entry.getValue()) { - if (transformed.getData() != null) { - result.put(transformed.getFileName(), transformed.getData()); - } else { - log.warn("Unable to remove resource " + transformed.getFileName() - + " as this is not supported in JBangf"); + + try { + BuildResult buildResult = builder.build().run(); + Map result = new HashMap<>(); + for (GeneratedClassBuildItem i : buildResult.consumeMulti(GeneratedClassBuildItem.class)) { + result.put(i.getName().replace(".", "/") + ".class", i.getClassData()); + } + for (GeneratedResourceBuildItem i : buildResult.consumeMulti(GeneratedResourceBuildItem.class)) { + result.put(i.getName(), i.getData()); + } + for (Map.Entry> entry : buildResult + .consume(TransformedClassesBuildItem.class).getTransformedClassesByJar().entrySet()) { + for (TransformedClassesBuildItem.TransformedClass transformed : entry.getValue()) { + if (transformed.getData() != null) { + result.put(transformed.getFileName(), transformed.getData()); + } else { + log.warn("Unable to remove resource " + transformed.getFileName() + + " as this is not supported in JBangf"); + } } } + resultMap.put("files", result); + final List javaargs = new ArrayList<>(); + javaargs.add("-Djava.util.logging.manager=org.jboss.logmanager.LogManager"); + javaargs.add( + "-Djava.util.concurrent.ForkJoinPool.common.threadFactory=io.quarkus.bootstrap.forkjoin.QuarkusForkJoinWorkerThreadFactory"); + resultMap.put("java-args", javaargs); + resultMap.put("main-class", buildResult.consume(MainClassBuildItem.class).getClassName()); + if (nativeRequested) { + resultMap.put("native-image", buildResult.consume(NativeImageBuildItem.class).getPath()); + } + } catch (RuntimeException e) { + throw e; + } catch (Exception e) { + throw new RuntimeException(e); } - resultMap.put("files", result); - final List javaargs = new ArrayList<>(); - javaargs.add("-Djava.util.logging.manager=org.jboss.logmanager.LogManager"); - javaargs.add( - "-Djava.util.concurrent.ForkJoinPool.common.threadFactory=io.quarkus.bootstrap.forkjoin.QuarkusForkJoinWorkerThreadFactory"); - resultMap.put("java-args", javaargs); - resultMap.put("main-class", buildResult.consume(MainClassBuildItem.class).getClassName()); - if (nativeRequested) { - resultMap.put("native-image", buildResult.consume(NativeImageBuildItem.class).getPath()); - } - } catch (Exception e) { - throw new RuntimeException(e); } } } diff --git a/core/deployment/src/main/java/io/quarkus/runner/bootstrap/AugmentActionImpl.java b/core/deployment/src/main/java/io/quarkus/runner/bootstrap/AugmentActionImpl.java index ebe88aa820873..43776e45eba47 100644 --- a/core/deployment/src/main/java/io/quarkus/runner/bootstrap/AugmentActionImpl.java +++ b/core/deployment/src/main/java/io/quarkus/runner/bootstrap/AugmentActionImpl.java @@ -138,29 +138,30 @@ public AugmentActionImpl(CuratedApplication curatedApplication, List[] targets = Arrays.stream(finalOutputs) - .map(new Function>() { - @Override - public Class apply(String s) { - try { - return (Class) Class.forName(s, false, classLoader); - } catch (ClassNotFoundException e) { - throw new RuntimeException(e); + try (QuarkusClassLoader classLoader = curatedApplication.createDeploymentClassLoader()) { + Class[] targets = Arrays.stream(finalOutputs) + .map(new Function>() { + @Override + public Class apply(String s) { + try { + return (Class) Class.forName(s, false, classLoader); + } catch (ClassNotFoundException e) { + throw new RuntimeException(e); + } } - } - }).toArray(Class[]::new); - BuildResult result = runAugment(true, Collections.emptySet(), null, classLoader, targets); + }).toArray(Class[]::new); + BuildResult result = runAugment(true, Collections.emptySet(), null, classLoader, targets); - writeDebugSourceFile(result); - try { - BiConsumer consumer = (BiConsumer) Class - .forName(resultHandler, false, classLoader) - .getConstructor().newInstance(); - consumer.accept(context, result); - } catch (InstantiationException | IllegalAccessException | NoSuchMethodException | ClassNotFoundException - | InvocationTargetException e) { - throw new RuntimeException(e); + writeDebugSourceFile(result); + try { + BiConsumer consumer = (BiConsumer) Class + .forName(resultHandler, false, classLoader) + .getConstructor().newInstance(); + consumer.accept(context, result); + } catch (InstantiationException | IllegalAccessException | NoSuchMethodException | ClassNotFoundException + | InvocationTargetException e) { + throw new RuntimeException(e); + } } } @@ -169,31 +170,32 @@ public AugmentResult createProductionApplication() { if (launchMode != LaunchMode.NORMAL) { throw new IllegalStateException("Can only create a production application when using NORMAL launch mode"); } - ClassLoader classLoader = curatedApplication.createDeploymentClassLoader(); - BuildResult result = runAugment(true, Collections.emptySet(), null, classLoader, ArtifactResultBuildItem.class, - DeploymentResultBuildItem.class); + try (QuarkusClassLoader classLoader = curatedApplication.createDeploymentClassLoader()) { + BuildResult result = runAugment(true, Collections.emptySet(), null, classLoader, ArtifactResultBuildItem.class, + DeploymentResultBuildItem.class); - writeDebugSourceFile(result); + writeDebugSourceFile(result); - JarBuildItem jarBuildItem = result.consumeOptional(JarBuildItem.class); - NativeImageBuildItem nativeImageBuildItem = result.consumeOptional(NativeImageBuildItem.class); - List artifactResultBuildItems = result.consumeMulti(ArtifactResultBuildItem.class); - BuildSystemTargetBuildItem buildSystemTargetBuildItem = result.consume(BuildSystemTargetBuildItem.class); + JarBuildItem jarBuildItem = result.consumeOptional(JarBuildItem.class); + NativeImageBuildItem nativeImageBuildItem = result.consumeOptional(NativeImageBuildItem.class); + List artifactResultBuildItems = result.consumeMulti(ArtifactResultBuildItem.class); + BuildSystemTargetBuildItem buildSystemTargetBuildItem = result.consume(BuildSystemTargetBuildItem.class); - // this depends on the fact that the order in which we can obtain MultiBuildItems is the same as they are produced - // we want to write result of the final artifact created - if (artifactResultBuildItems.isEmpty()) { - throw new IllegalStateException("No artifact results were produced"); - } - ArtifactResultBuildItem lastResult = artifactResultBuildItems.get(artifactResultBuildItems.size() - 1); - writeArtifactResultMetadataFile(buildSystemTargetBuildItem, lastResult); + // this depends on the fact that the order in which we can obtain MultiBuildItems is the same as they are produced + // we want to write result of the final artifact created + if (artifactResultBuildItems.isEmpty()) { + throw new IllegalStateException("No artifact results were produced"); + } + ArtifactResultBuildItem lastResult = artifactResultBuildItems.get(artifactResultBuildItems.size() - 1); + writeArtifactResultMetadataFile(buildSystemTargetBuildItem, lastResult); - return new AugmentResult(artifactResultBuildItems.stream() - .map(a -> new ArtifactResult(a.getPath(), a.getType(), a.getMetadata())) - .collect(Collectors.toList()), - jarBuildItem != null ? jarBuildItem.toJarResult() : null, - nativeImageBuildItem != null ? nativeImageBuildItem.getPath() : null, - nativeImageBuildItem != null ? nativeImageBuildItem.getGraalVMInfo().toMap() : Collections.emptyMap()); + return new AugmentResult(artifactResultBuildItems.stream() + .map(a -> new ArtifactResult(a.getPath(), a.getType(), a.getMetadata())) + .collect(Collectors.toList()), + jarBuildItem != null ? jarBuildItem.toJarResult() : null, + nativeImageBuildItem != null ? nativeImageBuildItem.getPath() : null, + nativeImageBuildItem != null ? nativeImageBuildItem.getGraalVMInfo().toMap() : Collections.emptyMap()); + } } private void writeDebugSourceFile(BuildResult result) { @@ -247,10 +249,11 @@ public StartupActionImpl createInitialRuntimeApplication() { if (launchMode == LaunchMode.NORMAL) { throw new IllegalStateException("Cannot launch a runtime application with NORMAL launch mode"); } - ClassLoader classLoader = curatedApplication.createDeploymentClassLoader(); - @SuppressWarnings("unchecked") - BuildResult result = runAugment(true, Collections.emptySet(), null, classLoader, NON_NORMAL_MODE_OUTPUTS); - return new StartupActionImpl(curatedApplication, result); + try (QuarkusClassLoader classLoader = curatedApplication.createDeploymentClassLoader()) { + @SuppressWarnings("unchecked") + BuildResult result = runAugment(true, Collections.emptySet(), null, classLoader, NON_NORMAL_MODE_OUTPUTS); + return new StartupActionImpl(curatedApplication, result); + } } @Override @@ -259,13 +262,14 @@ public StartupActionImpl reloadExistingApplication(boolean hasStartedSuccessfull if (launchMode != LaunchMode.DEVELOPMENT) { throw new IllegalStateException("Only application with launch mode DEVELOPMENT can restart"); } - ClassLoader classLoader = curatedApplication.createDeploymentClassLoader(); - @SuppressWarnings("unchecked") - BuildResult result = runAugment(!hasStartedSuccessfully, changedResources, classChangeInformation, classLoader, - NON_NORMAL_MODE_OUTPUTS); + try (QuarkusClassLoader classLoader = curatedApplication.createDeploymentClassLoader()) { + @SuppressWarnings("unchecked") + BuildResult result = runAugment(!hasStartedSuccessfully, changedResources, classChangeInformation, classLoader, + NON_NORMAL_MODE_OUTPUTS); - return new StartupActionImpl(curatedApplication, result); + return new StartupActionImpl(curatedApplication, result); + } } private BuildResult runAugment(boolean firstRun, Set changedResources,