From 299fd59cbb37135705aef96a037d3d728bc9e60f Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Thu, 14 May 2026 00:00:53 +0200 Subject: [PATCH] refactor(retention): use Framework.RunOnTick instead of synchronous .Wait() Retention sweep no longer blocks for ~194ms on Framework.Run().Wait(). The clear+refilter pair is now scheduled on the next framework tick, so it still runs on the framework thread (keeping the Tabs-list mutation serialisation invariant -- Plugin.Config.Tabs is plain List and AutoTellTabsService can mutate it from background paths) but does not block the sweep thread while the framework finishes the current frame. A new _isDisposing volatile bool is set as the first statement in DisposeAsync so a deferred tick that fires after teardown bails before it touches MessageManager / Log / static fields the dispose path has already cleared. The retention worker is IsBackground=true so plugin unload can race against a still-pending tick. The existing RetentionSweepLock / RetentionSweepRunning serialisation covers the not-two-sweeps-at-once invariant; we don't add a CTS here because RunOnTick is fire-and-forget and the framework service owns the tick lifecycle. v1.4.8 B3. Coverage via in-game smoke (frame-time trace during a retention sweep run) in Task 9 -- no Build-Suite test because the suite has no FakeFramework fixture and the change is a schedule-form swap rather than new behaviour. --- HellionChat/Plugin.cs | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/HellionChat/Plugin.cs b/HellionChat/Plugin.cs index e799129..35de71c 100755 --- a/HellionChat/Plugin.cs +++ b/HellionChat/Plugin.cs @@ -126,6 +126,12 @@ public sealed class Plugin : IAsyncDalamudPlugin // Idempotency guard — Dalamud may fire DisposeAsync twice in a reload race. private int _disposeStarted; + // Set in the first DisposeAsync statement so async callbacks scheduled + // via Framework.RunOnTick (v1.4.8 B3 retention sweep) can early-bail + // before they touch state that has already been torn down. Volatile + // because the tick reads it from a different thread than the writer. + private volatile bool _isDisposing; + internal int DeferredSaveFrames = -1; // Cancels the v1.4.8 FTS5 bulk-insert worker on plugin teardown. The @@ -440,6 +446,12 @@ public sealed class Plugin : IAsyncDalamudPlugin if (Interlocked.Exchange(ref _disposeStarted, 1) != 0) return; + // Set before any cleanup so deferred Framework.RunOnTick callbacks + // (B3 retention sweep) see the flag and bail out before they touch + // MessageManager / Log / static fields that the rest of this method + // is about to tear down. + _isDisposing = true; + Exception? failure = null; // Unsubscribe hooks first — mirrors the hooks-last subscribe order in LoadAsync. @@ -711,15 +723,31 @@ public sealed class Plugin : IAsyncDalamudPlugin if (deleted > 0) { Log.Information($"Retention sweep deleted {deleted} expired messages."); - // Run clear+refilter on the framework thread — FilterAllTabsAsync - // is fire-and-forget and would race the next sweep cycle. - Framework - .Run(() => + // Schedule on the next framework tick to avoid the ~194ms + // hitch from blocking with .Wait() while the framework + // finishes the current frame. Tabs-list mutation must + // stay on the framework thread because Plugin.Config.Tabs + // (Configuration.cs:222) is not lock-protected and + // AutoTellTabsService can mutate it from background paths. + // Pattern reference: SimpleTweaks + // Tweaks/Chat/CaseInsensitiveCommands.cs:45. + Framework.RunOnTick(() => + { + // The retention thread is IsBackground=true so plugin + // unload can fire while a scheduled tick is still + // pending; bail before touching anything torn down. + if (_isDisposing) + return; + try { MessageManager.ClearAllTabs(); MessageManager.FilterAllTabs(); - }) - .Wait(); + } + catch (Exception ex) + { + Log.Error(ex, "Retention sweep clear+refilter failed"); + } + }); } else {