From b66005daeaafa17c0b7360b593116712c0dc31b4 Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Sun, 17 May 2026 08:24:45 +0200 Subject: [PATCH] fix(di): stop double-disposing container singletons in Plugin.DisposeAsync Smoke 1 of C3 surfaced MessageManager.DisposeAsync throwing on unload: Plugin.DisposeAsync ran the manual MessageManager teardown (CTS cancel + dispose at MessageManager.cs:84-99), then awaited _lifecycle.DisposeAsync which routed Host.Dispose through the container, which hit MessageManager.DisposeAsync a second time and threw ObjectDisposedException on the already-disposed CTS. Plugin.DisposeAsync now drops every manual service dispose - the container owns those singletons end-to-end. The framework-thread block keeps the three calls the container has no handle on (TearDownCommands, GameFunctions.SetChatInteractable, WindowSystem.RemoveAllWindows), plus the static-class cleanups (EmoteCache.Dispose, InputHistoryService.Reset) stay outside the container entirely. This changes the teardown order versus v1.4.10: the container disposes in reverse-registration order, which puts Windows ahead of IPC services. The v1.4.10 ordering ("IPC before Windows so a final IPC event cannot hit a half-torn ChatLogWindow") is no longer enforced. Host.Dispose runs synchronously on the framework thread, so no Framework.Update or Draw event fires during teardown; the remaining risk is an external IPC plugin invoking a subscriber mid-dispose, which is not something v1.4.10 actually prevented either. --- HellionChat/Plugin.cs | 57 +++++++++++-------------------------------- 1 file changed, 14 insertions(+), 43 deletions(-) diff --git a/HellionChat/Plugin.cs b/HellionChat/Plugin.cs index 348aad4..cd1bf02 100755 --- a/HellionChat/Plugin.cs +++ b/HellionChat/Plugin.cs @@ -527,49 +527,21 @@ public sealed class Plugin : IAsyncDalamudPlugin } ); - // Unsubscribe AutoTellTabs before MessageManager goes away. - failure = CaptureFailure(failure, () => AutoTellTabsService?.Dispose()); - - // MessageManager has its own async dispose path (DB flush, thread shutdown). - if (MessageManager is not null) - { - failure = await CaptureFailureAsync( - failure, - () => MessageManager.DisposeAsync().AsTask() - ) - .ConfigureAwait(false); - } - - // Game-function / IPC / window cleanup must run on the framework thread. + // Framework-thread cleanup the container does not reach. TearDownCommands + // walks Plugin-private dictionaries; SetChatInteractable is a static + // call into game state; WindowSystem.RemoveAllWindows clears the + // backing List<> that AddWindow populated in PluginLifecycle.LoadAsync. try { await Framework .RunOnFrameworkThread(() => { - // TearDown slash-commands + UiBuilder hooks before windows - // tear down. Slash-commands holding handlers that reach - // the windows would otherwise see a half-torn Plugin. failure = CaptureFailure(failure, TearDownCommands); - failure = CaptureFailure( failure, () => GameFunctions.GameFunctions.SetChatInteractable(true) ); - - // IPC subscribers before windows — prevents a final IPC event - // from reaching a half-torn ChatLogWindow. - failure = CaptureFailure(failure, () => HonorificService?.Dispose()); - failure = CaptureFailure(failure, () => TypingIpc?.Dispose()); - failure = CaptureFailure(failure, () => ExtraChat?.Dispose()); - failure = CaptureFailure(failure, () => Ipc?.Dispose()); - failure = CaptureFailure(failure, () => WindowSystem?.RemoveAllWindows()); - failure = CaptureFailure(failure, () => ChatLogWindow?.Dispose()); - failure = CaptureFailure(failure, () => DbViewer?.Dispose()); - failure = CaptureFailure(failure, () => InputPreview?.Dispose()); - failure = CaptureFailure(failure, () => SettingsWindow?.Dispose()); - failure = CaptureFailure(failure, () => DebuggerWindow?.Dispose()); - failure = CaptureFailure(failure, () => SeStringDebugger?.Dispose()); }) .ConfigureAwait(false); } @@ -578,23 +550,22 @@ public sealed class Plugin : IAsyncDalamudPlugin failure ??= ex; } - // Pure-memory cleanups — no Framework / UI / IPC touch. - failure = CaptureFailure(failure, () => Functions?.Dispose()); - failure = CaptureFailure(failure, () => Commands?.Dispose()); - failure = CaptureFailure(failure, () => EmoteCache.Dispose()); - // Static input history would otherwise survive the plugin reload. - failure = CaptureFailure(failure, InputHistoryService.Reset); - - // Lifecycle stops the host (HostedService.StopAsync) and disposes it - // on the framework thread. Container reaches the same singletons that - // the manual block above already disposed; second Dispose() is a no-op - // for the IDisposable services we own. + // Lifecycle stops the host (HostedService.StopAsync) and disposes the + // container on the framework thread; that path disposes all the + // services + windows we used to dispose manually here. The smoke from + // C3 surfaced MessageManager.DisposeAsync as non-idempotent (CTS + // dispose at line 99 throws on a second call), so we hand the entire + // service teardown to the container instead of double-disposing. if (_lifecycle is not null) { failure = await CaptureFailureAsync(failure, () => _lifecycle.DisposeAsync().AsTask()) .ConfigureAwait(false); } + // Static-class cleanups the container has no handle on. + failure = CaptureFailure(failure, () => EmoteCache.Dispose()); + failure = CaptureFailure(failure, InputHistoryService.Reset); + if (failure is not null) ExceptionDispatchInfo.Capture(failure).Throw(); }