Merge 324fca2 into 8e55f0e

Author: H0zen

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);
     }
 }

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<proto::IClientLink> const& link,

src/proto/ClientConnection.cpp

@@ -62,7 +62,7 @@ std::vector<uint8_t> 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<uint8_t> 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<uint8> 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;
 }

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<SessionId> m_traceSession{INVALID_SESSION_ID};
     bool m_authStarted = false;
     std::atomic<bool> m_closed{false};
     net::Sender m_sender;

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<IClientLink> const& link,

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];

src/shared/Log/Log.h

@@ -311,16 +311,16 @@ class Log : public MaNGOS::Singleton<Log>
         /**
          * @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
          *