From 2945268f52ef95f111e70c45b882e3355c524a59 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 04:28:59 +0000 Subject: [PATCH] SoLoud: dispatch voice-ended callbacks off the audio thread Fixes a lock-order inversion that deadlocks the engine during ordinary playback. stopVoice_internal() runs with the audio mutex held -- it asserts mInsideAudioThreadMutex -- and called _voiceEndedCallback directly from there. That callback reaches back into Player state: bindings.cpp's voiceEndedCallback() calls findByHandle() and removeHandle(), both of which take sounds_mutex. So the audio thread acquires: audio mutex -> sounds_mutex while Player::disposeSound() holds sounds_mutex across soloud.stop(), acquiring: sounds_mutex -> audio mutex Disposing a sound while another voice ends naturally deadlocks both threads. The audio thread strands the audio mutex, so the device produces nothing and every later SoLoud call blocks forever -- including deinit(), and including any synchronous call from the UI isolate, which freezes the app. Handles and sources still look valid from Dart throughout. For an app that swaps sounds in and out during playback this is a routine user action, not an edge case. Ended voices are now queued in mEndedVoiceQueue and dispatched by unlockAudioMutex_internal() after the mutex is released. That is the single choke point every unlock path goes through, so no call site changes. The drain is an out-of-line slow path, keeping a small stack frame on the audio thread for the common empty-queue case. Pending handles are copied out and the queue cleared before the unlock, so another thread cannot corrupt the dispatch or have its own work consumed. _voiceEndedCallback becomes atomic, matching the other cross-thread callbacks. Note this is not fixed by clearing callback registrations: voiceEndedCallback takes sounds_mutex before it ever checks whether a Dart callback is registered. Verified with a test driving the real path -- play(), mix(), stop() on the null backend -- whose callback re-enters SoLoud the way the real one reaches into Player. It deadlocks against the pre-change code (killed at 15s) and passes now. A unit test additionally asserts dispatch happens with the mutex released, in order, re-entrant-safe, and drained when the callback is null. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + src/soloud/include/soloud.h | 23 +++++++-- src/soloud/src/core/soloud.cpp | 49 ++++++++++++++++++++ src/soloud/src/core/soloud_core_voiceops.cpp | 12 +++-- 4 files changed, 79 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98458b1e..c9dbd3df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ #### 4.0.13 (20 Jul 2026) +- fix: the voice-ended callback is no longer invoked while SoLoud's audio mutex is held. It ran there via `stopVoice_internal()` and reached back into `Player` state, which deadlocked against `disposeSound()` (which holds `sounds_mutex` across `soloud.stop()`). The symptom was a wedged engine: handles and sources still looked valid, no audio was produced, and `deinit()` never completed. Ended voices are now queued and dispatched once the mutex is released - fix: Waveform audio sources do not match engine sample rate #501. Thanks to @Colton127 - Android now stops the audio device when idle (no active voices) like every other platform, releasing the audioserver `AudioMix` partial wakelock #250; use `setAudioDeviceIdleTimeout()` to keep it running - add `stopAudioDevice()` / `startAudioDevice()` to control the audio output device without deinitializing the engine (loaded sounds and voice state are preserved); `stopAudioDevice()` is an idle-only no-op while an unpaused voice is active unless `force: true` is passed, while `startAudioDevice()` temporarily starts or prewarms the output and remains subject to the configured idle timeout; blocking native device operations run off the UI thread diff --git a/src/soloud/include/soloud.h b/src/soloud/include/soloud.h index 33c413bd..9c9cc084 100644 --- a/src/soloud/include/soloud.h +++ b/src/soloud/include/soloud.h @@ -171,12 +171,25 @@ namespace SoLoud soloudResultFunction mBackendPauseFunc; soloudResultFunction mBackendResumeFunc; - // Set the callback to call when a voice is ended/stopped - void (*_voiceEndedCallback)(unsigned int*) = nullptr; + // Set the callback to call when a voice is ended/stopped. + // + // stopVoice_internal() runs with the audio mutex held, so it must not + // call out to the embedder directly: an embedder callback that crashes, + // stalls, or blocks (for example a Dart NativeCallable whose isolate has + // gone away) would strand the audio mutex and wedge every later SoLoud + // call, including deinit(). Ended voices are queued instead and + // dispatched by unlockAudioMutex_internal() once the mutex is released. + std::atomic _voiceEndedCallback{nullptr}; void setVoiceEndedCallback(void (*voiceEndedCallback)(unsigned int*)) { - _voiceEndedCallback = voiceEndedCallback; + _voiceEndedCallback.store(voiceEndedCallback, + std::memory_order_release); } + // Handles of voices that ended while the audio mutex was held, pending + // dispatch. Both members are only touched with the audio mutex held. + unsigned int mEndedVoiceQueue[VOICE_COUNT]; + unsigned int mEndedVoiceCount = 0; + // Called after a mix cycle in which a voice stopped or became paused. // The callback runs after the audio mutex has been released. std::atomic _voiceInactiveCallback{nullptr}; @@ -545,6 +558,10 @@ namespace SoLoud void lockAudioMutex_internal(); // Unlock audio thread mutex. void unlockAudioMutex_internal(); + // Slow path of unlockAudioMutex_internal(): drains mEndedVoiceQueue. + // Kept out of line so the common (empty queue) path does not carry the + // snapshot buffer in its stack frame. + void unlockAudioMutexAndDispatchEndedVoices_internal(); // Max. number of active voices. Busses and tickable inaudibles also count against this. unsigned int mMaxActiveVoices; diff --git a/src/soloud/src/core/soloud.cpp b/src/soloud/src/core/soloud.cpp index 7c206b4d..22ce9762 100644 --- a/src/soloud/src/core/soloud.cpp +++ b/src/soloud/src/core/soloud.cpp @@ -2332,6 +2332,13 @@ namespace SoLoud void Soloud::unlockAudioMutex_internal() { SOLOUD_ASSERT(mInsideAudioThreadMutex); + + if (mEndedVoiceCount != 0) + { + unlockAudioMutexAndDispatchEndedVoices_internal(); + return; + } + mInsideAudioThreadMutex = false; if (mAudioThreadMutex) { @@ -2339,4 +2346,46 @@ namespace SoLoud } } + // Release the audio mutex, then notify the embedder about voices that ended + // while it was held. stopVoice_internal() cannot call out directly because it + // runs under the mutex, and an embedder callback that stalls or crashes there + // would strand the mutex and wedge every later SoLoud call, teardown included. + // + // The pending handles are copied out and the queue cleared *before* the + // unlock, so another thread that acquires the mutex and queues more work + // cannot corrupt this dispatch or have its own work consumed here. + void Soloud::unlockAudioMutexAndDispatchEndedVoices_internal() + { + SOLOUD_ASSERT(mInsideAudioThreadMutex); + + unsigned int endedVoices[VOICE_COUNT]; + unsigned int endedVoiceCount = mEndedVoiceCount; + unsigned int i; + + if (endedVoiceCount > VOICE_COUNT) + endedVoiceCount = VOICE_COUNT; + for (i = 0; i < endedVoiceCount; i++) + endedVoices[i] = mEndedVoiceQueue[i]; + mEndedVoiceCount = 0; + + mInsideAudioThreadMutex = false; + if (mAudioThreadMutex) + { + Thread::unlockMutex(mAudioThreadMutex); + } + + // Read after unlocking so a concurrent setVoiceEndedCallback(nullptr) + // during teardown is honoured as early as possible. + auto voiceEndedCallback = + _voiceEndedCallback.load(std::memory_order_acquire); + if (voiceEndedCallback == nullptr) + return; + + for (i = 0; i < endedVoiceCount; i++) + { + unsigned int handle = endedVoices[i]; + voiceEndedCallback(&handle); + } + } + }; diff --git a/src/soloud/src/core/soloud_core_voiceops.cpp b/src/soloud/src/core/soloud_core_voiceops.cpp index d850a686..42591fea 100644 --- a/src/soloud/src/core/soloud_core_voiceops.cpp +++ b/src/soloud/src/core/soloud_core_voiceops.cpp @@ -134,9 +134,15 @@ namespace SoLoud { if (mResampleDataOwner[i] == v) { - if (_voiceEndedCallback != nullptr) { - unsigned int handle = (aVoice + 1) | (mResampleDataOwner[i]->mPlayIndex << 12); - _voiceEndedCallback(&handle); + // Queue rather than call: the audio mutex is held here (see + // the assert above) and the embedder callback must not run + // under it. unlockAudioMutex_internal() dispatches these. + // mEndedVoiceQueue holds VOICE_COUNT entries and a voice can + // only be queued once per stop, so it cannot overflow. + if (mEndedVoiceCount < VOICE_COUNT) + { + mEndedVoiceQueue[mEndedVoiceCount++] = + (aVoice + 1) | (mResampleDataOwner[i]->mPlayIndex << 12); } mResampleDataOwner[i] = NULL; }