`Class.forName` causes early loading of vanilla classes

[sakurawald]

Mixin class are different from normal Java class.
The import statements in mixin class gets special treatment from mixin processor.
While static import cause the early class loading, which breaks the time sequence of class loading when mixin processor processes them.

The line

ChatPlus/src/main/java/cn/revaria/chatplus/mixin/ChatMixin.java

Line 18 in e434261

import static cn.revaria.chatplus.util.TextStyleFormatter.applyStyle;

will cause any other mixins who targets the imported classes loaded too early.

[CPTProgrammer]

Thanks for raising the issue!

But when I tested it, both import static and regular import produced identical bytecode. I also made a test mod to mixin the TextStyleFormatter class, and it worked fine.

Could you clarify what I might be misunderstanding? For example:

• What specific code needs changing?
• What's the intended outcome?

[sakurawald]

Thanks for raising the issue!

But when I tested it, both import static and regular import produced identical bytecode. I also made a test mod to mixin the TextStyleFormatter class, and it worked fine.

Could you clarify what I might be misunderstanding? For example:

• What specific code needs changing?
• What's the intended outcome?

You should remove the unused import, or the optimize phase of compiling will produce the same bytecode.

[sakurawald]

Could you clarify what I might be misunderstanding? For example:

The static import uses the Text class in Minecraft.

If other mods writes

@Mixin(Text.class)
public interface TextMixin {

}

Then the injector will fail due to early class loading.

[sakurawald]

• What specific code needs changing?

Replace

import static cn.revaria.chatplus.util.TextStyleFormatter.applyStyle;

with

import cn.revaria.chatplus.util.TextStyleFormatter;

And use qualified calls to the method.

[CPTProgrammer]

Thanks for the detailed explanation - you did point out something important.

I tried mixin the Text class and it does throw a MixinTargetAlreadyLoadedException. But this isn't caused by import static - as we confirmed earlier, import static and regular import produce identical bytecode.

The root cause is this line:

ChatPlus/src/main/java/cn/revaria/chatplus/plugin/MixinConfigPlugin.java

Line 26 in e434261

Class<?> mixinClass = Class.forName(mixinClassName);

Class.forName() forces premature loading and initialization of the ChatMixin class, which triggers the vanilla Minecraft Text class loading.

Currently working on a better implementation.

[sakurawald]

as we confirmed earlier

I don't think it is confirmed, but things are interesting.

The conclusion: the bytecode cheats us, even the javap -v -c or the IDEA bytecode viewer says the two file contains the identical bytecode, that's wrong.

Let me explain how to make the difference and how to compare it:

1. Clone the repo at 2c6471a8c00bb81872fbbda17c82a4892da60a8a commit, which is taged as v1.0.0 version. (It's the original version)
2. Run gradle build task to comlile the project and get the original jar file.
3. Make the modification as follows, and run the gradle build task again to get the modified jar file.
   
4. And magic happens, the modified jar works fine with TextMixin, but the original jar fails.

Now, we will de-compress the two jar files and compute the checksum to see the difference.

The difference shows the ChatMixin.class file are different.

And here is the difference at hex view for ChatMixin.class file:

So there are some in-visible flags inside the two .class files, and the bytecode viewer ignores them.

The static import will trigger the static initializer at once, but the import only triggers the static initializer when it's actually needed.

The static import does calls the Class.forName implicitly, the static import cause the applyStyle method to be loaded, and this method rquires the Text class as its argument, which causing the ClassLoader to find the Text class, and causing the class loaded too early error. There are special treatment for import statement, so things are happy if you use import statement.

[MC-XiaoHei]

Have you noticed a change in your line number? So naturally the bytecode changes
Can you provide your compile JDK and running JVM info?

[sakurawald]

Have you noticed a change in your line number? So naturally the bytecode changes Can you provide your compile JDK and running JVM info?

Of course, there are 2 difference, one for the line number change.
And the result is, the two jar file has different checksums, and different effects when mixin processor processes them.

[sakurawald]

Can you provide your compile JDK and running JVM info?

java 21 2023-09-19
Java(TM) SE Runtime Environment Oracle GraalVM 21+35.1 (build 21+35-jvmci-23.1-b15)
Java HotSpot(TM) 64-Bit Server VM Oracle GraalVM 21+35.1 (build 21+35-jvmci-23.1-b15, mixed mode, sharing)

With the default gradle wrapper.

[sakurawald]

Have you noticed a change in your line number? So naturally the bytecode changes

It doesn't matter.

The .class file is different, even the bytecode viewer says they are identical:

In this case, the @Mixin all starts at line 20, and get the same conclusion:

[MC-XiaoHei]

Can you provide the two class files that you have compiled?

[sakurawald]

The conclusion holds, these are 2 different class file, due to their checksum is different.
And the most important, the effect is different when you using these 2 jar files.
The bytecode is too high-level, to see what is going on underlying.

You are happy to see it: https://drive.google.com/file/d/1L7_WHbICA2APLdiBVasKkwN-dTW4Mj6n/view?usp=sharing

[MC-XiaoHei]

are you upload wrong jar?

also, graalvm has some miscompile problems, maybe you can try other jvm, such as oepnjdk, or adoptium jdk