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

Avoid initializing class when resolving lambda type. #3

Closed
wants to merge 1 commit into from
Closed

Avoid initializing class when resolving lambda type. #3

wants to merge 1 commit into from

Conversation

ZekerZhayard
Copy link

@ZekerZhayard ZekerZhayard commented Feb 21, 2021

Related to McModLauncher/modlauncher#67

Although Forge does not support OpenJ9, I think typetools, as an upstream library, should support it as much as possible.

This issue is not the same as jhalterman#47 which has been fixed in 8059d5e. Let's analyze what happened on FarmersDelight when using OpenJ9.
Considering a simple example below:
Main.java:

import java.lang.reflect.Member;
import java.util.function.Consumer;

import sun.misc.SharedSecrets;
import sun.reflect.ConstantPool;

public class Main {
    public static void main(String[] args) throws Throwable {
        Consumer<Object> consumer = Test::test;

        ConstantPool cp = SharedSecrets.getJavaLangAccess().getConstantPool(consumer.getClass());
        for (int i = cp.getSize() - 1; i >= 0; i--) {
            try {
                System.out.println("index: " + i + ", method: " + cp.getMethodAt(i) + ", info: " + Arrays.toString(cp.getMemberRefInfoAt(i)));
            } catch (Throwable ignored) {

            }
        }
        
        Test.test(null);
    }
}

Test.java:

public class Test {
    static {
        System.out.println("<clinit> was invoked");
        try {
            // force to throw exception.
            Test.class.getDeclaredField("abcde");
        } catch (Throwable t) {
            throw new RuntimeException();
        }
    }

    public static void test(Object o) {
        System.out.println("test was invoked");
    }
}

Test class will throw an exception when initializing.
When we run this use case with different JVMs, the console will output:

AdoptOpenJDK 8u282 with Hotspot ->
index: 18, method: public static void Test.test(java.lang.Object), info: [Test, test, (Ljava/lang/Object;)V]
index: 10, method: public java.lang.Object(), info: [java/lang/Object, <init>, ()V]
<clinit> was invoked
Exception in thread "main" java.lang.ExceptionInInitializerError
	at Main.main(Main.java:21)
Caused by: java.lang.RuntimeException
	at Test.<clinit>(Test.java:8)
	... 1 more

AdoptOpenJDK 8u282 with OpenJ9 ->
<clinit> was invoked
index: 5, method: null, info: [Test, test, (Ljava/lang/Object;)V]
index: 2, method: public java.lang.Object(), info: [java/lang/Object, <init>, ()V]
test was invoked

From the above output, we will find that OpenJ9 initializes the target class when calling ConstantPool#getMethodAt and swallows the exception, then it cannot obtain the corresponding method correctly.

If we change cp.getMethodAt(i) to cp.getMethodAtIfLoaded(i), the result will be:

AdoptOpenJDK 8u282 with Hotspot ->
index: 18, method: public static void Test.test(java.lang.Object), info: [Test, test, (Ljava/lang/Object;)V]
index: 10, method: public java.lang.Object(), info: [java/lang/Object, <init>, ()V]
<clinit> was invoked
Exception in thread "main" java.lang.ExceptionInInitializerError
	at Main.main(Main.java:20)
Caused by: java.lang.RuntimeException
	at Test.<clinit>(Test.java:8)
	... 1 more

AdoptOpenJDK 8u282 with OpenJ9 ->
index: 5, method: null, info: [Test, test, (Ljava/lang/Object;)V]
index: 2, method: public java.lang.Object(), info: [java/lang/Object, <init>, ()V]
test was invoked

We will find Test class was not initialized and even not initialized when Test.test being invoked (this looks like an openj9 issue (eclipse-openj9/openj9#12016) but I don’t think the problem is very serious ), so we can use cp.getMemberRefInfoAt and reflection to find target method.

However, if we comment out the throw statement in static block, these two JVMs will get the method correctly:

AdoptOpenJDK 8u282 with Hotspot ->
index: 18, method: public static void Test.test(java.lang.Object), info: [Test, test, (Ljava/lang/Object;)V]
index: 10, method: public java.lang.Object(), info: [java/lang/Object, <init>, ()V]
<clinit> was invoked
test was invoked

AdoptOpenJDK 8u282 with OpenJ9 ->
<clinit> was invoked
index: 5, method: public static void Test.test(java.lang.Object), info: [Test, test, (Ljava/lang/Object;)V]
index: 2, method: public java.lang.Object(), info: [java/lang/Object, <init>, ()V]
test was invoked

Specific this test case to FarmersDelight:
If we try to initialize CropPatchGeneration between these two lines:

https://github.com/vectorwing/FarmersDelight/blob/59e9a8e6e6e660c43ab8143aa447f099315d1478/src/main/java/vectorwing/farmersdelight/FarmersDelight.java#L34-L35

then run the mod with Hotspot, an exception will be thrown:

[modloading-worker-2/ERROR] [ne.mi.fm.ja.FMLModContainer/LOADING]: Failed to create mod instance. ModID: farmersdelight, class vectorwing.farmersdelight.FarmersDelight
java.lang.ExceptionInInitializerError: null
	at vectorwing.farmersdelight.FarmersDelight.<init>(FarmersDelight.java:35) ~[farmersdelight:1.16.3-0.3.2] {re:classloading}
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) ~[?:1.8.0_281] {}
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) ~[?:1.8.0_281] {}
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) ~[?:1.8.0_281] {}
	at java.lang.reflect.Constructor.newInstance(Constructor.java:423) ~[?:1.8.0_281] {}
	at java.lang.Class.newInstance(Class.java:442) ~[?:1.8.0_281] {}
	at net.minecraftforge.fml.javafmlmod.FMLModContainer.constructMod(FMLModContainer.java:81) ~[forge:36.0] {re:classloading}
	at net.minecraftforge.fml.ModContainer.lambda$buildTransitionHandler$4(ModContainer.java:120) ~[forge:?] {re:classloading}
	at java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1640) [?:1.8.0_281] {}
	at java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1632) [?:1.8.0_281] {}
	at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289) [?:1.8.0_281] {}
	at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1067) [?:1.8.0_281] {}
	at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1703) [?:1.8.0_281] {}
	at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:172) [?:1.8.0_281] {}
Caused by: java.lang.NullPointerException: Registry Object not present: farmersdelight:wild_cabbages
	at java.util.Objects.requireNonNull(Objects.java:290) ~[?:1.8.0_281] {}
	at net.minecraftforge.fml.RegistryObject.get(RegistryObject.java:120) ~[forge:?] {re:mixin,re:classloading}
	at vectorwing.farmersdelight.world.CropPatchGeneration.<clinit>(CropPatchGeneration.java:22) ~[farmersdelight:1.16.3-0.3.2] {re:classloading}
	... 14 more

The problem is obvious. When typetools analyzes the lambda under OpenJ9, it accidentally initializes the CropPatchGeneration class that should not be initialized, and swallowed the exception. Therefore, typetools cannot find the correct method in this line:

Member member = getConstantPoolMethodAt(constantPool, i);

This PR replaced ConstantPool#getMethodAt to ConstantPool#getMethodAtIfLoaded to avoid initializing class and if the method ha not been loaded, use ConstantPool#getMemberRefInfoAt and reflection to find the method.

@ZekerZhayard
Copy link
Author

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.

1 participant