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
• Merge after fix(milesaudiomanager): Fix audio request kill condition in MilesAudioManager::killAudioEventImmediately() #2784

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 RefCountMTClass RefCountClass 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 (or MilesAudioManager::stopPlayingAudio), 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
• Rename the second commit in regards to RefCountMTClass
• Rename the third commit because it is no longer accurate
• Fix condition in killAudioEventImmediately after fix(milesaudiomanager): Fix audio request kill condition in MilesAudioManager::killAudioEventImmediately() #2784 is merged
• Replicate in Generals

[greptile-apps]

Greptile Summary

This PR fixes a use-after-free race condition in MilesAudioManager where the MSS Timer thread's startNextLoop was directly transferring ownership of a raw AudioEventRTS* to a new AudioRequest while PlayingAudio still held the same pointer. The fix introduces shared reference-counted ownership via RefCountPtr<DynamicAudioEventRTS> in both AudioRequest and PlayingAudio, and defers the re-request to the main thread.

• DynamicAudioEventRTS now directly inherits AudioEventRTS (instead of embedding it as m_event) and adds RefCountClass for shared ownership via RefCountPtr; all callers across ThingTemplate, Drawable, PhysicsBehavior, and the audio manager are updated accordingly.
• startNextLoop (called on the MSS Timer thread) no longer allocates an AudioRequest directly; instead it sets m_rerequestOnNextUpdate = true and defers the RefCountPtr copy and request creation to rerequestPlayingAudio on the main thread.
• A new RefCountMTClass with interlocked ref-counting is introduced and scaffolded (including correct operator=), but DynamicAudioEventRTS currently uses the existing non-thread-safe RefCountClass since ref-count operations remain main-thread-only after the fix.

Confidence Score: 4/5

Safe to merge after the pre-existing killAudioEventImmediately handle-check fix (flagged in a prior review) is addressed; the core race-condition fix is mechanically sound.

The shared-ownership design is correct: all RefCountPtr Add_Ref/Release_Ref calls happen on the main thread, the timer thread only calls Peek(), and the Interlocked status transition provides the necessary ordering. The one remaining concern is that m_rerequestOnNextUpdate is written by the timer thread without volatile, which could theoretically cause the main thread to cache a stale false in a register-optimized build. The pre-existing killAudioEventImmediately AR_Play handle mismatch (flagged separately in a previous review) is the more impactful outstanding item.

MilesAudioManager.cpp / MilesAudioManager.h — killAudioEventImmediately AR_Play check and the m_rerequestOnNextUpdate volatile annotation.

Important Files Changed

Filename	Overview
Core/Libraries/Source/WWVegas/WWLib/refcount.h	Adds RefCountMTClass with thread-safe interlocked ref-counting and a correct copy-assignment operator (takes const RefCountMTClass&). RefCountMTClass is defined here but not yet used by DynamicAudioEventRTS (which uses RefCountClass); available for future use.
Core/Libraries/Source/WWVegas/WWLib/refcount.cpp	Implements RefCountMTClass::Add_Ref and Release_Ref using InterlockedIncrement/Decrement. Calls Delete_This when ref hits zero, consistent with RefCountClass pattern.
Core/GameEngine/Include/Common/AudioEventRTS.h	DynamicAudioEventRTS now directly inherits AudioEventRTS (instead of containing it as m_event) and adds RefCountClass for shared ownership. Delete_This override correctly routes to the memory pool via deleteInstance.
Core/GameEngine/Include/Common/AudioRequest.h	Replaces the raw-pointer/union (AudioEventRTS*, AudioHandle) with RefCountPtr m_pendingEvent and a separate AudioHandle. Removes the m_usePendingEvent flag, simplifying lifetime management.
Core/GameEngineDevice/Include/MilesAudioDevice/MilesAudioManager.h	PlayingAudio::m_audioEventRTS changed to RefCountPtr; m_cleanupAudioEventRTS replaced by m_rerequestOnNextUpdate. New rerequestPlayingAudio helper declared. m_rerequestOnNextUpdate should be volatile for cross-thread visibility.
Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp	Core race-condition fix: startNextLoop now defers rerequest via m_rerequestOnNextUpdate flag instead of directly creating a new AudioRequest from the timer thread. rerequestPlayingAudio copies the RefCountPtr on the main thread, avoiding the unsafe cross-thread ownership transfer.
Core/GameEngine/Source/Common/Audio/GameAudio.cpp	addAudioEvent now allocates DynamicAudioEventRTS via newInstance and wraps it in RefCountPtr::Create_NoAddRef; releaseAudioEventRTS removed; SoundManager::addAudioEvent now returns Bool to signal ownership transfer.
GeneralsMD/Code/GameEngine/Include/Common/ThingTemplate.h	AudioArray::m_audio changed to RefCountPtr[]; ctor/dtor simplified; copy constructor and operator= updated to use Create_NoAddRef for new instances while leaving the ref-count no-op assignment in place for existing slots.
Dependencies/Utility/Utility/interlocked_adapter.h	Adds InterlockedIncrement/Decrement wrappers inside the existing #if _MSC_VER < 1300 guard. The (long*) cast intentionally strips volatile to resolve to the pre-1300 MSVC Windows API signature (PLONG, no volatile), avoiding infinite recursion.

Sequence Diagram

sequenceDiagram
    participant MT as Main Thread
    participant TT as MSS Timer Thread
    participant DA as DynamicAudioEventRTS

    Note over MT: processRequestList → playAudioEvent
    MT->>DA: "Create_NoAddRef (refs=1)"
    MT->>DA: "RefCountPtr copy into PlayingAudio (refs=2)"
    MT->>DA: "RefCountPtr in AudioRequest released (refs=1)"

    Note over TT: notifyOfAudioCompletion → startNextLoop
    TT->>DA: Peek() — no ref change
    TT-->>MT: "sets m_rerequestOnNextUpdate=true, InterlockedCAS(m_status, PS_Stopping)"

    Note over MT: processPlayingList (next frame)
    MT->>DA: "rerequestPlayingAudio: RefCountPtr copy into new AudioRequest (refs=2)"
    MT->>MT: "stopPlayingAudio → releasePlayingAudio, PlayingAudio deleted (refs=1)"
    MT->>DA: "playAudioEvent: new PlayingAudio takes ownership (refs=1)"

Loading

_{Reviews (8): Last reviewed commit: "fixup! bugfix(milesaudiomanager): Preven..." | Re-trigger Greptile}

[xezon]

I revisited the implementation and added new fixup commits to simplify it, because I noticed that we can avoid adding the deferred audio requests container by using a new flag in PlayingAudio to tell main thread to create a new audio request when stopping the playing audio. This simplifies the whole thing.

The RefCountMTClass is now no longer used, but we can keep it anyway for future use cases.