fix(recorder): Fix memory leak of RecorderClass::m_crcInfo (TheSuperHackers#2713)

Author: Caball009

Generals/Code/GameEngine/Include/Common/Recorder.h

@@ -53,13 +53,39 @@ 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
+{
+protected:
+	// TheSuperHackers @info helmutbuhler 03/04/2025 CRC overview:
+	// Each peer periodically computes a CRC from its local game state and broadcasts it to all peers, including itself,
+	// to verify synchronization. CRC messages are received a few frames later in network games to avoid stalling every
+	// frame while waiting for all peers. This works because all peers compare the same received CRCs on the same frame.
+	//
+	// Replays are different: recorded CRC messages appear on the frame they were originally received, so directly
+	// comparing them against the current local state would mismatch. To handle this, local CRCs must be queued until the
+	// corresponding replay CRC messages arrive. This class implements that queue.
+	class CRCInfo
+	{
+	public:
+		CRCInfo();
+		CRCInfo(UnsignedInt localPlayer, Bool isMultiplayer);
+		void addCRC(UnsignedInt val);
+		UnsignedInt readCRC();
+		int GetQueueSize() const { return m_data.size(); }
+		UnsignedInt getLocalPlayer() const { return m_localPlayer; }
+		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;
+	};
 
-class RecorderClass : public SubsystemInterface {
 public:
 	struct ReplayHeader;
 
-public:
 	RecorderClass();																	///< Constructor.
 	virtual ~RecorderClass() override;													///< Destructor.
 
@@ -86,9 +112,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
@@ -158,6 +181,7 @@ class RecorderClass : public SubsystemInterface {
 
 	CullBadCommandsResult cullBadCommands(); ///< prevent the user from giving mouse commands that he shouldn't be able to do during playback.
 
+	CRCInfo m_crcInfo;
 	File* m_file;
 	AsciiString m_fileName;
 	Int m_currentFilePosition;

Generals/Code/GameEngine/Source/Common/Recorder.cpp

@@ -100,6 +100,50 @@ static FILE* openStatsLogFile()
 }
 #endif
 
+RecorderClass::CRCInfo::CRCInfo() :
+	m_sawCRCMismatch(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)
@@ -933,96 +977,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);
@@ -1031,10 +1000,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())
 		{
 			// Since we don't seem to have any *visible* desyncs when replaying games, but get this warning
 			// virtually every replay, the assumption is our CRC checking is faulty.  Since we're at the
@@ -1048,7 +1017,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");
@@ -1070,7 +1039,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;
@@ -1192,9 +1161,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);
 	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));

GeneralsMD/Code/GameEngine/Include/Common/Recorder.h

@@ -53,13 +53,39 @@ 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
+{
+protected:
+	// TheSuperHackers @info helmutbuhler 03/04/2025 CRC overview:
+	// Each peer periodically computes a CRC from its local game state and broadcasts it to all peers, including itself,
+	// to verify synchronization. CRC messages are received a few frames later in network games to avoid stalling every
+	// frame while waiting for all peers. This works because all peers compare the same received CRCs on the same frame.
+	//
+	// Replays are different: recorded CRC messages appear on the frame they were originally received, so directly
+	// comparing them against the current local state would mismatch. To handle this, local CRCs must be queued until the
+	// corresponding replay CRC messages arrive. This class implements that queue.
+	class CRCInfo
+	{
+	public:
+		CRCInfo();
+		CRCInfo(UnsignedInt localPlayer, Bool isMultiplayer);
+		void addCRC(UnsignedInt val);
+		UnsignedInt readCRC();
+		int GetQueueSize() const { return m_data.size(); }
+		UnsignedInt getLocalPlayer() const { return m_localPlayer; }
+		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;
+	};
 
-class RecorderClass : public SubsystemInterface {
 public:
 	struct ReplayHeader;
 
-public:
 	RecorderClass();																	///< Constructor.
 	virtual ~RecorderClass() override;													///< Destructor.
 
@@ -86,9 +112,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
@@ -158,6 +181,7 @@ class RecorderClass : public SubsystemInterface {
 
 	CullBadCommandsResult cullBadCommands(); ///< prevent the user from giving mouse commands that he shouldn't be able to do during playback.
 
+	CRCInfo m_crcInfo;
 	File* m_file;
 	AsciiString m_fileName;
 	Int m_currentFilePosition;