diff --git a/src/game/Debug/GdbServer/GdbBreakpoints.cpp b/src/game/Debug/GdbServer/GdbBreakpoints.cpp index 3659f820e..190a7a8f0 100644 --- a/src/game/Debug/GdbServer/GdbBreakpoints.cpp +++ b/src/game/Debug/GdbServer/GdbBreakpoints.cpp @@ -32,8 +32,10 @@ #include "GdbMonitor.h" #include "GdbServer.h" +#include #include #include +#include #include #include @@ -50,8 +52,12 @@ namespace GdbBp uint64 filter; }; + // g_entries is mutated on the world thread (monitor commands) but + // also iterated by Matches from foreign threads (network/DB call + // sites), so every access must hold g_entriesLock. + std::mutex g_entriesLock; std::vector g_entries; - uint64 g_hits = 0; + std::atomic g_hits{0}; // Canonical lower-case name per Event, indexed by enum value. Keep in // lockstep with the GdbEvent enum in shared/Debug/GdbEvents.h. @@ -75,6 +81,7 @@ namespace GdbBp static_cast(Event::Count), "kEventNames must match the Event enum"); + // Callers must hold g_entriesLock; do not lock here (self-deadlock). void Recount() { uint64 mask = 0; @@ -93,6 +100,7 @@ namespace GdbBp bool Matches(Event e, uint64 detail) { + std::lock_guard lock(g_entriesLock); for (const Entry& entry : g_entries) { if (entry.ev == e && (entry.filter == 0 || entry.filter == detail)) @@ -105,6 +113,7 @@ namespace GdbBp bool Arm(Event e, uint64 filter) { + std::lock_guard lock(g_entriesLock); for (const Entry& entry : g_entries) { if (entry.ev == e && entry.filter == filter) @@ -119,6 +128,7 @@ namespace GdbBp bool Disarm(Event e, uint64 filter) { + std::lock_guard lock(g_entriesLock); for (size_t i = 0; i < g_entries.size(); ++i) { if (g_entries[i].ev == e && g_entries[i].filter == filter) @@ -133,12 +143,14 @@ namespace GdbBp void DisarmAll() { + std::lock_guard lock(g_entriesLock); g_entries.clear(); Recount(); } void List(GdbMon::MonitorWriter& out) { + std::lock_guard lock(g_entriesLock); if (g_entries.empty()) { out.Str(" (no breakpoints armed)\n"); @@ -159,7 +171,7 @@ namespace GdbBp out.Line(); } out.Str(" hits="); - out.U64(g_hits); + out.U64(g_hits.load()); out.Line(); } diff --git a/src/game/Debug/GdbServer/GdbBreakpoints.h b/src/game/Debug/GdbServer/GdbBreakpoints.h index 3f737db91..6275c8af8 100644 --- a/src/game/Debug/GdbServer/GdbBreakpoints.h +++ b/src/game/Debug/GdbServer/GdbBreakpoints.h @@ -50,9 +50,10 @@ namespace GdbMon { class MonitorWriter; } * inspect game state via the `monitor` surface, then resume. * * The hot-path guard is a single relaxed atomic load of a per-event bitmask, - * so call sites cost effectively nothing when their event is not armed. All - * arming and matching happens on the world thread (monitor dispatch + game - * code), so the registry needs no locking. + * so call sites cost effectively nothing when their event is not armed. + * Arming happens on the world thread (monitor dispatch), but matching and + * hit-counting can also run from foreign threads (network/DB call sites), so + * the registry is serialised with a mutex and the hit counter is atomic. */ namespace GdbBp { diff --git a/src/game/Debug/GdbServer/GdbServer.cpp b/src/game/Debug/GdbServer/GdbServer.cpp index db216a224..1f42d7c63 100644 --- a/src/game/Debug/GdbServer/GdbServer.cpp +++ b/src/game/Debug/GdbServer/GdbServer.cpp @@ -311,10 +311,24 @@ void GdbServer::EnterStop(const char* reason) void GdbServer::EnterBreak(const char* reason) { + if (!m_enabled) + { + return; + } + // World-thread-only: EnterStop mutates unguarded RSP state and pauses + // the world by blocking this very thread. A breakpoint raised from the + // DB worker or an ACE network thread must not enter it; the hit was + // already counted by the caller, so just return. The acquire load pairs + // with the release store in OnWorldUpdate. + if (!m_worldThreadLatched.load(std::memory_order_acquire) || + std::this_thread::get_id() != m_worldThreadId) + { + return; + } // Only break when a debugger is attached — otherwise no one could resume // the server and the world would hang. Never nest stops (a breakpoint may // fire from a command executed while already stopped). - if (!m_enabled || m_inStop || !DebuggerAttached()) + if (m_inStop || !DebuggerAttached()) { return; } @@ -328,6 +342,14 @@ void GdbServer::OnWorldUpdate() return; } + // This is the world thread: latch its identity on the first tick so + // EnterBreak can reject breakpoints raised from foreign threads. + if (!m_worldThreadLatched.load(std::memory_order_acquire)) + { + m_worldThreadId = std::this_thread::get_id(); + m_worldThreadLatched.store(true, std::memory_order_release); + } + if (m_resetPending.exchange(false)) { GdbRsp::ResetSession(); diff --git a/src/game/Debug/GdbServer/GdbServer.h b/src/game/Debug/GdbServer/GdbServer.h index 35d235046..5bd4dab7f 100644 --- a/src/game/Debug/GdbServer/GdbServer.h +++ b/src/game/Debug/GdbServer/GdbServer.h @@ -36,6 +36,7 @@ #include #include #include +#include #include /** @@ -136,6 +137,11 @@ class GdbServer // True while inside the stop loop; blocks a breakpoint that fires from // a command run during the stop from nesting another stop. bool m_inStop = false; + // World-thread identity, latched on the first OnWorldUpdate tick so + // EnterBreak can reject breakpoints raised from foreign threads. The + // id is written once before the latch flag is released. + std::atomic m_worldThreadLatched{false}; + std::thread::id m_worldThreadId; // Set by AttachRsp (network thread); consumed by OnWorldUpdate so all // RSP engine state mutation happens on the world thread. std::atomic m_resetPending{false};