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

Use app classloader instead of platform classloader as parent #66

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SirYwell
Copy link
Contributor

@SirYwell SirYwell commented Mar 1, 2024

When using the platform classloader as parent, service loaders fail to look up services that are defined in a jdk module.

As an example, consider the following onEnable of a plugin:

    @Override
    public void onEnable() {
        System.out.println(RandomGenerator.getDefault());
    }

this will result in an exception:

[12:41:12 ERROR]: Error occurred while enabling Paper-Test-Plugin v1.0.0-SNAPSHOT (Is it up to date?)
java.lang.IllegalArgumentException: No implementation of the random number generator algorithm "L32X64MixRandom" is available
        at java.util.random.RandomGeneratorFactory.findProvider(RandomGeneratorFactory.java:229) ~[?:?]
        at java.util.random.RandomGeneratorFactory.of(RandomGeneratorFactory.java:257) ~[?:?]
        at java.util.random.RandomGenerator.of(RandomGenerator.java:124) ~[?:?]
        at java.util.random.RandomGenerator.getDefault(RandomGenerator.java:139) ~[?:?]
        at io.papermc.testplugin.TestPlugin.onEnable(TestPlugin.java:12) ~[test-plugin-1.0.0-SNAPSHOT.jar:?]
        at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:287) ~[paper-api-1.20.4-R0.1-SNAPSHOT.jar:?]
        at io.papermc.paper.plugin.manager.PaperPluginInstanceManager.enablePlugin(PaperPluginInstanceManager.java:188) ~[paper-1.20.4.jar:git-Paper-436]
        at io.papermc.paper.plugin.manager.PaperPluginManagerImpl.enablePlugin(PaperPluginManagerImpl.java:104) ~[paper-1.20.4.jar:git-Paper-436]
        at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:507) ~[paper-api-1.20.4-R0.1-SNAPSHOT.jar:?]
        at org.bukkit.craftbukkit.v1_20_R3.CraftServer.enablePlugin(CraftServer.java:639) ~[paper-1.20.4.jar:git-Paper-436]
        at org.bukkit.craftbukkit.v1_20_R3.CraftServer.enablePlugins(CraftServer.java:550) ~[paper-1.20.4.jar:git-Paper-436]
        at net.minecraft.server.dedicated.DedicatedServer.initServer(DedicatedServer.java:275) ~[paper-1.20.4.jar:git-Paper-436]
        at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1131) ~[paper-1.20.4.jar:git-Paper-436]
        at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:319) ~[paper-1.20.4.jar:git-Paper-436]
        at java.lang.Thread.run(Thread.java:1583) ~[?:?]

This is because the service loader doesn't find the proper implementations with the platform classloader being the direct parent of the context classloader. With this change, the service loader can properly look up the providers in the jdk.random module.

The behavior prior and after the change is the same on Java 17 and Java 21 respectively.

Some alternatives:

  • Plugins set the context classloader to the app classloader before accessing code that loads services from a jdk module
  • Plugins use reflections to load such services
  • Instead of using a URLClassLoader, use a custom ModuleLayer and load the jars as modules
  • Preload known services before calling the main method of the server

When using the platform classloader as parent, service loaders fail to look up services that are defined in a jdk module.
@yannicklamprecht
Copy link

Likely related bug. PaperMC/Paper#10114

Dreeam-qwq referenced this pull request in Winds-Studio/Leaf May 4, 2024
Leaf's own modified paperclip ovo

To fix a issue in Paperclip `https://github.com/PaperMC/Paperclip/pull/66` caused by a JAVA bug `https://bugs.openjdk.org/browse/JDK-8330005`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants