Refactor Plugin to IAsyncDalamudPlugin two-phase load

HellionChat/Plugin.cs

@@ -2,6 +2,7 @@ using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.IO;
+using System.Runtime.ExceptionServices;
 using HellionChat.Ipc;
 using HellionChat.Resources;
 using HellionChat.Ui;
@@ -17,7 +18,7 @@ using Dalamud.Interface.ImGuiFileDialog;
 namespace HellionChat;
 
 // ReSharper disable once ClassNeverInstantiated.Global
-public sealed class Plugin : IDalamudPlugin
+public sealed class Plugin : IAsyncDalamudPlugin
 {
     public const string PluginName = "Hellion Chat";
 
@@ -47,27 +48,35 @@ public sealed class Plugin : IDalamudPlugin
     public static FileDialogManager FileDialogManager { get; private set; } = null!;
 
     public readonly WindowSystem WindowSystem = new(PluginName);
-    public SettingsWindow SettingsWindow { get; }
-    public ChatLogWindow ChatLogWindow { get; }
-    public DbViewer DbViewer { get; }
-    public InputPreview InputPreview { get; }
-    public CommandHelpWindow CommandHelpWindow { get; }
-    public SeStringDebugger SeStringDebugger { get; }
-    public FirstRunWizard FirstRunWizard { get; }
-    public DebuggerWindow DebuggerWindow { get; }
 
-    internal Commands Commands { get; }
+    // v1.4.3: Phase-2 services need private setters now that LoadAsync
-    internal GameFunctions.GameFunctions Functions { get; }
+    // owns their construction. Phase-1-only services (Commands, Functions,
-    internal MessageManager MessageManager { get; }
+    // Ipc, ExtraChat, TypingIpc) keep their setters for symmetry.
-    internal AutoTellTabsService AutoTellTabsService { get; }
+    public SettingsWindow SettingsWindow { get; private set; } = null!;
-    internal IpcManager Ipc { get; }
+    public ChatLogWindow ChatLogWindow { get; private set; } = null!;
-    internal ExtraChat ExtraChat { get; }
+    public DbViewer DbViewer { get; private set; } = null!;
-    internal TypingIpc TypingIpc { get; }
+    public InputPreview InputPreview { get; private set; } = null!;
-    internal FontManager FontManager { get; }
+    public CommandHelpWindow CommandHelpWindow { get; private set; } = null!;
+    public SeStringDebugger SeStringDebugger { get; private set; } = null!;
+    public FirstRunWizard FirstRunWizard { get; private set; } = null!;
+    public DebuggerWindow DebuggerWindow { get; private set; } = null!;
+
+    internal Commands Commands { get; private set; } = null!;
+    internal GameFunctions.GameFunctions Functions { get; private set; } = null!;
+    internal MessageManager MessageManager { get; private set; } = null!;
+    internal AutoTellTabsService AutoTellTabsService { get; private set; } = null!;
+    internal IpcManager Ipc { get; private set; } = null!;
+    internal ExtraChat ExtraChat { get; private set; } = null!;
+    internal TypingIpc TypingIpc { get; private set; } = null!;
+    internal FontManager FontManager { get; private set; } = null!;
     internal Themes.ThemeRegistry ThemeRegistry { get; private set; } = null!;
     internal Ui.StatusBar StatusBar { get; private set; } = null!;
     internal Integrations.HonorificService HonorificService { get; private set; } = null!;
 
+    // (B3) Lightless idempotency guard — Dalamud may fire DisposeAsync twice
+    // in a reload race; second call short-circuits.
+    private int _disposeStarted;
+
     internal int DeferredSaveFrames = -1;
 
     // Serialises retention sweeps. The 24h auto-sweep on plugin load and
@@ -98,14 +107,17 @@ public sealed class Plugin : IDalamudPlugin
 
     public Plugin()
     {
+        // v1.4.3 Phase 1 — sync ctor: only work that doesn't touch
+        // Theme/Font/MessageManager/UI-windows lives here. Async Phase 2
+        // (LoadAsync) owns those. Hooks subscribe at the END of LoadAsync
+        // so Dalamud's first Draw tick never sees null services (B1).
+
         // Refuse to start if upstream Chat 2 is loaded — prevents IPC
         // channel collisions and double-replacement of the in-game chat
         // window. Throwing here makes Dalamud abort the load cleanly with
         // our localized message instead of crashing FFXIV mid-frame.
         ChatTwoConflictDetector.ThrowIfChatTwoIsLoaded(Interface);
 
-        try
-        {
         GameStarted = Process.GetCurrentProcess().StartTime.ToUniversalTime();
 
         // Hellion Chat: take over config + database from upstream ChatTwo
@@ -440,26 +452,12 @@ public sealed class Plugin : IDalamudPlugin
 
         FileDialogManager = new FileDialogManager();
 
+        // Phase-1 services: pure allocation, no Theme/Font/UI touch.
         Commands = new Commands();
         Functions = new GameFunctions.GameFunctions(this);
         Ipc = new IpcManager();
         TypingIpc = new TypingIpc(this);
         ExtraChat = new ExtraChat();
-            FontManager = new FontManager();
-
-            // v1.1.0 — Theme-Engine init. Custom-Themes liegen in
-            // pluginConfigs/HellionChat/themes/, lazy geladen beim ersten Get.
-            var customThemesDir = Path.Combine(Interface.ConfigDirectory.FullName, "themes");
-            Directory.CreateDirectory(customThemesDir);
-            SeedExampleThemeIfEmpty(customThemesDir);
-            ThemeRegistry = new Themes.ThemeRegistry(customThemesDir);
-            ThemeRegistry.Switch(Config.Theme);
-
-            // SelfTest hooks live alongside the live registry — the steps
-            // poll Active per frame and need the registry already wired.
-            SelfTestRegistry.RegisterTestSteps([
-                new SelfTests.ThemeSwitchSelfTestStep(this),
-            ]);
 
         // Plugin integrations register their IPC subscribers up-front so
         // Ready/Disposing events from the target plugins are caught from
@@ -469,7 +467,57 @@ public sealed class Plugin : IDalamudPlugin
 
         StatusBar = new Ui.StatusBar();
 
-            MessageManager = new MessageManager(this); // Does it require UI?
+        Interface.UiBuilder.DisableCutsceneUiHide = true;
+        Interface.UiBuilder.DisableGposeUiHide = true;
+
+        // Hellion Chat — daily retention sweep, off-thread so it never
+        // blocks plugin load. Skips itself when disabled or already ran
+        // within the past 24 hours. Pure fire-and-forget on a background
+        // thread, so it doesn't depend on Phase-2 services being live.
+        RunRetentionSweepIfDue();
+
+        if (Config.ShowEmotes)
+            _ = EmoteCache.LoadData(); // Fire-and-forget intentional, exceptions are caught inside
+    }
+
+    public async Task LoadAsync(CancellationToken cancellationToken)
+    {
+        cancellationToken.ThrowIfCancellationRequested();
+
+        try
+        {
+            // Group A: Font + Theme parallel (Q1=A). Both are CPU-bound,
+            // independent, and dominate the load-time profile. Everything
+            // else stays sequential to keep ordering simple.
+            // Q3=B transition: BuildFonts() is sync today; Task 5 converts
+            // FontManager itself to BuildFontsAsync.
+            var fontTask = Task.Run(() =>
+            {
+                FontManager = new FontManager();
+                FontManager.BuildFonts();
+            }, cancellationToken);
+
+            var themeTask = Task.Run(() =>
+            {
+                // v1.1.0 — Theme-Engine init. Custom-Themes liegen in
+                // pluginConfigs/HellionChat/themes/, lazy geladen beim ersten Get.
+                var customThemesDir = Path.Combine(Interface.ConfigDirectory.FullName, "themes");
+                Directory.CreateDirectory(customThemesDir);
+                SeedExampleThemeIfEmpty(customThemesDir);
+                ThemeRegistry = new Themes.ThemeRegistry(customThemesDir);
+                ThemeRegistry.Switch(Config.Theme);
+            }, cancellationToken);
+
+            await Task.WhenAll(fontTask, themeTask).ConfigureAwait(false);
+            cancellationToken.ThrowIfCancellationRequested();
+
+            // SelfTest hooks live alongside the live registry — the steps
+            // poll Active per frame and need the registry already wired.
+            SelfTestRegistry.RegisterTestSteps([
+                new SelfTests.ThemeSwitchSelfTestStep(this),
+            ]);
+
+            MessageManager = new MessageManager(this);
 
             // Hellion Chat — Auto-Tell-Tabs service. Subscribes to the
             // MessageManager's MessageProcessed event for live tells and
@@ -479,11 +527,6 @@ public sealed class Plugin : IDalamudPlugin
             AutoTellTabsService = new AutoTellTabsService(this, MessageManager, MessageManager.Store);
             AutoTellTabsService.Initialize();
 
-            // Hellion Chat — daily retention sweep, off-thread so it never
-            // blocks plugin load. Skips itself when disabled or already ran
-            // within the past 24 hours.
-            RunRetentionSweepIfDue();
-
             ChatLogWindow = new ChatLogWindow(this);
             SettingsWindow = new SettingsWindow(this);
             DbViewer = new DbViewer(this);
@@ -507,17 +550,24 @@ public sealed class Plugin : IDalamudPlugin
             if (!Config.FirstRunCompleted)
                 FirstRunWizard.IsOpen = true;
 
-            FontManager.BuildFonts();
-
-            Interface.UiBuilder.DisableCutsceneUiHide = true;
-            Interface.UiBuilder.DisableGposeUiHide = true;
-
             // let all the other components register, then initialize commands
             Commands.Initialise();
 
             if (Interface.Reason is not PluginLoadReason.Boot)
                 MessageManager.FilterAllTabsAsync();
 
+            #if !DEBUG
+            // Avoid 300ms hitch when sending first message by preloading the
+            // auto-translate cache. Don't do this in debug because it makes
+            // profiling difficult.
+            AutoTranslate.PreloadCache();
+            #endif
+
+            cancellationToken.ThrowIfCancellationRequested();
+
+            // (B1) Hooks last: every service and window must be live before
+            // Dalamud fires our first Draw / FrameworkUpdate tick. Anything
+            // earlier risks rendering against null FontManager / ThemeRegistry.
             Framework.Update += FrameworkUpdate;
             Interface.UiBuilder.Draw += Draw;
             Interface.LanguageChanged += LanguageChanged;
@@ -526,48 +576,86 @@ public sealed class Plugin : IDalamudPlugin
             // most useful landing place; OpenConfigUi is already wired to
             // the same toggle inside SettingsWindow.
             Interface.UiBuilder.OpenMainUi += OpenMainUi;
-
-            if (Config.ShowEmotes)
-                _ = EmoteCache.LoadData(); // Fire-and-forget intentional, exceptions are caught inside
-
-            #if !DEBUG
-            // Avoid 300ms hitch when sending first message by preloading the
-            // auto-translate cache. Don't do this in debug because it makes
-            // profiling difficult.
-            AutoTranslate.PreloadCache();
-            #endif
         }
-        catch (Exception ex)
+        catch
         {
-            Log.Error(ex, "Plugin load threw an error, turning off plugin");
+            // Mirror the v1.4.0 load-failure recovery: hand off to DisposeAsync
-            Dispose();
+            // so partially-built services are torn down. Swallow the cleanup
-
+            // exception so the original load failure stays the visible cause.
-            // Re-throw the exception to fail the plugin load.
+            try { await DisposeAsync().ConfigureAwait(false); }
+            catch { /* keep original failure */ }
             throw;
         }
     }
 
-    // Suppressing this warning because Dispose() is called in Plugin() if the
+    // Suppressing this warning because DisposeAsync may run after a partial
-    // load fails, so some values may not be initialized.
+    // LoadAsync, so some properties may not be initialized.
     [SuppressMessage("ReSharper", "ConditionalAccessQualifierIsNonNullableAccordingToAPIContract")]
-    public void Dispose()
+    public async ValueTask DisposeAsync()
     {
-        Interface.UiBuilder.OpenMainUi -= OpenMainUi;
+        // (B3) Idempotency guard — Dalamud may reload-race us; second
-        Interface.LanguageChanged -= LanguageChanged;
+        // call short-circuits so we don't double-dispose services.
-        Interface.UiBuilder.Draw -= Draw;
+        if (Interlocked.Exchange(ref _disposeStarted, 1) != 0)
-        Framework.Update -= FrameworkUpdate;
+            return;
-        GameFunctions.GameFunctions.SetChatInteractable(true);
 
-        // FrameworkUpdate would have fired the pending save in N frames,
+        Exception? failure = null;
-        // but we just unsubscribed it. -1 is the idle sentinel.
+
+        // Hooks unsubscribe FIRST so no Draw / FrameworkUpdate / LanguageChanged
+        // tick can fire while we're tearing services down. Mirrors the
+        // hooks-last subscribe order in LoadAsync.
+        failure = CaptureFailure(failure, () => Interface.UiBuilder.OpenMainUi -= OpenMainUi);
+        failure = CaptureFailure(failure, () => Interface.LanguageChanged -= LanguageChanged);
+        failure = CaptureFailure(failure, () => Interface.UiBuilder.Draw -= Draw);
+        failure = CaptureFailure(failure, () => Framework.Update -= FrameworkUpdate);
+
+        // v1.4.0 F5.3 — flush a pending DeferredSave before service teardown,
+        // since FrameworkUpdate just got unsubscribed and won't fire it.
+        failure = CaptureFailure(failure, () =>
+        {
             if (DeferredSaveFrames >= 0)
             {
                 SaveConfig();
                 DeferredSaveFrames = -1;
             }
+        });
 
+        // Auto-Tell-Tabs unsubscribes from MessageProcessed before MessageManager
+        // goes away. Pure-memory cleanup, no framework-thread pflicht.
+        failure = CaptureFailure(failure, () => AutoTellTabsService?.Dispose());
+
+        // v1.4.0 F6.2 — MessageManager has its own async dispose path
+        // (DB flush, pending-message thread shutdown). Run it before the
+        // framework-block so the worker threads are quiesced first.
+        if (MessageManager is not null)
+        {
+            failure = await CaptureFailureAsync(failure, () => MessageManager.DisposeAsync().AsTask())
+                .ConfigureAwait(false);
+        }
+
+        // (B4) Game-Function / IPC / UI-Window cleanup MUST run on the
+        // framework thread. WindowSystem mutations and IPC subscriber
+        // disposes touch Dalamud state that's only safe from the framework.
+        // Worker-thread DisposeAsync would race the next Draw tick.
+        failure = await CaptureFailureAsync(failure, async () =>
+        {
+            await Framework.RunOnFrameworkThread(() =>
+            {
+                // Game-Functions first — other services may still query
+                // chat-interactable state during their Dispose.
+                try { GameFunctions.GameFunctions.SetChatInteractable(true); } catch { /* swallowed */ }
+
+                // IPC subscribers — dispose before windows so any final
+                // event firing from the IPC source can't reach a half-torn
+                // ChatLogWindow.
                 HonorificService?.Dispose();
+                TypingIpc?.Dispose();
+                ExtraChat?.Dispose();
+                Ipc?.Dispose();
 
+                // Windows — RemoveAllWindows first, then per-window Dispose.
+                // Order matches the pre-v1.4.3 Dispose body byte-for-byte.
+                // CommandHelpWindow and FirstRunWizard don't implement
+                // IDisposable; their resources are reclaimed via WindowSystem.
                 WindowSystem?.RemoveAllWindows();
                 ChatLogWindow?.Dispose();
                 DbViewer?.Dispose();
@@ -575,19 +663,34 @@ public sealed class Plugin : IDalamudPlugin
                 SettingsWindow?.Dispose();
                 DebuggerWindow?.Dispose();
                 SeStringDebugger?.Dispose();
+            }).ConfigureAwait(false);
+        }).ConfigureAwait(false);
 
-        TypingIpc?.Dispose();
+        // Pure-memory cleanups — no Framework / UI / IPC touch, so they
-        ExtraChat?.Dispose();
+        // run on whatever thread DisposeAsync resumes on.
-        Ipc?.Dispose();
+        failure = CaptureFailure(failure, () => Functions?.Dispose());
-        // Dispose the Auto-Tell-Tabs service before MessageManager so it
+        failure = CaptureFailure(failure, () => Commands?.Dispose());
-        // can cleanly unsubscribe from the MessageProcessed event before
+        failure = CaptureFailure(failure, () => EmoteCache.Dispose());
-        // its source goes away.
-        AutoTellTabsService?.Dispose();
-        MessageManager?.DisposeAsync().AsTask().Wait();
-        Functions?.Dispose();
-        Commands?.Dispose();
 
-        EmoteCache.Dispose();
+        if (failure is not null)
+            ExceptionDispatchInfo.Capture(failure).Throw();
+    }
+
+    // Lightless-pattern capture helpers: run cleanup, remember the FIRST
+    // exception, keep going. Without these one mid-teardown failure would
+    // skip every cleanup behind it and leave services half-torn.
+    private static Exception? CaptureFailure(Exception? failure, Action action)
+    {
+        try { action(); }
+        catch (Exception ex) { failure ??= ex; }
+        return failure;
+    }
+
+    private static async ValueTask<Exception?> CaptureFailureAsync(Exception? failure, Func<Task> action)
+    {
+        try { await action().ConfigureAwait(false); }
+        catch (Exception ex) { failure ??= ex; }
+        return failure;
     }
 
     // Reads HellionThemeWindowOpacity from the pre-v13 backup the v12→v13