Skip to content

Commit 61d0b40

Browse files
authored
Store plugin loader separately from instance (#117393) (#117413)
Stable plugins do not have a plugin instance, so their loader cannot be retrieved by looking at the instance class (which is a placeholder). This commit adds back the class loader of each plugin to the LoadedPlugin record so that it can be closed by tests. closes #117220
1 parent 213ae57 commit 61d0b40

File tree

3 files changed

+14
-9
lines changed

3 files changed

+14
-9
lines changed

server/src/main/java/org/elasticsearch/plugins/PluginsService.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public StablePluginsRegistry getStablePluginRegistry() {
6262
* @param descriptor Metadata about the plugin, usually loaded from plugin properties
6363
* @param instance The constructed instance of the plugin's main class
6464
*/
65-
record LoadedPlugin(PluginDescriptor descriptor, Plugin instance) {
65+
record LoadedPlugin(PluginDescriptor descriptor, Plugin instance, ClassLoader classLoader) {
6666

6767
LoadedPlugin {
6868
Objects.requireNonNull(descriptor);
@@ -426,7 +426,7 @@ We need to pass a name though so that we can show that a plugin was loaded (via
426426
}
427427
plugin = loadPlugin(pluginClass, settings, configPath);
428428
}
429-
loadedPlugins.put(name, new LoadedPlugin(pluginBundle.plugin, plugin));
429+
loadedPlugins.put(name, new LoadedPlugin(pluginBundle.plugin, plugin, pluginLayer.pluginClassLoader()));
430430
} finally {
431431
privilegedSetContextClassLoader(cl);
432432
}

server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,8 @@ public void testExtensiblePlugin() {
466466
List.of(
467467
new PluginsService.LoadedPlugin(
468468
new PluginDescriptor("extensible", null, null, null, null, classname, null, List.of(), false, false, false, false),
469-
extensiblePlugin
469+
extensiblePlugin,
470+
null
470471
)
471472
)
472473
);
@@ -480,7 +481,8 @@ public void testExtensiblePlugin() {
480481
List.of(
481482
new PluginsService.LoadedPlugin(
482483
new PluginDescriptor("extensible", null, null, null, null, classname, null, List.of(), false, false, false, false),
483-
extensiblePlugin
484+
extensiblePlugin,
485+
null
484486
),
485487
new PluginsService.LoadedPlugin(
486488
new PluginDescriptor(
@@ -497,7 +499,8 @@ public void testExtensiblePlugin() {
497499
false,
498500
false
499501
),
500-
testPlugin
502+
testPlugin,
503+
null
501504
)
502505
)
503506
);
@@ -885,20 +888,22 @@ static final class Loader extends ClassLoader {
885888
// We can use the direct ClassLoader from the plugin because tests do not use any parent SPI ClassLoaders.
886889
static void closePluginLoaders(PluginsService pluginService) {
887890
for (var lp : pluginService.plugins()) {
888-
if (lp.instance().getClass().getClassLoader() instanceof URLClassLoader urlClassLoader) {
891+
if (lp.classLoader() instanceof URLClassLoader urlClassLoader) {
889892
try {
890893
PrivilegedOperations.closeURLClassLoader(urlClassLoader);
891894
} catch (IOException unexpected) {
892895
throw new UncheckedIOException(unexpected);
893896
}
894-
}
895-
if (lp.instance().getClass().getClassLoader() instanceof UberModuleClassLoader loader) {
897+
} else if (lp.classLoader() instanceof UberModuleClassLoader loader) {
896898
try {
897899
PrivilegedOperations.closeURLClassLoader(loader.getInternalLoader());
898900
} catch (Exception e) {
899901
throw new RuntimeException(e);
900902
}
903+
} else {
904+
throw new AssertionError("Cannot close unexpected classloader " + lp.classLoader());
901905
}
906+
902907
}
903908
}
904909

test/framework/src/main/java/org/elasticsearch/plugins/MockPluginsService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ protected void addServerExportsService(Map<String, List<ModuleQualifiedExportsSe
7272
if (logger.isTraceEnabled()) {
7373
logger.trace("plugin loaded from classpath [{}]", pluginInfo);
7474
}
75-
pluginsLoaded.add(new LoadedPlugin(pluginInfo, plugin));
75+
pluginsLoaded.add(new LoadedPlugin(pluginInfo, plugin, MockPluginsService.class.getClassLoader()));
7676
}
7777
loadExtensions(pluginsLoaded);
7878
this.classpathPlugins = List.copyOf(pluginsLoaded);

0 commit comments

Comments
 (0)