From 581cb506ccb110cfaeadb291d46a5e5aeb53b1c6 Mon Sep 17 00:00:00 2001 From: Liam Date: Fri, 23 Apr 2021 22:20:11 -0400 Subject: [PATCH] fix: unload plugins before unloading Dalamud to prevent crashes --- Dalamud/Dalamud.cs | 41 +++++++++++++-------------- Dalamud/EntryPoint.cs | 5 ++++ Dalamud/Game/Internal/Framework.cs | 28 ++++++++++++++++-- Dalamud/Interface/InterfaceManager.cs | 1 + 4 files changed, 52 insertions(+), 23 deletions(-) diff --git a/Dalamud/Dalamud.cs b/Dalamud/Dalamud.cs index 6d8e054d8..64aa81c0e 100644 --- a/Dalamud/Dalamud.cs +++ b/Dalamud/Dalamud.cs @@ -351,6 +351,26 @@ namespace Dalamud this.finishUnloadSignal?.WaitOne(); } + public void DisposePlugins() + { + // this must be done before unloading plugins, or it can cause a race condition + // due to rendering happening on another thread, where a plugin might receive + // a render call after it has been disposed, which can crash if it attempts to + // use any resources that it freed in its own Dispose method + this.InterfaceManager?.Dispose(); + + try + { + this.PluginManager.UnloadPlugins(); + } + catch (Exception ex) + { + Log.Error(ex, "Plugin unload failed."); + } + + this.DalamudUi?.Dispose(); + } + /// /// Dispose Dalamud subsystems. /// @@ -358,27 +378,6 @@ namespace Dalamud { try { - // this must be done before unloading plugins, to prevent crashes due to errors - // in plugin cleanup - this.Framework.DispatchUpdateEvents = false; - - // this must be done before unloading plugins, or it can cause a race condition - // due to rendering happening on another thread, where a plugin might receive - // a render call after it has been disposed, which can crash if it attempts to - // use any resources that it freed in its own Dispose method - this.InterfaceManager?.Dispose(); - - try - { - this.PluginManager.UnloadPlugins(); - } - catch (Exception ex) - { - Log.Error(ex, "Plugin unload failed."); - } - - this.DalamudUi?.Dispose(); - this.Framework?.Dispose(); this.ClientState?.Dispose(); diff --git a/Dalamud/EntryPoint.cs b/Dalamud/EntryPoint.cs index 2895a13e0..3c9bddcbd 100644 --- a/Dalamud/EntryPoint.cs +++ b/Dalamud/EntryPoint.cs @@ -59,6 +59,11 @@ namespace Dalamud // Run session dalamud.Start(); dalamud.WaitForUnload(); + if (dalamud.Framework.DispatchUpdateEvents) + { + dalamud.DisposePlugins(); + Thread.Sleep(100); + } dalamud.Dispose(); } diff --git a/Dalamud/Game/Internal/Framework.cs b/Dalamud/Game/Internal/Framework.cs index ebfdcb520..44f04a1b8 100644 --- a/Dalamud/Game/Internal/Framework.cs +++ b/Dalamud/Game/Internal/Framework.cs @@ -28,6 +28,8 @@ namespace Dalamud.Game.Internal { public delegate IntPtr OnDestroyDelegate(); + public delegate bool OnRealDestroyDelegate(IntPtr framework); + /// /// Event that gets fired every time the game framework updates. /// @@ -36,6 +38,8 @@ namespace Dalamud.Game.Internal { private Hook updateHook; private Hook destroyHook; + + private Hook realDestroyHook; /// /// A raw pointer to the instance of Client::Framework @@ -101,6 +105,10 @@ namespace Dalamud.Game.Internal { var pDestroy = Marshal.ReadIntPtr(vtable, IntPtr.Size * 3); this.destroyHook = new Hook(pDestroy, new OnDestroyDelegate(HandleFrameworkDestroy), this); + + var pRealDestroy = Marshal.ReadIntPtr(vtable, IntPtr.Size * 2); + this.realDestroyHook = + new Hook(pRealDestroy, new OnRealDestroyDelegate(HandleRealDestroy), this); } public void Enable() { @@ -109,6 +117,7 @@ namespace Dalamud.Game.Internal { this.updateHook.Enable(); this.destroyHook.Enable(); + this.realDestroyHook.Enable(); } public void Dispose() { @@ -117,6 +126,7 @@ namespace Dalamud.Game.Internal { this.updateHook.Dispose(); this.destroyHook.Dispose(); + this.realDestroyHook.Dispose(); } private bool HandleFrameworkUpdate(IntPtr framework) { @@ -168,11 +178,23 @@ namespace Dalamud.Game.Internal { return this.updateHook.Original(framework); } - private IntPtr HandleFrameworkDestroy() { - Log.Information("Framework::OnDestroy!"); + private bool HandleRealDestroy(IntPtr framework) + { + if (this.DispatchUpdateEvents) + { + Log.Information("Framework::Destroy!"); + this.dalamud.DisposePlugins(); + Log.Information("Framework::Destroy OK!"); + } this.DispatchUpdateEvents = false; + return this.realDestroyHook.Original(framework); + } + + private IntPtr HandleFrameworkDestroy() { + Log.Information("Framework::Free!"); + // Store the pointer to the original trampoline location var originalPtr = Marshal.GetFunctionPointerForDelegate(this.destroyHook.Original); @@ -180,6 +202,8 @@ namespace Dalamud.Game.Internal { this.dalamud.WaitForUnloadFinish(); + Log.Information("Framework::Free OK!"); + // Return the original trampoline location to cleanly exit return originalPtr; } diff --git a/Dalamud/Interface/InterfaceManager.cs b/Dalamud/Interface/InterfaceManager.cs index b987855de..6e1e63cf5 100644 --- a/Dalamud/Interface/InterfaceManager.cs +++ b/Dalamud/Interface/InterfaceManager.cs @@ -169,6 +169,7 @@ namespace Dalamud.Interface System.Threading.Thread.Sleep(500); this.scene?.Dispose(); + this.setCursorHook.Dispose(); this.presentHook.Dispose(); this.resizeBuffersHook.Dispose(); }