From b435def31d99acf1a362482591be70b5eb3fa014 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Thu, 13 May 2021 11:42:40 +1000 Subject: [PATCH] More test CL changes This allows non-class resources to be loaded parent first to work around ClassLoading issues in libraries. Fixes #17175 --- .../bootstrap/app/CuratedApplication.java | 68 ++++++++++++++++++- .../classloading/QuarkusClassLoader.java | 10 +++ .../src/main/resources/wrong-classloading.txt | 1 + ...correctClassloadingWorkaroundTestCase.java | 25 +++++++ .../src/main/resources/application.properties | 3 - .../io/quarkus/it/shared/SharedResource.java | 15 ++++ 6 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 integration-tests/main/src/main/resources/wrong-classloading.txt create mode 100644 integration-tests/main/src/test/java/io/quarkus/it/main/IncorrectClassloadingWorkaroundTestCase.java delete mode 100644 integration-tests/picocli-native/src/main/resources/application.properties 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 567117ec9057c..355e6af8f2823 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 @@ -2,14 +2,17 @@ import io.quarkus.bootstrap.BootstrapConstants; import io.quarkus.bootstrap.classloading.ClassPathElement; +import io.quarkus.bootstrap.classloading.ClassPathResource; import io.quarkus.bootstrap.classloading.MemoryClassPathElement; import io.quarkus.bootstrap.classloading.QuarkusClassLoader; import io.quarkus.bootstrap.model.AppArtifact; import io.quarkus.bootstrap.model.AppArtifactKey; import io.quarkus.bootstrap.model.AppDependency; import io.quarkus.bootstrap.model.AppModel; +import java.io.IOException; import java.io.Serializable; import java.nio.file.Path; +import java.security.ProtectionDomain; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -20,6 +23,8 @@ import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Function; +import java.util.jar.Manifest; +import java.util.stream.Collectors; /** * The result of the curate step that is done by QuarkusBootstrap. @@ -218,7 +223,7 @@ public synchronized QuarkusClassLoader getBaseRuntimeClassLoader() { } } else { for (Path root : quarkusBootstrap.getApplicationRoot()) { - builder.addBannedElement(ClassPathElement.fromPath(root)); + builder.addBannedElement(new ClassFilteredBannedElement(ClassPathElement.fromPath(root))); } } @@ -232,7 +237,7 @@ public synchronized QuarkusClassLoader getBaseRuntimeClassLoader() { } else { for (Path root : i.getArchivePath()) { hotReloadPaths.add(root); - builder.addBannedElement(ClassPathElement.fromPath(root)); + builder.addBannedElement(new ClassFilteredBannedElement(ClassPathElement.fromPath(root))); } } } @@ -331,4 +336,63 @@ public void close() { baseRuntimeClassLoader.close(); } } + + /** + * TODO: Fix everything in the universe to do loading properly + * + * This class exists because a lot of libraries do getClass().getClassLoader.getResource() + * instead of using the context class loader, which breaks tests as these resources are present in the + * top CL and not the base CL that is used to load libraries. + * + * This yucky yucky hack works around this, by allowing non-class files to be loaded parent first, so they + * will be loaded from the application ClassLoader. + * + * Note that the underlying reason for this 'banned element' existing in the first place + * is because other libraries do Class Loading wrong in different ways, and attempt to load + * from the TCCL as a fallback instead of as the first priority, so we need to have the banned element + * to prevent a load from the application ClassLoader (which won't work). + * + */ + static class ClassFilteredBannedElement implements ClassPathElement { + + private final ClassPathElement delegate; + + ClassFilteredBannedElement(ClassPathElement delegate) { + this.delegate = delegate; + } + + @Override + public Path getRoot() { + return delegate.getRoot(); + } + + @Override + public ClassPathResource getResource(String name) { + if (!name.endsWith(".class")) { + return null; + } + return delegate.getResource(name); + } + + @Override + public Set getProvidedResources() { + return delegate.getProvidedResources().stream().filter(s -> s.endsWith(".class")).collect(Collectors.toSet()); + } + + @Override + public ProtectionDomain getProtectionDomain(ClassLoader classLoader) { + return delegate.getProtectionDomain(classLoader); + } + + @Override + public Manifest getManifest() { + return delegate.getManifest(); + } + + @Override + public void close() throws IOException { + delegate.close(); + } + } + } 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 a0ab908b9b243..c9a88c539fe13 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 @@ -567,6 +567,16 @@ public void close() { log.error("Failed to close " + element, e); } } + for (ClassPathElement element : bannedElements) { + //note that this is a 'soft' close + //all resources are closed, however the CL can still be used + //but after close no resources will be held past the scope of an operation + try (ClassPathElement ignored = element) { + //the close() operation is implied by the try-with syntax + } catch (Exception e) { + log.error("Failed to close " + element, e); + } + } ResourceBundle.clearCache(this); } diff --git a/integration-tests/main/src/main/resources/wrong-classloading.txt b/integration-tests/main/src/main/resources/wrong-classloading.txt new file mode 100644 index 0000000000000..50a5cb6b1b485 --- /dev/null +++ b/integration-tests/main/src/main/resources/wrong-classloading.txt @@ -0,0 +1 @@ +Wrong Classloading \ No newline at end of file diff --git a/integration-tests/main/src/test/java/io/quarkus/it/main/IncorrectClassloadingWorkaroundTestCase.java b/integration-tests/main/src/test/java/io/quarkus/it/main/IncorrectClassloadingWorkaroundTestCase.java new file mode 100644 index 0000000000000..03d5dbc561e86 --- /dev/null +++ b/integration-tests/main/src/test/java/io/quarkus/it/main/IncorrectClassloadingWorkaroundTestCase.java @@ -0,0 +1,25 @@ +package io.quarkus.it.main; + +import static org.hamcrest.Matchers.is; + +import org.junit.jupiter.api.Test; + +import io.quarkus.test.junit.QuarkusTest; +import io.restassured.RestAssured; + +//https://github.com/quarkusio/quarkus/issues/17175 +@QuarkusTest +public class IncorrectClassloadingWorkaroundTestCase { + + /** + * libraries should load from the Thread Context Class Loader + * we test that even if libraries do the wrong thing our workaround still works + * without the need to force flat Class-Path + */ + @Test + public void testClassloadingStillWorksWhenLibrariesLoadFromWrongCL() { + RestAssured.when().get("/shared/classloading").then() + .body(is("Wrong Classloading")); + } + +} diff --git a/integration-tests/picocli-native/src/main/resources/application.properties b/integration-tests/picocli-native/src/main/resources/application.properties deleted file mode 100644 index fb60b142eb2e6..0000000000000 --- a/integration-tests/picocli-native/src/main/resources/application.properties +++ /dev/null @@ -1,3 +0,0 @@ - -#picocli attempts to load resource bundles from its own CL rather than the TCCL -quarkus.test.flat-class-path=true \ No newline at end of file diff --git a/integration-tests/shared-library/src/main/java/io/quarkus/it/shared/SharedResource.java b/integration-tests/shared-library/src/main/java/io/quarkus/it/shared/SharedResource.java index ffc88873eeecf..0dcf3ce84f797 100644 --- a/integration-tests/shared-library/src/main/java/io/quarkus/it/shared/SharedResource.java +++ b/integration-tests/shared-library/src/main/java/io/quarkus/it/shared/SharedResource.java @@ -1,5 +1,8 @@ package io.quarkus.it.shared; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; + import javax.ws.rs.GET; import javax.ws.rs.Path; @@ -10,4 +13,16 @@ public class SharedResource { public String shared() { return "Shared Resource"; } + + //https://github.com/quarkusio/quarkus/issues/17175 + @GET + @Path("/classloading") + public String loadFromWrongClassLoader() throws Exception { + //this is wrong, libraries should load from the Thread Context Class Loader + //we test that even if libraries do the wrong thing our workaround still works + //without the need to force flat Class-Path + try (InputStream is = getClass().getClassLoader().getResourceAsStream("wrong-classloading.txt")) { + return new String(is.readAllBytes(), StandardCharsets.UTF_8); + } + } }