bugfix(aigroup): Prevent game crash when a player is selected in Replay playback (2)

[Caball009]

• Follow up for PR [GEN][ZH] Prevent game crash when a player is selected in Replay playback #1212

With the fix from #1212 AIGroupPtr currentlySelectedGroup would be accessed even if it had been destroyed and its memory deallocated. This is a use-after-free bug. It relies on the memory not being overwritten on deallocation.

If macro MEMORYPOOL_DEBUG (enabled by RTS_DEBUG) is enabled, freed memory is overwritten with a garbage value, so the game crashes when currentlySelectedGroup is dereferenced (at line 2132).

TODO:

• Tweak TSH comment.
• Replicate in Generals.

[greptile-apps]

Greptile Summary

This PR fixes a use-after-free bug introduced by PR #1212, where currentlySelectedGroup could be dereferenced after its memory was already freed. The previous guard checked getCount() == 0 on the potentially-freed pointer, which was unsafe.

• doesGroupExist added to both Generals and GeneralsMD builds: The new method does a pointer-identity scan of m_groupList without dereferencing the group, making it safe to call even when the pointer is stale.
• Guard updated in GameLogicDispatch.cpp: The check now calls TheAI->doesGroupExist(currentlySelectedGroup) instead of accessing a member of the potentially-freed group, closing the use-after-free window.

Confidence Score: 5/5

Safe to merge — the fix correctly eliminates a use-after-free by replacing a dereference of a potentially freed pointer with a safe pointer-identity list scan.

The guard in GameLogicDispatch.cpp now calls doesGroupExist before any member access, and doesGroupExist itself only compares pointer values without dereferencing the candidate. Both Generals and GeneralsMD builds receive the matching header declaration and implementation, closing the compilation gap called out on the previous PR.

No files require special attention.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp	Replaces unsafe use-after-free guard (getCount() == 0) with TheAI->doesGroupExist(), preventing dereference of freed memory.
Generals/Code/GameEngine/Include/GameLogic/AI.h	Adds doesGroupExist declaration to the Generals build header, needed for compilation under RETAIL_COMPATIBLE_AIGROUP.
Generals/Code/GameEngine/Source/GameLogic/AI/AI.cpp	Implements doesGroupExist via a pointer-identity search of m_groupList — no dereference of the candidate pointer.
GeneralsMD/Code/GameEngine/Include/GameLogic/AI.h	Adds doesGroupExist declaration to the Zero Hour build header, mirroring the Generals change.
GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AI.cpp	Implements doesGroupExist for Zero Hour, identical to the Generals implementation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[logicMessageDispatcher called] --> B{currentlySelectedGroup != null?}
    B -- No --> Z[Continue to destroyGroup block]
    B -- Yes --> C{TheAI->doesGroupExist currentlySelectedGroup?}
    C -- No: group was freed --> R[return early — use-after-free avoided]
    C -- Yes: group is live --> D[Safe to dereference: currentlySelectedGroup->getAllIDs]
    D --> E[Deselect / reselect drawables]
    E --> Z
    Z --> F{currentlySelectedGroup != null?}
    F -- Yes --> G[TheAI->destroyGroup — group still in list]
    F -- No --> H[End]
    G --> H

Loading

_{Reviews (4): Last reviewed commit: "Made 'AI::doesGroupExist' member functio..." | Re-trigger Greptile}

[xezon]

Makes sense. I have not thought of that.

[Caball009]

Replicated in Generals.

[xezon]

Looking good

[Caball009]

Rebased to include the fix for the CI Replay checker.