mcmodlauncher / modlauncher

Java 21 mod launcher

Java 8 mod launcher for Minecraft

Intended as a replacement for LegacyLauncher, using Java 8 and 9 compatible constructs, in theory.

NOTE

This is strictly for initial development and review of ideas. It will not be this github repo once complete, it will not be these packages once complete. Please note this well - this is a holding area as I develop out the initial plans, a more permanent home has yet to be identified

TODO

• Everything
• Some things
• Make it work
• Make it compile

In seriousness, areas that need to be reviewed: the details of voting contexts, environment contexts. The documentation on Voting should be OK, I think it reflects how I believe it should work.

Things I want to have:

• a universal access transformer - access transformers are a bit of a special bunny, and everyone needs them all the time. Having it baked right into the basic framework removes a huge burden from a lot of places.
• logging. I want the logging to be very solid. If we die, I want a log file to tell us why immediately, no speculations.
• there is probably a third "initialization cycle" in the launcherservice that isn't represented yet - for once the classloader has stood up.
• we probably still need to do the signed but not signed jar hacks from legacylauncher.

Tests

I want everything to have a test, if possible. I hate how unreproducible legacylauncher is. With Java9 ripping the rug out of how everything is supposed to work, having a broad test suite that represents "yup, it works" will be invaluable.

On contributions

This code is often sitting on my desktop in a half working state as I take time to work on it. Time is on our side, 1.13 is the earliest this will land, and that is some time away yet. PRs, although welcome and invited, might be based on the repo as it existed some time before, and I will have taken the code in a new direction as I figure out things. At present, I'm not likely to accept a PR as a result. Once we get past the "initial conception" stages, where it is properly working and able to fulfill the role I'm giving it (soon, I hope), I anticipate seeking some PRs to implement enhancement features.

On licensing

There's some complexity involved in this, because Mojang are interested in this themselves, and licensing may need to conform to their standards. At present, this code is therefore unlicensed and as such "all rights reserved". Contributions and contributors should take note of this. It's not ideal (LGPLv2.1 is my preferred target license for platform libs such as this, as you all know) but I do intend to address this before it goes live.

modlauncher's Issues

ClassLoader fails to load java classes with null byte naming

As the title says, loading a mod jar which has null characters in the file name will make Minecraft crash when using this (Latest Forge 1.16.1), the normal Minecraft loader does not do this, nor does other loaders like Fabric.

The main error is: Error: java.nio.file.InvalidPathException: Path: nul character not allowed: followed by the path itself.

The whole stacktrace:

java.nio.file.InvalidPathException: Path: nul character not allowed: <null character>/<the path to my class>
    at com.sun.nio.zipfs.ZipPath.normalize(ZipPath.java:448)
    at com.sun.nio.zipfs.ZipPath.<init>(ZipPath.java:76)
    at com.sun.nio.zipfs.ZipPath.<init>(ZipPath.java:67)
    at com.sun.nio.zipfs.ZipFileSystem.getPath(ZipFileSystem.java:186)
    at com.sun.nio.zipfs.ZipFileSystem.getPath(ZipFileSystem.java:80)
    at net.minecraftforge.fml.loading.moddiscovery.ModDiscoverer$MinecraftLocator.findPathJar(ModDiscoverer.java:200)
    at net.minecraftforge.fml.loading.moddiscovery.ModDiscoverer$MinecraftLocator.findPath(ModDiscoverer.java:187)
    at net.minecraftforge.fml.loading.moddiscovery.ModFile.findResource(ModFile.java:181)
    at net.minecraftforge.fml.loading.LoadingModList.findURLForResource(LoadingModList.java:126)
    at net.minecraftforge.fml.loading.FMLCommonLaunchHandler.lambda$getClassLoaderLocatorFunction$5(FMLCommonLaunchHandler.java:132)
    at net.minecraftforge.fml.loading.FMLCommonLaunchHandler$$Lambda$409/664839586.apply(Unknown Source)
    at cpw.mods.modlauncher.TransformationServicesHandler.lambda$alternate$1(TransformationServicesHandler.java:52)
    at cpw.mods.modlauncher.TransformationServicesHandler$$Lambda$414/651235118.apply(Unknown Source)
    at cpw.mods.modlauncher.TransformationServicesHandler.lambda$alternate$1(TransformationServicesHandler.java:52)
    at cpw.mods.modlauncher.TransformationServicesHandler$$Lambda$414/651235118.apply(Unknown Source)
    at cpw.mods.modlauncher.TransformingClassLoader.lambda$alternate$10(TransformingClassLoader.java:85)
    at cpw.mods.modlauncher.TransformingClassLoader$$Lambda$420/295055909.apply(Unknown Source)
    at cpw.mods.modlauncher.TransformingClassLoader$DelegatedClassLoader.findClass(TransformingClassLoader.java:218)
    at cpw.mods.modlauncher.TransformingClassLoader.loadClass(TransformingClassLoader.java:126)
    at cpw.mods.modlauncher.TransformingClassLoader.loadClass(TransformingClassLoader.java:96)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:411)
    at java.net.FactoryURLClassLoader.loadClass(URLClassLoader.java:814)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
    at <my class calling a new MyClass()>```
 
The null character is a common in obfuscation programs, and forge with modlauncher cannot load it.

Anything not in the inital classpath cannot provide a ITransformationService

The ITransformationService map is populated very early in the cycle, so only anything that is not on the launch class path wil lnot be scanned for ITransformationServices.
This mean 3rd party class transformation providers (like Mixin, Optifine, etc) have no access to ILaunchPluginService (which is intended IIRC) AND no access to ITransformationService (which they should have IMO, as it has a much ITransformers are much more locked down)

ILaunchPluginService: preLaunch callback?

Preamble

It's taken me a long time to get the new Mixin version to a point where I'm happy with it, mostly due to personal commitments, so I'd like to apologise for the radio silence from my end. The approach I have taken while developing Mixin for ModLauncher is to work around things as they are, since I felt that until I was happy with the engineering of the solution that asking for changes to ModLauncher was frivolous. I wanted to make sure I was asking for the right things.

To this end, I've wrapped the work-arounds and reflection-based shims into a single class which I've named Internals. This issue pertains to one of the duties offloaded to Internals, identifying the moment immediately prior to startup.

The Issue

As you may recall, Mixin processing happens in phases in order that Mixins can be loaded, conformed to the current environment, and are then ready for application during the unfolding of the lifecycle phase.

With the current design, Mixin begins the only phase (DEFAULT) as the game starts. Mixins which were previously obtained by the platform agents - eg. from FML mod containers, classpath entries, or the command line - are then selected into the environment ready for application.

Currently the only way I have discovered to detect this event is via the rather crufty method of log watching, and simply hunting for the "Launching Target" message that ML emits prior to start.

This isn't the most elegant solution in the world, so it would be helpful if either ILaunchPluginService or ITransformationService could be notified immediately prior to the call to launch service launch with some kind of preLaunch callback.

Would this be possible? I can think of reasons it might not be desirable to put it into ITransformationService but if it could be added to ILaunchPluginService that would allow me to nuke this particular hack.

---

As a side note, obviously Internals contains some other cruft, but rather than make a "kitchen sink" issue I figured it was more appropriate to list these as separate issues for your consideration.

Cannot get jdk class byte from ITransformerLoader

Sometimes we want to get parent class byte for analysis, but ITransformerLoader delegates to buildTransformedClassNodeFor and it can only load class from TCL. It is dull to always write a try catch block.

https://github.com/cpw/modlauncher/blob/3cf110cd0b2a5bcd8a44057fd0513e38a4b06f97/src/main/java/cpw/mods/modlauncher/TransformerClassWriter.java#L147-L164

Game won't start

Hello,
I can't start a game and the logs led me here.

Latest: https://hasteb.in/ipeyavof.md
Debug: https://hasteb.in/zogidibe.md

Greetings
TajemnikTV

Crash on startup

log https://gist.github.com/FrostDelta123/1c81b1ea4833cf9876b1488b45190b05

Client-args

"--launchTarget", "fmlclient", "--fml.forgeVersion", "28.0.49", "--fml.mcVersion", "1.14.4", "--fml.forgeGroup", "net.minecraftforge", "--fml.mcpVersion", "20190719.225934"

Transformer voting and ordering

Currently the transformer voting is a bit pointless as it doesn't have any useful context on which to base the vote. The ITransformerVotingContext is empty, casting to VotingContext only gives the class name and sha1 which is not enough when working with modified classes.

Ideally the voting would need a full context (parsed node, transformers applied, transformers left, etc.). The parsed node should be read-only or it should be recreated for each transformer to avoid side effects.

It is clear that the general recommendation is to write core mods in JavaScript and only as absolutely last resort fall back to transformers. The examples below are general, not specific to transformers. The JS core mods could also benefit from adding vote support in a similar way.

Transformer ordering is important as there are some transformers that may want to:

• execute first (optifine)
• execute after another transformer (optifabric after optifine)
• delay or apply different patches depending on the other transformers (replaymod with or without optifine).
• execute last, in order to have the last word

In order for a transformer to specify its order preferences the vote results could be extended.

Vote results:

• NOW - must patch in this round (strong yes)
• YES - can patch in this round
• COULD - could patch in this round, but prefer to skip (weak yes)
• NO - can't patch in this round
• NEVER - can't patch in any round (skip all subsequent rounds)
• FAIL - catastrophic failure, abort transformation

Winners are NOW (highest priority), YES (normal priority) and COULD (lowest priority).

Normal transformers should only use YES/NO.
Complex transformers could additionally use NOW/COULD/NEVER to select the round at which to be applied relative to other transformers.

The decision to crash (FAIL) should be left to the transformer as it is the only one that knows if the patch is required or not. A round which ends with all transformers voting NO shouldn't cause any special action.

After the class transformation is complete, all transformers should be informed by calling "boolean transformFinished(Node, List applied, Set skipped)". The transformer can return false if its required patch hasn't been applied or if the Node state is broken in some way. This should abort the class transforming and crash similar to FAIL.

Currently the voting is per class and the transformer can't make general decisions until the transforming has started, which may be too late (some classes may have already been patched in a wrong way). For example a transformer may deactivate itself early or switch to a different set of patches if it sees that another specific transformer is active.

To solve this a new ITransformer method can be added boolean init(Map<TransformerID,<Set<ClassNames>>) so the transformer knows which other transformers are going to patch which classes. Returning false would deactivate the transformer and remove it from the voting.

At the beginning the list of transformers should be sorted in a stable way (most trivial is by transformer id). All subsequent operations (init, voting, application of equal winners) should be done in this order. This would immensly simplify bug reproduction and bug fixing as the same set of transformers would always behave in the same way.

Error when computing hierarchy for class with interface that should be stripped

This issue is somewhat complex to understand, so I'll try to explain it by example:
The enviroment is a dedicated server.
A transformer transforms the class CrossbowItem. This means modlauncher will re-compute the frames for this class. One of the frames requires the common superclass of FireworkRocketEntity and another class, so getSupers (https://github.com/cpw/modlauncher/blob/2f3da1afba50a180f02c9e700e582cdcb59ce42b/src/main/java/cpw/mods/modlauncher/TransformerClassWriter.java#L110) is called, which then calls computeHierarchy when FireworkRocketEntity has not been classloaded yet (https://github.com/cpw/modlauncher/blob/2f3da1afba50a180f02c9e700e582cdcb59ce42b/src/main/java/cpw/mods/modlauncher/TransformerClassWriter.java#L76) computeHierarchy reads the class metadata, and eventually reaches the visit (https://github.com/cpw/modlauncher/blob/2f3da1afba50a180f02c9e700e582cdcb59ce42b/src/main/java/cpw/mods/modlauncher/TransformerClassWriter.java#L134) Then it calls computeHierarchy for all interfaces that FireworkRocketEntity implements (https://github.com/cpw/modlauncher/blob/2f3da1afba50a180f02c9e700e582cdcb59ce42b/src/main/java/cpw/mods/modlauncher/TransformerClassWriter.java#L144). Modlauncher sees the interface IRendersAsItem, which is not present on the dedicated server, and crashes trying to lookup it (https://github.com/cpw/modlauncher/blob/2f3da1afba50a180f02c9e700e582cdcb59ce42b/src/main/java/cpw/mods/modlauncher/TransformerClassWriter.java#L94). Normally, the interface would not even be in the bytecode, but as there is a forge patch for FireworkRocketEntity, the binarypatcher also adds the interface (which is normally stripped by the obfuscator). This is not a problem in normal classloading however, as the RuntimeDistCleaner would remove the interface from the class before the ClassWriter or any other code would see it. But when loading a class for hiearchy lookup, no class transformers are run on the class, and so the interface will not be removed.

Possible solutions:
1: Define a new method on Launch Plugins that is being called when a Launch Plugin should perform class metadata transformation (such as changing super, adding/removing interfaces etc)
2: Just ignore when classes cannot be found (this would be more of a hack fix IMO)

Example log of the error:
https://gist.github.com/Shadows-of-Fire/64fb62022cdd6c17b675a71663a0ba4f

Not compatible with Java 16

Due to JEP 396 you cannot access internal apis now.

There are 3 problems preventing modlauncher from running in JDK 16:

1. grossjava9hacks tries to access ucp field via reflection
2. SecureJarHandler has direct reference to sun api
3. SecureJarHandler tries to access jv field via reflection

INameMappingService isn't exposed in API jar

It would be nice to pull out INameMappingService, and a way to look one up for a specific naming target (Environment.findNameMapping(target)) into the API jar. So that things like AccessTransformer don't have to have a hard dep on the non-api jar.

OpenJ9 detection fails for certain builds of the VM

Encountered in vectorwing/FarmersDelight#190, which seems to make the modded game crash because of a typetools bug.

https://github.com/cpw/modlauncher/blob/3cf110cd0b2a5bcd8a44057fd0513e38a4b06f97/src/main/java/cpw/mods/modlauncher/ValidateLibraries.java#L29

The builds used by these users, coming from AdoptOpenJDK seem to contain the string OpenJ9 in properties java.vm.name, java.vm.info and java.vm.vendor, but not the checked java.vendor (which is AdoptOpenJDK).

An answer to a question about detecting OpenJ9 on Adopt's github repo here seems to point at java.vm.name for this.

getResources broken

getResources is broken. See MinecraftForge/MinecraftForge#6029

Obtaining class bytecode

This issue follows on from #35 and getting rid of hacky things in the Internals class which works around the limitations using reflection for the time being.

Background

Mixin obtains ClassNodes for three purposes:

• Obtaining mixin classes ready for pre-processing and application
• Obtaining class metadata to conform and validate mixins prior to application, and populate class metadata (see note of ClassInfo here)
• Obtaining mixin inner class data to synthesize target-conformed inner classes for mixins as they are applied (eg. WorldMixin$1 being resynthesized to World$MixinInner$1)

In general the ClassNode needs to pass through the transformer chain. Under LaunchWrapper the approach was to create a delegation list of transformers which were not re-entrant and simply apply them manually.

The current hack

The current horrendous, reflection-based hack to achieve this is executed as follows:

• The ModLauncherBytecodeProvider is registered as an interceptor which is a delegate of the launch plugin.
• When a ClassNode is requested, the bytecode provider stuffs an entry into a Map for the name of the class. The provider then simulates the class load using a mixture of logic stolen from the delegating class loader, and reflection into the class transformer.
• Since launch plugins can process classes passing through the chain, the interceptor can detect and retain the ClassNode it's looking for.
• Depending on the state of the runTransformers flag, the interceptor throws an exception either BEFORE or AFTER transformation, including the captured ClassNode as an argument
• The interceptor catches the exception and receives and returns the ClassNode

This is obviously pretty horrible, but it's the only sensible way I could think of to faithfully replicate the class loading process to both run the transformers and retain the working ClassNode without replicating all the ML logic externally.

Possible Solutions

I'm throwing myself on the mercy of the court here, because I know this is some kind of crime against humanity. Some possible solutions might be:

• Provide this action as a service - currently the Mixin subsystem is engineered in such a way that obtaining bytecode is the sole responsibility of the IClassBytecodeProvider service interface. If this functionality could be internalised to ML then the hacky service can go away and simply delegate to the official one.

• Allow launch plugins to do more - obviously a drawback with a service is that other transformers might abuse this power. One solution might be to only expose this capability to launch plugins (either have them receive delegates which can execute specialised actions, or the ClassTransformer itself?

• Formalise the class metadata service into escrow - another solution might be to formalise what ClassInfo is doing into a pluggable ML service, which itself has the power to obtain the bytecode. This would be nice to have, but the drawback is the complexity of specifying such a service in a reasonable timescale.

I'm sure there are other ways but these are just some initial thoughts.

The loaded classes will be reloaded when switching context ClassLoader to TransformingClassLoader

javadoc missing for ITransformationService.runScan

see title

Use checker framework instead of jsr305

[NPE] Cannot invoke "joptsimple.OptionSet.valueOf(joptsimple.OptionSpec)" because "this.optionSet" is null

766836f introduced a crash that causes ModLauncher 9 to exit every time with a null pointer exception.

The stack trace on Forge 1.18.1:

Exception in thread "main" java.lang.NullPointerException: Cannot invoke "joptsimple.OptionSet.valueOf(joptsimple.OptionSpec)" because "this.optionSet" is null
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.ArgumentHandler.lambda$setArgs$0(ArgumentHandler.java:46)
	at java.base/java.util.concurrent.ConcurrentHashMap.computeIfAbsent(ConcurrentHashMap.java:1708)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.api.TypesafeMap.computeIfAbsent(TypesafeMap.java:52)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.api.TypesafeMap.computeIfAbsent(TypesafeMap.java:47)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.Environment.computePropertyIfAbsent(Environment.java:67)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.ArgumentHandler.setArgs(ArgumentHandler.java:46)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.Launcher.run(Launcher.java:85)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.Launcher.main(Launcher.java:77)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.BootstrapLaunchConsumer.accept(BootstrapLaunchConsumer.java:26)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.BootstrapLaunchConsumer.accept(BootstrapLaunchConsumer.java:23)
	at [email protected]/cpw.mods.bootstraplauncher.BootstrapLauncher.main(BootstrapLauncher.java:90)

This issue currently affects any Forge userdev environments on 1.17 and 1.18 as ModLauncher is dynamically versioned with 9.0.+ in MinecraftForge/EventBus, MinecraftForge/CoreMods, and FMLLoader.

For userdev, the current workaround involves adding the following snippet to the dependencies {} block in build.gradle:

afterEvaluate {
    minecraft('cpw.mods:modlauncher:9.0.7') {
        force = true
    }
}

Signed mod jars significantly slow down class loading

I recently noticed that the "mod construction" phase took quite long for Immersive Engineering while profiling startup for a modpack, even when compared to mods of a similar size. Most of that time is spent loading classes. With help from @sfPlayer1 I tracked this down to jar signing:
ModLauncher parses the manifest every time it loads a class from a jar file (here). We seem to be one of the few teams that still uses signed jars in 1.16, so IE has a much larger manifest than most mods (roughly 735 kB compared to 25 bytes without signing). Storing the loaded manifest would significantly improve load times for signed mods and at least slightly for unsigned ones.

Startup times (time between the message "ModLauncher running[...]" and the last message of the form "[minecraft/AtlasTexture]: Created: [...]"):
Without any mods: 42-43 s (2 launches)
With signed IE jar: 75-76 s or 32-34 additional seconds (3 launches)
With unsigned IE jar: 55-58 s or 12-16 additional seconds (2 launches)

Forge version: 32.0.107
ModLauncher version: 6.1.1+74+master.966c698

Allow ILaunchPluginServices to process class bytes if they prefer

Although most launch plugins will be using ASM to transform classes, launch plugins should still have the option of directly parsing class bytes, for several reasons:

• For efficiency, rather than working with a ClassNode, a launch plugin could create a custom class visitor which doesn't read the whole class, for example by returning a MethodWriter as their method visitor, in which case ASM will give the bytes directly to the MethodWriter (see lines 1123-1139 of ClassReader.java)
• A launch plugin may want to use something other than ASM to transform their class, just load a cached transformed class from a file, or replace the class with null (in the case of a SideOnly transformer)
• A launch plugin may want to just save the class to a file, for debugging, without actually transforming it

I suggest adding methods byte[] processClassBytes(ClassNode classNode, final Type classType) and boolean handlesClassBytes(Type classType, final boolean isEmpty) to the ILaunchPluginService interface.

This way, a ClassNode would only be created if there is a plugin that wants to process the ClassNode, and not just plugins that process the class bytes. Plugins handling class bytes should run either before or after all plugins handling class nodes.

Classes from the TransformationService JAR are not found by the classloader

A mod JAR with ITransformationService can be loaded from mods folder.
Transformers are correctly processed and the transformed classes are also loaded without problems.
However if the transformed classes reference other classes from the mod JAR then the class loading fails, because the JAR classes are not visible.

For example if net.minecraft.util.MathHelper is patched to reference net.optifine.util.MathUtils, then the class loading fails with:

java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
    at cpw.mods.modlauncher.LaunchServiceHandlerDecorator.launch(LaunchServiceHandlerDecorator.java:39)
    at cpw.mods.modlauncher.LaunchServiceHandler.launch(LaunchServiceHandler.java:50)
    at cpw.mods.modlauncher.LaunchServiceHandler.launch(LaunchServiceHandler.java:68)
    at cpw.mods.modlauncher.Launcher.run(Launcher.java:77)
    at cpw.mods.modlauncher.Launcher.main(Launcher.java:62)
Caused by: java.lang.reflect.InvocationTargetException
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at net.minecraftforge.fml.loading.FMLClientLaunchProvider.lambda$launchService$0(FMLClientLaunchProvider.java:51)
    at net.minecraftforge.fml.loading.FMLClientLaunchProvider$$Lambda$308/261748192.call(Unknown Source)
    at cpw.mods.modlauncher.LaunchServiceHandlerDecorator.launch(LaunchServiceHandlerDecorator.java:37)
    ... 4 more
Caused by: java.lang.NoClassDefFoundError: net/optifine/util/MathUtils
    at net.minecraft.util.math.MathHelper.<clinit>(MathHelper.java:21)
    at net.minecraft.util.Util.func_215078_k(SourceFile:87)
    at net.minecraft.util.Util.<clinit>(SourceFile:53)
    at net.minecraft.client.main.Main.main(SourceFile:56)
    ... 11 more
Caused by: java.lang.ClassNotFoundException: net.optifine.util.MathUtils
    at java.lang.ClassLoader.findClass(ClassLoader.java:530)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
    at cpw.mods.modlauncher.TransformingClassLoader.loadClass(TransformingClassLoader.java:102)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
    ... 15 more

The class net.optifine.util.MathUtils is included in the mod JAR using SRG naming, so it should be loadable.

Unable to transform class related to record.

Consider the following use case:

package com.example;
public record ExampleRecord(String name) {}

package com.example.mixin;

import com.example.ExampleRecord;
import net.minecraft.client.main.Main;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;

@Mixin(Main.class)
public class MixinMain {
    @Inject(method = "main", at = @At("HEAD"))
    private static void inject$main(String[] args, CallbackInfo ci) {
        try {
            ExampleRecord record = new ExampleRecord("");
        } catch (Throwable t) {}
    }
}

The above code will throw the following exception:

Exception in thread "main" java.lang.RuntimeException: java.lang.UnsupportedOperationException: Record requires ASM8
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.LaunchServiceHandlerDecorator.launch(LaunchServiceHandlerDecorator.java:39)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.LaunchServiceHandler.launch(LaunchServiceHandler.java:53)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.LaunchServiceHandler.launch(LaunchServiceHandler.java:71)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.Launcher.run(Launcher.java:106)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.Launcher.main(Launcher.java:77)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.BootstrapLaunchConsumer.accept(BootstrapLaunchConsumer.java:26)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.BootstrapLaunchConsumer.accept(BootstrapLaunchConsumer.java:23)
	at [email protected]/cpw.mods.bootstraplauncher.BootstrapLauncher.main(BootstrapLauncher.java:90)
Caused by: java.lang.UnsupportedOperationException: Record requires ASM8
	at [email protected]/org.objectweb.asm.ClassVisitor.visitRecordComponent(ClassVisitor.java:305)
	at [email protected]/org.objectweb.asm.ClassReader.readRecordComponent(ClassReader.java:930)
	at [email protected]/org.objectweb.asm.ClassReader.accept(ClassReader.java:708)
	at [email protected]/org.objectweb.asm.ClassReader.accept(ClassReader.java:401)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.TransformerClassWriter.computeHierarchyFromFile(TransformerClassWriter.java:151)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.TransformerClassWriter.computeHierarchy(TransformerClassWriter.java:112)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.TransformerClassWriter.getSupers(TransformerClassWriter.java:83)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.TransformerClassWriter.getCommonSuperClass(TransformerClassWriter.java:63)
	at [email protected]/org.objectweb.asm.SymbolTable.addMergedType(SymbolTable.java:1202)
	at [email protected]/org.objectweb.asm.Frame.merge(Frame.java:1299)
	at [email protected]/org.objectweb.asm.Frame.merge(Frame.java:1197)
	at [email protected]/org.objectweb.asm.MethodWriter.computeAllFrames(MethodWriter.java:1610)
	at [email protected]/org.objectweb.asm.MethodWriter.visitMaxs(MethodWriter.java:1546)
	at [email protected]/org.objectweb.asm.tree.MethodNode.accept(MethodNode.java:769)
	at [email protected]/org.objectweb.asm.tree.MethodNode.accept(MethodNode.java:649)
	at [email protected]/org.objectweb.asm.tree.ClassNode.accept(ClassNode.java:452)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.ClassTransformer.transform(ClassTransformer.java:133)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.TransformingClassLoader.maybeTransformClassBytes(TransformingClassLoader.java:50)
	at [email protected]/cpw.mods.cl.ModuleClassLoader.readerToClass(ModuleClassLoader.java:110)
	at [email protected]/cpw.mods.cl.ModuleClassLoader.lambda$findClass$16(ModuleClassLoader.java:213)
	at [email protected]/cpw.mods.cl.ModuleClassLoader.loadFromModule(ModuleClassLoader.java:223)
	at [email protected]/cpw.mods.cl.ModuleClassLoader.findClass(ModuleClassLoader.java:213)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:631)
	at java.base/java.lang.Class.forName(Class.java:543)
	at MC-BOOTSTRAP/[email protected]/net.minecraftforge.fml.loading.targets.ForgeClientUserdevLaunchHandler.lambda$launchService$0(ForgeClientUserdevLaunchHandler.java:38)
	at MC-BOOTSTRAP/[email protected]/cpw.mods.modlauncher.LaunchServiceHandlerDecorator.launch(LaunchServiceHandlerDecorator.java:37)
	... 7 more

This is caused by SuperCollectingVisitor using ASM7 which does not support record:
https://github.com/cpw/modlauncher/blob/8569cdfcbc41cf97d7fcf94e34789072af64c0a8/src/main/java/cpw/mods/modlauncher/TransformerClassWriter.java#L168-L170

TransformerClassWriter is not Thread safe

See https://github.com/cpw/modlauncher/blob/b1a340bec0f5ab3cb3338c50d4e8add5932ad414/src/main/java/cpw/mods/modlauncher/TransformerClassWriter.java#L37
These HashMaps are not synchronised, and as a result, updates to maps can be lost if it is just being rescaled.
This can lead to NPEs like https://pastebin.com/SJ7LQhh3 or https://pastebin.com/u9KphWtF which seem to be impossible due to a put before the get ensuring a value is always present, but when a HashMap gets a new value, it might have to resize the table array, which can lead to race conditions as two threads resize the map and think they got the right copy, but actually only one of these maps will become the new instance.

See HashMap contract:

<p><strong>Note that this implementation is not synchronized.</strong>
If multiple threads access a hash map concurrently, and at least one of
the threads modifies the map structurally, it <i>must</i> be
synchronized externally

This can be fixed pretty easily by replacing the 3 HashMaps with ConcurrentHashMaps.
Unfortunally, it's not too easy to make a unit test for this, as it requires precise timing and many classes to hit the issue consistent, but I hope my explination makes it clear.

License

Currently, modlauncher has no license - much like LaunchWrapper. This is something that will concern a number of people - including myself who may well be using this in the future as a LaunchWrapper replacement. This may also be a blocker for potential contributors.

As I am sure you are aware relicensing after the fact can prove to be a rather long, tedious procedure. My point is, with only one contributor (other than yourself), you are in a perfect position to be picking a license.

I would like to see modlauncher have a permissive license such as MIT or BSD 3-Clause - however, I'm sure you'll pick a suitable license.

`ClassCastException` due to transformers that implement nonparameterized interfaces

ModLauncher assumes that all generic interfaces of transformers are parameterized. If it gets to a nonparameterized interface before ITransformer (or if the implemented ITransformer is nonparameterized), then the cast will fail and a ClassCastException will be thrown.

Readme needs updating

Specifically the portion regarding licensing needs to be updated. In 866a8b3, the project became LGPL, but the readme wasn't updated to reflect that.

Server crash

Ran a latest version forge server
log: https://paste.dimdev.org/kuqeyagocu.md

Broken feature: ITransformationService::additionalClassesLocator

In the move from java 8 to java 9+ ITransformationService::additionalClassesLocator() was broken. I was previously using this to register additional jars for transforming. I don't know of a good workaround at present.

The Java 9+ code calls the method but discards the result, which is unhelpful.

The issue is here:

modlauncher/src/main/java/cpw/mods/modlauncher/TransformationServicesHandler.java

Line 59 in 386f465

final List<Function<String, Optional<URL>>> classLocatorList = serviceLookup.values().stream().map(TransformationServiceDecorator::getClassLoader).filter(Objects::nonNull).collect(Collectors.toList());

I asked about this on the forge discord and it was apparently a known issue but I can't see an issue for it

Getting class bytes of other classes in a transformer

Why do transformers depend on bytecode of other classes?

• Finding superclasses/interfaces
• • Stackmap generation (getCommonSuperClass()) - required when adding new control flow
• • SpongePowered/Mixin - ClassInfo
• Find existing Methods and fields
• • javassist - needs symbols in other classes to compile source code against them
• • SpongePowered/Mixin - ClassInfo

This list is probably incomplete.

Current situation - dangerous and slow

Existing mods which depend on previous class bytes do this in one of three ways:

• Load the class they need info on, causing that class to be fully transformed
• • "Reentrant transformer"
• • Causes classloading order to change
• • infinite recursion if two transformers do this on classes and both depend on the class bytes of each other's class
    can't be used in general on all classes due to infinite recursion risk.
    always acceptable if used only for superclass/interface loading as calling defineClass will cause these classes to get loaded anyway
• Get class bytes from disk, run through earlier transformers again
• • Slow
• • Runs transformers multiple times
• • Changes classloading order with reentrant transformer, may fail completely
• • Depends on reflection into LaunchClassLoader
• Get class bytes from disk, run only through some hardcoded non-reentrant transformers
• • Slow
• • Wrong info - if a transformer before you which isn't one of your hardcoded transformers changes super classes/adds fields you won't know about this
• • Depends on reflection into LaunchClassLoader

Current known reentrant/retransforming transformers

• net.minecraftforge.fml.common.asm.transformers.EventSubscriptionTransformer - reentrant
• net.minecraftforge.fml.common.asm.transformers.TerminalTransformer - reentrant
• org.minimallycorrect.modpatcher.api.ModPatcherTransformer$ClassTransformer - retransforms using hardcoded transformer list only
• org.spongepowered.asm.mixin.transformer.Proxy - retransforms using non-reentrant transformers only (hardcoded/additionally runtime detected)

This list is probably incomplete.

getCurrentClassBytes(name) - simple, less broken solution

The simplest solution is that when requesting the current class bytes by name inside a transformer, we run that class through all transformers until it reaches the current transformer, then store this result, then return it.

When it later comes to time to transform this class normally, the cached partial transformation will be reused, and we won't re-run the earlier transformers.

A problem occurs when you need to access a class which has already been fully transformed or has been partially transformed further than the current transformer.
Do you run it through all the earlier transformers again? Do you give it the already fully transformed result? Do we store all intermediate transformation results?

The first option causes poor performance, the second is wrong and could cause subtle bugs (different result depending on classloading order), the third causes awful memory usage.

Multi pass transformation before minecraft launch - big change from the current way, more correct?

An alternative would be to change transformation to run in passes and on all classes.
We would run all classes through transformer A. All classes through transformer B, and so on. The results would be stored in memory until the final class, then stored to disk as binpatches (or the final classes? Is there a copyright issue here? Don't think so as it's only on disk on the user's machine).
This process would happen only on the first launch, or when a new transformer is added (new mod with transformer), or a transformer signals that the cache should be invalidated. (Similar to #3)

Existing transformers which don't depend on classes existing already could continue to work. Transformers which are reentrant or retransforming would need updated.

InvocationTargetException when using ModLauncher with OpenLauncherLib

I'm currently working on a custom Minecraft Launcher for MC1.13.2 with Forge.
I got a lot of problems when doing it, but I approach the goal.

When i try to run ModLauncher, i'm getting this error :

Exception in thread "Client thread" [08:22:33.652] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
[08:22:33.652] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: 	at cpw.mods.modlauncher.LaunchServiceHandlerDecorator.launch(LaunchServiceHandlerDecorator.java:21)
[08:22:33.652] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: 	at cpw.mods.modlauncher.LaunchServiceHandler.launch(LaunchServiceHandler.java:32)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: 	at cpw.mods.modlauncher.LaunchServiceHandler.launch(LaunchServiceHandler.java:50)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: 	at cpw.mods.modlauncher.Launcher.run(Launcher.java:57)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: 	at cpw.mods.modlauncher.Launcher.main(Launcher.java:43)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: Caused by: java.lang.reflect.InvocationTargetException
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	at java.lang.reflect.Method.invoke(Method.java:498)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	at net.minecraftforge.fml.loading.FMLClientLaunchProvider.lambda$launchService$0(FMLClientLaunchProvider.java:51)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	at cpw.mods.modlauncher.LaunchServiceHandlerDecorator.launch(LaunchServiceHandlerDecorator.java:19)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	... 4 more
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: Caused by: java.lang.IncompatibleClassChangeError: vtable stub
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:174)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.state.EnumProperty.func_177708_a(SourceFile:75)
[08:22:33.653] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.state.EnumProperty.func_177709_a(SourceFile:71)
[08:22:33.654] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.state.properties.BlockStateProperties.<clinit>(SourceFile:38)
[08:22:33.654] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.fluid.FlowingFluid.<clinit>(SourceFile:34)
[08:22:33.654] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.fluid.Fluid.func_207195_i(Fluid.java:104)
[08:22:33.654] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.init.Bootstrap.func_151354_b(Bootstrap.java:404)
[08:22:33.654] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.client.Minecraft.<init>(Minecraft.java:334)
[08:22:33.654] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.client.main.Main.main(SourceFile:144)
[08:22:33.654] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	... 10 more

After some search i found an answer who say that he haven't the error with the JVM argument -Xin. But i'm still getting an almost identical error message :

Exception in thread "Client thread" [08:25:57.412] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
[08:25:57.412] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: 	at cpw.mods.modlauncher.LaunchServiceHandlerDecorator.launch(LaunchServiceHandlerDecorator.java:21)
[08:25:57.412] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: 	at cpw.mods.modlauncher.LaunchServiceHandler.launch(LaunchServiceHandler.java:32)
[08:25:57.413] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: 	at cpw.mods.modlauncher.LaunchServiceHandler.launch(LaunchServiceHandler.java:50)
[08:25:57.413] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: 	at cpw.mods.modlauncher.Launcher.run(Launcher.java:57)
[08:25:57.413] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: 	at cpw.mods.modlauncher.Launcher.main(Launcher.java:43)
[08:25:57.413] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: Caused by: java.lang.reflect.InvocationTargetException
[08:25:57.413] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[08:25:57.414] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[08:25:57.414] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[08:25:57.414] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	at java.lang.reflect.Method.invoke(Method.java:498)
[08:25:57.414] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	at net.minecraftforge.fml.loading.FMLClientLaunchProvider.lambda$launchService$0(FMLClientLaunchProvider.java:51)
[08:25:57.414] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	at cpw.mods.modlauncher.LaunchServiceHandlerDecorator.launch(LaunchServiceHandlerDecorator.java:19)
[08:25:57.414] [Client thread/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1061]: 	... 4 more
[08:25:57.415] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: Caused by: java.lang.IncompatibleClassChangeError
[08:25:57.415] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:174)
[08:25:57.415] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
[08:25:57.415] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
[08:25:57.415] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
[08:25:57.415] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
[08:25:57.416] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
[08:25:57.416] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
[08:25:57.416] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.state.EnumProperty.func_177708_a(SourceFile:75)
[08:25:57.416] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.state.EnumProperty.func_177709_a(SourceFile:71)
[08:25:57.416] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.state.properties.BlockStateProperties.<clinit>(SourceFile:38)
[08:25:57.416] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.fluid.FlowingFluid.<clinit>(SourceFile:34)
[08:25:57.417] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.fluid.Fluid.func_207195_i(Fluid.java:104)
[08:25:57.417] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.init.Bootstrap.func_151354_b(Bootstrap.java:404)
[08:25:57.417] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.client.Minecraft.<init>(Minecraft.java:334)
[08:25:57.417] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	at net.minecraft.client.main.Main.main(SourceFile:144)
[08:25:57.417] [Client thread/INFO] [STDERR/]: [java.lang.Throwable:printStackTrace:643]: 	... 10 more

Java Version : 1.8.0_212
MC Version : 1.13.2
Forge Version : 1.13.2-25.0.215

I'm using Livartan OpenLauncherLib

I really hope you have an idea on how to fix that and why is it happening !

Thanks in advance.

Need to reimplement test harness and testing paradigm if we stick with modular setup

See java9-modularity/gradle-modules-plugin#189

Classloader filtering of the javax package causes problems

Delegating all javax package loadClass calls to the parent classloader is not a good idea. This is because some libraries bundle specific javax packages that are only available in the Java EE runtime into the library jars so that they run on Java SE as well. Specifically, I was trying to use the Dagger 2 library with my mod which bundles the javax.inject package into the final jar. Since your classloader refuses to load javax classes, this will result in a ClassDefNotFound for Dagger 2 and any other dependency injection library.

d08c9b1

Dealing with different classloaders

Hey!
I want to call a mod init method from the Minecraft class but apparently it loads the mod in a different classloader:
MC: cpw.mods.modlauncher.TransformingClassLoader
INIT: sun.misc.Launcher$AppClassLoader

How am I supposed to deal with such a situation? Because in my mod's classloader context (or however you call it) I'm not able to access the Minecraft instance (it's null) so my mod becomes useless. I would really appreciate some advice on this.

Thanks in advance!

PR #23 is causing LinkageErrors

PR #23 is causing a LinkageError during forge loading.

Update ASM to 7.0

Update ASM to 7

Improve transformer performance by skipping unnecessary work

See closed PR #16 for more discussion. I am not a fan of using null in this fashion however. I will investigate different ways to do this.

Allow registering additional packages in the transformable layer for generated classes

A regression due to the introduction of module-based transformation has occurred where synthetic classes cannot be placed into a package that does not already exist in the transformable module layer.

The ModuleClassLoader will only pass class load requests to transformers for classes in packages it already knows about, so Mixin's strategy of requesting a class in a package it controls will not work unchanged.

Some options for solutions are:

• launch plugins need to be restricted to only adding classes to existing packages
• launch plugins and/or transformation services should be able to provide additional packages when the transformable module layer is built.

Java has added a hidden classes feature which allows adding new classes to a module at runtime, but that is often not suitable for bytecode transformations because hidden classes cannot be directly referenced, and is such unlikely to be a solution for this issue.

Initial discussion from #non-api-modding

``` Smelly² — 09/29/2021 I'm trying to port my mixins over to 1.17.1, and I'm having a really weird crash happening when the game is running The logs say there were no errors when processing one of my specific mixins, but later in game I'm getting this: java.lang.NoClassDefFoundError: org/spongepowered/asm/synthetic/args/Args$1 Does anybody know what would cause this? I also noticed earlier in the logs it says, [mixin/]: ArgsClassGenerator assigning org.spongepowered.asm.synthetic.args.Args$1 for descriptor (JLjava/util/List;Ljava/util/Optional;)V But I'm guessing that step didn't go as planned? nessie❦ — 09/29/2021 i suggest giving us more information like full log files and your mixin and potentially your buildscript Mumfrey — 09/30/2021 Args$1 is a mixin-generated synthetic subclass of Args which is created when you use the @ModifyArgs injector It seems like mixin never gets the request to generate the class though, as it seems like it's failing earlier in modlauncher I think we might need @cpw on this one it seems on the surface like SJH is deciding the class doesn't exist and then instead of ML passing it through the rest of the transformers/plugins with isEmpty set, it's just bombing out before that but that's just what I can glean from reading the stack trace either way, the culprit in your code will be modifyargs, but it's nothing you've done wrong Smelly² — 09/30/2021 Yeah, I suspected something like that to be the cause cpw — 09/30/2021 ? ZekerZhayard (ping) — 09/30/2021 We think there is currently no way to generate a non-existent class through LaunchPlugin or TransformationService 👀 cpw — 09/30/2021 is it a bug? that should work... LlamaLad7 — 09/30/2021 doesn't seem to when Mixin tries to make args subclasses we get ArgsClassGenerator assigning org.spongepowered.asm.synthetic.args.Args$1 for descriptor (Lcom/mojang/authlib/yggdrasil/YggdrasilAuthenticationService;Lnet/minecraft/client/main/GameConfig;)V

and then a CNFE
Mumfrey — 09/30/2021
and I confirmed this morning the request for that class never hits the service
what's weird is that inner classes which get repatriated, effectively generated, seem to work fine
cpw — 09/30/2021
is it going into the wrong classloader?
ZekerZhayard (ping) — 09/30/2021
https://github.com/MinecraftForge/securejarhandler/blob/main/src/main/java/cpw/mods/cl/ModuleClassLoader.java#L127-L134
TCL seems to only look for classes with existing package names, and throws CFNE directly in other cases
cpw — 09/30/2021
yeah. you won't be able to birth a class into a package that's not already associated with a module
module package lists are fixed at module creation time. there's no way around that.
Mumfrey — 09/30/2021
ok but I changed the package it generates the classed in to the same as the Args class itself, and it still bombs
shouldn't org.spongepowered.asm.mixin.injection.invoke.arg be associated with a module?
since a real class exists in it
cpw — 09/30/2021
it should be.
Mumfrey — 09/30/2021
if I set the package it generates the classes in to net.minecraft.client it works, if I use any existing mixin package it bombs
if I use an arbitrary package it bombs, eg. foo.bar
I did wonder if it's because my Mixin is dev-time and is being loaded from a flat dir rather than jar, but I tried using org.objectweb.asm and it bombs too
ZekerZhayard (ping) — 09/30/2021
I think only the package located in TCL is available, both asm and mixin are located in MCL
cpw — 09/30/2021
it's probably because mixin is loaded at the non-transformable level
you have to target a package in a module in the GAME layer
Mumfrey — 09/30/2021
tbh I have no idea how the module hackery works
cpw — 09/30/2021
yeah. its complicated for sure.
Mumfrey — 09/30/2021
all I can tell you is mixin synthesizes two types of class: repatriated inner classes, and args classes
cpw — 09/30/2021
i basically partitioned it into four layers. The top layer is where mixin and shit loads.
Mumfrey — 09/30/2021
the args classes are the ones that break because the transformer never gets the chance to generate them
cpw — 09/30/2021
the bottom layer (GAME) is where transforming happens
so yeah, if your targetting a package NOT in GAME layer, you won't have much success
Mumfrey — 09/30/2021
so how can that package be added to the GAME layer?
cpw — 09/30/2021
good question. that might be a thing we need to add to the API
to allow you to generate synthetic packages
probably, it'll be a new target type for the thingy thing.
Mumfrey — 09/30/2021
well the args subclasses are generated in a package of their own so that they don't conflict with anything, so just that one package needs to be permitted
or find some way to enable the old behaviour
cpw — 09/30/2021
old behaviour won't be possible sadly
not without a giant ctrl-z
and then resolving all the bullshit j16 added differently
anyway, i'll try and add it when i finally get out of this giant burnout
Mumfrey — 09/30/2021
so there's no way for it to fail over for nonexistent classes?

<details>

Writing custom audit trail entries

I've been looking at having Mixin write more information to the audit trail to document its activities and have hit a small snag. While the audit trail is easy to retrieve, it's only possible to write the 3 existing record types via addReason, addPluginAuditTrail and addTransformerAuditTrail.

Whilst transformer actions can be traced reasonably, it would be useful if launch plugins could be supplied with a delegate to add their own (scoped) audit trail entries which could be recorded under (for example) PLUGIN_ACTIVITY as Type or simply recorded under the existing PLUGIN type with plugin-specific detail (D?) included as the context.

For example if I apply 3 mixins to a class, being able to do something like

pluginAuditTrail.addActivity(className, "apply", "mixin.name");
// or maybe 
pluginAuditTrail.addActivity(className, "+", "mixin.name"); // shorter is better?

(edited because I forgot class name argument)

ideally we'd end up with something like this in the audit trail:

net.minecraft.client.gui.screen.MainMenuScreen {
   PLUGIN:mixin:A
   PLUGIN:mixin:D:+:mixins.foo.bar.ScreenMixin1
   PLUGIN:mixin:D:+:com.blah.ScreenMixin
}

I don't particularly want to go crazy with detail, but just get some of the more significant events recorded in the audit trail in a consistent manner.

FileNotFoundException on first launch

See 9438249#r33902825
Searching for additional locators on first launch fails since this is so early that the mods dir is not yet there because proper Forge has not done any initialisation.
A fix would be to either ignore the folder if missing, or to create it. IDK which is better so I haven't PR'd this yet.

Incorrect logic for additional resource name check

In document of method additionalResourcesLocator it says:

Rules:
The Strings in the set must not end with ".class". Conflicts with other ITransformationServices will result in an immediate crash.

But inside TransformationServiceDecorator, it checks the name with this logic:

final Set<String> badResourceNames = resNames.stream().
                    filter(s -> !s.endsWith(".class") || resourceNames.contains(s)).
                    collect(Collectors.toSet());

It seems inverted to me. Am I getting something wrong?

Whole-class transformers run after (and overwrite) method & field transformers

This is more an issue with OptiFine but I don't see how OptiFine can fix it without changes to ModLauncher.

Currently class transformers run after method transformers, so OptiFine's class transformations overwrite my method transformations. Without transformer ordering (#32) there is no way to make sure that my transformer runs before OptiFine's.

I think that class transformers should run before field & method transformers as they are more likely to be doing large scale transformations. Field & method transformers are more likely to be used for smaller & more directed injections.

This is directly related to NoCubes/issues/61

Incompatibility between modlauncher 8.1.x (mc 1.16) and Java 8 u321+

The latest update to Java 8 applied a change to the sun.security.util.ManifestEntryVerifier class constructor, adding a parameter to it.

This breaks binary compatibility with modlauncher 8.1.x code:

modlauncher/src/main/java/cpw/mods/modlauncher/SecureJarHandler.java

Line 66 in 3cf110c

ManifestEntryVerifier mev = new ManifestEntryVerifier(manifest);

This change appears to also have been made to the other LTS branches (jdk11 and jdk17) as of last october:

• JDK 8: openjdk/jdk8u@9edb3c7
• JDK 11: openjdk/jdk11u@9512c47
• JDK 17: openjdk/jdk17u@f89c59f

JDK 16 is not affected, and being EOL, will never be.

Stack trace (trimmed):

Exception in thread "main" [15:56:10] [main/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:-1]: java.lang.NoSuchMethodError: sun.security.util.ManifestEntryVerifier.<init>(Ljava/util/jar/Manifest;)V
[15:56:10] [main/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:-1]:         at cpw.mods.modlauncher.SecureJarHandler.createCodeSource(SecureJarHandler.java:66)
[15:56:10] [main/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:-1]:         at cpw.mods.modlauncher.TransformingClassLoader$DelegatedClassLoader.findClass(TransformingClassLoader.java:275)
[15:56:10] [main/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:-1]:         at cpw.mods.modlauncher.TransformingClassLoader.loadClass(TransformingClassLoader.java:136)
[15:56:10] [main/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:-1]:         at cpw.mods.modlauncher.TransformingClassLoader.loadClass(TransformingClassLoader.java:98)
[15:56:10] [main/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:-1]:         at java.lang.ClassLoader.loadClass(Unknown Source)

Rare crash when defining packages

java.lang.IllegalArgumentException: net.minecraft.client.network
	at java.lang.ClassLoader.definePackage(ClassLoader.java:1594)
	at cpw.mods.modlauncher.TransformingClassLoader$DelegatedClassLoader.definePackage(TransformingClassLoader.java:270)
	at cpw.mods.modlauncher.TransformingClassLoader$DelegatedClassLoader.findClass(TransformingClassLoader.java:221)
	at cpw.mods.modlauncher.TransformingClassLoader.loadClass(TransformingClassLoader.java:76)
	at cpw.mods.modlauncher.TransformingClassLoader$DelegatedClassLoader.loadClass(TransformingClassLoader.java:168)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at net.minecraft.network.NetworkSystem$2.initChannel(SourceFile:128)
	at io.netty.channel.ChannelInitializer.initChannel(ChannelInitializer.java:113)
	at io.netty.channel.ChannelInitializer.handlerAdded(ChannelInitializer.java:105)
	at io.netty.channel.DefaultChannelPipeline.callHandlerAdded0(DefaultChannelPipeline.java:637)
	at io.netty.channel.DefaultChannelPipeline.access$000(DefaultChannelPipeline.java:46)
	at io.netty.channel.DefaultChannelPipeline$PendingHandlerAddedTask.execute(DefaultChannelPipeline.java:1487)
	at io.netty.channel.DefaultChannelPipeline.callHandlerAddedForAllHandlers(DefaultChannelPipeline.java:1161)
	at io.netty.channel.DefaultChannelPipeline.invokeHandlerAddedIfNeeded(DefaultChannelPipeline.java:686)
	at io.netty.channel.AbstractChannel$AbstractUnsafe.register0(AbstractChannel.java:510)
	at io.netty.channel.AbstractChannel$AbstractUnsafe.access$200(AbstractChannel.java:423)
	at io.netty.channel.AbstractChannel$AbstractUnsafe$1.run(AbstractChannel.java:482)
	at io.netty.util.concurrent.AbstractEventExecutor.safeExecute$$$capture(AbstractEventExecutor.java:163)
	at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java)
	at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:404)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:465)
	at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:884)
	at java.lang.Thread.run(Thread.java:748)

Only happened once for me so far, but seems like an issue with modlauncher.

ModLauncher prints accessToken arguments to log

ModLauncher currently simple logs all arguments before starting (see https://github.com/cpw/modlauncher/blob/2ac5413d0fa889aae60b4ed6af32c619fca79e68/src/main/java/cpw/mods/modlauncher/LaunchServiceHandler.java#L34), which also includes the access token.
There is already a forge patch to prevent this for minecraft, so modlauncher should also prevent it.

This needs to get official before 1.13 releases

Hi there,
as 1.13 is very close now, we have to make this the official replacement for legacylauncher, or else other minecraft basemods like optifine will most likely continue using launchwrapper, which breaks compat with other basemods. So we should get Mojang to adopt this and write Transformation Services for their "old stuff" (so basically rewrite these for this system). Upside for Mojang: Compat with J9+ for their old versions. Upside for us: An established new class transformation framework for minecraft 1.13+

Invalid audit trails added when logging excetions

When logging an exception that contains classes loaded from child/another classloaders, invalid re:classloading trails added to log.

Example code:

        Exception exception = new Exception();
        StackTraceElement[] stackTrace = new StackTraceElement[1];
        stackTrace[0] = new StackTraceElement("test.aa", "test.b", "cc", 111);
        exception.setStackTrace(stackTrace);
        for (int i = 0; i < 3; i++) {
            getLogger().log(Level.SEVERE, "test" + i, exception);
        }

gets a log like that:

[13:22:20.377] [Server thread/ERROR] [Test/]: test0 java.lang.Exception: null
	at test.aa.test.b(cc:111) ~[?:?] {re:classloading}

[13:22:20.385] [Server thread/ERROR] [Test/]: test1 java.lang.Exception: null
	at test.aa.test.b(cc:111) ~[?:?] {re:classloading,re:classloading}

[13:22:20.386] [Server thread/ERROR] [Test/]: test2 java.lang.Exception: null
	at test.aa.test.b(cc:111) ~[?:?] {re:classloading,re:classloading,re:classloading}

Edit
It turns out that when logging an exception, log4j wants to load classes that appears in stacktrace. TransformingCL loads it multiple times and thrown multiple times ClassNotFoundException, and add multiple times audit reasons.

Before throwing a ClassNotFoundException, maybe it should remove the reason of this class.
https://github.com/cpw/modlauncher/blob/b1a340bec0f5ab3cb3338c50d4e8add5932ad414/src/main/java/cpw/mods/modlauncher/TransformingClassLoader.java#L252-L256

Finish name transformation service

See MinecraftForge/MinecraftForge#5620. Accesstransformer needs to know about alternative naming schemes.

how 1.13 forge mcpconfig mcp

Vanilla command line arguments inaccesible

As far as I can tell, ITransformationServices have no way to access any vanilla command line arguments, such as the screen size. It can only access arguments for that specific service.

Discussion: Should Minecraft's own dependencies be added to the transform path by modlauncher

Ok, this has been changed to a discussion as I phrased it as if it were a bug (having seen people implying this was an already intended feature, and me assuming that minecraft's libraries would be registered along with it's own jar)

The discussion at hand is two fold:

• Are minecrafts libraries considered to be in the same class of special jars as the core code and as such should be added to the transformer path
• If not, this should be moved to FML to discuss as to whether FML should/shouldn't add them itself (as they aren't core code so should be handeld on a loader by loader basis)

Specifically, this asks should they be added to classBytesFinder here

Expose package filters to ILaunchPluginService

Currently only the ILaunchHandlerService is able to interact with ITransformingClassLoader#addTargetPackageFilter(Predicate), an omission I noticed while passing mod metadata from a IModLocator through to a ModContainer (using metadata other than nightconfig, which is excluded by Forge).

There seems to be other scenarios where this is required too, such as MixinBootstrap - where they hack around the API limitation by using the Thread context class loader.

public void initializeLaunch(ITransformerLoader transformerLoader, Path[] specialPaths) {
    TransformingClassLoader classLoader = (TransformingClassLoader) Thread.currentThread().getContextClassLoader();
    classLoader.addTargetPackageFilter(name -> SKIP_PACKAGES.stream().noneMatch(name::startsWith));
}