From 079e28022683892032958138029c0a176f4bc5e2 Mon Sep 17 00:00:00 2001 From: JonKazama-Hellion Date: Thu, 7 May 2026 00:42:26 +0200 Subject: [PATCH 1/9] fix(messagestore): drop GC.Collect from Dispose, rely on Pooling=false --- HellionChat/MessageStore.cs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/HellionChat/MessageStore.cs b/HellionChat/MessageStore.cs index a257f3a..85c43e8 100644 --- a/HellionChat/MessageStore.cs +++ b/HellionChat/MessageStore.cs @@ -131,11 +131,22 @@ internal class MessageStore : IDisposable public void Dispose() { + // Order matters: Close releases the WAL/SHM files, Dispose then + // hands the underlying connection state back to the pool. We + // intentionally configure Pooling = false in Connect(), so there + // is no pool to clear globally, which keeps HellionChat's reload + // from disturbing other plugins' SQLite connections (which is + // what SqliteConnection.ClearAllPools() would do, since it acts + // provider-wide). + // + // We used to call GC.Collect() + GC.WaitForPendingFinalizers() + // here as a defensive flush, but with Pooling = false there is + // nothing left to collect that the explicit Close hasn't already + // released. The GC calls were heap pressure on every plugin + // reload and reached into other plugins' object graphs because + // GC.Collect is process-wide. Connection.Close(); Connection.Dispose(); - // Closing the connection doesn't immediately release the file. - GC.Collect(); - GC.WaitForPendingFinalizers(); } private SqliteConnection Connect() From 8c624a0032fbaeaaf365beb2b38bb645ee8483ad Mon Sep 17 00:00:00 2001 From: JonKazama-Hellion Date: Thu, 7 May 2026 00:54:43 +0200 Subject: [PATCH 2/9] fix(threads): mark PendingMessage thread as background, document RetentionSweep rationale --- HellionChat/MessageManager.cs | 10 +++++++++- HellionChat/Plugin.cs | 5 +++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/HellionChat/MessageManager.cs b/HellionChat/MessageManager.cs index c655b0a..983591f 100644 --- a/HellionChat/MessageManager.cs +++ b/HellionChat/MessageManager.cs @@ -66,7 +66,15 @@ internal class MessageManager : IAsyncDisposable Store = new MessageStore(DatabasePath()); - PendingMessageThread = new Thread(() => ProcessPendingMessages(PendingThreadCancellationToken.Token)); + // IsBackground = true so a stuck worker never blocks plugin unload. + // The worker has its own cancellation path via PendingThreadCancellationToken, + // and DisposeAsync waits up to 10s for cooperative shutdown. The + // background flag is the safety net for the case where cooperative + // shutdown fails to drain the queue in time. + PendingMessageThread = new Thread(() => ProcessPendingMessages(PendingThreadCancellationToken.Token)) + { + IsBackground = true, + }; PendingMessageThread.Start(); ContentIdResolverHook = Plugin.GameInteropProvider.HookFromAddress(RaptureLogModule.MemberFunctionPointers.AddMsgSourceEntry, ContentIdResolver); diff --git a/HellionChat/Plugin.cs b/HellionChat/Plugin.cs index 9744593..d0d349c 100755 --- a/HellionChat/Plugin.cs +++ b/HellionChat/Plugin.cs @@ -691,6 +691,11 @@ public sealed class Plugin : IDalamudPlugin policy[(int)(ushort)type] = days; var defaultDays = Config.RetentionDefaultDays; + // IsBackground = true for the same reason as PendingMessageThread: + // a stuck sweep must never block plugin unload. RunRetentionSweepIfDue + // guards the run-frequency, and the sweep itself uses the framework's + // cooperative cancellation pattern. The background flag is the safety + // net if the sweep ever takes longer than expected. new Thread(() => { // Bail out cheaply if a manual sweep is already in flight; the From c9dfd024b2dd53fe30b43290d4237209cd09afd0 Mon Sep 17 00:00:00 2001 From: JonKazama-Hellion Date: Thu, 7 May 2026 01:10:50 +0200 Subject: [PATCH 3/9] docs(comments): trim verbose dispose and thread rationale Match the new HellionChat comment-length convention: 1-3 lines for standard pitfall notes, 5+ only for non-trivial workarounds. The previous Dispose comment was 14 lines of textbook prose, which veered into AI-slop territory and would rot on the next refactor. --- HellionChat/MessageManager.cs | 7 ++----- HellionChat/MessageStore.cs | 18 ++++-------------- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/HellionChat/MessageManager.cs b/HellionChat/MessageManager.cs index 983591f..40dfb6e 100644 --- a/HellionChat/MessageManager.cs +++ b/HellionChat/MessageManager.cs @@ -66,11 +66,8 @@ internal class MessageManager : IAsyncDisposable Store = new MessageStore(DatabasePath()); - // IsBackground = true so a stuck worker never blocks plugin unload. - // The worker has its own cancellation path via PendingThreadCancellationToken, - // and DisposeAsync waits up to 10s for cooperative shutdown. The - // background flag is the safety net for the case where cooperative - // shutdown fails to drain the queue in time. + // IsBackground so a stuck worker never blocks plugin unload. + // Cooperative cancel via PendingThreadCancellationToken first, background flag is the safety net. PendingMessageThread = new Thread(() => ProcessPendingMessages(PendingThreadCancellationToken.Token)) { IsBackground = true, diff --git a/HellionChat/MessageStore.cs b/HellionChat/MessageStore.cs index 85c43e8..7ae1fc1 100644 --- a/HellionChat/MessageStore.cs +++ b/HellionChat/MessageStore.cs @@ -131,20 +131,10 @@ internal class MessageStore : IDisposable public void Dispose() { - // Order matters: Close releases the WAL/SHM files, Dispose then - // hands the underlying connection state back to the pool. We - // intentionally configure Pooling = false in Connect(), so there - // is no pool to clear globally, which keeps HellionChat's reload - // from disturbing other plugins' SQLite connections (which is - // what SqliteConnection.ClearAllPools() would do, since it acts - // provider-wide). - // - // We used to call GC.Collect() + GC.WaitForPendingFinalizers() - // here as a defensive flush, but with Pooling = false there is - // nothing left to collect that the explicit Close hasn't already - // released. The GC calls were heap pressure on every plugin - // reload and reached into other plugins' object graphs because - // GC.Collect is process-wide. + // Pooling=false (set in Connect) avoids ClearAllPools, which is + // provider-wide and would touch other plugins' SQLite connections. + // GC.Collect was here as a defensive flush; removed because explicit + // Close already releases everything we hold. Connection.Close(); Connection.Dispose(); } From 72d568e5b373d691c886b552e5c7b31508b79844 Mon Sep 17 00:00:00 2001 From: JonKazama-Hellion Date: Thu, 7 May 2026 07:41:50 +0200 Subject: [PATCH 4/9] fix(emotecache): replace async void Load with async Task tracker --- HellionChat/EmoteCache.cs | 41 ++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/HellionChat/EmoteCache.cs b/HellionChat/EmoteCache.cs index 97d5dc8..eff10ad 100644 --- a/HellionChat/EmoteCache.cs +++ b/HellionChat/EmoteCache.cs @@ -1,4 +1,5 @@ -using System.Numerics; +using System.Collections.Concurrent; +using System.Numerics; using System.Text.Json; using System.Text.Json.Serialization; using Dalamud.Interface.Textures; @@ -73,6 +74,19 @@ public static class EmoteCache private static CancellationTokenSource Cts = new(); internal static CancellationToken Token => Cts.Token; + // Drain target for in-flight loads on Dispose; without this an orphan + // continuation could still write to a torn-down Texture/Frames field. + private static readonly ConcurrentBag PendingLoads = new(); + + internal static void TrackLoad(Task loadTask, string emoteCode) + { + PendingLoads.Add(loadTask.ContinueWith(t => + { + if (t.IsFaulted) + Plugin.Log.Error(t.Exception!, $"EmoteCache load failed for {emoteCode}"); + }, TaskScheduler.Default)); + } + public static async Task LoadData() { if (State is not LoadingState.Unloaded) @@ -135,10 +149,20 @@ public static class EmoteCache public static void Dispose() { - // Cancel in-flight downloads / texture creates so the async-void - // Load methods bail out before they touch a disposed TextureProvider. Cts.Cancel(); + // 5s upper bound; anything still running gets abandoned. + try + { + Task.WaitAll(PendingLoads.ToArray(), TimeSpan.FromSeconds(5)); + } + catch (AggregateException) + { + // Faults already logged in TrackLoad. + } + + while (PendingLoads.TryTake(out _)) { } + foreach (var emote in EmoteImages.Values) emote.InnerDispose(); } @@ -233,11 +257,12 @@ public static class EmoteCache public ImGuiEmote Prepare(Emote emote) { var ct = EmoteCache.Token; - Task.Run(() => Load(emote, ct), ct); + // Task.Run keeps the sync prefix off the ImGui render thread. + EmoteCache.TrackLoad(Task.Run(() => LoadAsyncTracked(emote, ct), ct), emote.Code); return this; } - private async void Load(Emote emote, CancellationToken ct) + private async Task LoadAsyncTracked(Emote emote, CancellationToken ct) { try { @@ -251,8 +276,6 @@ public static class EmoteCache } catch (OperationCanceledException) { - // Plugin disposed mid-load; the EmoteImages entry is also - // being torn down, no extra cleanup needed. } catch (Exception ex) { @@ -310,11 +333,11 @@ public static class EmoteCache public ImGuiGif Prepare(Emote emote) { var ct = EmoteCache.Token; - Task.Run(() => Load(emote, ct), ct); + EmoteCache.TrackLoad(Task.Run(() => LoadAsyncTracked(emote, ct), ct), emote.Code); return this; } - private async void Load(Emote emote, CancellationToken ct) + private async Task LoadAsyncTracked(Emote emote, CancellationToken ct) { try { From 93329087a90a9a8b8681037c04b0ccdf38c46659 Mon Sep 17 00:00:00 2001 From: JonKazama-Hellion Date: Thu, 7 May 2026 07:52:14 +0200 Subject: [PATCH 5/9] fix(messagemanager): warn loudly when DisposeAsync 10s timeout hits --- HellionChat/MessageManager.cs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/HellionChat/MessageManager.cs b/HellionChat/MessageManager.cs index 40dfb6e..adcc0b1 100644 --- a/HellionChat/MessageManager.cs +++ b/HellionChat/MessageManager.cs @@ -90,19 +90,22 @@ internal class MessageManager : IAsyncDisposable Plugin.ChatGui.ChatMessageUnhandled -= ChatMessage; await PendingThreadCancellationToken.CancelAsync(); - var timeout = 10_000; // 10s - while (timeout > 0) - { - if (!PendingMessageThread.IsAlive) - break; - timeout -= 100; + // 10s cooperative window; Thread.Abort is gone since .NET 5, so a + // stuck worker has to ride out the next AppDomain unload. + var deadline = TimeSpan.FromSeconds(10); + var stopwatch = Stopwatch.StartNew(); + while (stopwatch.Elapsed < deadline && PendingMessageThread.IsAlive) await Task.Delay(100); - Plugin.Log.Debug("Sleeping because PendingMessageThread thread still alive"); - } - // CancellationTokenSource owns an unmanaged WaitHandle; dispose after the - // worker thread has drained, otherwise it leaks across plugin reloads. + if (PendingMessageThread.IsAlive) + Plugin.Log.Warning( + "PendingMessageThread did not observe cancellation within 10s. " + + "Worker remains on a background thread; next plugin reload releases it. " + + "If this recurs, file a bug with /xllog after the previous reload."); + + // CTS owns an unmanaged WaitHandle; dispose even if the worker is + // alive — it checks IsCancellationRequested via the linked token. PendingThreadCancellationToken.Dispose(); Store.Dispose(); From e5bf375b42afb3ac21860e8b43a7bd9d395e2d21 Mon Sep 17 00:00:00 2001 From: JonKazama-Hellion Date: Thu, 7 May 2026 07:53:52 +0200 Subject: [PATCH 6/9] fix(plugin): flush DeferredSaveFrames in Dispose before service teardown --- HellionChat/Plugin.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/HellionChat/Plugin.cs b/HellionChat/Plugin.cs index d0d349c..f5f5cbd 100755 --- a/HellionChat/Plugin.cs +++ b/HellionChat/Plugin.cs @@ -536,6 +536,14 @@ public sealed class Plugin : IDalamudPlugin Framework.Update -= FrameworkUpdate; GameFunctions.GameFunctions.SetChatInteractable(true); + // FrameworkUpdate would have fired the pending save in N frames, + // but we just unsubscribed it. -1 is the idle sentinel. + if (DeferredSaveFrames >= 0) + { + SaveConfig(); + DeferredSaveFrames = -1; + } + HonorificService?.Dispose(); WindowSystem?.RemoveAllWindows(); From 3f7e86b32e6249e9ee6d3e89b61b2382f1d31914 Mon Sep 17 00:00:00 2001 From: JonKazama-Hellion Date: Thu, 7 May 2026 08:03:57 +0200 Subject: [PATCH 7/9] fix(migration): pull HellionThemeWindowOpacity from pre-v13 backup in v13->v14 --- HellionChat/Plugin.cs | 51 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/HellionChat/Plugin.cs b/HellionChat/Plugin.cs index f5f5cbd..b4bd86f 100755 --- a/HellionChat/Plugin.cs +++ b/HellionChat/Plugin.cs @@ -245,11 +245,24 @@ public sealed class Plugin : IDalamudPlugin // HellionThemeEnabled-Flag wird deprecated und nur noch ein Release // als Safety-Net im JSON behalten. Window-Opacity wandert von // HellionThemeWindowOpacity in das neue WindowOpacity-Feld. + // + // v1.4.0 (F5.4): Pre-v13-Backup wird gelesen, HellionThemeWindowOpacity + // ins neue Feld gezogen. Override nur wenn WindowOpacity noch beim + // Default sitzt — sonst hat der User in der Zwischenzeit (z.B. via + // WindowAlpha → WindowOpacity in v15→v16) explizit etwas gesetzt. if (Config.Version < 14) { Config.Theme = "hellion-arctic"; - // v1.2.0: alter Opacity-Wert wird nicht mehr migriert (Field entfernt). - // User die direkt v13 → v15 springen bekommen den Default 0.85. + + var oldThemeOpacity = TryReadPreV13ThemeOpacity(); + if (oldThemeOpacity is { } legacy + && Math.Abs(Config.WindowOpacity - 0.85f) < 0.001f) + { + Config.WindowOpacity = Math.Clamp(legacy, 0.5f, 1.0f); + Log.Information( + $"Migrated pre-v13 HellionThemeWindowOpacity {legacy} to WindowOpacity {Config.WindowOpacity}"); + } + Config.ReduceMotion = false; Config.UseCompactDensity = false; Config.Version = 14; @@ -568,6 +581,40 @@ public sealed class Plugin : IDalamudPlugin EmoteCache.Dispose(); } + // Reads HellionThemeWindowOpacity from the pre-v13 backup the v12→v13 + // block writes alongside the live config. Null when absent, unreadable, + // or schema-incompatible — all valid steady states (fresh install, + // backup pruned, pre-v12 config). Errors log at Warning so a corrupted + // backup stays visible in /xllog without breaking the migration. + private static float? TryReadPreV13ThemeOpacity() + { + var pluginConfigsDir = Interface.ConfigDirectory.Parent?.FullName; + if (pluginConfigsDir is null) + return null; + + var backupPath = Path.Combine(pluginConfigsDir, $"{Interface.InternalName}.json.pre-v13-backup"); + if (!File.Exists(backupPath)) + return null; + + try + { + using var stream = File.OpenRead(backupPath); + using var doc = System.Text.Json.JsonDocument.Parse(stream); + if (doc.RootElement.TryGetProperty("HellionThemeWindowOpacity", out var prop) + && prop.ValueKind == System.Text.Json.JsonValueKind.Number + && prop.TryGetSingle(out var value)) + { + return value; + } + return null; + } + catch (Exception ex) + { + Log.Warning(ex, "HellionChat: pre-v13 backup lookup failed, defaulting WindowOpacity"); + return null; + } + } + private static void MigrateFromChatTwoLayout() { var pluginConfigsDir = Interface.ConfigDirectory.Parent?.FullName; From f8a734d93f53b063ceb5634e32168770e52a6010 Mon Sep 17 00:00:00 2001 From: JonKazama-Hellion Date: Thu, 7 May 2026 19:04:20 +0200 Subject: [PATCH 8/9] chore: bump version to 1.4.0 and document Critical Lifecycle Fixes --- HellionChat/HellionChat.csproj | 2 +- HellionChat/HellionChat.yaml | 39 ++++++++++++++++++++++++++++++++++ README.md | 4 ++-- docs/CHANGELOG.md | 34 +++++++++++++++++++++++++++++ docs/ROADMAP.md | 21 ++++++++++++------ repo.json | 14 ++++++------ 6 files changed, 98 insertions(+), 16 deletions(-) diff --git a/HellionChat/HellionChat.csproj b/HellionChat/HellionChat.csproj index 1190e83..98f58fa 100644 --- a/HellionChat/HellionChat.csproj +++ b/HellionChat/HellionChat.csproj @@ -4,7 +4,7 @@ 0.1.0 is our bootstrap release; the underlying Chat 2 base is called out in the yaml changelog so users can see what it derives from. --> - 1.3.0 + 1.4.0 enable enable