diff --git a/src/game/Debug/GdbServer/GdbServer.cpp b/src/game/Debug/GdbServer/GdbServer.cpp index db216a224..f01f20cc6 100644 --- a/src/game/Debug/GdbServer/GdbServer.cpp +++ b/src/game/Debug/GdbServer/GdbServer.cpp @@ -151,15 +151,22 @@ void GdbServer::FeedRsp(const uint8* data, uint32 len) m_inbound.emplace_back(data, data + len); } -void GdbServer::SubmitMonitorLine(void* ctx, MonWriter writer, const char* line) +void GdbServer::SubmitMonitorLine(void* ctx, MonWriter writer, MonRefFn addRef, + MonRefFn release, const char* line) { - if (line == nullptr || writer == nullptr) + if (line == nullptr || writer == nullptr || addRef == nullptr || + release == nullptr) { return; } + // Pin ctx alive until DrainMonitorRequests (or queue teardown) releases + // it — otherwise a socket close racing the world-thread drain would + // free ctx while this request still references it. + addRef(ctx); MonitorReq req; req.ctx = ctx; req.writer = writer; + req.release = release; req.line = line; std::lock_guard guard(m_monLock); m_monitor.push_back(std::move(req)); @@ -194,6 +201,9 @@ void GdbServer::DrainMonitorRequests() { req.writer(req.ctx, w.Data()); } + // req's destructor releases the pin taken in SubmitMonitorLine here, + // now that the write attempt (safe even if the socket already + // started closing) is done. } } diff --git a/src/game/Debug/GdbServer/GdbServer.h b/src/game/Debug/GdbServer/GdbServer.h index 35d235046..fb8e61dcb 100644 --- a/src/game/Debug/GdbServer/GdbServer.h +++ b/src/game/Debug/GdbServer/GdbServer.h @@ -63,6 +63,10 @@ class GdbServer public: typedef void (*RspWriter)(void* ctx, const uint8* data, uint32 len); typedef void (*MonWriter)(void* ctx, const char* text); + /// Pins/unpins the lifetime of a monitor connection's @p ctx (network + /// thread's reference-counted socket) so a queued request can never + /// outlive the object it targets; see SubmitMonitorLine. + typedef void (*MonRefFn)(void* ctx); static GdbServer& Instance(); @@ -83,8 +87,14 @@ class GdbServer void FeedRsp(const uint8* data, uint32 len); /// Submit one plain-text monitor line ("mangos ...") for execution on - /// the world thread; the reply is delivered via @p writer. - void SubmitMonitorLine(void* ctx, MonWriter writer, const char* line); + /// the world thread; the reply is delivered via @p writer. @p addRef + /// is called synchronously (network thread) to pin @p ctx alive for + /// as long as the request is queued/in-flight; @p release is called + /// exactly once, after the request has been serviced or dropped, to + /// undo that pin. This prevents a use-after-free when the socket + /// closes before the world thread drains the request. + void SubmitMonitorLine(void* ctx, MonWriter writer, MonRefFn addRef, + MonRefFn release, const char* line); // --- world thread side --------------------------------------------- @@ -116,10 +126,62 @@ class GdbServer void EnterStop(const char* reason); void CaptureContext(GdbRsp::RegSnapshot& out); + // Move-only: owns one pinning reference (taken by SubmitMonitorLine + // via addRef) on ctx and releases it exactly once — on normal + // servicing, on drop, or when leftover requests are torn down along + // with the queue itself (e.g. at shutdown) — so ctx can never be + // freed while a request still points at it. struct MonitorReq { + MonitorReq() = default; + MonitorReq(const MonitorReq&) = delete; + MonitorReq& operator=(const MonitorReq&) = delete; + + MonitorReq(MonitorReq&& other) noexcept + : ctx(other.ctx), writer(other.writer), + release(other.release), line(std::move(other.line)) + { + other.ctx = nullptr; + other.writer = nullptr; + other.release = nullptr; + } + + MonitorReq& operator=(MonitorReq&& other) noexcept + { + if (this != &other) + { + ReleasePin(); + ctx = other.ctx; + writer = other.writer; + release = other.release; + line = std::move(other.line); + other.ctx = nullptr; + other.writer = nullptr; + other.release = nullptr; + } + return *this; + } + + ~MonitorReq() + { + ReleasePin(); + } + + /// Undo the pinning reference exactly once; safe to call + /// unconditionally (no-op after the first call or on a + /// moved-from instance). + void ReleasePin() + { + if (release != nullptr) + { + release(ctx); + release = nullptr; + } + } + void* ctx = nullptr; MonWriter writer = nullptr; + MonRefFn release = nullptr; std::string line; }; diff --git a/src/mangosd/GdbServerThread.cpp b/src/mangosd/GdbServerThread.cpp index 9c0130b26..b23b5ce4b 100644 --- a/src/mangosd/GdbServerThread.cpp +++ b/src/mangosd/GdbServerThread.cpp @@ -277,6 +277,7 @@ class GdbMonSocket : public GdbSocketBase if (i > lineStart) { sGdbServer.SubmitMonitorLine(this, &GdbMonSocket::WriteTextThunk, + &GdbMonSocket::AddRefThunk, &GdbMonSocket::ReleaseThunk, inputBuffer + lineStart); } lineStart = i + 1; @@ -309,6 +310,19 @@ class GdbMonSocket : public GdbSocketBase static_cast(ctx)->sendCStr(text); } + // Pin/unpin this socket's ACE reference count around a queued + // monitor request so GdbServer can safely hold ctx across the + // network->world thread hop even if the connection closes first. + static void AddRefThunk(void* ctx) + { + static_cast(ctx)->add_reference(); + } + + static void ReleaseThunk(void* ctx) + { + static_cast(ctx)->remove_reference(); + } + char inputBuffer[MON_BUFF_SIZE]; uint32 inputBufferLen; };