Skip to content

fix(memory): Fix memory leak for RecorderClass::m_crcInfo #2713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Caball009 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix(memory): Fix memory leak for RecorderClass::m_crcInfo #2713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
34a7d6f
Moved 'CRCInfo' class from .cpp to .h file.
Caball009 May 14, 2026
59ae549
Fixed compilation.
Caball009 May 14, 2026
c5e3079
Changed 'm_crcInfo' from 'CRCInfo*' to 'CRCInfo'.
Caball009 May 14, 2026
2eaa020
Moved functions to .cpp file.
Caball009 May 15, 2026
ed1352d
Made 'CRCInfo' class part of 'RecorderClass' class, and made some twe…
Caball009 May 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 40 additions & 6 deletions GeneralsMD/Code/GameEngine/Include/Common/Recorder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,50 @@ enum RecorderModeType CPP_11(: Int) {
RECORDERMODETYPE_NONE // this is a valid state to be in on the shell map, or in saved games
};

class CRCInfo;

class RecorderClass : public SubsystemInterface {
Copy link
Copy Markdown

@xezon xezon May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While at it

class RecorderClass : public SubsystemInterface
{

protected:
// TheSuperHackers @info helmutbuhler 03/04/2025
// Some info about CRC:
// In each game, each peer periodically calculates a CRC from the local gamestate and sends that
// in a message to all peers (including itself) so that everyone can check that the crc is synchronous.
// In a network game, there is a delay between sending the CRC message and receiving it. This is
// necessary because if you were to wait each frame for all messages from all peers, things would go
// horribly slow.
// But this delay is not a problem for CRC checking because everyone receives the CRC in the same frame
// and every peer just makes sure all the received CRCs are equal.
// While playing replays, this is a problem however: The CRC messages in the replays appear on the frame
// they were received, which can be a few frames delayed if it was a network game. And if we were to
// compare those with the local gamestate, they wouldn't sync up.
// So, in order to fix this, we need to queue up our local CRCs,
// so that we can check it with the crc messages that come later.
// This class is basically that queue.
Copy link
Copy Markdown

@xezon xezon May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While at it, perhaps compact the format of the massive comment a bit.

class CRCInfo
{
public:
CRCInfo();
CRCInfo(UnsignedInt localPlayer, Bool isMultiplayer);
void addCRC(UnsignedInt val);
UnsignedInt readCRC();

int GetQueueSize() const { return m_data.size(); }

UnsignedInt getLocalPlayer() { return m_localPlayer; }

Copy link
Copy Markdown

@xezon xezon May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can remove the empty lines here for compactness.

void setSawCRCMismatch() { m_sawCRCMismatch = TRUE; }
Bool sawCRCMismatch() const { return m_sawCRCMismatch; }

protected:
Bool m_sawCRCMismatch;
Bool m_skippedOne;
UnsignedInt m_localPlayer;
std::list<UnsignedInt> m_data;
};

CRCInfo m_crcInfo;

public:
struct ReplayHeader;

public:
RecorderClass(); ///< Constructor.
virtual ~RecorderClass() override; ///< Destructor.

Expand All @@ -86,9 +123,6 @@ class RecorderClass : public SubsystemInterface {

public:
void handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool fromPlayback);
protected:
CRCInfo *m_crcInfo;
public:

// read in info relating to a replay, conditionally setting up m_file for playback
struct ReplayHeader
Expand Down
139 changes: 54 additions & 85 deletions GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,50 @@ static FILE* openStatsLogFile()
}
#endif

RecorderClass::CRCInfo::CRCInfo() :
m_sawCRCMismatch(FALSE),
Copy link
Copy Markdown

@xezon xezon May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While at it, prefer format like:

RecorderClass::CRCInfo::CRCInfo()
	: m_sawCRCMismatch(FALSE)
	, m_skippedOne(FALSE)

m_skippedOne(FALSE),
m_localPlayer(0)
{}

RecorderClass::CRCInfo::CRCInfo(UnsignedInt localPlayer, Bool isMultiplayer)
{
m_sawCRCMismatch = FALSE;
m_skippedOne = !isMultiplayer;
m_localPlayer = localPlayer;
}

void RecorderClass::CRCInfo::addCRC(UnsignedInt val)
{
// TheSuperHackers @fix helmutbuhler 03/04/2025
// In Multiplayer, the first MSG_LOGIC_CRC message somehow doesn't make it through the network.
// Perhaps this happens because the network is not yet set up on frame 0.
// So we also don't queue up the first local crc message, otherwise the crc
// messages wouldn't match up anymore and we'd desync immediately during playback.
if (!m_skippedOne)
{
m_skippedOne = TRUE;
return;
}

m_data.push_back(val);
//DEBUG_LOG(("CRCInfo::addCRC() - crc %8.8X pushes list to %d entries (full=%d)", val, m_data.size(), !m_data.empty()));
}

UnsignedInt RecorderClass::CRCInfo::readCRC()
{
if (m_data.empty())
{
DEBUG_LOG(("CRCInfo::readCRC() - bailing, full=0, size=%d", m_data.size()));
return 0;
}

UnsignedInt val = m_data.front();
m_data.pop_front();
//DEBUG_LOG(("CRCInfo::readCRC() - returning %8.8X, full=%d, size=%d", val, !m_data.empty(), m_data.size()));
return val;
}

void RecorderClass::logGameStart(AsciiString options)
{
if (!m_file)
Expand Down Expand Up @@ -935,96 +979,21 @@ AsciiString RecorderClass::getCurrentReplayFilename()
return AsciiString::TheEmptyString;
}

// TheSuperHackers @info helmutbuhler 03/04/2025
// Some info about CRC:
// In each game, each peer periodically calculates a CRC from the local gamestate and sends that
// in a message to all peers (including itself) so that everyone can check that the crc is synchronous.
// In a network game, there is a delay between sending the CRC message and receiving it. This is
// necessary because if you were to wait each frame for all messages from all peers, things would go
// horribly slow.
// But this delay is not a problem for CRC checking because everyone receives the CRC in the same frame
// and every peer just makes sure all the received CRCs are equal.
// While playing replays, this is a problem however: The CRC messages in the replays appear on the frame
// they were received, which can be a few frames delayed if it was a network game. And if we were to
// compare those with the local gamestate, they wouldn't sync up.
// So, in order to fix this, we need to queue up our local CRCs,
// so that we can check it with the crc messages that come later.
// This class is basically that queue.
class CRCInfo
{
public:
CRCInfo(UnsignedInt localPlayer, Bool isMultiplayer);
void addCRC(UnsignedInt val);
UnsignedInt readCRC();

int GetQueueSize() const { return m_data.size(); }

UnsignedInt getLocalPlayer() { return m_localPlayer; }

void setSawCRCMismatch() { m_sawCRCMismatch = TRUE; }
Bool sawCRCMismatch() const { return m_sawCRCMismatch; }

protected:

Bool m_sawCRCMismatch;
Bool m_skippedOne;
std::list<UnsignedInt> m_data;
UnsignedInt m_localPlayer;
};

CRCInfo::CRCInfo(UnsignedInt localPlayer, Bool isMultiplayer)
{
m_localPlayer = localPlayer;
m_skippedOne = !isMultiplayer;
m_sawCRCMismatch = FALSE;
}

void CRCInfo::addCRC(UnsignedInt val)
{
// TheSuperHackers @fix helmutbuhler 03/04/2025
// In Multiplayer, the first MSG_LOGIC_CRC message somehow doesn't make it through the network.
// Perhaps this happens because the network is not yet set up on frame 0.
// So we also don't queue up the first local crc message, otherwise the crc
// messages wouldn't match up anymore and we'd desync immediately during playback.
if (!m_skippedOne)
{
m_skippedOne = TRUE;
return;
}

m_data.push_back(val);
//DEBUG_LOG(("CRCInfo::addCRC() - crc %8.8X pushes list to %d entries (full=%d)", val, m_data.size(), !m_data.empty()));
}

UnsignedInt CRCInfo::readCRC()
{
if (m_data.empty())
{
DEBUG_LOG(("CRCInfo::readCRC() - bailing, full=0, size=%d", m_data.size()));
return 0;
}

UnsignedInt val = m_data.front();
m_data.pop_front();
//DEBUG_LOG(("CRCInfo::readCRC() - returning %8.8X, full=%d, size=%d", val, !m_data.empty(), m_data.size()));
return val;
}

Bool RecorderClass::sawCRCMismatch() const
{
return m_crcInfo->sawCRCMismatch();
return m_crcInfo.sawCRCMismatch();
}

void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool fromPlayback)
{
if (fromPlayback)
{
//DEBUG_LOG(("RecorderClass::handleCRCMessage() - Adding CRC of %X from %d to m_crcInfo", newCRC, playerIndex));
m_crcInfo->addCRC(newCRC);
m_crcInfo.addCRC(newCRC);
return;
}

Int localPlayerIndex = m_crcInfo->getLocalPlayer();
Int localPlayerIndex = m_crcInfo.getLocalPlayer();
Bool samePlayer = FALSE;
AsciiString playerName;
playerName.format("player%d", localPlayerIndex);
Expand All @@ -1033,10 +1002,10 @@ void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool f
samePlayer = TRUE;
if (samePlayer || (localPlayerIndex < 0))
{
UnsignedInt playbackCRC = m_crcInfo->readCRC();
UnsignedInt playbackCRC = m_crcInfo.readCRC();
//DEBUG_LOG(("RecorderClass::handleCRCMessage() - Comparing CRCs of InGame:%8.8X Replay:%8.8X Frame:%d from Player %d",
// playbackCRC, newCRC, TheGameLogic->getFrame()-m_crcInfo->GetQueueSize()-1, playerIndex));
if (TheGameLogic->getFrame() > 0 && newCRC != playbackCRC && !m_crcInfo->sawCRCMismatch())
// playbackCRC, newCRC, TheGameLogic->getFrame()-m_crcInfo.GetQueueSize()-1, playerIndex));
if (TheGameLogic->getFrame() > 0 && newCRC != playbackCRC && !m_crcInfo.sawCRCMismatch())
{
//Kris: Patch 1.01 November 10, 2003 (integrated changes from Matt Campbell)
// Since we don't seem to have any *visible* desyncs when replaying games, but get this warning
Expand All @@ -1051,7 +1020,7 @@ void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool f
// TheSuperHackers @info helmutbuhler 03/04/2025
// Note: We subtract the queue size from the frame number. This way we calculate the correct frame
// the mismatch first happened in case the NetCRCInterval is set to 1 during the game.
const UnsignedInt mismatchFrame = TheGameLogic->getFrame() - m_crcInfo->GetQueueSize() - 1;
const UnsignedInt mismatchFrame = TheGameLogic->getFrame() - m_crcInfo.GetQueueSize() - 1;

// Now also prints a UI message for it.
const UnicodeString mismatchDetailsStr = TheGameText->FETCH_OR_SUBSTITUTE("GUI:CRCMismatchDetails", L"InGame:%8.8X Replay:%8.8X Frame:%d");
Expand All @@ -1073,7 +1042,7 @@ void RecorderClass::handleCRCMessage(UnsignedInt newCRC, Int playerIndex, Bool f
TheGameLogic->setGamePaused(pause, pauseMusic, pauseInput);

// Mark this mismatch as seen when we had the chance to pause once.
m_crcInfo->setSawCRCMismatch();
m_crcInfo.setSawCRCMismatch();
}
}
return;
Expand Down Expand Up @@ -1195,9 +1164,9 @@ Bool RecorderClass::playbackFile(AsciiString filename)
#endif

Bool isMultiplayer = m_gameInfo.getSlot(header.localPlayerIndex)->getIP() != 0;
m_crcInfo = NEW CRCInfo(header.localPlayerIndex, isMultiplayer);
m_crcInfo = CRCInfo(header.localPlayerIndex, isMultiplayer);
Comment thread
Caball009 marked this conversation as resolved.
REPLAY_CRC_INTERVAL = m_gameInfo.getCRCInterval();
DEBUG_LOG(("Player index is %d, replay CRC interval is %d", m_crcInfo->getLocalPlayer(), REPLAY_CRC_INTERVAL));
DEBUG_LOG(("Player index is %d, replay CRC interval is %d", m_crcInfo.getLocalPlayer(), REPLAY_CRC_INTERVAL));

Int difficulty = 0;
m_file->read(&difficulty, sizeof(difficulty));
Expand Down
Loading