fix(milesaudiomanager): Prevent multithread crashing in MilesAudioManager

[xezon]

Merge with Rebase

• Fixes Race condition in MilesAudioManager::findPlayingAudioFrom #2715

This change is split into 2 commits. The first cleans up some junk in Miles Audio Manager. And the second applies everything necessary for the fix.

It fixes a rare data race in MilesAudioManager, by preventing accessing potentially invalidated iterators in MilesAudioManager::findPlayingAudioFrom, which runs on the MSS Timer thread. This potentially is no issue in regular builds, because an invalidated iterator might still be usable when the memory was not reused.

Note that it NOT possible to simply defer the event processing coming in MilesAudioManager::notifyOfAudioCompletion to Main thread, because it wants to continue looping sounds immediately, and any delays will introduce audible stutter.

Note that the stop playing audio step is often seperated from the release playing audio step, because it is illegal to call into Miles while holding the new critical section due Miles holding its own internal mutex during callbacks to MilesAudioManager::notifyOfAudioCompletion and Miles API functions.

TODO

• Test and listen
• Add pull id to commits
• Replicate in Generals

[greptile-apps]

Greptile Summary

This PR addresses a multithreading data race in MilesAudioManager where the MSS Timer thread could access invalidated iterators in findPlayingAudioFrom. The fix introduces a two-phase stop/release design: stopPlayingAudio atomically transitions status through PS_Playing → PS_Stopping → PS_Stopped using InterlockedCompareExchange, while list erasure is deferred to a locked phase so the timer thread always sees valid iterators.

• Adds CriticalSectionClass guards per list, protecting only structural mutations (push/erase) while Miles API calls are made outside the lock, correctly avoiding deadlock with Miles' own internal mutex.
• Removes processStoppedList and m_stoppedAudio; stopped items are now released inline via releasePlayingAudioInListIfStopped at the end of each process pass.
• Cleans up pervasive null-pointer guards on list elements and refactors music-track queries through a new findActiveMusic helper that respects the deferred-stop state.

Confidence Score: 5/5

Safe to merge. The two-phase stop/release design with per-list critical sections correctly serialises timer-thread iteration against main-thread structural modifications without holding the lock during Miles API calls.

The threading model is sound: the CAS sequence in stopPlayingAudio guarantees exactly one thread calls releaseMilesHandles, the critical sections protect only list push/erase so Miles' own internal mutex is never held concurrently, and the deferred-erasure pattern keeps iterator validity for the timer thread. The one behavioural change found (plain else instead of else-if PAT_3DSample in the completion-notify block) is a minor defensive regression that cannot trigger under any current sound-type configuration.

MilesAudioManager.cpp — specifically the stopPlayingAudio completion-notify block and the deferred-erasure interactions in processPlayingList / processFadingList.

Important Files Changed

Filename	Overview
Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp	Core of the fix: introduces two-phase stop/release with CAS atomics, per-list critical sections, and deferred erasure. One minor defensive regression found (else-if → else in notify completion block).
Core/GameEngineDevice/Include/MilesAudioDevice/MilesAudioManager.h	Adds PS_Stopping state, CriticalSectionClass members, new helpers, and updates API signatures for nextMusicTrack/prevMusicTrack to return AsciiString.
Dependencies/Utility/Utility/interlocked_adapter.h	New inline MSVC <1300 compatibility shim for typed InterlockedCompareExchange, correctly scoped to 32-bit-only old MSVC.
Core/GameEngine/Include/Common/GameAudio.h	nextMusicTrack/prevMusicTrack return AsciiString; getMusicTrackName() removed. Clean API update.

Sequence Diagram

sequenceDiagram
    participant MT as Main Thread
    participant MSS as MSS Timer Thread
    participant Miles as Miles SDK

    Note over MT,MSS: Normal audio completion flow
    Miles->>MSS: EOS callback → notifyOfAudioCompletion()
    MSS->>MSS: findPlayingAudioFrom() [holds ListCS]
    MSS->>MSS: CAS PS_Playing → PS_Stopping
    MSS-->>Miles: return (Miles mutex released)

    Note over MT: Next processPlayingList() frame
    MT->>MT: First pass (no lock): sees PS_Stopping
    MT->>MT: stopPlayingAudio(): CAS PS_Stopping → PS_Stopped
    MT->>Miles: releaseMilesHandles() — nulls handles, sets PAT_INVALID
    MT->>MT: Second pass (holds ListCS): releasePlayingAudioInListIfStopped()
    MT->>MT: Erase PS_Stopped from list, delete PlayingAudio

    Note over MT,MSS: Main-thread stop (e.g. stopAudioEvent)
    MT->>MT: stopPlayingAudio(): CAS PS_Playing→PS_Stopping→PS_Stopped
    MT->>Miles: releaseMilesHandles() — nulls handles
    Note over MT: Erasure deferred to next processPlayingList()
    MT->>MT: releasePlayingAudioInListIfStopped() [holds ListCS]

Loading

Prompt To Fix All With AI

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

---

### Issue 1 of 1
Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp:1050-1054
The original `releasePlayingAudio` guarded the 3D-sample completion notification with `else if (release->m_type == PAT_3DSample)`. The new `stopPlayingAudio` uses a plain `else`, so any `AT_SoundEffect` item whose type is `PAT_Stream` (an unlikely but possible audio-data misconfiguration) will incorrectly call `notifyOf3DSampleCompletion` instead of being silently skipped. Restoring the explicit type check keeps the same defensive behaviour as the original.

```suggestion
		} else if (release->m_type == PAT_3DSample) {
			if (release->m_3DSample) {
				m_sound->notifyOf3DSampleCompletion();
			}
		}
```

_{Reviews (12): Last reviewed commit: "fixup! fix(milesaudiomanager): Prevent m..." | Re-trigger Greptile}

[Caball009]

Could you rebase this to get the CI replay check working again?

[xezon]

Could you rebase this to get the CI replay check working again?

Done

[xezon]

Rebased. I played a bit of the campaign and it sounds ok.

[Mauller]

Looks good overall, need another look over it and a test, but i couldn't see any significant issues, just a few small points.