From 324fca2305f7a2a702917a58a1775fe6391c0331 Mon Sep 17 00:00:00 2001 From: H0zen Date: Thu, 6 Aug 2026 15:32:23 +0300 Subject: [PATCH] [Log] Attribute each packet dump to its session instead of "SOCKET: 0" WorldGateway::TracePacket passed a hardcoded 0 for the socket field, so every packet of every client was logged under the same id. With two clients connected the dump is one interleaved stream that cannot be taken apart, which defeats the purpose: the question a packet dump is usually asked is what ONE observer was handed, and when. Thread the connection's SessionId through IWorldGateway::TracePacket and print it as SESSION. The field is 0 only for the pre-auth handshake, which has no session yet. ClientConnection keeps an atomic copy of the id for tracing. SendPacket traces while holding m_sendOrderLock, and reading m_session there would take m_sessionLock under it, inverting the order HandleAuthSession uses. Co-Authored-By: Claude Opus 5 --- src/game/Server/WorldGateway.cpp | 4 ++-- src/game/Server/WorldGateway.h | 2 +- src/proto/ClientConnection.cpp | 9 ++++++--- src/proto/ClientConnection.h | 5 +++++ src/proto/IWorldGateway.h | 6 +++++- src/shared/Log/Log.cpp | 17 +++++++++-------- src/shared/Log/Log.h | 8 ++++---- 7 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/game/Server/WorldGateway.cpp b/src/game/Server/WorldGateway.cpp index fa789ce04..c6d108fad 100644 --- a/src/game/Server/WorldGateway.cpp +++ b/src/game/Server/WorldGateway.cpp @@ -85,11 +85,11 @@ bool WorldGateway::FilterAuthPacket(WorldPacket& packet) return true; } -void WorldGateway::TracePacket(WorldPacket const& packet, bool incoming) +void WorldGateway::TracePacket(proto::SessionId session, WorldPacket const& packet, bool incoming) { if (sLog.IsPacketLoggingEnabled()) { - sLog.outWorldPacketDump(0, packet.GetOpcode(), + sLog.outWorldPacketDump(session, packet.GetOpcode(), LookupOpcodeName(packet.GetOpcode()), &packet, incoming); } } diff --git a/src/game/Server/WorldGateway.h b/src/game/Server/WorldGateway.h index d96aa0a71..6af085b8c 100644 --- a/src/game/Server/WorldGateway.h +++ b/src/game/Server/WorldGateway.h @@ -38,7 +38,7 @@ class WorldGateway final : public proto::IWorldGateway { public: bool FilterAuthPacket(WorldPacket& packet) override; - void TracePacket(WorldPacket const& packet, bool incoming) override; + void TracePacket(proto::SessionId session, WorldPacket const& packet, bool incoming) override; proto::AuthLookup LookupAccount(proto::AuthRequest const& request) override; proto::SessionId Attach(proto::AuthRequest const& request, std::shared_ptr const& link, diff --git a/src/proto/ClientConnection.cpp b/src/proto/ClientConnection.cpp index 8d0c3f6dd..f3892e83d 100644 --- a/src/proto/ClientConnection.cpp +++ b/src/proto/ClientConnection.cpp @@ -62,7 +62,7 @@ std::vector ClientConnection::onConnect() { WorldPacket challenge(SMSG_AUTH_CHALLENGE, 4); challenge << m_seed; - m_gateway.TracePacket(challenge, false); + m_gateway.TracePacket(INVALID_SESSION_ID, challenge, false); return EncodePacket(challenge); } catch (...) @@ -99,7 +99,8 @@ std::vector ClientConnection::onData( for (size_t i = 0; i < packets.size() && !m_closed.load(); ++i) { - m_gateway.TracePacket(packets[i], true); + m_gateway.TracePacket(m_traceSession.load(std::memory_order_relaxed), + packets[i], true); if (!HandlePacket(packets[i])) { Close(); @@ -126,6 +127,7 @@ void ClientConnection::onClose() session = m_session; m_session = INVALID_SESSION_ID; } + m_traceSession.store(INVALID_SESSION_ID, std::memory_order_relaxed); if (session != INVALID_SESSION_ID) { try @@ -153,7 +155,7 @@ void ClientConnection::SendPacket(WorldPacket const& packet) return; } - m_gateway.TracePacket(packet, false); + m_gateway.TracePacket(m_traceSession.load(std::memory_order_relaxed), packet, false); std::vector const frame = PacketCodec::Encode(packet, [this](uint8* header, std::size_t len) { @@ -290,6 +292,7 @@ bool ClientConnection::HandleAuthSession(WorldPacket& packet) m_gateway.Detach(session); return false; } + m_traceSession.store(session, std::memory_order_relaxed); return true; } diff --git a/src/proto/ClientConnection.h b/src/proto/ClientConnection.h index 0d0f9ac31..cd08cd777 100644 --- a/src/proto/ClientConnection.h +++ b/src/proto/ClientConnection.h @@ -79,6 +79,11 @@ class ClientConnection final : public net::ISession, public IClientLink std::mutex m_sessionLock; uint32 m_seed; SessionId m_session = INVALID_SESSION_ID; + + /// The same id, readable without m_sessionLock: SendPacket traces while holding + /// m_sendOrderLock, and taking m_sessionLock under it would invert the order + /// HandleAuthSession uses. Tracing only -- m_session stays the authority. + std::atomic m_traceSession{INVALID_SESSION_ID}; bool m_authStarted = false; std::atomic m_closed{false}; net::Sender m_sender; diff --git a/src/proto/IWorldGateway.h b/src/proto/IWorldGateway.h index 8b1b81632..9eb6c38c2 100644 --- a/src/proto/IWorldGateway.h +++ b/src/proto/IWorldGateway.h @@ -95,7 +95,11 @@ class IWorldGateway public: virtual ~IWorldGateway() = default; virtual bool FilterAuthPacket(WorldPacket& packet) = 0; - virtual void TracePacket(WorldPacket const& packet, bool incoming) = 0; + + /// `session` is INVALID_SESSION_ID for the pre-auth handshake only. Without it a dump + /// cannot say which of several connected clients a packet belongs to, which is most of + /// what a packet dump is for. + virtual void TracePacket(SessionId session, WorldPacket const& packet, bool incoming) = 0; virtual AuthLookup LookupAccount(AuthRequest const& request) = 0; virtual SessionId Attach(AuthRequest const& request, std::shared_ptr const& link, diff --git a/src/shared/Log/Log.cpp b/src/shared/Log/Log.cpp index 40b641f59..485eb2bed 100644 --- a/src/shared/Log/Log.cpp +++ b/src/shared/Log/Log.cpp @@ -1225,7 +1225,7 @@ void Log::outErrorScriptLib(const char* err, ...) } } -void Log::outWorldPacketDump(uint32 socket, uint32 opcode, char const* opcodeName, ByteBuffer const* packet, bool incoming) +void Log::outWorldPacketDump(uint32 session, uint32 opcode, char const* opcodeName, ByteBuffer const* packet, bool incoming) { if (!worldLogfile) { @@ -1237,18 +1237,19 @@ void Log::outWorldPacketDump(uint32 socket, uint32 opcode, char const* opcodeNam outTimestamp(worldLogfile); // Build the whole hex dump into one buffer and emit it with a single - // fwrite, instead of one fprintf PER BYTE (16+ stdio calls per row). Output - // is byte-identical to the previous format. Durability via Flush()/shutdown. + // fwrite, instead of one fprintf PER BYTE (16+ stdio calls per row). The hex + // body is byte-identical to that format. Durability via Flush()/shutdown. std::string out; out.reserve(packet->size() * 3 + packet->size() / 16 + 128); - // header[512] is ample for the fixed text plus a (short, compile-time) - // opcode-name constant; snprintf is bounded, so output stays byte-identical - // to the previous fprintf for every real opcode name. + // SESSION, not SOCKET: the field carried a hardcoded 0 for every packet of every + // client, so two clients logged as one interleaved stream that could not be taken + // apart. header[512] is ample for the fixed text plus a (short, compile-time) + // opcode-name constant, and snprintf is bounded. char header[512]; - snprintf(header, sizeof(header), "\n%s:\nSOCKET: %u\nLENGTH: %zu\nOPCODE: %s (0x%.4X)\nDATA:\n", + snprintf(header, sizeof(header), "\n%s:\nSESSION: %u\nLENGTH: %zu\nOPCODE: %s (0x%.4X)\nDATA:\n", incoming ? "CLIENT" : "SERVER", - socket, packet->size(), opcodeName, opcode); + session, packet->size(), opcodeName, opcode); out += header; char hexbuf[4]; diff --git a/src/shared/Log/Log.h b/src/shared/Log/Log.h index 082209559..6039f36ae 100644 --- a/src/shared/Log/Log.h +++ b/src/shared/Log/Log.h @@ -311,16 +311,16 @@ class Log : public MaNGOS::Singleton /** * @brief any log level * - * Called from WorldGateway::Deliver (incoming) and WorldSession::SendPacket - * (outgoing) -- see IsPacketLoggingEnabled()'s comment below. + * Called from WorldGateway::TracePacket, i.e. from ClientConnection on the + * network thread in both directions -- see IsPacketLoggingEnabled() below. * - * @param socket + * @param session which client stream the packet belongs to; 0 pre-auth * @param opcode * @param opcodeName * @param packet * @param incoming */ - void outWorldPacketDump(uint32 socket, uint32 opcode, char const* opcodeName, ByteBuffer const* packet, bool incoming); + void outWorldPacketDump(uint32 session, uint32 opcode, char const* opcodeName, ByteBuffer const* packet, bool incoming); /** * @brief any log level *