fix(headless): Fix use-after-free crash when replay ends in headless mode in debug build by bobtista · Pull Request #2219 · TheSuperHackers/GeneralsGameCode

bobtista · 2026-01-29T23:12:34Z

Summary

Fix particle accumulation in headless replay by using ParticleSystemManager::update() instead of reset(). Using update() has slightly more CPU overhead than reset() but is semantically correct - particles finish their lifecycle naturally.

Test plan

Run headless replay to completion (-replay test.rep -headless)
Verify no crash, no particle accumulation slowdown

greptile-apps · 2026-01-29T23:16:08Z

Greptile Overview

Greptile Summary

Changed ParticleSystemManager::reset() to update() in headless mode to prevent particle accumulation while avoiding potential issues during shutdown.

Key changes:

Replaced TheParticleSystemManager->reset() with update() in GameClient.cpp:790
Simplified and updated comment to reflect the change
update() only cleans up finished particle systems, while reset() deletes all systems immediately

Note: The PR description mentions setting DX8Wrapper_IsWindowed to false in headless mode, but this change is not present in the commits. The description also mentions "DrawModules hold raw pointers to particle systems," but commit 026e633 (Feb 1) already replaced raw pointers with ParticleSystemID for safe lookups, making the described use-after-free scenario unlikely with the current codebase.

The update() approach is semantically correct for headless mode - it allows particles to finish naturally rather than abruptly deleting them, which is cleaner even though both approaches would work safely with ID-based lookups.

Confidence Score: 5/5

This PR is safe to merge with minimal risk
The change from reset() to update() is straightforward and semantically correct. The modification affects only headless replay mode, making it low-risk for normal gameplay. While the PR description mentions issues that may have been resolved in previous commits, the actual code change is sound and improves particle cleanup behavior.
No files require special attention

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp	Changed from ParticleSystemManager::reset() to update() for headless mode particle cleanup, simplified comment

xezon

Fast shutdown is a common way to exit apps, but it is not a good sign to do this to avoid problems here. Where is the problem exactly?

bobtista · 2026-02-03T18:38:21Z

Fast shutdown is a common way to exit apps, but it is not a good sign to do this to avoid problems here. Where is the problem exactly?

Did a bunch of testing and found the crash was:
The crash was caused by GameClient::updateHeadless() calling TheParticleSystemManager->reset() each frame during headless replay playback.

updateHeadless() calls TheParticleSystemManager->reset() every frame
reset() deletes ALL ParticleSystems immediately (memory filled with DEADBEEF)
DrawModules (W3DTruckDraw, W3DTankDraw, W3DTankTruckDraw) still hold raw pointers to those ParticleSystems (m_dustEffect, m_dirtEffect, m_powerslideEffect)
When replay ends and game shuts down, DrawModule destructors call tossEmitters()
tossEmitters() calls m_dustEffect->destroy() on already-freed memory
CRASH when ParticleSystem::destroy() tries to set m_isDestroyed = true at offset 0x338 on DEADBEEF

Pushed two fixes - first, we use update() instead of reset(), which leaves active particles intact.

Second, I was getting memory leak debug asserts even with ignoreAsserts enabled. It seems that:
During shutdown, TheGlobalData is destroyed before the memory leak report assertion fires. The ignoringAsserts() function checks TheGlobalData->m_headless and TheGlobalData->m_debugIgnoreAsserts, but both fail when TheGlobalData is null. Additionally, DX8Wrapper_IsWindowed defaults to true and is never changed in headless mode (DX8 is never initialized), so the !DX8Wrapper_IsWindowed check also fails.

The second fix is to set DX8Wrapper_IsWindowed = false at startup in headless mode. There is no DX8 window in headless mode, so the variable should reflect that. The existing !DX8Wrapper_IsWindowed check in ignoringAsserts() then handles the entire process lifetime, including shutdown.

Mauller · 2026-02-04T08:41:09Z

After the changes in #2235 to use and search for ParticleID's instead of holding pointers, is this valid anymore?
Checking since your comment talks about the altered draw modules that now use ID's to lookup their particle systems.

xezon · 2026-02-08T11:02:03Z

Is this change still relevant?

Edit: Looks like it still is relevant.

xezon · 2026-02-08T11:03:18Z

GeneralsMD/Code/Main/WinMain.cpp

+			DX8Wrapper_IsWindowed = false;
+		}
+		else if (initializeAppWindows(hInstance, nCmdShow, TheGlobalData->m_windowed) == false)
 			return exitcode;


Style:

{
}

xezon · 2026-02-08T11:04:47Z

GeneralsMD/Code/Main/WinMain.cpp

-		if(!TheGlobalData->m_headless && initializeAppWindows(hInstance, nCmdShow, TheGlobalData->m_windowed) == false)
+		// TheSuperHackers @fix bobtista 02/03/2026 Set DX8Wrapper_IsWindowed to false in headless
+		// mode so that ignoringAsserts() works correctly throughout the entire process lifetime,
+		// including during shutdown after TheGlobalData has been destroyed.


This looks like a change that could also have been separate of the particles change.

Do you mean a separate PR?

Yes it looks to be a separate change, right?

Ok I split it out

xezon · 2026-02-08T11:08:23Z

GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp

 	// the particles accumulate and slow things down a lot and can even cause a crash on
 	// longer replays.
-	TheParticleSystemManager->reset();
+	// TheSuperHackers @fix bobtista 02/02/2026 Use update() instead of reset() to avoid


Please combine this comment with the previous comment. Provide a rationale why update is preferred over reset, other than the dangling pointers whch should no longer be problem after #2235

xezon · 2026-02-08T11:08:31Z

GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp

+	// (W3DTruckDraw, W3DTankDraw, etc.) hold raw pointers to particle systems. When those
+	// DrawModules are later destroyed during game shutdown, they crash accessing freed memory.
+	// update() only cleans up finished particle systems, leaving active ones intact.
+	TheParticleSystemManager->update();


I suspect the downside of this update is that it will require more CPU processing than just resetting. But otherwise for logical correctness this seems like the better thing to do.

…articleSystemManager::update() instead of reset()

greptile-apps

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-12T17:16:30Z

GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp

+	// TheSuperHackers @info helmutbuhler 03/05/2025 bobtista 02/02/2026
+	// Update particles to prevent accumulation in headless mode. update() has slightly more
+	// CPU overhead than reset() but is semantically correct - particles finish naturally.
+	TheParticleSystemManager->update();


Comment says update() has "slightly more CPU overhead" but this depends on active particle count - with many particles, update() processes each one while reset() just bulk-deletes. Consider removing "slightly more CPU overhead" to avoid confusion.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review. Path: GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp Line: 786:789 Comment: Comment says `update()` has "slightly more CPU overhead" but this depends on active particle count - with many particles, `update()` processes each one while `reset()` just bulk-deletes. Consider removing "slightly more CPU overhead" to avoid confusion. <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> How can I resolve this? If you propose a fix, please make it concise.

xezon reviewed Jan 30, 2026

View reviewed changes

bobtista force-pushed the bobtista/fix-debug-replay-crash branch from 0fc66f8 to 3198cff Compare February 3, 2026 18:41

bobtista changed the title ~~Fix use-after-free crash when replay ends in headless mode in debug build~~ fix(headless): Fix use-after-free crash when replay ends in headless mode in debug build Feb 3, 2026

xezon reviewed Feb 8, 2026

View reviewed changes

xezon added Gen Relates to Generals ZH Relates to Zero Hour Debug Is mostly debug functionality Fix Is fixing something, but is not user facing labels Feb 8, 2026

bobtista added 2 commits February 12, 2026 12:12

fix(particle): Fix use-after-free crash in headless replay by using P…

adf466c

…articleSystemManager::update() instead of reset()

Simplify particle system update comment

3d7c95f

bobtista force-pushed the bobtista/fix-debug-replay-crash branch from 7a3e413 to 3d7c95f Compare February 12, 2026 17:12

greptile-apps bot reviewed Feb 12, 2026

View reviewed changes

Conversation

bobtista commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

greptile-apps bot commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Overview

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

bobtista commented Feb 3, 2026

Uh oh!

Mauller commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xezon commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xezon Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

xezon Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

bobtista Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

xezon Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

bobtista Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

xezon Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

xezon Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bobtista commented Jan 29, 2026 •

edited

Loading

greptile-apps bot commented Jan 29, 2026 •

edited

Loading

Mauller commented Feb 4, 2026 •

edited

Loading

xezon commented Feb 8, 2026 •

edited

Loading