Skip to content

fix(memory): Fix various memory leaks (2) #2710

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 11 commits into
base: main
Choose a base branch
from
Open

fix(memory): Fix various memory leaks (2) #2710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8f1afe4
Added deletes to prevent potential memory leaks.
Caball009 May 13, 2026
707f194
Fixed 'BattlePlan bonus' memory leak.
Caball009 May 13, 2026
8fe08f6
Fixed memory leak in line renderer texture.
Caball009 May 13, 2026
d713e62
Fixed removed 'Upgrade' memory leak.
Caball009 May 13, 2026
41a3d91
Fixed script list memory leaks.
Caball009 May 13, 2026
d87c02e
Fixed animation memory leak.
Caball009 May 13, 2026
2b1c2d9
Fixed 'QuitMenu' memory leak.
Caball009 May 13, 2026
a49c810
Fixed 'AudioRequest' audio event memory leak.
Caball009 May 13, 2026
2e0fe76
Fixed 'getNoSoundMarker' memory leak.
Caball009 May 14, 2026
229839f
Added workaround for 'TheSkirmishGameInfo' and 'TheChallengeGameInfo'…
Caball009 May 14, 2026
08a2a42
Addressed feedback & made tweaks.
Caball009 May 14, 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
5 changes: 4 additions & 1 deletion Core/GameEngine/Source/Common/Audio/AudioRequest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,8 @@

AudioRequest::~AudioRequest()
{

if (m_usePendingEvent)
Comment thread
xezon marked this conversation as resolved.
{
delete m_pendingEvent;
}
}
38 changes: 29 additions & 9 deletions Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,16 +262,36 @@ void GameLogic::clearGameData( Bool showScoreScreen )
TheScriptActions->closeWindows(FALSE); // Close victory or defeat windows.

Bool shellGame = FALSE;
if ((!isInShellGame() || !isInGame()) && showScoreScreen && !TheGlobalData->m_headless)
if (showScoreScreen)
{
shellGame = TRUE;
TheTransitionHandler->setGroup("FadeWholeScreen");
TheShell->push("Menus/ScoreScreen.wnd");
TheShell->showShell(FALSE); // by passing in false, we don't want to run the Init on the shell screen we just pushed on
TheTransitionHandler->reverse("FadeWholeScreen");

void FixupScoreScreenMovieWindow();
FixupScoreScreenMovieWindow();
if ((!isInShellGame() || !isInGame()) && !TheGlobalData->m_headless)
{
shellGame = TRUE;
TheTransitionHandler->setGroup("FadeWholeScreen");
TheShell->push("Menus/ScoreScreen.wnd");
TheShell->showShell(FALSE); // by passing in false, we don't want to run the Init on the shell screen we just pushed on
TheTransitionHandler->reverse("FadeWholeScreen");

void FixupScoreScreenMovieWindow();
FixupScoreScreenMovieWindow();

destroyQuitMenu();
}

// TheSuperHackers @info The game info may have been allocated on save game load.
// Deallocate it here until there's a better place to do it.
if (TheSkirmishGameInfo)
{
delete TheSkirmishGameInfo;
TheSkirmishGameInfo = nullptr;
TheGameInfo = nullptr;
}
else if (TheChallengeGameInfo)
{
delete TheChallengeGameInfo;
TheChallengeGameInfo = nullptr;
TheGameInfo = nullptr;
}
Comment on lines +283 to +294
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot May 14, 2026

Choose a reason for hiding this comment

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

P1 TheChallengeGameInfo deletion breaks the challenge campaign "Next Mission" flow

clearGameData(TRUE) (which shows the score screen) now unconditionally deletes TheChallengeGameInfo. In ScoreScreen.cpp, startNextCampaignGame() immediately asserts TheChallengeGameInfo != nullptr (line 200) before re-configuring it for the next mission. Since clearGameData() runs before the score screen updates, TheChallengeGameInfo is already null by the time the player clicks "Next Mission", causing an assertion failure / crash on every challenge campaign mission transition.

The same applies to TheSkirmishGameInfo being set to null before ScoreScreenUpdate() uses TheGameInfo (line 428 of ScoreScreen.cpp) to start score-screen music, silently breaking score-screen music for all regular skirmish games.

A targeted fix would guard these deletions to only run when the game info was allocated during a save-game load (e.g. by checking an additional flag or by only deleting when isLoadingSave() was true).

Prompt To Fix With AI 
This is a comment left during a code review.
Path: Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp
Line: 283-294

Comment:
**`TheChallengeGameInfo` deletion breaks the challenge campaign "Next Mission" flow**

`clearGameData(TRUE)` (which shows the score screen) now unconditionally deletes `TheChallengeGameInfo`. In `ScoreScreen.cpp`, `startNextCampaignGame()` immediately asserts `TheChallengeGameInfo != nullptr` (line 200) before re-configuring it for the next mission. Since `clearGameData()` runs _before_ the score screen updates, `TheChallengeGameInfo` is already null by the time the player clicks "Next Mission", causing an assertion failure / crash on every challenge campaign mission transition.

The same applies to `TheSkirmishGameInfo` being set to null before `ScoreScreenUpdate()` uses `TheGameInfo` (line 428 of `ScoreScreen.cpp`) to start score-screen music, silently breaking score-screen music for all regular skirmish games.

A targeted fix would guard these deletions to only run when the game info was allocated during a save-game load (e.g. by checking an additional flag or by only deleting when `isLoadingSave()` was true).

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Author

@Caball009 Caball009 May 14, 2026

Choose a reason for hiding this comment

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

Good point.

A targeted fix would guard these deletions to only run when the game info was allocated during a save-game load (e.g. by checking an additional flag or by only deleting when isLoadingSave() was true).

This doesn't fix the crash that happens when restarting a challenge match from the 'score screen'. I could use some suggestions where these deallocations are better suited to take place.

}

TheGameEngine->reset();
Expand Down
5 changes: 5 additions & 0 deletions Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2236,6 +2236,11 @@ void MilesAudioManager::processRequestList()

if (!req->m_requiresCheckForSample || checkForSample(req)) {
processRequest(req);

// TheSuperHackers @info Deallocating the audio event is no longer responsibility of this request,
// because it was just processed. Reset fields to avoid use after free and double free.
req->m_usePendingEvent = false;
req->m_pendingEvent = nullptr;
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.

Who is deallocating it?

Copy link
Copy Markdown
Author

@Caball009 Caball009 May 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I'll probably rewrite this a bit to make it clearer, and put in a separate PR. MilesAudioManager::playAudioEvent acts as a 'sink' function that can take ownership of the audio event.

}
deleteInstance(req);
it = m_audioRequests.erase(it);
Expand Down
78 changes: 40 additions & 38 deletions Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DModelDraw.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2180,51 +2180,53 @@ void W3DModelDraw::adjustAnimation(const ModelConditionInfo* prevState, Real pre

const W3DAnimationInfo& animInfo = m_curState->m_animations[m_whichAnimInCurState];

HAnimClass* animHandle = animInfo.getAnimHandle(); // note that this now returns an ADDREFED handle, which must be released by the caller!
if (m_renderObject && animHandle)
if (m_renderObject)
{
Int startFrame = 0;
if (m_curState->m_mode == RenderObjClass::ANIM_MODE_ONCE_BACKWARDS ||
m_curState->m_mode == RenderObjClass::ANIM_MODE_LOOP_BACKWARDS)
HAnimClass* animHandle = animInfo.getAnimHandle(); // note that this now returns an ADDREFED handle, which must be released by the caller!
if (animHandle)
{
startFrame = animHandle->Get_Num_Frames()-1;
}
Int startFrame = 0;
if (m_curState->m_mode == RenderObjClass::ANIM_MODE_ONCE_BACKWARDS ||
m_curState->m_mode == RenderObjClass::ANIM_MODE_LOOP_BACKWARDS)
{
startFrame = animHandle->Get_Num_Frames()-1;
}

if (testFlagBit(m_curState->m_flags, RANDOMIZE_START_FRAME))
{
startFrame = GameClientRandomValue(0, animHandle->Get_Num_Frames()-1);
}
else if (testFlagBit(m_curState->m_flags, START_FRAME_FIRST))
{
startFrame = 0;
}
else if (testFlagBit(m_curState->m_flags, START_FRAME_LAST))
{
startFrame = animHandle->Get_Num_Frames()-1;
}
// order is important here: MAINTAIN_FRAME_ACROSS_STATES is overridden by the other bits, above.
else if (isAnyMaintainFrameFlagSet(m_curState->m_flags) &&
prevState &&
prevState != m_curState &&
isAnyMaintainFrameFlagSet(prevState->m_flags) &&
isCommonMaintainFrameFlagSet(m_curState->m_flags, prevState->m_flags) &&
prevAnimFraction >= 0.0)
{
startFrame = REAL_TO_INT(prevAnimFraction * animHandle->Get_Num_Frames()-1);
}
if (testFlagBit(m_curState->m_flags, RANDOMIZE_START_FRAME))
{
startFrame = GameClientRandomValue(0, animHandle->Get_Num_Frames()-1);
}
else if (testFlagBit(m_curState->m_flags, START_FRAME_FIRST))
{
startFrame = 0;
}
else if (testFlagBit(m_curState->m_flags, START_FRAME_LAST))
{
startFrame = animHandle->Get_Num_Frames()-1;
}
// order is important here: MAINTAIN_FRAME_ACROSS_STATES is overridden by the other bits, above.
else if (isAnyMaintainFrameFlagSet(m_curState->m_flags) &&
prevState &&
prevState != m_curState &&
isAnyMaintainFrameFlagSet(prevState->m_flags) &&
isCommonMaintainFrameFlagSet(m_curState->m_flags, prevState->m_flags) &&
prevAnimFraction >= 0.0)
{
startFrame = REAL_TO_INT(prevAnimFraction * animHandle->Get_Num_Frames()-1);
}

m_renderObject->Set_Animation(animHandle, startFrame, m_curState->m_mode);
REF_PTR_RELEASE(animHandle);
animHandle = nullptr;
m_renderObject->Set_Animation(animHandle, startFrame, m_curState->m_mode);
REF_PTR_RELEASE(animHandle);
animHandle = nullptr;

if (m_renderObject->Class_ID() == RenderObjClass::CLASSID_HLOD)
{
HLodClass *hlod = (HLodClass*)m_renderObject;
Real factor = GameClientRandomValueReal( m_curState->m_animMinSpeedFactor, m_curState->m_animMaxSpeedFactor );
hlod->Set_Animation_Frame_Rate_Multiplier( factor );
if (m_renderObject->Class_ID() == RenderObjClass::CLASSID_HLOD)
{
HLodClass *hlod = (HLodClass*)m_renderObject;
Real factor = GameClientRandomValueReal( m_curState->m_animMinSpeedFactor, m_curState->m_animMaxSpeedFactor );
hlod->Set_Animation_Frame_Rate_Multiplier( factor );
}
}
}

}
else
{
Expand Down
62 changes: 42 additions & 20 deletions Core/Libraries/Source/WWVegas/WW3D2/hcanim.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,10 +293,6 @@ int HCompressedAnimClass::Load_W3D(ChunkLoadClass & cload)
/*
** Now, read in all of the other chunks (motion channels).
*/
TimeCodedMotionChannelClass * tc_chan;
AdaptiveDeltaMotionChannelClass * ad_chan;
TimeCodedBitChannelClass * newbitchan;

while (cload.Open_Chunk()) {

switch (cload.Cur_Chunk_ID()) {
Expand All @@ -306,7 +302,8 @@ int HCompressedAnimClass::Load_W3D(ChunkLoadClass & cload)
switch ( Flavor ) {

case ANIM_FLAVOR_TIMECODED:

{
TimeCodedMotionChannelClass* tc_chan = nullptr;
if (!read_channel(cload,&tc_chan)) {
goto Error;
}
Expand All @@ -322,8 +319,11 @@ int HCompressedAnimClass::Load_W3D(ChunkLoadClass & cload)
}

break;
}

case ANIM_FLAVOR_ADAPTIVE_DELTA:
{
AdaptiveDeltaMotionChannelClass* ad_chan = nullptr;
if (!read_channel(cload,&ad_chan)) {
goto Error;
}
Expand All @@ -338,10 +338,13 @@ int HCompressedAnimClass::Load_W3D(ChunkLoadClass & cload)
WWDEBUG_SAY(("ERROR! animation %s indexes a bone not present in the model. Please re-export!",Name));
}
break;
}
}
break;

case W3D_CHUNK_COMPRESSED_BIT_CHANNEL:
{
TimeCodedBitChannelClass* newbitchan = nullptr;
if (!read_bit_channel(cload,&newbitchan)) {
goto Error;
}
Expand All @@ -357,6 +360,7 @@ int HCompressedAnimClass::Load_W3D(ChunkLoadClass & cload)
}

break;
}

default:
break;
Expand Down Expand Up @@ -387,20 +391,32 @@ int HCompressedAnimClass::Load_W3D(ChunkLoadClass & cload)
*=============================================================================================*/
bool HCompressedAnimClass::read_channel(ChunkLoadClass & cload,TimeCodedMotionChannelClass * * newchan)
{
*newchan = W3DNEW TimeCodedMotionChannelClass;
bool result = (*newchan)->Load_W3D(cload);

return result;

TimeCodedMotionChannelClass* channel = W3DNEW TimeCodedMotionChannelClass;
if (channel->Load_W3D(cload))
{
*newchan = channel;
return true;
}
else
{
delete channel;
return false;
}
}

bool HCompressedAnimClass::read_channel(ChunkLoadClass & cload,AdaptiveDeltaMotionChannelClass * * newchan)
{
*newchan = W3DNEW AdaptiveDeltaMotionChannelClass;
bool result = (*newchan)->Load_W3D(cload);

return result;

AdaptiveDeltaMotionChannelClass* channel = W3DNEW AdaptiveDeltaMotionChannelClass;
if (channel->Load_W3D(cload))
{
*newchan = channel;
return true;
}
else
{
delete channel;
return false;
}
}


Expand Down Expand Up @@ -483,11 +499,17 @@ void HCompressedAnimClass::add_channel(AdaptiveDeltaMotionChannelClass * newchan
*=============================================================================================*/
bool HCompressedAnimClass::read_bit_channel(ChunkLoadClass & cload,TimeCodedBitChannelClass * * newchan)
{
*newchan = W3DNEW TimeCodedBitChannelClass;
bool result = (*newchan)->Load_W3D(cload);

return result;

TimeCodedBitChannelClass* channel = W3DNEW TimeCodedBitChannelClass;
if (channel->Load_W3D(cload))
{
*newchan = channel;
return true;
}
else
{
delete channel;
return false;
}
}


Expand Down
2 changes: 1 addition & 1 deletion Core/Libraries/Source/WWVegas/WW3D2/seglinerenderer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ void SegLineRendererClass::Set_Texture_Tile_Factor(float factor)
///@todo: I raised this number and didn't see much difference on our min-spec. -MW
const static float MAX_LINE_TILING_FACTOR = 50.0f;
if (factor > MAX_LINE_TILING_FACTOR) {
WWDEBUG_SAY(("Texture (%s) Tile Factor (%.2f) too large in SegLineRendererClass!", Get_Texture()->Get_Texture_Name().str(), TextureTileFactor));
WWDEBUG_SAY(("Texture (%s) Tile Factor (%.2f) too large in SegLineRendererClass!", Peek_Texture()->Get_Texture_Name().str(), TextureTileFactor));
factor = MAX_LINE_TILING_FACTOR;
} else {
factor = MAX(factor, 0.0f);
Expand Down
10 changes: 5 additions & 5 deletions GeneralsMD/Code/GameEngine/Source/Common/RTS/Player.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3167,13 +3167,13 @@ void Player::removeUpgrade( const UpgradeTemplate *upgradeTemplate )
if( upgrade->getStatus() == UPGRADE_STATUS_COMPLETE )
onUpgradeRemoved();

if( ThePlayerList->getLocalPlayer() == this )
{
TheControlBar->markUIDirty();
}
deleteInstance(upgrade);

if( ThePlayerList->getLocalPlayer() == this )
{
TheControlBar->markUIDirty();
}
}

}


Expand Down
6 changes: 6 additions & 0 deletions GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameStateMap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ static void embedPristineMap( AsciiString map, Xfer *xfer )
if( file->read( buffer, fileSize ) != fileSize )
{

delete[] buffer;

DEBUG_CRASH(( "embedPristineMap - Error reading from file '%s'", map.str() ));
throw SC_INVALID_DATA;

Expand Down Expand Up @@ -161,6 +163,8 @@ static void embedInUseMap( AsciiString map, Xfer *xfer )
if( fread( buffer, 1, fileSize, fp ) != fileSize )
{

delete[] buffer;

DEBUG_CRASH(( "embedInUseMap - Error reading from file '%s'", map.str() ));
throw SC_INVALID_DATA;

Expand Down Expand Up @@ -216,6 +220,8 @@ static void extractAndSaveMap( AsciiString mapToSave, Xfer *xfer )
if( fwrite( buffer, 1, dataSize, fp ) != dataSize )
{

delete[] buffer;

DEBUG_CRASH(( "extractAndSaveMap - Error writing to file '%s'", mapToSave.str() ));
throw SC_INVALID_DATA;

Expand Down
16 changes: 6 additions & 10 deletions GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,17 +116,13 @@ static_assert(ARRAY_SIZE(TheDrawableIconNames) == MAX_ICONS + 1, "Incorrect arra
*
* OK, so it's a bit of a hack, but it saves memory in every Drawable
*/
static DynamicAudioEventInfo * getNoSoundMarker()
{
static DynamicAudioEventInfo * marker = nullptr;
class DynamicAudioEventInfoStatic : public DynamicAudioEventInfo
{};
static DynamicAudioEventInfoStatic s_noSoundMarker;

if ( marker == nullptr )
{
// Initialize first time function is called
marker = newInstance( DynamicAudioEventInfo );
}

return marker;
static DynamicAudioEventInfo* getNoSoundMarker()
{
return &s_noSoundMarker;
}


Expand Down
7 changes: 6 additions & 1 deletion GeneralsMD/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,12 @@ void SidesList::prepareForMP_or_Skirmish()
break;
}
}
if (curSide == -1) continue;
if (curSide == -1)
{
deleteInstance(scripts[i]);
scripts[i] = nullptr;
continue;
}

deleteInstance(getSkirmishSideInfo(curSide)->getScriptList());
getSkirmishSideInfo(curSide)->setScriptList(scripts[i]);
Expand Down
1 change: 1 addition & 0 deletions GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/BattlePlanUpdate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ BattlePlanUpdate::~BattlePlanUpdate()
TheAudio->removeAudioEvent( m_holdTheLineUnpack.getPlayingHandle() );
TheAudio->removeAudioEvent( m_holdTheLinePack.getPlayingHandle() );

deleteInstance(m_bonuses);
}

// ------------------------------------------------------------------------------------------------
Expand Down
Loading
Loading