From e3ce41306e7c96795388025ca7b7bfe4be4da207 Mon Sep 17 00:00:00 2001 From: JonKazama-Hellion Date: Sun, 3 May 2026 22:08:02 +0200 Subject: [PATCH] fix(threading): protect AutoTranslate cache and bound framework waits - Util/AutoTranslate.cs introduces a single EntriesLock object and serializes every read and write of the static Entries dictionary and ValidEntries hash set behind it. PreloadCache spawns a worker thread that fills both while the main thread reads them via the Matching / ReplaceWithPayload / StartsWithCommand entry points; without the lock the underlying collection access was undefined. AllEntries() splits into a thin lock wrapper plus a private BuildEntriesLocked() helper that runs under the lock - Ui/SettingsTabs/Privacy.cs bounds the .Wait() on the framework refresh after a manual retention sweep and after the privacy cleanup. A hung framework tick previously could deadlock the background worker thread. Five-second timeout, log on miss --- HellionChat/Ui/SettingsTabs/Privacy.cs | 29 ++++++++++++++------ HellionChat/Util/AutoTranslate.cs | 38 ++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/HellionChat/Ui/SettingsTabs/Privacy.cs b/HellionChat/Ui/SettingsTabs/Privacy.cs index 30e859c..4a0f2c0 100644 --- a/HellionChat/Ui/SettingsTabs/Privacy.cs +++ b/HellionChat/Ui/SettingsTabs/Privacy.cs @@ -455,11 +455,18 @@ internal sealed class Privacy : ISettingsTab if (deleted > 0) { - Plugin.Framework.Run(() => + // Bound the wait so a hung framework tick can't deadlock + // the background retention worker. Five seconds is well + // beyond a normal frame; if we time out we log and let + // the next FilterAllTabsAsync call recover the state. + if (!Plugin.Framework.Run(() => + { + Plugin.MessageManager.ClearAllTabs(); + Plugin.MessageManager.FilterAllTabsAsync(); + }).Wait(TimeSpan.FromSeconds(5))) { - Plugin.MessageManager.ClearAllTabs(); - Plugin.MessageManager.FilterAllTabsAsync(); - }).Wait(); + Plugin.Log.Warning("Retention sweep: framework refresh timed out after 5s."); + } } WrapperUtil.AddNotification(string.Format(HellionStrings.Retention_Success, deleted), NotificationType.Success); @@ -615,11 +622,17 @@ internal sealed class Privacy : ISettingsTab var deleted = Plugin.MessageManager.Store.CleanupRetainOnly(allowed); Plugin.Log.Information($"Privacy cleanup: deleted {deleted} messages"); - Plugin.Framework.Run(() => + // Bound the wait so a hung framework tick can't deadlock + // the background cleanup worker. See the matching comment in + // the retention path above for rationale. + if (!Plugin.Framework.Run(() => + { + Plugin.MessageManager.ClearAllTabs(); + Plugin.MessageManager.FilterAllTabsAsync(); + }).Wait(TimeSpan.FromSeconds(5))) { - Plugin.MessageManager.ClearAllTabs(); - Plugin.MessageManager.FilterAllTabsAsync(); - }).Wait(); + Plugin.Log.Warning("Privacy cleanup: framework refresh timed out after 5s."); + } WrapperUtil.AddNotification(string.Format(HellionStrings.Cleanup_Success, deleted), NotificationType.Success); } diff --git a/HellionChat/Util/AutoTranslate.cs b/HellionChat/Util/AutoTranslate.cs index 7e2619c..d3532d1 100644 --- a/HellionChat/Util/AutoTranslate.cs +++ b/HellionChat/Util/AutoTranslate.cs @@ -19,6 +19,12 @@ internal static class AutoTranslate private static readonly Dictionary> Entries = new(); private static readonly HashSet<(uint, uint)> ValidEntries = []; + // Serializes all reads and writes against Entries / ValidEntries. + // PreloadCache spawns a worker thread that fills both, while the main + // thread reads them via Matching / ReplaceWithPayload / StartsWithCommand + // — without this lock the HashSet/Dictionary access is undefined. + private static readonly object EntriesLock = new(); + private static Parser> selector)> Parser() { var sheetName = Any @@ -68,9 +74,17 @@ internal static class AutoTranslate private static List AllEntries() { - if (Entries.TryGetValue(Plugin.DataManager.Language, out var entries)) - return entries; + lock (EntriesLock) + { + if (Entries.TryGetValue(Plugin.DataManager.Language, out var entries)) + return entries; + return BuildEntriesLocked(); + } + } + + private static List BuildEntriesLocked() + { var shouldAdd = ValidEntries.Count == 0; var parser = Parser(); @@ -229,7 +243,10 @@ internal static class AutoTranslate return; // populate the list of valid entries - if (ValidEntries.Count == 0) + bool needBuild; + lock (EntriesLock) + needBuild = ValidEntries.Count == 0; + if (needBuild) AllEntries(); var start = -1; @@ -244,7 +261,10 @@ internal static class AutoTranslate var parts = tag[4..^1].Split(',', 2); if (parts.Length == 2 && uint.TryParse(parts[0], out var group) && uint.TryParse(parts[1], out var key)) { - var payload = ValidEntries.Contains((group, key)) ? CreateFixedTranslation(group, key) : []; + bool isValid; + lock (EntriesLock) + isValid = ValidEntries.Contains((group, key)); + var payload = isValid ? CreateFixedTranslation(group, key) : []; var oldBytes = bytes.ToArray(); var lengthDiff = payload.Length - (i - start); @@ -271,7 +291,10 @@ internal static class AutoTranslate return false; // populate the list of valid entries - if (ValidEntries.Count == 0) + bool needBuild; + lock (EntriesLock) + needBuild = ValidEntries.Count == 0; + if (needBuild) AllEntries(); for (var i = 0; i < search.Length; i++) @@ -289,7 +312,10 @@ internal static class AutoTranslate var parts = tag[4..^1].Split(',', 2); if (parts.Length == 2 && uint.TryParse(parts[0], out var group) && uint.TryParse(parts[1], out var key)) { - if (!ValidEntries.Contains((group, key))) + bool isValid; + lock (EntriesLock) + isValid = ValidEntries.Contains((group, key)); + if (!isValid) return false; var evaluated = Plugin.Evaluator.Evaluate(new ReadOnlySeString(CreateFixedTranslation(group, key))).ToString();