Skip to content

Commit

Permalink
Merge pull request #17200 from stuartwdouglas/17175
Browse files Browse the repository at this point in the history
More test CL changes
  • Loading branch information
gastaldi authored May 13, 2021
2 parents e2696e4 + b435def commit c133787
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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)));
}
}

Expand All @@ -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)));
}
}
}
Expand Down Expand Up @@ -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<String> 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();
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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);

}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Wrong Classloading
Original file line number Diff line number Diff line change
@@ -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"));
}

}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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);
}
}
}

0 comments on commit c133787

Please sign in to comment.