[1.17/Java 16] DistExecutor::validateSafeReferent crashes in dev

[malte0811]

Minecraft Version: 1.17.1

Forge Version: 37.0.33

Logs: https://gist.github.com/malte0811/497c75ee3eb8641a6758ea2864cae330

Steps to Reproduce:

1. Add this class to an MDK

package com.example.examplemod;

import net.minecraft.client.Minecraft;
import net.minecraft.world.level.Level;

public class ClientOnlyClass {
	public Level getClientLevel() {
		return Minecraft.getInstance().level;
	}
}

2. Add DistExecutor.safeRunWhenOn(Dist.CLIENT, () -> ClientOnlyClass::new); to the example mod constructor.
3. Launch a dev server (./gradlew runServer or IDE run configs)
4. Observe a crash

Description of issue: In 1.16 (more specifically in Java 8, I've seen a similar crash in 1.16 with a newer Java version) this would produce an Exception of some sort. In 1.17/Java 16 it results in an Error instead, which is not caught in DistExecutor::validateSafeReferent.

[ChampionAsh5357]

This affects all JDKs after 8. It seems to be a result of the resolution of Bug 8051045 as all invokedynamic instructions now wrap the exception in a BootstrapMethodError.

[marchermans]

I don't think that example is a valid reference. Since that ClientClass::new is not a static reference. Which is needed for a valid safe reference.

[ChampionAsh5357]

Event if its not, the same error occurs. Take for example this modified case where we use ClientOnlyClass::test instead:

public class ClientOnlyClass {

    public static void test() {

    }

    public World getClientWorld() {
        return Minecraft.getInstance().level;
    }
}

The generated bytecode for the supplied safe referent still results in an invokedynamic instruction which will wrap the exception as seen by this console output. I'm using 1.16.5 with j10 for this test to prove the issue stems from JDK and not version. Rest assured, it still causes an issue in 1.17.1 if you run the test case yourself.

[TheCurle]

Indeed, the original idea of validateSafeReferent was to ensure that a Supplier to a Runnable would have the protections of the invokestatic instruction.

A newer Java version replaced that with invokedynamic for lambdas and method references, which means the original idea is dead. We're discussing internally what to do with this, but whatever we decide upon,

if (FMLEnvironment.dist == Dist.CLIENT) ClientHandler.doStuff();

will always be more robust and reliable.