Skip to content

Commit

Permalink
Fix memory leak in WorldSocket.cpp
Browse files Browse the repository at this point in the history
  • Loading branch information
@namreeb
Jose Quinteiro authored and namreeb committed Oct 28, 2016
1 parent 409f11f commit 964387f
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 43 deletions.
50 changes: 22 additions & 28 deletions src/game/WorldSession.cpp
View file
Expand Up @@ -97,10 +97,6 @@ WorldSession::~WorldSession()
///- unload player if not unloaded
if (_player)
LogoutPlayer(true);

///- empty incoming packet queue
for (auto const packet : m_recvQueue)
delete packet;
}

void WorldSession::SizeError(WorldPacket const& packet, uint32 size) const
Expand Down Expand Up @@ -161,28 +157,28 @@ void WorldSession::SendPacket(WorldPacket const* packet)
}

/// Add an incoming packet to the queue
void WorldSession::QueuePacket(WorldPacket* new_packet)
void WorldSession::QueuePacket(std::unique_ptr<WorldPacket> new_packet)
{
std::lock_guard<std::mutex> guard(m_recvQueueLock);
m_recvQueue.push_back(new_packet);
m_recvQueue.push_back(std::move(new_packet));
}

/// Logging helper for unexpected opcodes
void WorldSession::LogUnexpectedOpcode(WorldPacket* packet, const char* reason)
void WorldSession::LogUnexpectedOpcode(WorldPacket const& packet, const char* reason)
{
sLog.outError("SESSION: received unexpected opcode %s (0x%.4X) %s",
packet->GetOpcodeName(),
packet->GetOpcode(),
packet.GetOpcodeName(),
packet.GetOpcode(),
reason);
}

/// Logging helper for unexpected opcodes
void WorldSession::LogUnprocessedTail(WorldPacket* packet)
void WorldSession::LogUnprocessedTail(WorldPacket &packet)
{
sLog.outError("SESSION: opcode %s (0x%.4X) have unprocessed tail data (read stop at " SIZEFMTD " from " SIZEFMTD ")",
packet->GetOpcodeName(),
packet->GetOpcode(),
packet->rpos(), packet->wpos());
packet.GetOpcodeName(),
packet.GetOpcode(),
packet.rpos(), packet.wpos());
}

/// Update the WorldSession (triggered by World update)
Expand All @@ -194,7 +190,7 @@ bool WorldSession::Update(PacketFilter& updater)
/// not process packets if socket already closed
while (m_Socket && !m_Socket->IsClosed() && !m_recvQueue.empty())
{
WorldPacket *packet = m_recvQueue.front();
auto const packet = std::move(m_recvQueue.front());
m_recvQueue.pop_front();

/*#if 1
Expand All @@ -213,35 +209,35 @@ bool WorldSession::Update(PacketFilter& updater)
{
// skip STATUS_LOGGEDIN opcode unexpected errors if player logout sometime ago - this can be network lag delayed packets
if (!m_playerRecentlyLogout)
LogUnexpectedOpcode(packet, "the player has not logged in yet");
LogUnexpectedOpcode(*packet, "the player has not logged in yet");
}
else if (_player->IsInWorld())
ExecuteOpcode(opHandle, packet);
ExecuteOpcode(opHandle, *packet);

// lag can cause STATUS_LOGGEDIN opcodes to arrive after the player started a transfer
break;
case STATUS_LOGGEDIN_OR_RECENTLY_LOGGEDOUT:
if (!_player && !m_playerRecentlyLogout)
{
LogUnexpectedOpcode(packet, "the player has not logged in yet and not recently logout");
LogUnexpectedOpcode(*packet, "the player has not logged in yet and not recently logout");
}
else
// not expected _player or must checked in packet hanlder
ExecuteOpcode(opHandle, packet);
ExecuteOpcode(opHandle, *packet);
break;
case STATUS_TRANSFER:
if (!_player)
LogUnexpectedOpcode(packet, "the player has not logged in yet");
LogUnexpectedOpcode(*packet, "the player has not logged in yet");
else if (_player->IsInWorld())
LogUnexpectedOpcode(packet, "the player is still in world");
LogUnexpectedOpcode(*packet, "the player is still in world");
else
ExecuteOpcode(opHandle, packet);
ExecuteOpcode(opHandle, *packet);
break;
case STATUS_AUTHED:
// prevent cheating with skip queue wait
if (m_inQueue)
{
LogUnexpectedOpcode(packet, "the player not pass queue yet");
LogUnexpectedOpcode(*packet, "the player not pass queue yet");
break;
}

Expand All @@ -250,7 +246,7 @@ bool WorldSession::Update(PacketFilter& updater)
if (packet->GetOpcode() != CMSG_SET_ACTIVE_VOICE_CHANNEL)
m_playerRecentlyLogout = false;

ExecuteOpcode(opHandle, packet);
ExecuteOpcode(opHandle, *packet);
break;
case STATUS_NEVER:
sLog.outError("SESSION: received not allowed opcode %s (0x%.4X)",
Expand Down Expand Up @@ -287,8 +283,6 @@ bool WorldSession::Update(PacketFilter& updater)
KickPlayer();
}
}

delete packet;
}

// check if we are safe to proceed with logout
Expand Down Expand Up @@ -947,14 +941,14 @@ void WorldSession::SendRedirectClient(std::string& ip, uint16 port)
SendPacket(&pkt);
}

void WorldSession::ExecuteOpcode(OpcodeHandler const& opHandle, WorldPacket* packet)
void WorldSession::ExecuteOpcode(OpcodeHandler const& opHandle, WorldPacket &packet)
{
// need prevent do internal far teleports in handlers because some handlers do lot steps
// or call code that can do far teleports in some conditions unexpectedly for generic way work code
if (_player)
_player->SetCanDelayTeleport(true);

(this->*opHandle.handler)(*packet);
(this->*opHandle.handler)(packet);

if (_player)
{
Expand All @@ -967,7 +961,7 @@ void WorldSession::ExecuteOpcode(OpcodeHandler const& opHandle, WorldPacket* pac
_player->TeleportTo(_player->m_teleport_dest, _player->m_teleport_options);
}

if (packet->rpos() < packet->wpos() && sLog.HasLogLevelOrHigher(LOG_LVL_DEBUG))
if (packet.rpos() < packet.wpos() && sLog.HasLogLevelOrHigher(LOG_LVL_DEBUG))
LogUnprocessedTail(packet);
}

Expand Down
11 changes: 6 additions & 5 deletions src/game/WorldSession.h
View file
Expand Up @@ -32,6 +32,7 @@

#include <deque>
#include <mutex>
#include <memory>

struct ItemPrototype;
struct AuctionEntry;
Expand Down Expand Up @@ -294,7 +295,7 @@ class WorldSession
void LogoutPlayer(bool Save);
void KickPlayer();

void QueuePacket(WorldPacket* new_packet);
void QueuePacket(std::unique_ptr<WorldPacket> new_packet);

bool Update(PacketFilter& updater);

Expand Down Expand Up @@ -881,11 +882,11 @@ class WorldSession
bool VerifyMovementInfo(MovementInfo const& movementInfo, ObjectGuid const& guid) const;
void HandleMoverRelocation(MovementInfo& movementInfo);

void ExecuteOpcode(OpcodeHandler const& opHandle, WorldPacket* packet);
void ExecuteOpcode(OpcodeHandler const& opHandle, WorldPacket & packet);

// logging helper
void LogUnexpectedOpcode(WorldPacket* packet, const char* reason);
void LogUnprocessedTail(WorldPacket* packet);
void LogUnexpectedOpcode(WorldPacket const& packet, const char* reason);
void LogUnprocessedTail(WorldPacket &packet);

uint32 m_GUIDLow; // set logged or recently logout player (while m_playerRecentlyLogout set)
Player * _player;
Expand All @@ -911,7 +912,7 @@ class WorldSession
AddonsList m_addonsList;

std::mutex m_recvQueueLock;
std::deque<WorldPacket *> m_recvQueue;
std::deque<std::unique_ptr<WorldPacket>> m_recvQueue;
};
#endif
/// @}
8 changes: 4 additions & 4 deletions src/game/WorldSocket.cpp
View file
Expand Up @@ -97,7 +97,7 @@ void WorldSocket::SendPacket(const WorldPacket& pct, bool immediate)
return;

// Dump outgoing packet.
sLog.outWorldPacketDump(GetRemoteEndpoint().c_str(), pct.GetOpcode(), pct.GetOpcodeName(), &pct, false);
sLog.outWorldPacketDump(GetRemoteEndpoint().c_str(), pct.GetOpcode(), pct.GetOpcodeName(), pct, false);

ServerPktHeader header(pct.size() + 2, pct.GetOpcode());
m_crypt.EncryptSend((uint8*)header.header, header.getHeaderLength());
Expand Down Expand Up @@ -192,15 +192,15 @@ bool WorldSocket::ProcessIncomingData()
if (IsClosed())
return false;

WorldPacket *pct = new WorldPacket(opcode, validBytesRemaining);
std::unique_ptr<WorldPacket> pct(new WorldPacket(opcode, validBytesRemaining));

if (validBytesRemaining)
{
pct->append(InPeak(), validBytesRemaining);
ReadSkip(validBytesRemaining);
}

sLog.outWorldPacketDump(GetRemoteEndpoint().c_str(), pct->GetOpcode(), pct->GetOpcodeName(), pct, true);
sLog.outWorldPacketDump(GetRemoteEndpoint().c_str(), pct->GetOpcode(), pct->GetOpcodeName(), *pct, true);

try
{
Expand Down Expand Up @@ -231,7 +231,7 @@ bool WorldSocket::ProcessIncomingData()
return false;
}

m_session->QueuePacket(pct);
m_session->QueuePacket(std::move(pct));

return true;
}
Expand Down
10 changes: 5 additions & 5 deletions src/shared/Log.cpp
View file
Expand Up @@ -888,7 +888,7 @@ void Log::outErrorScriptLib(const char* err, ...)
fflush(stderr);
}

void Log::outWorldPacketDump(const char* socket, uint32 opcode, char const* opcodeName, ByteBuffer const* packet, bool incoming)
void Log::outWorldPacketDump(const char* socket, uint32 opcode, char const* opcodeName, ByteBuffer const &packet, bool incoming)
{
if (!worldLogfile)
return;
Expand All @@ -899,13 +899,13 @@ void Log::outWorldPacketDump(const char* socket, uint32 opcode, char const* opco

fprintf(worldLogfile, "\n%s:\nSOCKET: %s\nLENGTH: %u\nOPCODE: %s (0x%.4X)\nDATA:\n",
incoming ? "CLIENT" : "SERVER",
socket, static_cast<uint32>(packet->size()), opcodeName, opcode);
socket, static_cast<uint32>(packet.size()), opcodeName, opcode);

size_t p = 0;
while (p < packet->size())
while (p < packet.size())
{
for (size_t j = 0; j < 16 && p < packet->size(); ++j)
fprintf(worldLogfile, "%.2X ", (*packet)[p++]);
for (size_t j = 0; j < 16 && p < packet.size(); ++j)
fprintf(worldLogfile, "%.2X ", packet[p++]);

fprintf(worldLogfile, "\n");
}
Expand Down
2 changes: 1 addition & 1 deletion src/shared/Log.h
View file
Expand Up @@ -162,7 +162,7 @@ class Log : public MaNGOS::Singleton<Log, MaNGOS::ClassLevelLockable<Log, std::m
// any log level
void outErrorScriptLib(const char* str, ...) ATTR_PRINTF(2, 3);

void outWorldPacketDump(const char* socket, uint32 opcode, char const* opcodeName, ByteBuffer const* packet, bool incoming);
void outWorldPacketDump(const char* socket, uint32 opcode, char const* opcodeName, ByteBuffer const &packet, bool incoming);
// any log level
void outCharDump(const char* str, uint32 account_id, uint32 guid, const char* name);
void outRALog(const char* str, ...) ATTR_PRINTF(2, 3);
Expand Down

0 comments on commit 964387f

Please sign in to comment.