Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load stable plugins as synthetic modules #91869

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/91869.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 91869
summary: Load stable plugins as synthetic modules
area: Infra/Plugins
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -523,12 +523,33 @@ static LayerAndLoader createPlugin(
extendedPlugins.stream().map(LoadedPlugin::layer)
).toList();
return createPluginModuleLayer(bundle, pluginParentLoader, parentLayers);
} else if (plugin.isStable()) {
logger.debug(() -> "Loading bundle: " + plugin.getName() + ", non-modular as synthetic module");
return LayerAndLoader.ofLoader(
UberModuleClassLoader.getInstance(
pluginParentLoader,
ModuleLayer.boot(),
"synthetic." + toModuleName(plugin.getName()),
bundle.allUrls,
Set.of("org.elasticsearch.server") // TODO: instead of denying server, allow only jvm + stable API modules
)
);
} else {
logger.debug(() -> "Loading bundle: " + plugin.getName() + ", non-modular");
return LayerAndLoader.ofLoader(URLClassLoader.newInstance(bundle.urls.toArray(URL[]::new), pluginParentLoader));
}
}

// package-visible for testing
static String toModuleName(String name) {
String result = name.replaceAll("\\W+", ".") // replace non-alphanumeric character strings with dots
.replaceAll("(^[^A-Za-z_]*)", "") // trim non-alpha or underscore characters from start
.replaceAll("\\.$", "") // trim trailing dot
.toLowerCase(Locale.getDefault());
assert ModuleSupport.isPackageName(result);
return result;
}

private static void checkDeprecations(
PluginIntrospector inspector,
List<PluginDescriptor> pluginDescriptors,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,13 @@ protected Class<?> loadClass(String cn, boolean resolve) throws ClassNotFoundExc
}
}

// For testing in cases where code must be given access to an unnamed module
void addReadsSystemClassLoaderUnnamedModule() {
moduleController.layer()
.modules()
.forEach(module -> moduleController.addReads(module, ClassLoader.getSystemClassLoader().getUnnamedModule()));
}

/**
* Returns the package name for the given class name
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.TestEnvironment;
import org.elasticsearch.index.IndexModule;
import org.elasticsearch.plugin.analysis.api.CharFilterFactory;
import org.elasticsearch.plugins.scanners.PluginInfo;
import org.elasticsearch.plugins.spi.BarPlugin;
import org.elasticsearch.plugins.spi.BarTestService;
import org.elasticsearch.plugins.spi.TestService;
Expand Down Expand Up @@ -823,6 +825,23 @@ public Reader create(Reader reader) {
assertThat(pluginInfos.get(0).descriptor().getName(), equalTo("stable-plugin"));
assertThat(pluginInfos.get(0).descriptor().isStable(), is(true));

// check ubermodule classloader usage
Collection<PluginInfo> stablePluginInfos = pluginService.getStablePluginRegistry()
.getPluginInfosForExtensible("org.elasticsearch.plugin.analysis.api.CharFilterFactory");
assertThat(stablePluginInfos, hasSize(1));
ClassLoader stablePluginClassLoader = stablePluginInfos.stream().findFirst().orElseThrow().loader();
assertThat(stablePluginClassLoader, instanceOf(UberModuleClassLoader.class));

if (CharFilterFactory.class.getModule().isNamed() == false) {
// test frameworks run with stable api classes on classpath, so we
// have no choice but to let our class read the unnamed module that
// owns the stable api classes
((UberModuleClassLoader) stablePluginClassLoader).addReadsSystemClassLoaderUnnamedModule();
}

Class<?> stableClass = stablePluginClassLoader.loadClass("p.A");
assertThat(stableClass.getModule().getName(), equalTo("synthetic.stable.plugin"));

// TODO should we add something to pluginInfos.get(0).pluginApiInfo() ?
} finally {
closePluginLoaders(pluginService);
Expand All @@ -838,6 +857,20 @@ public void testCanCreateAClassLoader() {
assertEquals(this.getClass().getClassLoader(), loader.getParent());
}

public void testToModuleName() {
assertThat(PluginsService.toModuleName("module.name"), equalTo("module.name"));
assertThat(PluginsService.toModuleName("module-name"), equalTo("module.name"));
assertThat(PluginsService.toModuleName("module-name1"), equalTo("module.name1"));
assertThat(PluginsService.toModuleName("1module-name"), equalTo("module.name"));
assertThat(PluginsService.toModuleName("module-name!"), equalTo("module.name"));
assertThat(PluginsService.toModuleName("module!@#name!"), equalTo("module.name"));
assertThat(PluginsService.toModuleName("!module-name!"), equalTo("module.name"));
assertThat(PluginsService.toModuleName("module_name"), equalTo("module_name"));
assertThat(PluginsService.toModuleName("-module-name-"), equalTo("module.name"));
assertThat(PluginsService.toModuleName("_module_name"), equalTo("_module_name"));
assertThat(PluginsService.toModuleName("_"), equalTo("_"));
}

static final class Loader extends ClassLoader {
Loader(ClassLoader parent) {
super(parent);
Expand Down