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.
This commit is contained in:
+14
-43
@@ -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();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user