-
Notifications
You must be signed in to change notification settings - Fork 128
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
Reflection is broken on the JavaPlugin instance as it is instantiated as a proxy class #755
Comments
I should probably mention that, while the example I'm giving is about package names, this is only an example and this issue is occurring in other places. |
We use the proxy to get control over how and which classloader loads the 'main' plugin class. Paper changed some internals that we were abusing to work around needing. This change should have been marked as a 3.0.0 release in order to indicate that breakages are somewhat expected, but the pipeline broke in an unexpected way causing it to be released as part of the 2.x.y series. It's probably possible in some way to make byte buddy/the classloader believe that the proxy class is in the same package as the original. Would such a solution fix your use case? Alternatively, perhaps you can change your tests so that they use the parent class instead of the plugin class directly? |
I saw what happened with the 3.0.0 release, but don't worry it didn't really affect me. Tricking the classloader and bytebuddy into thinking the proxy class is in the same package as the original would indeed fix my issue. But I don't think it would be a very good fix, as it would only fix my very specific problem, and potentially won't fix other issues that would involve reflections over that proxy class. I did the workaround you're describing in my tests, and it works. Now that I think about it, wouldn't it be possible to simply replace the getClass of the proxy class and return the getClass of the parent class, and (hopefully) that would magically work? |
Chiming in that this is impacting Multiverse as well. We are using dependency injection and similarly rely on reflection that breaks because of the use of the proxy. |
I've looked into this a bit, I almost have a solution for this; I am able to change the class contextClassLoader for the thread that solves the test with my own implementation of a class loader. The problem comes with the actuall implementation (I believe at least). The problem from what I can see is that the classloader that is going to be used when loading a new class is inherited from the current class as written here. This means that I can not go back to using another classloader for loading a class, as all class-loading instansiated after loading that class will also be loaded using that other class loader. That seems fine by me; this far it is seems possible to just load everything from our own custom class loader. Here's where the issues arise; some classes has been loaded by the previous classloader (the classloader which we replaced wih our own implementation). Any class which is inheriting from that class has to be linked to that class, which only can be done using the previous class loader. So to say it is necessary to return the same class instance of any class that has already been loaaded before the class loader was changed. Okay, and from that, one might think, why not look into the previous class loader and check if the class already has been loaded there? This is not possible since Java 9 unfortunately, as the method that does that has a protected access modifier, which can not be changed as ClassLoader is a core java method. (I made a draft pull request for this, see #1003) |
Is there an existing issue for this?
Are you using the latest version of MockBukkit?
Minecraft Version
1.19
Describe the bug
Since updating to Paper 1.19.4, and MockBukkit to its latest version, most of my tests broke because some of the initialization logic in the plugin class uses its class instance to do reflection.
With this update, some of the logic for initializing a plugin has been changed, and in particular those lines: https://github.com/MockBukkit/MockBukkit/pull/747/files#diff-4a7fdcf888c1e81d04a702b47342e96a583b026f1036b44599ab74e4e91ceb38R83-R89. With this, the plugin instance is no longer an instance of the actual plugin class, but rather of a proxy class that extends the plugin class. The result is that most reflection don't work the way it used to.
In particular, for my use case, I use the plugin's package name to find classes in subpackages and automagically inject them in my plugin's runtime. However, the package name is now empty, and it broke.
I could do a workaround by checking it on the superclass instance, but that would imply adding checks for all usage of reflection on this class for whether this is a test environment or not, and that isn't ideal.
Reproducible Test
instance.getClass().getPackageName()
Anything else?
I don't know ByteBuddy enough to fix this issue myself, I don't know how to "transfer" all the reflection data to the proxy instance, but I'm hoping that someone with more ByteBuddy knowledge than me will be able to.
The text was updated successfully, but these errors were encountered: