snehashisp commented on code in PR #18403:
URL: https://github.com/apache/kafka/pull/18403#discussion_r1908232685


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/ReflectionScanner.java:
##########
@@ -76,8 +78,21 @@ private static <T> String versionFor(Class<? extends T> 
pluginKlass) throws Refl
 
     @Override
     protected PluginScanResult scanPlugins(PluginSource source) {
+        // By default, java and classgraph uses parent first classloading, 
hence if a plugin is loaded by the classpath
+        // loader, and then by an isolated plugin loader, the default 
precedence will always load the classpath version.
+        // This breaks isolation and hence connect uses isolated plugin 
loaders, which are child first classloaders.
+        // Therefore, we override the classloader order to be child first, so 
that the isolated plugin loader is used first.
+        // In addition, we need to explicitly specify the full classloader 
order, as classgraph only scans the classes available
+        // in the classloaders and not the entire parent chain. Due to this 
reason if a plugin is extending a class present
+        // in classpath/application it will not be able to find the parent 
class unless we explicitly specify the classloader order.
+        List<ClassLoader> classLoaderOrder = new ArrayList<>();
+        ClassLoader cl = source.loader();
+        while (cl != null) {
+            classLoaderOrder.add(cl);
+            cl = cl.getParent();
+        }

Review Comment:
   > So the order of the specified classloaders does change the classloading 
order of the plugins, even though it doesn't change the scanning behavior.
   
   Yes, that's why with `addClassLoader` which uses 
`environmentClassLoaderDelegationOrder` in `ClassGraphClassLoader` always finds 
the files in the classpath loader (AppClassLoader) before PluginClassLoder has 
a chance to kick in. 
   
   Also, yeah maybe with a special handler for PluginClassLoader we can avoid 
having to pass the entire class loader chain in the reverse order. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to