Use PluginClassLoader#libraryLoader for Bukkit libraries

[SkytAsul]

By doing this, the plugin's "real" classloader won't get polluted by libraries and this would prevent issues when multiple versions of the same library are loaded by multiple plugins.

E.g.:

• Plugin A loads library X through Libby
• Plugin B loads library X using the "libraries" section of plugin.yml
• Plugin C depends on B and wants to load a class of library X. It expects to get a class from the plugin B's version of library X, but since the "global" classpath is polluted with plugin A's libraries loaded by Libby, we end up with the version of X from A.

By loading the Libby libraries in the "libraries" classloader, we automatically get the benefit that classes are only fetched in class loaders that are either 1. plugin classloader or 2. library classloader of (transitive) dependencies (https://hub.spigotmc.org/stash/projects/SPIGOT/repos/bukkit/browse/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java#205).

This issue is blocking for me because my plugins, which don't use Libby in any way, are being polluted by another unrelated plugin using Libby.
I am open for discussion and can propose changes in a Pull Request.

[LoneDev6]

Seems a good solution to me, might it be possible to change the classloader without editing the Libby source?

[frengor]

E.g.:

• Plugin A loads library X through Libby
• Plugin B loads library X using the "libraries" section of plugin.yml
• Plugin C depends on B and wants to load a class of library X. It expects to get a class from the plugin B's version of library X, but since the "global" classpath is polluted with plugin A's libraries loaded by Libby, we end up with the version of X from A.

The culprit here is plugin A, which is not relocating the library X. When loading in the plugin's classloader the libraries must always be relocated exactly to avoid this issue.

I do agree though that loading into the "libraries" classloader would be ideal, however it is not straightforward. If the plugin.yml does not contain any libraries it is set to null and the field is final, so we cannot set it. We made a PR to Paper for exactly this issue, but it was not accepted (we haven't tried to make one for Spigot).

Seems a good solution to me, might it be possible to change the classloader without editing the Libby source?

You can create your custom LibraryManager or use a custom classloader directly, for example here I'm manually adding libraries to an IsolatedClassLoader. You can also replace the IsolatedClassLoader with a URLClassLoader which has as parent the plugin's classloader, so that when the libraries inside the URLClassLoader load a class from MC or your plugin they have access to it (the only exception is that your plugin cannot load automatically classes from the URLClassLoader, you need to use loadClass).

[SkytAsul]

I do agree though that loading into the "libraries" classloader would be ideal, however it is not straightforward. If the plugin.yml does not contain any libraries it is set to null and the field is final, so we cannot set it.

Modifying a final field using reflection is possible afaik, why not do it?

[frengor]

Modifying a final field using reflection is possible afaik, why not do it?

It is possible, but it is not guaranteed to work. From Field#set docs (emphasis added):

If the underlying field is final, the method throws an IllegalAccessException unless setAccessible(true) has succeeded for this Field object and the field is non-static. Setting a final field in this way is meaningful only during deserialization or reconstruction of instances of classes with blank final fields, before they are made available for access by other parts of a program. Use in any other context may have unpredictable effects, including cases in which other parts of a program continue to use the original value of this field.

[SkytAsul]

Ah, my bad.

Then, only use the library classloader if it exists and encourage users to load Libby via the libraries section, so the classloader is created :)
(I know it's not ideal :/)

Maybe a PR to Spigot will be accepted though.