bugfix(milesaudiomanager): Use reference counted DynamicAudioEventRTS class in AudioRequest and PlayingAudio to prevent race conditions when sharing audio event data in MilesAudioManager::startNextLoop()

[xezon]

Merge with Rebase

• Fixes Crash in MilesAudioManager::stopPlayingAudio() #2770

This change has 3 commits to work towards fixing race conditions in MilesAudioManager concerning a shared AudioEventRTS instance in classes AudioRequest and PlayingAudio.

The first commit implements a new RefCountMTClass which is fundamentally identical to RefCountClass, except it has a thread safe counter and all the debug functionality is omitted.

The second commit adds the new RefCountMTClass to DynamicAudioEventRTS to allow for shared ownership. All existing users of DynamicAudioEventRTS accomodate it and will now use RefCountPtr for automatic reference counting.

The third commits replaces AudioEventRTS* with RefCountPtr<DynamicAudioEventRTS> in AudioRequest and PlayingAudio to allow sharing the audio event data between them. This is needed, because ownership will be shared in function MilesAudioManager::startNextLoop, where previously AudioRequest was given the sole authority to delete the AudioEventRTS while PlayingAudio still kept a pointer to it. Now both classes need to release their reference count before the audio event data is deleted.

This likely was also a problem in retail, because AudioEventRTS is heap allocated, not pool allocated.

TODO

• Add pull ids to commit titles
• Replicate in Generals

[greptile-apps]

Greptile Summary

This PR fixes a race condition in MilesAudioManager::startNextLoop where a PlayingAudio and the newly-queued AudioRequest both referenced the same heap-allocated AudioEventRTS, but only AudioRequest was responsible for deletion — leaving PlayingAudio with a dangling pointer once the request was processed. Shared ownership is now expressed via RefCountPtr<DynamicAudioEventRTS> backed by the new thread-safe RefCountMTClass, and deferred requests are routed through a mutex-protected staging vector before being merged onto the main request list.

• Adds RefCountMTClass (thread-safe ref counting via Interlocked*) and makes DynamicAudioEventRTS inherit from it directly as AudioEventRTS, eliminating the nested m_event wrapper.
• Replaces the union {AudioEventRTS*; AudioHandle} + m_usePendingEvent pattern in AudioRequest with separate RefCountPtr<DynamicAudioEventRTS> and AudioHandle fields, and removes m_cleanupAudioEventRTS from PlayingAudio.
• Introduces appendDeferredAudioRequest (mutex-protected) and transferDeferredAudioRequests so the MSS Timer callback can safely queue a deferred restart without touching m_audioRequests directly.

Confidence Score: 3/5

The core race condition fix is sound, but killAudioEventImmediately still cannot cancel a deferred AR_Play request because it checks m_handleToInteractOn (always AHSV_Error for play requests) rather than m_pendingEvent->getPlayingHandle().

The primary fix (shared RefCountPtr ownership across PlayingAudio and AudioRequest, plus the deferred request queue) is architecturally correct and eliminates the original dangling-pointer race. However, killAudioEventImmediately was not updated to match the new field layout: it still uses req->m_handleToInteractOn == audioEvent for AR_Play requests, which is permanently AHSV_Error, so any deferred restart cannot be killed immediately.

Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp — the killAudioEventImmediately AR_Play branch needs its check updated to match the new m_pendingEvent-based layout.

Important Files Changed

Filename	Overview
Core/Libraries/Source/WWVegas/WWLib/refcount.h	Adds RefCountMTClass with thread-safe ref counting via InterlockedIncrement/Decrement. Copy constructor correctly resets to NumRefs(1) and copy-assign is a no-op, both matching the intent.
Core/GameEngine/Include/Common/AudioEventRTS.h	DynamicAudioEventRTS now inherits directly from AudioEventRTS and RefCountMTClass; eliminates the nested m_event member. Delete_This() override correctly uses deleteInstance for memory pool cleanup.
Core/GameEngine/Include/Common/AudioRequest.h	Replaces the union {AudioEventRTS*; AudioHandle} + m_usePendingEvent pattern with separate RefCountPtr<DynamicAudioEventRTS> and AudioHandle fields. Constructor correctly initialises m_handleToInteractOn to AHSV_Error.
Core/GameEngine/Source/Common/Audio/GameAudio.cpp	Adds appendDeferredAudioRequest (mutex-protected) and transferDeferredAudioRequests. The const overload of transferDeferredAudioRequests uses const_cast to mutate non-mutable members — functional but unconventional.
Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp	Core of the race-condition fix: startNextLoop now shares m_audioEventRTS into the deferred request via RefCountPtr, removing the dangling-pointer hazard. However, killAudioEventImmediately still checks m_handleToInteractOn for AR_Play requests, which is always AHSV_Error, so deferred play requests can never be killed.
Core/GameEngineDevice/Include/MilesAudioDevice/MilesAudioManager.h	Removes m_cleanupAudioEventRTS flag from PlayingAudio and switches m_audioEventRTS to RefCountPtr<DynamicAudioEventRTS>; ownership is now automatic.
GeneralsMD/Code/GameEngine/Include/Common/ThingTemplate.h	Migrates AudioArray from raw pointer management to RefCountPtr; constructor/destructor are now empty because RefCountPtr handles lifetime.
GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp	Replaces manual deleteInstance/null-out patterns for m_ambientSound with Clear() and direct member access through the now-direct AudioEventRTS base.
Dependencies/Utility/Utility/interlocked_adapter.h	Adds compatibility wrappers for InterlockedIncrement and InterlockedDecrement that accept long volatile*, bridging old SDK declarations that took non-volatile LONG*.
Core/Libraries/Source/WWVegas/WWLib/win.h	Centralises mmsys.h and interlocked_adapter.h includes so all consumers of win.h automatically get the interlocked compatibility shims.

Sequence Diagram

sequenceDiagram
    participant MSS as MSS Timer Thread
    participant MA as MilesAudioManager
    participant DA as DynamicAudioEventRTS

    Note over MA,DA: PlayingAudio holds RefCountPtr to DA
    MSS->>MA: notifyOfAudioCompletion()
    MA->>MA: startNextLoop(looping)
    MA->>MA: "req->m_pendingEvent = looping->m_audioEventRTS"
    Note over DA: refs bumped to 2
    MA->>MA: appendDeferredAudioRequest(req) [lock]
    MA->>MA: InterlockedCAS status to PS_Stopping
    Note over DA: PlayingAudio released, refs drop to 1
    Note over MA: Main loop tick
    MA->>MA: transferDeferredAudioRequests() [lock]
    MA->>MA: "processRequestList -> playAudioEvent(req)"
    MA->>MA: "audio->m_audioEventRTS = req->m_pendingEvent"
    Note over DA: refs bumped to 2, then req released, refs=1
    Note over DA: New PlayingAudio owns sole ref

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp:914-920
**`AR_Play` kill check is always false**

For AR_Play requests, `m_handleToInteractOn` is initialised to `AHSV_Error` (0) in the `AudioRequest` constructor and is never written for play requests — the playing handle lives in `m_pendingEvent->getPlayingHandle()`. As a result, any deferred restart request queued by `startNextLoop` (or any other AR_Play request) is never matched and cancelled by `killAudioEventImmediately`, so the sound can start playing again even after an explicit kill call. The `isCurrentlyPlaying` path was correctly updated in this PR to use `req->m_pendingEvent->getPlayingHandle() == handle`, but this path was missed.

### Issue 2 of 2
Core/GameEngine/Include/Common/GameAudio.h:327-328
`transferDeferredAudioRequests() const` uses `const_cast` to mutate `m_audioRequests` and `m_deferredAudioRequests`, which are not declared `mutable`. This could be avoided by marking those two members (and `m_deferredAudioRequestsCS`) as `mutable`, which is the idiomatic C++ way to express logically-const members that need physical mutation.

```suggestion
		void transferDeferredAudioRequests();
```

_{Reviews (4): Last reviewed commit: "bugfix(milesaudiomanager): Use reference..." | Re-trigger Greptile}