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

Clear ReflectionHelper cache #4220

Merged
merged 2 commits into from
May 24, 2024
Merged

Conversation

Solaestas
Copy link
Contributor

What is the bug?

When a mod is not fully unloaded (quite common), MonoMod.Utils.ReflectionHelper retains the previously loaded mod assembly. After force reloading the mod, MonoMod incorrectly uses the cached data to generate dynamic methods. This can result in generic methods like GetInstance<T> or GetGlobalPlayer<T> returning null where T is a type from mod assembly

These often occur when you want to patch another mod or avoiding EmitDelegate for performance

Example:

public class TestSystem : ModSystem
{
	public record JsonData(string Name);

	public override void Load()
	{
		IL_Main.Update += IL_Main_Update;
		JsonConvert.SerializeObject(new JsonData("Prevent fully unloaded"));
	}

	private void IL_Main_Update(ILContext il)
	{
		var cursor = new ILCursor(il);
		var method = ModContent.GetInstance<TestSystem>;
		cursor.Emit(OpCodes.Call, method.Method);
		cursor.EmitDelegate((TestSystem system) =>
		{
			Mod.Logger.Info($"Emit Call return is null: {system is null}");
			system = ModContent.GetInstance<TestSystem>();
			Mod.Logger.Info($"Emit Delegate return is null: {system is null}");
		});
	}
}

Expected output

  • Before Reload: (false, false)
  • After Reload: (true, false)

How did you fix the bug?

Clear the cache dictionary

Are there alternatives to your fix?

Replace emitting calling a generic method info with EmitDelegate

Remove generic method calling while patching another mod, which is troublesome

@JavidPack
Copy link
Collaborator

Works as described.

@Chicken-Bones
Copy link
Member

While we do need to clear the caches for unloading anyway, clearing them is not a complete solution to the issue presented. I investigated it thoroughly and have added failing test cases to the MonoMod repo to investigate a more complete fix.

In summary, the issue can happen any time a generic method is reused with two different type parameters which have the same full name.

I'm happy to accept this as a temporary mitigation, and to help with cleanup.

@Solaestas Solaestas reopened this May 23, 2024
@Chicken-Bones Chicken-Bones merged commit 3e0686e into tModLoader:1.4.4 May 24, 2024
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.

None yet

3 participants