Skip to content

bugfix(gui): Remove destroyed windows from modal stack - #3224

Open
tintinhamans wants to merge 1 commit into
TheSuperHackers:mainfrom
tintinhamans:arctic/bugfix/modal-stack-dangling-pointer
Open

bugfix(gui): Remove destroyed windows from modal stack#3224
tintinhamans wants to merge 1 commit into
TheSuperHackers:mainfrom
tintinhamans:arctic/bugfix/modal-stack-dangling-pointer

Conversation

@tintinhamans

Copy link
Copy Markdown

A destroyed window could stay buried in the modal stack. The game could later use that dead pointer and crash.

This removes destroyed windows from the whole stack. Hidden windows keep the old behavior so they can still be shown again.

Applied to Generals and Zero Hour.

Closes #3223

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Remove destroyed windows from GUI modal stacks

🐞 Bug fix 🕐 10-20 Minutes

Grey Divider

AI Description

• Remove every modal-stack entry referencing a window when destruction begins.
• Preserve hidden-window behavior by limiting modal cleanup to destruction.
• Apply identical lifecycle handling to Generals and Zero Hour.
Diagram

graph TD
  A["Destroy window"] --> B["winDestroy"] --> C["Modal cleanup"] --> D[("Modal stack")]
  B --> E["Destroy queue"] --> F["Memory release"]
Loading
High-Level Assessment

The dedicated destruction-only helper is the best fit: it removes buried and duplicate references immediately while preserving winUnsetModal's top-of-stack contract and hidden-window behavior. Reusing or broadening winUnsetModal would change its public semantics, while cleanup during deferred deletion would leave stale references alive longer.

Files changed (4) +44 / -10

Bug fix (4) +44 / -10
GameWindowManager.hDeclare modal-stack destruction cleanup +1/-0

Declare modal-stack destruction cleanup

• Adds the protected helper used to purge references to a window during destruction in Generals.

Generals/Code/GameEngine/Include/GameClient/GameWindowManager.h

GameWindowManager.cppPurge destroyed Generals windows from the full modal stack +21/-5

Purge destroyed Generals windows from the full modal stack

• Calls modal cleanup when destruction begins instead of only unsetting the current modal head. The pointer-to-pointer traversal removes all matching entries, including buried or duplicate references, before deferred deletion.

Generals/Code/GameEngine/Source/GameClient/GUI/GameWindowManager.cpp

GameWindowManager.hDeclare Zero Hour modal-stack destruction cleanup +1/-0

Declare Zero Hour modal-stack destruction cleanup

• Adds the protected helper used to purge references to a window during destruction in Zero Hour.

GeneralsMD/Code/GameEngine/Include/GameClient/GameWindowManager.h

GameWindowManager.cppPurge destroyed Zero Hour windows from the full modal stack +21/-5

Purge destroyed Zero Hour windows from the full modal stack

• Mirrors the Generals fix by removing every modal entry for a window as soon as destruction begins. This prevents deferred deletion from leaving dangling modal pointers.

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

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@tintinhamans
tintinhamans force-pushed the arctic/bugfix/modal-stack-dangling-pointer branch from 0f7d994 to c1cee73 Compare August 26, 2026 23:20
@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown

Greptile Summary

This PR prevents destroyed GUI windows from remaining as dangling entries in the modal stack in both Generals variants.

  • Adds a helper that traverses the complete modal stack and removes every entry associated with a destroyed window.
  • Invokes the cleanup when a window enters the deferred destruction path.
  • Preserves the existing top-only modal behavior for hidden windows.

Confidence Score: 5/5

The PR appears safe to merge with no actionable defects identified.

The destruction path now removes all modal nodes before deferred window deletion, while every destroy-list insertion passes through that cleanup and the unlink loop safely handles buried and duplicate entries.

Important Files Changed

Filename Overview
Generals/Code/GameEngine/Include/GameClient/GameWindowManager.h Declares the protected modal-stack cleanup helper used by the Generals window manager.
Generals/Code/GameEngine/Source/GameClient/GUI/GameWindowManager.cpp Moves modal cleanup to winDestroy and safely unlinks every stack node referencing the destroyed window.
GeneralsMD/Code/GameEngine/Include/GameClient/GameWindowManager.h Declares the equivalent cleanup helper for the Zero Hour window manager.
GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GameWindowManager.cpp Applies the same complete modal-stack cleanup to the Zero Hour destruction path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[winDestroy called] --> B[Mark window destroyed]
    B --> C[Remove every matching modal-stack node]
    C --> D[Queue window for deferred destruction]
    D --> E[processDestroyList]
    E --> F[Send destroy notification]
    F --> G[Delete window safely]
Loading

Reviews (1): Last reviewed commit: "bugfix(gui): Remove destroyed windows fr..." | Re-trigger Greptile

@tintinhamans
tintinhamans requested a review from bobtista August 27, 2026 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Destroying a non-top modal window leaves a dangling pointer

1 participant