BepInEx / HarmonyX

Harmony built on top of MonoMod.RuntimeDetours with additional features
MIT License
356 stars 44 forks source link

Double execution of `TargetMethods` #76

Closed Neutron3529 closed 11 months ago

Neutron3529 commented 1 year ago

This should be a bug confirmed by this discord thread.

It is said that this bug will be fixed in the upcoming harmony 2.3 lib.

using System;
using HarmonyLib;
using System.Reflection;
namespace Test {
    internal class Program {
        static void Main(string[] args) {
            var harmony=new Harmony("fine");
            harmony.PatchAll();
        }
    }
    [HarmonyPatch]
    class Test {
        static IEnumerable<MethodBase> TargetMethods() {
            Console.WriteLine("Hello World!");
            yield break;
        }
    }
}

this simple program would generate 2 hello worlds, add another harmony.PatchAll(); would generate 4 hello worlds in total.

This program could print stacktrace, thus might be helpful:

using System;
using HarmonyLib;
using System.Reflection;
namespace Test {
    internal class Program {
        static void Main(string[] args) {
            var harmony=new Harmony("fine");
            for(int i=0;i<2;i++){
                Test.state=i;
                Console.WriteLine("Patching.. Set state to {Test.state}");
                try{
                    harmony.PatchAll();
                } catch(Exception ex){
                    Test.logexception(ex);
                }
            }
        }
    }
    [HarmonyPatch]
    class Test {
        public static int state=-1;
        public static void logexception(Exception e){
            Console.WriteLine(_strexception(e,0));
        }
        static string _strexception(Exception e, int depth){
           var padding=depth==0?"\n":"\n"+new string(' ',depth*4);
           return ("\n"+e.GetType().Name+": "+e.Message+"\n"+e.StackTrace).Replace("\n",padding)+(e.InnerException==null?"":_strexception(e.InnerException,depth+1));
        }
        static IEnumerable<MethodBase> TargetMethods() {
            Console.WriteLine($"{state}");
            if(state!=1){
                throw new Exception("Hello World!");
            } else {
                state++;
            }
            yield break;
        }
    }
}

It is said that, it was calling Any(c => c == null) as a pre-check on the enumeration and later a .ToList().

Related fix is here

P.S. Sorry for that I cannot fork this repo thus I could not create any PR for this BUG.