From ba4a1fee7a26674d5c2185c495c5c384e27fc8b6 Mon Sep 17 00:00:00 2001
From: Copilot <223556219+Copilot@users.noreply.github.com>
Date: Sun, 31 May 2026 20:21:09 -0700
Subject: [PATCH] Detach RED4ext hooks at the top of App::Shutdown

App::Shutdown cleared m_systems while RED4ext's own game-side
detours were still attached. The detach lived in App::Destruct,
which runs much later from DLL_PROCESS_DETACH. In the QuickExit
path Destruct doesn't even run before the chained quick_exit
fires, so the hooks stay live during teardown.

In-flight detours on InitScripts, LoadScripts, and
CGameApplication_AddState call back through App::Get()->GetXxxSystem(),
which does m_systems.at(...) and throws std::out_of_range on an
empty vector. No try/catch on the game side, so the throw becomes
std::terminate and the user sees an unhandled-exception dialog on
what should be a clean quit.

Move the detach into a new App::DetachHooks() member, call it at
the top of App::Shutdown before any system is torn down, and keep a
defence-in-depth call in App::Destruct for the case where Shutdown
was never reached. DetachHooks tracks attach/detach state in a new
m_hooksAttached flag so the double-call from the normal path is a
no-op. AttachHooks drops its const qualifier so it can set the
flag on success.

Fixes #141

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 src/dll/App.cpp | 61 ++++++++++++++++++++++++++++++++++---------------
 src/dll/App.hpp |  4 +++-
 2 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/src/dll/App.cpp b/src/dll/App.cpp
index 4d9a8c3e..39333524 100644
--- a/src/dll/App.cpp
+++ b/src/dll/App.cpp
@@ -117,23 +117,12 @@ void App::Destruct()
 {
     spdlog::info("RED4ext is terminating...");
 
-    // Detaching hooks here and not in dtor, since the dtor can be called by CRT when the processes exists. We don't
-    // really care if this will be called or not when the game exist ungracefully.
-
-    spdlog::trace("Detaching the hooks...");
-
-    DetourTransaction transaction;
-    if (transaction.IsValid())
+    // Defence in depth: if Shutdown was not called (e.g. the DLL is being unloaded without a
+    // QuickExit), make sure the hooks come off before g_app's destructor runs. DetachHooks is a
+    // no-op when the hooks have already been detached.
+    if (auto* app = g_app.get())
     {
-        auto success = Hooks::WinMain::Detach() && Hooks::QuickExit::Detach() && Hooks::CGameApplication::Detach() &&
-                       Hooks::ExecuteProcess::Detach() && Hooks::InitScripts::Detach() &&
-                       Hooks::LoadScripts::Detach() && Hooks::ValidateScripts::Detach() &&
-                       Hooks::AssertionFailed::Detach() && Hooks::CollectSaveableSystems::Detach() &&
-                       Hooks::gsmState_SessionActive::Detach();
-        if (success)
-        {
-            transaction.Commit();
-        }
+        app->DetachHooks();
     }
 
     g_app.reset(nullptr);
@@ -167,6 +156,11 @@ void App::Shutdown()
 {
     spdlog::info("RED4ext is shutting down...");
 
+    // Detach RED4ext's own game-side hooks before tearing down any system. Otherwise an in-flight
+    // hook callback on another thread can call back into App::GetXxxSystem() after m_systems has
+    // been cleared and throw an uncaught std::out_of_range into the game.
+    DetachHooks();
+
     for (auto& system : m_systems | std::ranges::views::reverse)
     {
         system->Shutdown();
@@ -214,7 +208,7 @@ const Paths* App::GetPaths() const
     return &m_paths;
 }
 
-bool App::AttachHooks() const
+bool App::AttachHooks()
 {
     spdlog::trace("Attaching hooks...");
 
@@ -228,9 +222,38 @@ bool App::AttachHooks() const
                    Hooks::ExecuteProcess::Attach() && Hooks::InitScripts::Attach() && Hooks::LoadScripts::Attach() &&
                    Hooks::ValidateScripts::Attach() && Hooks::AssertionFailed::Attach() &&
                    Hooks::CollectSaveableSystems::Attach() && Hooks::gsmState_SessionActive::Attach();
-    if (success)
+    if (success && transaction.Commit())
+    {
+        m_hooksAttached = true;
+        return true;
+    }
+
+    return false;
+}
+
+bool App::DetachHooks()
+{
+    if (!m_hooksAttached)
+    {
+        return true;
+    }
+
+    spdlog::trace("Detaching the hooks...");
+
+    DetourTransaction transaction;
+    if (!transaction.IsValid())
+    {
+        return false;
+    }
+
+    auto success = Hooks::WinMain::Detach() && Hooks::QuickExit::Detach() && Hooks::CGameApplication::Detach() &&
+                   Hooks::ExecuteProcess::Detach() && Hooks::InitScripts::Detach() && Hooks::LoadScripts::Detach() &&
+                   Hooks::ValidateScripts::Detach() && Hooks::AssertionFailed::Detach() &&
+                   Hooks::CollectSaveableSystems::Detach() && Hooks::gsmState_SessionActive::Detach();
+    if (success && transaction.Commit())
     {
-        return transaction.Commit();
+        m_hooksAttached = false;
+        return true;
     }
 
     return false;
diff --git a/src/dll/App.hpp b/src/dll/App.hpp
index 6393a773..d55c6f2b 100644
--- a/src/dll/App.hpp
+++ b/src/dll/App.hpp
@@ -32,7 +32,8 @@ class App
 private:
     App();
 
-    bool AttachHooks() const;
+    bool AttachHooks();
+    bool DetachHooks();
 
     template<typename T, typename... Args, typename = std::enable_if_t<std::is_base_of_v<ISystem, T>>>
     inline void AddSystem(Args&&... args)
@@ -48,4 +49,5 @@ class App
     DevConsole m_devConsole;
 
     std::vector<std::unique_ptr<ISystem>> m_systems;
+    bool m_hooksAttached = false;
 };