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

Support Multi-Release plugin JARs #4841

Closed
Jannyboy11 opened this issue Dec 3, 2020 · 3 comments
Closed

Support Multi-Release plugin JARs #4841

Jannyboy11 opened this issue Dec 3, 2020 · 3 comments
Labels
status: accepted Disputed bug is accepted as valid or Feature accepted as desired to be added. type: bug Something doesn't work as it was intended to.

Comments

@Jannyboy11
Copy link
Contributor

Jannyboy11 commented Dec 3, 2020

With the announcement that Paper is going to require Java 11 or newer from Minecraft 1.17 onwards, I asked whether Multi-Release JAR files would be supported by the classloader. The answer I got from one of the Paper developers was yes, but currently they don't work correctly.

The rationale behind this request is as follows. Say I have a plugin that uses some deprecrated internal Java API for the lack of a better alternative on Java 8. On Java 11 there is a good replacement, but obviously if I'd use that, then my plugin couldn't run anymore on Java 8. The solution is to create a Multi-Release JAR file which contains multiple versions of the same class. The JVM would then pick up the most modern version of the class that is not newer than the Java version of the JVM itself. This works for jars on the classpath and module-path, but Bukkit's JavaPluginLoader ignores this possibility entirely.
I am not sure whether this issue should be filed on SpigotMC's JIRA or here, but Spigot doesn't have an explicit stance (yet) to support this and they're not moving off Java 8 so this seemed a better place.

Steps/models to reproduce:

I compiled a demo plugin JAR with just two versions of a class containing a string constant. One for "Release 8", and one for "Release 11". The plugin's onEnable() method logs this string constant. When running Paper with Java 15 it still logs "Release 8" indicating that the classloader picked up the wrong version.

What behaviour is expected:

A log message "Release 11"

What behaviour is observed:

A log message "Release 8"

Plugin list:

MultiRelease

Attachment now updated to get the string via a method call, preventing inlining by the front-end compiler.
MultiRelease-1.0-SNAPSHOT.zip

Paper version:

> version
[16:50:13 INFO]: This server is running Paper version git-Paper-31a6e3f89 (MC: 1.16.4) (Implementing API version 1.16.4-R0.1-SNAPSHOT)
[16:50:13 INFO]: Checking version, please wait...
[16:50:14 INFO]: Previous version: git-Paper-7358bf02 (MC: 1.16.4)
[16:50:14 INFO]: You are running the latest version

Anything else:

To detect whether a JAR file is Multi-Release, one can inspect the META-INF/manifest.MF file inside the jar and look for the property Multi-Release: true. The location of the 'newer' class would be META-INF/versions/<release>/my/package/MyPlugin.class. The JEP page contains a lot more info.

@JRoy
Copy link
Member

JRoy commented Dec 3, 2020

Say I have a plugin that uses some deprecrated internal Java API for the lack of a better alternative on Java 8.

Should probably address this problem then creating this monstrosity to possibly introduce more

@aurorasmiles aurorasmiles added the type: feature Request for a new Feature. label Dec 3, 2020
@Proximyst
Copy link
Contributor

This is definitely due upstream, but I'll mark this accepted as it is definitely a bug to ignore this part of Java entirely.

@Proximyst Proximyst added status: accepted Disputed bug is accepted as valid or Feature accepted as desired to be added. type: bug Something doesn't work as it was intended to. and removed type: feature Request for a new Feature. labels Dec 3, 2020
@A248
Copy link
Contributor

A248 commented Dec 4, 2020

Apparently this is caused by the same plugin bytecode rewriting PluginClassLoader is infamous for. PluginClassLoader has to locate the jar entry manually in order to perform its devious call to UnsafeValues. This breaks multi-release jars because the way the jar entry is located was never intended to handle multi-release rules.

                String path = name.replace('.', '/').concat(".class"); // No multi-release logic
                JarEntry entry = jar.getJarEntry(path);

                if (entry != null) {
                    byte[] classBytes;

                    try (InputStream is = jar.getInputStream(entry)) {
                        classBytes = ByteStreams.toByteArray(is);
                    } catch (IOException ex) {
                        throw new ClassNotFoundException(name, ex);
                    }

                    classBytes = loader.server.getUnsafe().processClass(description, path, classBytes);

Indeed, testing the demo plugin on an older server implementation prints "Release 11", because these versions correctly delegated jar entry location to URLClassLoader, which has proper support. Unfortunately the days of not having plugin bytecode rewritten are long gone.

@DenWav DenWav closed this as completed in f15abda Dec 4, 2020
Proximyst pushed a commit to mikroskeem/Paper that referenced this issue Dec 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepted Disputed bug is accepted as valid or Feature accepted as desired to be added. type: bug Something doesn't work as it was intended to.
Projects
None yet
Development

No branches or pull requests

6 participants