Skip to content

App::Shutdown clears m_systems before detaching RED4ext's own hooks #141

@JustinMDotNet

Description

@JustinMDotNet

Summary

App::Shutdown clears m_systems while RED4ext's own game-side detours are still attached. The detours are not removed until App::Destruct, which is a separate later call. Any in-flight hook callback that runs in that window calls App::Get()->GetXxxSystem(), which evaluates m_systems.at(...) on an empty vector and throws an uncaught std::out_of_range straight into the game.

Where

  • src/dll/App.cpp:166-180Shutdown walks systems in reverse, then m_systems.clear();.
  • src/dll/App.cpp:116-144Destruct runs a separate DetourTransaction to remove the hooks. It only runs from the normal exit path; QuickExit does not call it.
  • src/dll/App.cpp:182-210 — every GetXxxSystem() does m_systems.at(static_cast<size_t>(...)), which throws on an empty vector.
  • src/dll/Hooks/QuickExit.cpp:15-36_QuickExit_fnc calls app->Shutdown() and then chains the original quick_exit. Hooks remain installed for the rest of process teardown.

Hooks that demonstrably reach back into App::GetXxxSystem():

  • src/dll/Hooks/InitScripts.cpp:27App::Get()->GetScriptCompilationSystem()
  • src/dll/Hooks/LoadScripts.cpp:20 — same
  • src/dll/Hooks/CGameApplication.cpp:19-77States::*::Attach calls (the state hooks indirectly need the systems)
  • Anything plugin-installed via HookingSystem that calls back into App::Get() from the detour body

Why it matters

Scenario:

  1. quick_exit fires on the main thread.
  2. _QuickExit_fnc calls App::Get()->Shutdown(). m_systems is now empty.
  3. Before the chained quick_exit actually returns, another thread (or a queued continuation on the same thread) hits one of the still-installed detours, e.g. _CBaseEngine_LoadScripts.
  4. The detour calls App::Get()->GetScriptCompilationSystem()m_systems.at(static_cast<size_t>(ESystemType::Script))std::out_of_range.
  5. No try/catch on the game side. std::terminate, ugly exit dialog, "RED4ext crashed the game on quit" report from the user.

Suggested fix

Two reasonable options, both fine:

  1. Detach hooks first. Move the DetourTransaction + Hooks::*::Detach() block from the top of App::Destruct to the top of App::Shutdown. Then the hooks are gone before any system is shut down.
  2. Make accessors safe. Change every GetXxxSystem() to return nullptr when m_systems is empty (or when m_systems.size() <= index), and audit every detour body to handle a null system. This is a bigger change but also defends against any future bug in this area.

Option 1 is simpler and fixes the actual ordering bug. Option 2 is defense in depth.

Severity

High. Reachable from a normal quick_exit path. Bug surfaces as "the game crashes on quit," which is exactly the kind of thing users blame the loader for whether or not it is RED4ext's fault.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions