Fix ClassLoader getParent() hack

This replaces the hack where QuarkusClassLoader.getParent() returns null, and fixes it with some smaller more limited hacks around the problematic API's. Fixes #14898
quarkusio · Feb 11, 2021 · c545fc2 · c545fc2
1 parent 57eaa7d
commit c545fc2
Show file tree

Hide file tree

Showing 8 changed files with 218 additions and 16 deletions.
diff --git a/...quarkus/mutiny/reactive/operators/deployment/MutinyReactiveStreamsOperatorsProcessor.java b/...quarkus/mutiny/reactive/operators/deployment/MutinyReactiveStreamsOperatorsProcessor.java
@@ -6,7 +6,10 @@
 
 import io.quarkus.deployment.annotations.BuildProducer;
 import io.quarkus.deployment.annotations.BuildStep;
+import io.quarkus.deployment.annotations.ExecutionTime;
+import io.quarkus.deployment.annotations.Record;
 import io.quarkus.deployment.builditem.nativeimage.ServiceProviderBuildItem;
+import io.quarkus.mutiny.reactive.operators.runtime.ReactiveStreamsOperatorsRecorder;
 import io.smallrye.mutiny.streams.Engine;
 
 public class MutinyReactiveStreamsOperatorsProcessor {
@@ -19,4 +22,10 @@ public void build(BuildProducer<ServiceProviderBuildItem> serviceProvider) {
                 ReactiveStreamsFactoryImpl.class.getName()));
     }
 
+    @BuildStep
+    @Record(ExecutionTime.STATIC_INIT)
+    void classLoadingHack(ReactiveStreamsOperatorsRecorder reactiveStreamsOperatorsRecorder) {
+        reactiveStreamsOperatorsRecorder.classLoaderHack();
+    }
+
 }
diff --git a/...n/java/io/quarkus/mutiny/reactive/operators/runtime/ReactiveStreamsOperatorsRecorder.java b/...n/java/io/quarkus/mutiny/reactive/operators/runtime/ReactiveStreamsOperatorsRecorder.java
@@ -0,0 +1,58 @@
+package io.quarkus.mutiny.reactive.operators.runtime;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.util.Enumeration;
+
+import org.eclipse.microprofile.reactive.streams.operators.core.ReactiveStreamsEngineResolver;
+import org.eclipse.microprofile.reactive.streams.operators.spi.ReactiveStreamsFactoryResolver;
+
+import io.quarkus.runtime.annotations.Recorder;
+
+@Recorder
+public class ReactiveStreamsOperatorsRecorder {
+
+    /**
+     * ClassLoader hack to work around reactive streams API issue
+     * see https://github.com/eclipse/microprofile-reactive-streams-operators/pull/130
+     *
+     * This must be deleted when Reactive Streams Operators 1.1 is merged
+     */
+    public void classLoaderHack() {
+        ClassLoader cl = Thread.currentThread().getContextClassLoader();
+        try {
+            Thread.currentThread().setContextClassLoader(new ClassLoader(null) {
+                @Override
+                public Class<?> loadClass(String name) throws ClassNotFoundException {
+                    return cl.loadClass(name);
+                }
+
+                @Override
+                protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+                    return cl.loadClass(name);
+                }
+
+                @Override
+                public URL getResource(String name) {
+                    return cl.getResource(name);
+                }
+
+                @Override
+                public Enumeration<URL> getResources(String name) throws IOException {
+                    return cl.getResources(name);
+                }
+
+                @Override
+                public InputStream getResourceAsStream(String name) {
+                    return cl.getResourceAsStream(name);
+                }
+            });
+            ReactiveStreamsFactoryResolver.instance();
+            ReactiveStreamsEngineResolver.instance();
+        } finally {
+            Thread.currentThread().setContextClassLoader(cl);
+        }
+
+    }
+}
diff --git a/...mallrye/reactivestreamoperators/deployment/SmallRyeReactiveStreamsOperatorsProcessor.java b/...mallrye/reactivestreamoperators/deployment/SmallRyeReactiveStreamsOperatorsProcessor.java
@@ -7,8 +7,11 @@
 import io.quarkus.deployment.Feature;
 import io.quarkus.deployment.annotations.BuildProducer;
 import io.quarkus.deployment.annotations.BuildStep;
+import io.quarkus.deployment.annotations.ExecutionTime;
+import io.quarkus.deployment.annotations.Record;
 import io.quarkus.deployment.builditem.FeatureBuildItem;
 import io.quarkus.deployment.builditem.nativeimage.ServiceProviderBuildItem;
+import io.quarkus.smallrye.reactivestreamsoperators.runtime.ReactiveStreamsOperatorsRecorder;
 import io.smallrye.reactive.streams.Engine;
 
 public class SmallRyeReactiveStreamsOperatorsProcessor {
@@ -22,4 +25,10 @@ public void build(BuildProducer<ServiceProviderBuildItem> serviceProvider,
                 ReactiveStreamsFactoryImpl.class.getName()));
     }
 
+    @BuildStep
+    @Record(ExecutionTime.STATIC_INIT)
+    void classLoadingHack(ReactiveStreamsOperatorsRecorder reactiveStreamsOperatorsRecorder) {
+        reactiveStreamsOperatorsRecorder.classLoaderHack();
+    }
+
 }
diff --git a/...o/quarkus/smallrye/reactivestreamsoperators/runtime/ReactiveStreamsOperatorsRecorder.java b/...o/quarkus/smallrye/reactivestreamsoperators/runtime/ReactiveStreamsOperatorsRecorder.java
@@ -0,0 +1,58 @@
+package io.quarkus.smallrye.reactivestreamsoperators.runtime;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.util.Enumeration;
+
+import org.eclipse.microprofile.reactive.streams.operators.core.ReactiveStreamsEngineResolver;
+import org.eclipse.microprofile.reactive.streams.operators.spi.ReactiveStreamsFactoryResolver;
+
+import io.quarkus.runtime.annotations.Recorder;
+
+@Recorder
+public class ReactiveStreamsOperatorsRecorder {
+
+    /**
+     * ClassLoader hack to work around reactive streams API issue
+     * see https://github.com/eclipse/microprofile-reactive-streams-operators/pull/130
+     *
+     * This must be deleted when Reactive Streams Operators 1.1 is merged
+     */
+    public void classLoaderHack() {
+        ClassLoader cl = Thread.currentThread().getContextClassLoader();
+        try {
+            Thread.currentThread().setContextClassLoader(new ClassLoader(null) {
+                @Override
+                public Class<?> loadClass(String name) throws ClassNotFoundException {
+                    return cl.loadClass(name);
+                }
+
+                @Override
+                protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+                    return cl.loadClass(name);
+                }
+
+                @Override
+                public URL getResource(String name) {
+                    return cl.getResource(name);
+                }
+
+                @Override
+                public Enumeration<URL> getResources(String name) throws IOException {
+                    return cl.getResources(name);
+                }
+
+                @Override
+                public InputStream getResourceAsStream(String name) {
+                    return cl.getResourceAsStream(name);
+                }
+            });
+            ReactiveStreamsFactoryResolver.instance();
+            ReactiveStreamsEngineResolver.instance();
+        } finally {
+            Thread.currentThread().setContextClassLoader(cl);
+        }
+
+    }
+}
diff --git a/...oyment/src/main/java/io/quarkus/smallrye/openapi/deployment/SmallRyeOpenApiProcessor.java b/...oyment/src/main/java/io/quarkus/smallrye/openapi/deployment/SmallRyeOpenApiProcessor.java
@@ -180,6 +180,12 @@ RouteBuildItem handler(LaunchModeBuildItem launch,
                 .build();
     }
 
+    @BuildStep
+    @Record(ExecutionTime.STATIC_INIT)
+    void classLoaderHack(OpenApiRecorder recorder) {
+        recorder.classLoaderHack();
+    }
+
     @BuildStep
     void additionalBean(BuildProducer<AdditionalBeanBuildItem> additionalBeanProducer) {
         additionalBeanProducer.produce(AdditionalBeanBuildItem.builder()

diff --git a/...ye-openapi/runtime/src/main/java/io/quarkus/smallrye/openapi/runtime/OpenApiRecorder.java b/...ye-openapi/runtime/src/main/java/io/quarkus/smallrye/openapi/runtime/OpenApiRecorder.java
@@ -1,5 +1,12 @@
 package io.quarkus.smallrye.openapi.runtime;
 
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.util.Enumeration;
+
+import org.eclipse.microprofile.openapi.spi.OASFactoryResolver;
+
 import io.quarkus.runtime.ShutdownContext;
 import io.quarkus.runtime.annotations.Recorder;
 import io.vertx.core.Handler;
@@ -9,7 +16,6 @@
 public class OpenApiRecorder {
 
     public Handler<RoutingContext> handler(OpenApiRuntimeConfig runtimeConfig) {
-
         if (runtimeConfig.enable) {
             return new OpenApiHandler();
         } else {
@@ -27,4 +33,45 @@ public void run() {
         });
     }
 
+    /**
+     * ClassLoader hack to work around reactive streams API issue
+     * see https://github.com/eclipse/microprofile-open-api/pull/470
+     * <p>
+     * This must be deleted when it is fixed upstream
+     */
+    public void classLoaderHack() {
+        ClassLoader cl = Thread.currentThread().getContextClassLoader();
+        try {
+            Thread.currentThread().setContextClassLoader(new ClassLoader(null) {
+                @Override
+                public Class<?> loadClass(String name) throws ClassNotFoundException {
+                    return cl.loadClass(name);
+                }
+
+                @Override
+                protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+                    return cl.loadClass(name);
+                }
+
+                @Override
+                public URL getResource(String name) {
+                    return cl.getResource(name);
+                }
+
+                @Override
+                public Enumeration<URL> getResources(String name) throws IOException {
+                    return cl.getResources(name);
+                }
+
+                @Override
+                public InputStream getResourceAsStream(String name) {
+                    return cl.getResourceAsStream(name);
+                }
+            });
+            OASFactoryResolver.instance();
+        } finally {
+            Thread.currentThread().setContextClassLoader(cl);
+        }
+
+    }
 }
diff --git a/...ts/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java b/...ts/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java
@@ -42,6 +42,7 @@ public class QuarkusClassLoader extends ClassLoader implements Closeable {
     private final String name;
     private final List<ClassPathElement> elements;
     private final ConcurrentMap<ClassPathElement, ProtectionDomain> protectionDomains = new ConcurrentHashMap<>();
+    private final ConcurrentMap<String, Package> definedPackages = new ConcurrentHashMap<>();
     private final ClassLoader parent;
     /**
      * If this is true it will attempt to load from the parent first
@@ -87,15 +88,7 @@ public class QuarkusClassLoader extends ClassLoader implements Closeable {
     private volatile boolean driverLoaded;
 
     private QuarkusClassLoader(Builder builder) {
-        //we need the parent to be null
-        //as MP has super broken class loading where it attempts to resolve stuff from the parent
-        //will hopefully be fixed in 1.4
-        //e.g. https://github.com/eclipse/microprofile-config/issues/390
-        //e.g. https://github.com/eclipse/microprofile-reactive-streams-operators/pull/130
-        //to further complicate things we also have https://github.com/quarkusio/quarkus/issues/8985
-        //where getParent must work to load JDK services on JDK9+
-        //to get around this we pass in the platform ClassLoader, if it exists
-        super(PLATFORM_CLASS_LOADER);
+        super(new GetPackageBlockingClassLoader(builder.parent));
         this.name = builder.name;
         this.elements = builder.elements;
         this.bannedElements = builder.bannedElements;
@@ -440,23 +433,25 @@ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundE
 
     private void definePackage(String name, ClassPathElement classPathElement) {
         final String pkgName = getPackageNameFromClassName(name);
-        if ((pkgName != null) && getPackage(pkgName) == null) {
+        //we can't use getPackage here
+        //if can return a package from the parent
+        if ((pkgName != null) && definedPackages.get(pkgName) == null) {
             synchronized (getClassLoadingLock(pkgName)) {
-                if (getPackage(pkgName) == null) {
+                if (definedPackages.get(pkgName) == null) {
                     Manifest mf = classPathElement.getManifest();
                     if (mf != null) {
                         Attributes ma = mf.getMainAttributes();
-                        definePackage(pkgName, ma.getValue(Attributes.Name.SPECIFICATION_TITLE),
+                        definedPackages.put(pkgName, definePackage(pkgName, ma.getValue(Attributes.Name.SPECIFICATION_TITLE),
                                 ma.getValue(Attributes.Name.SPECIFICATION_VERSION),
                                 ma.getValue(Attributes.Name.SPECIFICATION_VENDOR),
                                 ma.getValue(Attributes.Name.IMPLEMENTATION_TITLE),
                                 ma.getValue(Attributes.Name.IMPLEMENTATION_VERSION),
-                                ma.getValue(Attributes.Name.IMPLEMENTATION_VENDOR), null);
+                                ma.getValue(Attributes.Name.IMPLEMENTATION_VENDOR), null));
                         return;
                     }
 
                     // this could certainly be improved to use the actual manifest
-                    definePackage(pkgName, null, null, null, null, null, null, null);
+                    definedPackages.put(pkgName, definePackage(pkgName, null, null, null, null, null, null, null));
                 }
             }
         }
@@ -706,4 +701,24 @@ static final class ClassLoaderState {
             this.parentFirstResources = parentFirstResources;
         }
     }
+
+    /**
+     * Horrible JDK8 hack
+     *
+     * getPackage() on JDK8 will always delegate to the parent, so QuarkusClassLoader will never define a package,
+     * which can cause issues if you then try and read package annotations as they will be from the wrong ClassLoader.
+     *
+     * We add this ClassLoader into the mix to block the getPackage() delegation.
+     */
+    private static class GetPackageBlockingClassLoader extends ClassLoader {
+
+        protected GetPackageBlockingClassLoader(ClassLoader parent) {
+            super(parent);
+        }
+
+        @Override
+        protected Package getPackage(String name) {
+            return null;
+        }
+    }
 }
diff --git a/integration-tests/injectmock/src/main/java/io/quarkus/it/mockbean/SuffixService.java b/integration-tests/injectmock/src/main/java/io/quarkus/it/mockbean/SuffixService.java
@@ -5,7 +5,7 @@
 @ApplicationScoped
 public class SuffixService {
 
-    public String getSuffix() {
+    String getSuffix() {
         return "";
     }
 }