search

pardeike / Harmony

A library for patching, replacing and decorating .NET and Mono methods during runtime
https://www.patreon.com/pardeike
MIT License
5.15k stars 485 forks source link

UnpatchAll fails when there was both a prefix and postfix with a __state parameter #175

Closed boformer closed 5 years ago

boformer commented 5 years ago

Describe the bug

When I apply a patch that consists of a prefix and a postfix, where a __state parameter is used to share data between the prefix and postfix.

Further Investigation showed that this also happens when I unpatch the Methods manually, first the prefix and then the postfix (That's exactly what UnpatchAll does).

Unpatching works correctly if I unpatch the prefix first, then the postfix, e.g.:

harmony.Unpatch(typeof(TrainTrackBridgeAI).GetMethod("GetNodeBuilding"), HarmonyPatchType.Postfix, HarmonyId);
harmony.Unpatch(typeof(TrainTrackBridgeAI).GetMethod("GetNodeBuilding"), HarmonyPatchType.Prefix, HarmonyId);

So I propose that UnpatchAll should also be changed to do the unpatching in this order!

To Reproduce

The patch class that triggers the error in my mod: https://gist.github.com/boformer/8788e1b0fcfea4c17583adccbc156255

Stack trace:

KeyNotFoundException: The given key was not present in the dictionary.
  at System.Collections.Generic.Dictionary`2[System.String,System.Reflection.Emit.LocalBuilder].get_Item (System.String key) [0x00000] in <filename unknown>:0 
  at Harmony.MethodPatcher.EmitCallParameter (System.Reflection.Emit.ILGenerator il, System.Reflection.MethodBase original, System.Reflection.MethodInfo patch, System.Collections.Generic.Dictionary`2 variables, Boolean allowFirsParamPassthrough) [0x00000] in <filename unknown>:0 
  at Harmony.MethodPatcher+<>c__DisplayClass20_0.<AddPostfixes>b__1 (System.Reflection.MethodInfo fix) [0x00000] in <filename unknown>:0 
  at Harmony.CollectionExtensions.Do[MethodInfo] (IEnumerable`1 sequence, System.Action`1 action) [0x00000] in <filename unknown>:0 
  at Harmony.MethodPatcher.AddPostfixes (System.Reflection.Emit.ILGenerator il, System.Reflection.MethodBase original, System.Collections.Generic.List`1 postfixes, System.Collections.Generic.Dictionary`2 variables, Boolean passthroughPatches) [0x00000] in <filename unknown>:0 
  at Harmony.MethodPatcher.CreatePatchedMethod (System.Reflection.MethodBase original, System.String harmonyInstanceID, System.Collections.Generic.List`1 prefixes, System.Collections.Generic.List`1 postfixes, System.Collections.Generic.List`1 transpilers) [0x00000] in <filename unknown>:0 
Rethrow as Exception: Exception from HarmonyInstance "boformer.NetworkSkins" patching TrainTrackBridgeAI.GetNodeBuilding(System.UInt16, NetNode&, BuildingInfo&, System.Single&)
  at Harmony.MethodPatcher.CreatePatchedMethod (System.Reflection.MethodBase original, System.String harmonyInstanceID, System.Collections.Generic.List`1 prefixes, System.Collections.Generic.List`1 postfixes, System.Collections.Generic.List`1 transpilers) [0x00000] in <filename unknown>:0 
  at Harmony.PatchFunctions.UpdateWrapper (System.Reflection.MethodBase original, Harmony.PatchInfo patchInfo, System.String instanceID) [0x00000] in <filename unknown>:0 
  at Harmony.PatchProcessor.Unpatch (System.Reflection.MethodInfo patch) [0x00000] in <filename unknown>:0 
  at Harmony.HarmonyInstance.Unpatch (System.Reflection.MethodBase original, System.Reflection.MethodInfo patch) [0x00000] in <filename unknown>:0 
  at Harmony.HarmonyInstance+<>c__DisplayClass12_1.<UnpatchAll>b__1 (Harmony.Patch patchInfo) [0x00000] in <filename unknown>:0 
  at Harmony.CollectionExtensions.Do[Patch] (IEnumerable`1 sequence, System.Action`1 action) [0x00000] in <filename unknown>:0 
  at Harmony.CollectionExtensions.DoIf[Patch] (IEnumerable`1 sequence, System.Func`2 condition, System.Action`1 action) [0x00000] in <filename unknown>:0 
  at Harmony.HarmonyInstance.UnpatchAll (System.String harmonyID) [0x00000] in <filename unknown>:0

Runtime environment (please complete the following information):

pardeike commented 5 years ago

This seems to be fixed with this line in 2.0: https://github.com/pardeike/Harmony/blob/c92d4468b45276499e4df8dadfbc71e73ac130ae/Harmony/Internal/MethodPatcher.cs#L338

DaEgi01 commented 5 years ago

any chance to get a 1.2.0.2 with the bugfix before the 2.0 release? :D

pardeike commented 5 years ago

Probably not

boformer commented 5 years ago

@pardeike Still having issues with this in latest master, getting the following exception on UnpatchAll:

FormatException: Method TrainTrackBridgeAI.GetNodeBuilding(System.UInt16, NetNode&, BuildingInfo&, System.Single&) cannot be patched. Reason: Invalid IL code in (wrapper dynamic-method) TrainTrackBridgeAI:GetNodeBuilding_Patch1 (TrainTrackBridgeAI,uint16,NetNode&,BuildingInfo&,single&): IL_00ae: call      0x00000023

  at Harmony.PatchFunctions.UpdateWrapper (System.Reflection.MethodBase original, Harmony.PatchInfo patchInfo, System.String instanceID) [0x00000] in <filename unknown>:0 
  at Harmony.PatchProcessor.Unpatch (System.Reflection.MethodInfo patch) [0x00000] in <filename unknown>:0 
  at Harmony.HarmonyInstance.Unpatch (System.Reflection.MethodBase original, System.Reflection.MethodInfo patch) [0x00000] in <filename unknown>:0 
  at Harmony.HarmonyInstance+<>c__DisplayClass13_1.<UnpatchAll>b__1 (Harmony.Patch patchInfo) [0x00000] in <filename unknown>:0 
  at Harmony.CollectionExtensions.Do[Patch] (IEnumerable`1 sequence, System.Action`1 action) [0x00000] in <filename unknown>:0 
  at Harmony.CollectionExtensions.DoIf[Patch] (IEnumerable`1 sequence, System.Func`2 condition, System.Action`1 action) [0x00000] in <filename unknown>:0 
  at Harmony.HarmonyInstance.UnpatchAll (System.String harmonyID) [0x00000] in <filename unknown>:0

when I change UnpatchAll in Harmony to this (first revert postfixes, then prefixes), it works:

public void UnpatchAll(string harmonyID = null)
{
    bool IDCheck(Patch patchInfo) => harmonyID == null || patchInfo.owner == harmonyID;

    var originals = GetPatchedMethods().ToList();
    foreach (var original in originals)
    {
        var info = GetPatchInfo(original);
        info.Postfixes.DoIf(IDCheck, patchInfo => Unpatch(original, patchInfo.patch));
        info.Prefixes.DoIf(IDCheck, patchInfo => Unpatch(original, patchInfo.patch));
        info.Transpilers.DoIf(IDCheck, patchInfo => Unpatch(original, patchInfo.patch));
    }
}

So maybe reopen until the issue is solved...

pardeike commented 5 years ago

Can you tell me the signature of your Postfix?

boformer commented 5 years ago

https://github.com/boformer/NetworkSkins2/blob/d7c63cb9b69c6db9c2f73d0388a2d2abf26a734f/NetworkSkins/Patches/TrainTrackBridgeAI/TrainTrackBridgeAiGetNodeBuildingPatch.cs#L26-L29

pardeike commented 5 years ago

Have you tried to make TrainTrackBridgePillarPatcherState a class and remove the '?' from ref TrainTrackBridgePillarPatcherState? __state ?

pardeike commented 5 years ago

Nevermind, I've hopefully fixed the problem with https://github.com/pardeike/Harmony/commit/10ca9fa787c4be7e516d3f2dc60598708625fbf2

boformer commented 5 years ago

Can confirm that it solved this particular problem!