keith/ForgePatch
1
0
Fork 0
Code Issues Pull requests Projects Releases 2 Wiki Activity

Improve the DistExecutor API and introduce some "almost completely" safe mechanisms to handle sided lambdas.

Browse source 
In general, use sided lambdas safely by embedding them in a separately loaded class. There's a whole twitch stream
dedicated to investigating this problem. See link in DistExecutor if you're interested.

Signed-off-by: cpw <cpw+github@weeksfamily.ca>
This commit is contained in:
cpw 2020-05-24 17:40:52 -04:00
parent 6fd01a13ee
commit a39f1cbe1a
No known key found for this signature in database
GPG key ID: 8EB3DF749553B1B7
6 changed files with 228 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
src/fmllauncher/java/net/minecraftforge/fml/loading/FMLClientLaunchProvider.java
View file

@ -22,21 +22,16 @@ package net.minecraftforge.fml.loading;
import cpw.mods.modlauncher.api.IEnvironment;
import cpw.mods.modlauncher.api.ILaunchHandlerService;
import cpw.mods.modlauncher.api.ITransformingClassLoader;
import cpw.mods.modlauncher.api.ITransformingClassLoaderBuilder;
import net.minecraftforge.api.distmarker.Dist;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Callable;
import static net.minecraftforge.fml.loading.LogMarkers.CORE;
public class FMLClientLaunchProvider extends FMLCommonLaunchHandler implements ILaunchHandlerService
{
private static final Logger LOGGER = LogManager.getLogger();
@ -88,4 +83,9 @@ public class FMLClientLaunchProvider extends FMLCommonLaunchHandler implements I
public Path[] getPaths() {
return FMLLoader.getMCPaths();
}
@Override
public boolean isProduction() {
return true;
}
}

4
src/fmllauncher/java/net/minecraftforge/fml/loading/FMLCommonLaunchHandler.java
View file

@ -150,4 +150,8 @@ public abstract class FMLCommonLaunchHandler
.peek(p->LOGGER.debug(LOADING, "Adding {} as a library to the transforming classloader", p))
.forEach(additionalLibraries::add);
}
public boolean isProduction() {
return false;
}
}

1
src/fmllauncher/java/net/minecraftforge/fml/loading/FMLEnvironment.java
View file

@ -30,6 +30,7 @@ public class FMLEnvironment
{
public static final Dist dist = FMLLoader.getDist();
public static final String naming = FMLLoader.getNaming();
public static final boolean production = FMLLoader.isProduction() || System.getProperties().containsKey("production");
static void setupInteropEnvironment(IEnvironment environment) {
environment.computePropertyIfAbsent(IEnvironment.Keys.NAMING.get(), v->naming);

6
src/fmllauncher/java/net/minecraftforge/fml/loading/FMLLoader.java
View file

@ -84,6 +84,7 @@ public class FMLLoader
private static FMLCommonLaunchHandler commonLaunchHandler;
public static Runnable progressWindowTick;
public static BackgroundScanHandler backgroundScanHandler;
private static boolean production;
static void onInitialLoad(IEnvironment environment, Set<String> otherServices) throws IncompatibleEnvironmentException
{
@ -181,6 +182,7 @@ public class FMLLoader
commonLaunchHandler = (FMLCommonLaunchHandler)launchHandler.get();
naming = commonLaunchHandler.getNaming();
dist = commonLaunchHandler.getDist();
production = commonLaunchHandler.isProduction();
progressWindowTick = EarlyProgressVisualization.INSTANCE.accept(dist);
StartupMessageManager.modLoaderConsumer().ifPresent(c->c.accept("Early Loading!"));
accessTransformer.getExtension().accept(Pair.of(naming, "srg"));
@ -296,4 +298,8 @@ public class FMLLoader
public static String launcherHandlerName() {
return launchHandlerName;
}
public static boolean isProduction() {
return production;
}
}

5
src/fmllauncher/java/net/minecraftforge/fml/loading/FMLServerLaunchProvider.java
View file

@ -83,4 +83,9 @@ public class FMLServerLaunchProvider extends FMLCommonLaunchHandler implements I
public Path[] getPaths() {
return FMLLoader.getMCPaths();
}
@Override
public boolean isProduction() {
return true;
}
}

208
src/main/java/net/minecraftforge/fml/DistExecutor.java
View file

@ -21,23 +21,51 @@ package net.minecraftforge.fml;
import net.minecraftforge.api.distmarker.Dist;
import net.minecraftforge.fml.loading.FMLEnvironment;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import java.io.Serializable;
import java.lang.invoke.SerializedLambda;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Objects;
import java.util.concurrent.Callable;
import java.util.function.Supplier;
/**
* Use to execute code conditionally based on sidedness.
* <ul>
* <li>When you want to call something on one side and return a result {@link #safeCallWhenOn(Dist, Supplier)}</li>
* <li>When you want to call one thing on one side, another thing on the other and return a result {@link #safeRunForDist(Supplier, Supplier)}</li>
* <li>When you want to run something on one side {@link #safeRunWhenOn(Dist, Supplier)}</li>
* </ul>
*/
public final class DistExecutor
{
private static final Logger LOGGER = LogManager.getLogger();
private DistExecutor() {}
/**
* Run the callable in the supplier only on the specified {@link Side}
* Run the callable in the supplier only on the specified {@link Side}.
* This method is NOT sided-safe and special care needs to be taken in code using this method that implicit class
* loading is not triggered by the Callable.
*
* This method can cause unexpected ClassNotFound exceptions.
*
* Use {@link #safeCallWhenOn(Dist, Supplier)} where possible.
*
* @param dist The dist to run on
* @param toRun A supplier of the callable to run (Supplier wrapper to ensure classloading only on the appropriate dist)
* @param <T> The return type from the callable
* @return The callable's result
* @deprecated use {@link #safeCallWhenOn(Dist, Supplier)} instead. This remains for advanced use cases.
*/
@Deprecated
public static <T> T callWhenOn(Dist dist, Supplier<Callable<T>> toRun) {
return unsafeCallWhenOn(dist, toRun);
}
public static <T> T unsafeCallWhenOn(Dist dist, Supplier<Callable<T>> toRun) {
if (dist == FMLEnvironment.dist) {
try
{
@ -51,12 +79,95 @@ public final class DistExecutor
return null;
}
/**
* Call the SafeCallable when on the correct {@link Dist}.
*
* <strong>The lambda supplied here is required to be a method reference to a method defined in
* another class, otherwise an invalid SafeReferent error will be thrown</strong>
* @param dist the dist which this will run on
* @param toRun the SafeCallable to run and return the result from
* @param <T> The type of the SafeCallable
* @return the result of the SafeCallable or null if on the wrong side
*/
public static <T> T safeCallWhenOn(Dist dist, Supplier<SafeCallable<T>> toRun) {
validateSafeReferent(toRun);
return callWhenOn(dist, toRun::get);
}
/**
* Runs the supplied Runnable on the speicified side. Same warnings apply as {@link #callWhenOn(Dist, Supplier)}.
*
* This method can cause unexpected ClassNotFound exceptions.
*
* @see #callWhenOn(Dist, Supplier)
* @param dist Dist to run this code on
* @param toRun The code to run
* @deprecated use {@link #safeRunWhenOn(Dist, Supplier)} where possible. Advanced uses only.
*/
@Deprecated
public static void runWhenOn(Dist dist, Supplier<Runnable> toRun) {
unsafeRunWhenOn(dist, toRun);
}
/**
* Runs the supplied Runnable on the speicified side. Same warnings apply as {@link #unsafeCallWhenOn(Dist, Supplier)}.
*
* This method can cause unexpected ClassNotFoundException problems in common scenarios. Understand the pitfalls of
* the way the class verifier works to load classes before using this.
*
* Use {@link #safeRunWhenOn(Dist, Supplier)} if you can.
*
* @see #unsafeCallWhenOn(Dist, Supplier)
* @param dist Dist to run this code on
* @param toRun The code to run
*/
public static void unsafeRunWhenOn(Dist dist, Supplier<Runnable> toRun) {
if (dist == FMLEnvironment.dist) {
toRun.get().run();
}
}
/**
* Call the supplied SafeRunnable when on the correct Dist.
* @param dist The dist to run on
* @param toRun The code to run
*/
public static void safeRunWhenOn(Dist dist, Supplier<SafeRunnable> toRun) {
validateSafeReferent(toRun);
if (dist == FMLEnvironment.dist) {
toRun.get().run();
}
}
/**
* Executes one of the two suppliers, based on which side is active.
*
* <p>
* Example (replacement for old SidedProxy):<br/>
* {@code Proxy p = DistExecutor.runForDist(()->ClientProxy::new, ()->ServerProxy::new);}
*
* NOTE: the double supplier is required to avoid classloading the secondary target.
*
* @param clientTarget The supplier supplier to run when on the {@link Dist#CLIENT}
* @param serverTarget The supplier supplier to run when on the {@link Dist#DEDICATED_SERVER}
* @param <T> The common type to return
* @return The returned instance
* @deprecated Use {@link #safeRunForDist(Supplier, Supplier)}
*/
@Deprecated
public static <T> T runForDist(Supplier<Supplier<T>> clientTarget, Supplier<Supplier<T>> serverTarget) {
return unsafeRunForDist(clientTarget, serverTarget);
}
/**
* Unsafe version of {@link #safeRunForDist(Supplier, Supplier)}. Use only when you know what you're doing
* and understand why the verifier can cause unexpected ClassNotFoundException crashes even when code is apparently
* not sided. Ensure you test both sides fully to be confident in using this.
*
* @param clientTarget The supplier supplier to run when on the {@link Dist#CLIENT}
* @param serverTarget The supplier supplier to run when on the {@link Dist#DEDICATED_SERVER}
* @param <T> The common type to return
* @return The returned instance
*/
public static <T> T unsafeRunForDist(Supplier<Supplier<T>> clientTarget, Supplier<Supplier<T>> serverTarget) {
switch (FMLEnvironment.dist)
{
case CLIENT:
@ -67,4 +178,99 @@ public final class DistExecutor
throw new IllegalArgumentException("UNSIDED?");
}
}
/**
* Executes one of the two suppliers, based on which side is active.
*
* <p>
* Example (replacement for old SidedProxy):<br/>
* {@code Proxy p = DistExecutor.safeRunForDist(()->ClientProxy::new, ()->ServerProxy::new);}
*
* NOTE: the double supplier is required to avoid classloading the secondary target.
*
* @param clientTarget The supplier supplier to run when on the {@link Dist#CLIENT}
* @param serverTarget The supplier supplier to run when on the {@link Dist#DEDICATED_SERVER}
* @param <T> The common type to return
* @return The returned instance
*/
public static <T> T safeRunForDist(Supplier<SafeSupplier<T>> clientTarget, Supplier<SafeSupplier<T>> serverTarget) {
validateSafeReferent(clientTarget);
validateSafeReferent(serverTarget);
switch (FMLEnvironment.dist)
{
case CLIENT:
return clientTarget.get().get();
case DEDICATED_SERVER:
return serverTarget.get().get();
default:
throw new IllegalArgumentException("UNSIDED?");
}
}
/**
* A safe referent. This will assert that it is being called via a separated class method reference. This will
* avoid the common pitfalls of {@link #callWhenOn(Dist, Supplier)} above.
*
* SafeReferents assert that they are defined as a separate method outside the scope of the calling class.
*
* <strong>Implementations need to be defined in a separate class to the referring site, with appropriate
* visibility to be accessible at the callsite (generally, avoid private methods).</strong>
*
* <p>
* Valid:<br/>
*
* {@code DistExecutor.safeCallWhenOn(Dist.CLIENT, ()->AnotherClass::clientOnlyMethod);}
*
* <p>
* Invalid:<br/>
*
* {@code DistExecutor.safeCallWhenOn(Dist.CLIENT, ()->()->Minecraft.getInstance().world);}
*/
public interface SafeReferent {}
/**
* SafeCallable version of {@link SafeReferent}.
* @see SafeReferent
* @param <T> The return type of the Callable
*/
public interface SafeCallable<T> extends SafeReferent, Callable<T>, Serializable {}
/**
* SafeSupplier version of {@link SafeReferent}
* @param <T> The return type of the Supplier
*/
public interface SafeSupplier<T> extends SafeReferent, Supplier<T>, Serializable {}
/**
* SafeRunnable version of {@link SafeReferent}
*/
public interface SafeRunnable extends SafeReferent, Runnable, Serializable {}
private static final void validateSafeReferent(Supplier<? extends SafeReferent> safeReferentSupplier) {
if (FMLEnvironment.production) return;
final SafeReferent setter;
try {
setter = safeReferentSupplier.get();
} catch (Exception e) {
// Typically a class cast exception, just return out, expected.
return;
}
for (Class<?> cl = setter.getClass(); cl != null; cl = cl.getSuperclass()) {
try {
Method m = cl.getDeclaredMethod("writeReplace");
m.setAccessible(true);
Object replacement = m.invoke(setter);
if (!(replacement instanceof SerializedLambda))
break;// custom interface implementation
SerializedLambda l = (SerializedLambda) replacement;
if (Objects.equals(l.getCapturingClass(), l.getImplClass())) {
LOGGER.fatal("Detected unsafe referent usage, please view the code at {}",Thread.currentThread().getStackTrace()[3]);
throw new RuntimeException("Unsafe Referent usage found in safe referent method");
}
} catch (NoSuchMethodException e) {
} catch (IllegalAccessException | InvocationTargetException e) {
break;
}
}
}
}