From 0e95424704c598ae1a0763dd5781b01aa85df9f2 Mon Sep 17 00:00:00 2001 From: goaaats <16760685+goaaats@users.noreply.github.com> Date: Wed, 12 Jan 2022 18:33:40 +0100 Subject: [PATCH] fix: make HookManager.TrackedHooks thread-safe, might fix invalid CLR state --- Dalamud/Hooking/AsmHook.cs | 4 ++-- Dalamud/Hooking/Hook.cs | 2 +- Dalamud/Hooking/Internal/HookManager.cs | 3 ++- .../Interface/Internal/Windows/PluginStatWindow.cs | 13 +++++++++++-- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/Dalamud/Hooking/AsmHook.cs b/Dalamud/Hooking/AsmHook.cs index 35eac4efd..e12d7f4f8 100644 --- a/Dalamud/Hooking/AsmHook.cs +++ b/Dalamud/Hooking/AsmHook.cs @@ -49,7 +49,7 @@ namespace Dalamud.Hooking this.statsMethod.GetILGenerator().Emit(OpCodes.Ret); var dele = this.statsMethod.CreateDelegate(typeof(Action)); - HookManager.TrackedHooks.Add(new HookInfo(this, dele, Assembly.GetCallingAssembly())); + HookManager.TrackedHooks.TryAdd(Guid.NewGuid(), new HookInfo(this, dele, Assembly.GetCallingAssembly())); } /// @@ -79,7 +79,7 @@ namespace Dalamud.Hooking this.statsMethod.GetILGenerator().Emit(OpCodes.Ret); var dele = this.statsMethod.CreateDelegate(typeof(Action)); - HookManager.TrackedHooks.Add(new HookInfo(this, dele, Assembly.GetCallingAssembly())); + HookManager.TrackedHooks.TryAdd(Guid.NewGuid(), new HookInfo(this, dele, Assembly.GetCallingAssembly())); } /// diff --git a/Dalamud/Hooking/Hook.cs b/Dalamud/Hooking/Hook.cs index 430527ed0..a61e366f8 100644 --- a/Dalamud/Hooking/Hook.cs +++ b/Dalamud/Hooking/Hook.cs @@ -75,7 +75,7 @@ namespace Dalamud.Hooking this.hookImpl = ReloadedHooks.Instance.CreateHook(detour, address.ToInt64()); } - HookManager.TrackedHooks.Add(new HookInfo(this, detour, callingAssembly)); + HookManager.TrackedHooks.TryAdd(Guid.NewGuid(), new HookInfo(this, detour, callingAssembly)); } /// diff --git a/Dalamud/Hooking/Internal/HookManager.cs b/Dalamud/Hooking/Internal/HookManager.cs index fde7ba3ba..d9237c156 100644 --- a/Dalamud/Hooking/Internal/HookManager.cs +++ b/Dalamud/Hooking/Internal/HookManager.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Runtime.InteropServices; @@ -28,7 +29,7 @@ namespace Dalamud.Hooking.Internal /// /// Gets a static list of tracked and registered hooks. /// - internal static List TrackedHooks { get; } = new(); + internal static ConcurrentDictionary TrackedHooks { get; } = new(); /// /// Gets a static dictionary of original code for a hooked address. diff --git a/Dalamud/Interface/Internal/Windows/PluginStatWindow.cs b/Dalamud/Interface/Internal/Windows/PluginStatWindow.cs index 257b67857..9bceef857 100644 --- a/Dalamud/Interface/Internal/Windows/PluginStatWindow.cs +++ b/Dalamud/Interface/Internal/Windows/PluginStatWindow.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Reflection; @@ -174,6 +175,8 @@ namespace Dalamud.Interface.Internal.Windows ImGui.EndTabItem(); } + var toRemove = new List(); + if (ImGui.BeginTabItem("Hooks")) { ImGui.Columns(4); @@ -204,10 +207,13 @@ namespace Dalamud.Interface.Internal.Windows ImGui.Separator(); ImGui.Separator(); - foreach (var trackedHook in HookManager.TrackedHooks) + foreach (var (guid, trackedHook) in HookManager.TrackedHooks) { try { + if (trackedHook.Hook.IsDisposed) + toRemove.Add(guid); + if (!this.showDalamudHooks && trackedHook.Assembly == Assembly.GetExecutingAssembly()) continue; @@ -265,7 +271,10 @@ namespace Dalamud.Interface.Internal.Windows if (ImGui.IsWindowAppearing()) { - HookManager.TrackedHooks.RemoveAll(h => h.Hook.IsDisposed); + foreach (var guid in toRemove) + { + HookManager.TrackedHooks.TryRemove(guid, out _); + } } ImGui.EndTabBar();