fix: Attempt to better handle hook disposal (#1803)

- Use a Weak Concurrent Collection to track scoped hooks
- Make `Hook`s remove themselves from the Tracked Hook list.

Dalamud/Hooking/AsmHook.cs

@@ -20,6 +20,8 @@ public sealed class AsmHook : IDisposable, IDalamudHook
     private bool isEnabled = false;
 
     private DynamicMethod statsMethod;
+    
+    private Guid hookId = Guid.NewGuid();
 
     /// <summary>
     /// Initializes a new instance of the <see cref="AsmHook"/> class.
@@ -44,7 +46,7 @@ public sealed class AsmHook : IDisposable, IDalamudHook
         this.statsMethod.GetILGenerator().Emit(OpCodes.Ret);
         var dele = this.statsMethod.CreateDelegate(typeof(Action));
 
-        HookManager.TrackedHooks.TryAdd(Guid.NewGuid(), new HookInfo(this, dele, Assembly.GetCallingAssembly()));
+        HookManager.TrackedHooks.TryAdd(this.hookId, new HookInfo(this, dele, Assembly.GetCallingAssembly()));
     }
 
     /// <summary>
@@ -70,7 +72,7 @@ public sealed class AsmHook : IDisposable, IDalamudHook
         this.statsMethod.GetILGenerator().Emit(OpCodes.Ret);
         var dele = this.statsMethod.CreateDelegate(typeof(Action));
 
-        HookManager.TrackedHooks.TryAdd(Guid.NewGuid(), new HookInfo(this, dele, Assembly.GetCallingAssembly()));
+        HookManager.TrackedHooks.TryAdd(this.hookId, new HookInfo(this, dele, Assembly.GetCallingAssembly()));
     }
 
     /// <summary>
@@ -116,6 +118,8 @@ public sealed class AsmHook : IDisposable, IDalamudHook
 
         this.IsDisposed = true;
 
+        HookManager.TrackedHooks.TryRemove(this.hookId, out _);
+
         if (this.isEnabled)
         {
             this.isEnabled = false;

Dalamud/Hooking/Hook.cs

@@ -71,6 +71,11 @@ public abstract class Hook<T> : IDalamudHook where T : Delegate
     /// <inheritdoc/>
     public virtual string BackendName => throw new NotImplementedException();
     
+    /// <summary>
+    /// Gets the unique GUID for this hook.
+    /// </summary>
+    protected Guid HookId { get; } = Guid.NewGuid();
+    
     /// <summary>
     /// Remove a hook from the current process.
     /// </summary>

Dalamud/Hooking/Internal/FunctionPointerVariableHook.cs

@@ -100,7 +100,7 @@ internal class FunctionPointerVariableHook<T> : Hook<T>
 
             unhooker.TrimAfterHook();
 
-            HookManager.TrackedHooks.TryAdd(Guid.NewGuid(), new HookInfo(this, detour, callingAssembly));
+            HookManager.TrackedHooks.TryAdd(this.HookId, new HookInfo(this, detour, callingAssembly));
         }
     }
 
@@ -137,6 +137,8 @@ internal class FunctionPointerVariableHook<T> : Hook<T>
 
         this.Disable();
 
+        HookManager.TrackedHooks.TryRemove(this.HookId, out _);
+
         var index = HookManager.MultiHookTracker[this.Address].IndexOf(this);
         HookManager.MultiHookTracker[this.Address][index] = null;
 

Dalamud/Hooking/Internal/GameInteropProviderPluginScoped.cs

@@ -1,5 +1,4 @@
-using System.Collections.Concurrent;
-using System.Diagnostics;
+using System.Diagnostics;
 using System.Linq;
 
 using Dalamud.Game;
@@ -7,6 +6,7 @@ using Dalamud.IoC;
 using Dalamud.IoC.Internal;
 using Dalamud.Plugin.Internal.Types;
 using Dalamud.Plugin.Services;
+using Dalamud.Utility;
 using Dalamud.Utility.Signatures;
 using Serilog;
 
@@ -25,7 +25,7 @@ internal class GameInteropProviderPluginScoped : IGameInteropProvider, IInternal
     private readonly LocalPlugin plugin;
     private readonly SigScanner scanner;
 
-    private readonly ConcurrentBag<IDalamudHook> trackedHooks = new();
+    private readonly WeakConcurrentCollection<IDalamudHook> trackedHooks = new();
 
     /// <summary>
     /// Initializes a new instance of the <see cref="GameInteropProviderPluginScoped"/> class.

Dalamud/Hooking/Internal/MinHookHook.cs

@@ -35,7 +35,7 @@ internal class MinHookHook<T> : Hook<T> where T : Delegate
 
             unhooker.TrimAfterHook();
 
-            HookManager.TrackedHooks.TryAdd(Guid.NewGuid(), new HookInfo(this, detour, callingAssembly));
+            HookManager.TrackedHooks.TryAdd(this.HookId, new HookInfo(this, detour, callingAssembly));
         }
     }
 
@@ -76,6 +76,8 @@ internal class MinHookHook<T> : Hook<T> where T : Delegate
             HookManager.MultiHookTracker[this.Address][index] = null;
         }
 
+        HookManager.TrackedHooks.TryRemove(this.HookId, out _);
+
         base.Dispose();
     }
 

Dalamud/Hooking/Internal/ReloadedHook.cs

@@ -30,7 +30,7 @@ internal class ReloadedHook<T> : Hook<T> where T : Delegate
 
             unhooker.TrimAfterHook();
 
-            HookManager.TrackedHooks.TryAdd(Guid.NewGuid(), new HookInfo(this, detour, callingAssembly));
+            HookManager.TrackedHooks.TryAdd(this.HookId, new HookInfo(this, detour, callingAssembly));
         }
     }
 
@@ -63,6 +63,8 @@ internal class ReloadedHook<T> : Hook<T> where T : Delegate
         if (this.IsDisposed)
             return;
 
+        HookManager.TrackedHooks.TryRemove(this.HookId, out _);
+
         this.Disable();
 
         base.Dispose();