This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new d755b397b9 correctly merge plugin classloaders, fixes #8133 (#8141)
d755b397b9 is described below
commit d755b397b980467718c615fcdec2e36e8dd712bb
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Fri Aug 28 15:28:25 2026 +0200
correctly merge plugin classloaders, fixes #8133 (#8141)
---
.../apache/hop/core/plugins/PluginRegistry.java | 40 +++++++++------
.../hop/core/plugins/PluginRegistryUnitTest.java | 59 ++++++++++++++++++++++
2 files changed, 83 insertions(+), 16 deletions(-)
diff --git a/core/src/main/java/org/apache/hop/core/plugins/PluginRegistry.java
b/core/src/main/java/org/apache/hop/core/plugins/PluginRegistry.java
index 17897c95d6..aaa781f558 100644
--- a/core/src/main/java/org/apache/hop/core/plugins/PluginRegistry.java
+++ b/core/src/main/java/org/apache/hop/core/plugins/PluginRegistry.java
@@ -28,6 +28,7 @@ import java.net.URLClassLoader;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
@@ -355,6 +356,11 @@ public class PluginRegistry {
}
}
+ /**
+ * Add the libraries of a plugin to a class loader it shares with other
plugins. Libraries the
+ * class loader already carries are skipped, so this is safe to call every
time a plugin joins an
+ * existing class loader.
+ */
private void addToClassLoader(IPlugin plugin, HopURLClassLoader ucl)
throws MalformedURLException {
String[] patterns = parentClassloaderPatternMap.get(plugin);
@@ -363,10 +369,14 @@ public class PluginRegistry {
((HopSelectiveParentFirstClassLoader) ucl).addPatterns(patterns);
}
+ Set<URL> present = new HashSet<>(Arrays.asList(ucl.getURLs()));
for (String jarFile : plugin.getLibraries()) {
File jarfile = new File(jarFile);
- ucl.addURL(
- new URL(URLDecoder.decode(jarfile.toURI().toURL().toString(),
StandardCharsets.UTF_8)));
+ URL url =
+ new URL(URLDecoder.decode(jarfile.toURI().toURL().toString(),
StandardCharsets.UTF_8));
+ if (present.add(url)) {
+ ucl.addURL(url);
+ }
}
}
@@ -903,13 +913,14 @@ public class PluginRegistry {
inverseClassLoaderLookup.computeIfAbsent(ucl, k -> new
HashSet<>()).add(plugin);
classLoaderGroupsMap.put(plugin.getClassLoaderGroup(), ucl);
} else {
- // we have a classloader, but does it have this plugin?
- try {
-
ucl.loadClass(plugin.getClassMap().values().iterator().next());
- } catch (ClassNotFoundException ignored) {
- // missed, add it to the classloader
- addToClassLoader(plugin, (HopURLClassLoader) ucl);
- }
+ // The group's class loader was created for whichever plugin
of the group was
+ // asked for first, so it doesn't necessarily carry this
plugin's libraries.
+ // Checking whether the plugin's own class loads isn't good
enough: two plugins
+ // in the same jar pass that check while still needing
different libraries. A
+ // database plugin adds the shared JDBC folders, the bulk
loader transform next
+ // to it doesn't, and the loser of that race ends up without
its driver (#8133).
+ //
+ addToClassLoader(plugin, (HopURLClassLoader) ucl);
}
} else {
// fallthrough folder based plugin
@@ -922,13 +933,10 @@ public class PluginRegistry {
inverseClassLoaderLookup.computeIfAbsent(ucl, k -> new
HashSet<>()).add(plugin);
folderBasedClassLoaderMap.put(plugin.getPluginDirectory().toString(), ucl);
} else {
- // we have a classloader, but does it have this plugin?
- try {
-
ucl.loadClass(plugin.getClassMap().values().iterator().next());
- } catch (ClassNotFoundException ignored) {
- // missed, add it to the classloader
- addToClassLoader(plugin, (HopURLClassLoader) ucl);
- }
+ // Same as above: make sure the shared class loader
carries the libraries of
+ // every plugin using it, not just of the one that created
it.
+ //
+ addToClassLoader(plugin, (HopURLClassLoader) ucl);
}
} else {
ucl = classLoaders.get(plugin);
diff --git
a/core/src/test/java/org/apache/hop/core/plugins/PluginRegistryUnitTest.java
b/core/src/test/java/org/apache/hop/core/plugins/PluginRegistryUnitTest.java
index f41f07d814..349e8e8895 100644
--- a/core/src/test/java/org/apache/hop/core/plugins/PluginRegistryUnitTest.java
+++ b/core/src/test/java/org/apache/hop/core/plugins/PluginRegistryUnitTest.java
@@ -32,7 +32,14 @@ import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
+import java.io.File;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.nio.file.Path;
+import java.util.Arrays;
import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
import java.util.UUID;
import org.apache.hop.core.exception.HopPluginClassMapException;
import org.apache.hop.core.exception.HopPluginException;
@@ -45,6 +52,7 @@ import org.apache.hop.core.row.value.ValueMetaPluginType;
import org.apache.hop.junit.rules.RestoreHopEnvironmentExtension;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
/** Unit test for {@link PluginRegistry} */
@ExtendWith(RestoreHopEnvironmentExtension.class)
@@ -131,6 +139,57 @@ class PluginRegistryUnitTest {
assertNotEquals(ucl, registry.getClassLoader(mockPlugin1));
}
+ /**
+ * A plugin joining an existing class loader group must get its own
libraries added to the shared
+ * class loader. The plugin that creates the group's class loader first
doesn't necessarily carry
+ * the same libraries - a database plugin adds the shared JDBC folders, the
bulk loader transform
+ * sharing its group doesn't - and the loser of that race used to end up
without its driver (issue
+ * #8133).
+ */
+ @Test
+ void testPluginClassloaderGroupKeepsLibrariesOfEveryMember(@TempDir Path
tempDir)
+ throws Exception {
+ PluginRegistry registry = PluginRegistry.getInstance();
+ File driverJar = tempDir.resolve("driver.jar").toFile();
+
+ // Both plugins live in the same jar, but only the second one needs the
driver.
+ //
+ IPlugin withoutLibraries = mockGroupPlugin("noLibs", String.class,
List.of());
+ IPlugin withDriver =
+ mockGroupPlugin("withDriver", Integer.class,
List.of(driverJar.getAbsolutePath()));
+
+ registry.registerPlugin(BasePluginType.class, withoutLibraries);
+ registry.registerPlugin(BasePluginType.class, withDriver);
+
+ // The plugin without libraries creates the group's class loader first.
+ //
+ URLClassLoader ucl = (URLClassLoader)
registry.getClassLoader(withoutLibraries);
+ assertEquals(ucl, registry.getClassLoader(withDriver));
+
+ URL driverUrl = driverJar.toURI().toURL();
+ assertTrue(
+ Arrays.asList(ucl.getURLs()).contains(driverUrl),
+ "the shared class loader should carry the libraries of every plugin in
the group");
+
+ // Asking again must not add the same jar a second time.
+ //
+ registry.getClassLoader(withDriver);
+ assertEquals(
+ 1, Arrays.stream(ucl.getURLs()).filter(driverUrl::equals).count(),
"duplicated library");
+ }
+
+ private static IPlugin mockGroupPlugin(String id, Class<?> mainClass,
List<String> libraries) {
+ IPlugin plugin = mock(IPlugin.class);
+ when(plugin.getIds()).thenReturn(new String[] {id});
+ when(plugin.matches(id)).thenReturn(true);
+ when(plugin.getName()).thenReturn(id);
+ when(plugin.getClassMap()).thenReturn(Map.of(IPluginType.class,
mainClass.getName()));
+ when(plugin.getLibraries()).thenReturn(libraries);
+ when(plugin.getClassLoaderGroup()).thenReturn("sharedGroup");
+ doReturn(BasePluginType.class).when(plugin).getPluginType();
+ return plugin;
+ }
+
@Test
void testClassloadingPluginNoClassRegistered() {
PluginRegistry registry = PluginRegistry.getInstance();
