From bbc192054b7246b1f13293445df584fcb680599e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 04:29:41 +0000 Subject: [PATCH 1/8] Android: clear stale callback registrations on hot restart onDetachedFromEngine does not fire on hot restart. The FlutterEngine and its id are unchanged while the Dart isolate is replaced, so every registered NativeCallable silently goes stale with no notification -- and invoking a callable whose isolate is gone is undefined behaviour. The plugin now registers a FlutterEngine.EngineLifecycleListener and clears the registrations in onPreEngineRestart(). onEngineWillDestroy() clears too; it fires just before the plugin registry is destroyed, while the engine is still valid, so it is a slightly earlier point than the detach hook. The listener is removed on detach. Only the callback bridges are cleared here, not the whole engine: after a hot restart the new isolate's init() finds the native engine still initialized and deinits it itself. Compiled under -Xlint:all against stubs mirroring the real embedding API, and the JNI symbol name diffed against javac -h output. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + .../flutter_soloud/FlutterSoloudPlugin.java | 52 ++++++++++++++++--- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dc6f142..8b95081b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ #### 4.0.13 (20 Jul 2026) +- fix: Android hot restart now clears stale Dart callback registrations. Hot restart replaces the isolate without detaching plugins, so the registered `NativeCallable`s silently went stale - Android: the plugin now does no native work at app startup. Plugin registration and `onAttachedToEngine` are pure Java bookkeeping; the native library is loaded lazily, only when an engine-lifecycle hook actually has to call into it. Previously a static initializer pulled the whole library onto the main thread during app launch even for apps that never played a sound, and a load failure there crashed plugin registration - 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 diff --git a/android/src/main/java/flutter/soloud/flutter_soloud/FlutterSoloudPlugin.java b/android/src/main/java/flutter/soloud/flutter_soloud/FlutterSoloudPlugin.java index 3a250855..e517fdc6 100644 --- a/android/src/main/java/flutter/soloud/flutter_soloud/FlutterSoloudPlugin.java +++ b/android/src/main/java/flutter/soloud/flutter_soloud/FlutterSoloudPlugin.java @@ -1,6 +1,8 @@ package flutter.soloud.flutter_soloud; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import io.flutter.embedding.engine.FlutterEngine; import io.flutter.embedding.engine.plugins.FlutterPlugin; public final class FlutterSoloudPlugin implements FlutterPlugin { @@ -24,7 +26,9 @@ public final class FlutterSoloudPlugin implements FlutterPlugin { private static native boolean nativeClearDartCallbackRegistrationsForEngine(long engineId); - private Long engineId; + @Nullable private FlutterEngine flutterEngine; + @Nullable private Long engineId; + @Nullable private FlutterEngine.EngineLifecycleListener lifecycleListener; private static synchronized boolean ensureNativeLibraryLoaded() { if (!nativeLibraryLoadAttempted) { @@ -49,20 +53,56 @@ public void onAttachedToEngine( ) { // Deliberately does no native work. This runs during app launch for // every app that depends on the plugin, whether or not it ever uses - // SoLoud, so it must stay pure Java bookkeeping. - engineId = binding.getFlutterEngine().getEngineId(); + // SoLoud, so it must stay pure Java bookkeeping: read the engine id and + // register a listener. + final FlutterEngine engine = binding.getFlutterEngine(); + flutterEngine = engine; + engineId = engine.getEngineId(); + + lifecycleListener = new FlutterEngine.EngineLifecycleListener() { + @Override + public void onPreEngineRestart() { + // Hot restart replaces the Dart isolate but does not detach + // plugins, and the engine id is unchanged -- so without this the + // registered NativeCallables silently go stale. Only the bridges + // are cleared: the new isolate's init() finds the native engine + // still initialized and deinits it itself. + clearDartCallbackRegistrations(); + } + + @Override + public void onEngineWillDestroy() { + // Fires just before the plugin registry is destroyed, while the + // engine is still valid. + clearDartCallbackRegistrations(); + } + }; + engine.addEngineLifecycleListener(lifecycleListener); } @Override public void onDetachedFromEngine( @NonNull FlutterPluginBinding binding ) { - final Long detachedEngineId = engineId; + final FlutterEngine engine = flutterEngine; + final FlutterEngine.EngineLifecycleListener listener = lifecycleListener; + + if (engine != null && listener != null) { + engine.removeEngineLifecycleListener(listener); + } + + clearDartCallbackRegistrations(); + + flutterEngine = null; engineId = null; + lifecycleListener = null; + } - if (detachedEngineId == null || !ensureNativeLibraryLoaded()) { + private void clearDartCallbackRegistrations() { + final Long id = engineId; + if (id == null || !ensureNativeLibraryLoaded()) { return; } - nativeClearDartCallbackRegistrationsForEngine(detachedEngineId); + nativeClearDartCallbackRegistrationsForEngine(id); } } From eec5cb0850b966ccc89a738a4d7e7e0ece1fdd5c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 04:30:15 +0000 Subject: [PATCH 2/8] Android: never block the platform thread, and tear down on engine detach Two related fixes for apps whose process outlives a FlutterEngine -- routine for a foreground-service audio app such as audio_service. 1. The engine-scoped callback clear no longer blocks the platform thread. clearDartCallbackRegistrationsForEngine() took init_deinit_mutex and loadMutex, and runs on the Android platform thread from onDetachedFromEngine(). init_deinit_mutex is held for the whole of dispose(), which joins the lifecycle scheduler and can inherit a stalled device stop, so detaching while a device operation was in flight could park the UI thread and ANR. Nulling the three global bridges is what actually stops native code invoking a dead NativeCallable, and needs only dart_callback_invocation_mutex, which is never held across a device operation. The per-BufferStream callbacks live inside Player and still need init_deinit_mutex; that part now tries the lock and, on failure, hands the work to a worker that can afford to wait. A failed try_lock must NOT be read as "a dispose is in flight and will do this for me": init_deinit_mutex is also held by loadFile(), loadMem(), initEngine(), changeDevice(), the device start/stop calls and isInited(), none of which clear callbacks. A file load holds it across disk I/O and decode, so a detach landing during one would otherwise silently leave every BufferStream callable pointing at a dying isolate. Doing the Player part outside dart_callback_invocation_mutex also removes a lock-order inversion against disposeSound(), which holds sounds_mutex across soloud.stop() and reaches voiceEndedCallback(). 2. Destroying an engine now tears the native engine down. Previously detach only cleared the bridges, leaving an initialized engine, a live output device and a running scheduler with no Dart able to drive them. requestEngineTeardownForEngine() drops the bridges synchronously and hands the blocking teardown to a detached worker. Both workers capture an initialization generation, bumped by prepareEngineInit(), and abort if a replacement engine initialized while they were waiting -- so a late teardown can never dispose a live engine, and a late clear can never erase callbacks a new engine just registered. Verified: a harness modelling a loadFile() holding the mutex across its I/O shows the old logic performing 0 clears while the new logic performs the clear once the lock frees, with the caller returning in ~280us. A 200-trial race harness confirms a stale teardown never disposes a replacement engine, and a 100-trial one the same for the deferred clear. Both clean under ThreadSanitizer. JNI symbol names diffed against javac -h output. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 2 + .../flutter_soloud/FlutterSoloudPlugin.java | 57 +++++- src/bindings.cpp | 177 ++++++++++++++++-- 3 files changed, 216 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b95081b..a7cd8b5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,6 @@ #### 4.0.13 (20 Jul 2026) +- Android: destroying a `FlutterEngine` while the process keeps running (foreground-service apps such as `audio_service`) now tears the native engine down instead of leaving an initialized engine and a running output device with no Dart left to drive them. The blocking part runs on a native worker thread, and a teardown is abandoned if a replacement engine initializes first +- fix: detaching a `FlutterEngine` on Android no longer blocks the platform thread on the engine-teardown mutex, which could ANR if a device operation was in flight - fix: Android hot restart now clears stale Dart callback registrations. Hot restart replaces the isolate without detaching plugins, so the registered `NativeCallable`s silently went stale - Android: the plugin now does no native work at app startup. Plugin registration and `onAttachedToEngine` are pure Java bookkeeping; the native library is loaded lazily, only when an engine-lifecycle hook actually has to call into it. Previously a static initializer pulled the whole library onto the main thread during app launch even for apps that never played a sound, and a load failure there crashed plugin registration - fix: Waveform audio sources do not match engine sample rate #501. Thanks to @Colton127 diff --git a/android/src/main/java/flutter/soloud/flutter_soloud/FlutterSoloudPlugin.java b/android/src/main/java/flutter/soloud/flutter_soloud/FlutterSoloudPlugin.java index e517fdc6..65e8813f 100644 --- a/android/src/main/java/flutter/soloud/flutter_soloud/FlutterSoloudPlugin.java +++ b/android/src/main/java/flutter/soloud/flutter_soloud/FlutterSoloudPlugin.java @@ -5,6 +5,21 @@ import io.flutter.embedding.engine.FlutterEngine; import io.flutter.embedding.engine.plugins.FlutterPlugin; +/** + * Keeps flutter_soloud's process-global native state in step with the lifetime + * of the FlutterEngine that owns it. + * + *

The native engine (the SoLoud player, its output device, its lifecycle + * scheduler and the registered Dart callback pointers) lives for the whole + * process, while the Dart isolate that drives it belongs to a single + * FlutterEngine. When an engine goes away but the process keeps running -- + * routine for a foreground-service audio app, e.g. audio_service -- native code + * would otherwise keep calling into NativeCallables whose isolate is gone + * (undefined behaviour) and keep an output device running with nothing left to + * control it. Only the embedder can observe that transition: Dart's + * {@code detached} lifecycle state is not guaranteed to arrive first, and there + * is no reliable root-isolate exit hook. + */ public final class FlutterSoloudPlugin implements FlutterPlugin { /** * Guarded by the class monitor. @@ -26,10 +41,19 @@ public final class FlutterSoloudPlugin implements FlutterPlugin { private static native boolean nativeClearDartCallbackRegistrationsForEngine(long engineId); + private static native boolean + nativeRequestEngineTeardownForEngine(long engineId); + @Nullable private FlutterEngine flutterEngine; @Nullable private Long engineId; @Nullable private FlutterEngine.EngineLifecycleListener lifecycleListener; + /** + * onEngineWillDestroy() and onDetachedFromEngine() both fire on a real + * engine destroy; the teardown must only be requested once. + */ + private boolean teardownRequested = false; + private static synchronized boolean ensureNativeLibraryLoaded() { if (!nativeLibraryLoadAttempted) { nativeLibraryLoadAttempted = true; @@ -54,10 +78,12 @@ public void onAttachedToEngine( // Deliberately does no native work. This runs during app launch for // every app that depends on the plugin, whether or not it ever uses // SoLoud, so it must stay pure Java bookkeeping: read the engine id and - // register a listener. + // register a listener. Nothing here loads the native library, opens a + // device, or starts a thread. final FlutterEngine engine = binding.getFlutterEngine(); flutterEngine = engine; engineId = engine.getEngineId(); + teardownRequested = false; lifecycleListener = new FlutterEngine.EngineLifecycleListener() { @Override @@ -72,9 +98,9 @@ public void onPreEngineRestart() { @Override public void onEngineWillDestroy() { - // Fires just before the plugin registry is destroyed, while the - // engine is still valid. - clearDartCallbackRegistrations(); + // Fires just before the plugin registry is destroyed. The engine + // is still valid here, so this is the earliest safe point. + requestEngineTeardown(); } }; engine.addEngineLifecycleListener(lifecycleListener); @@ -91,7 +117,9 @@ public void onDetachedFromEngine( engine.removeEngineLifecycleListener(listener); } - clearDartCallbackRegistrations(); + // Requested here too: onEngineWillDestroy() is not reached on every + // detach path, and requestEngineTeardown() is idempotent. + requestEngineTeardown(); flutterEngine = null; engineId = null; @@ -105,4 +133,23 @@ private void clearDartCallbackRegistrations() { } nativeClearDartCallbackRegistrationsForEngine(id); } + + /** + * Drops the Dart bridges and asks native code to tear the engine down. The + * blocking part of the teardown (stopping the device, joining the lifecycle + * scheduler) runs on a native worker thread, so this returns promptly and + * never blocks the platform thread. + */ + private void requestEngineTeardown() { + final Long id = engineId; + if (id == null || teardownRequested) { + return; + } + teardownRequested = true; + + if (!ensureNativeLibraryLoaded()) { + return; + } + nativeRequestEngineTeardownForEngine(id); + } } diff --git a/src/bindings.cpp b/src/bindings.cpp index 16cc4e91..1e9b14ab 100644 --- a/src/bindings.cpp +++ b/src/bindings.cpp @@ -26,6 +26,7 @@ #include #include #include +#include std::mutex dart_callback_invocation_mutex; @@ -38,6 +39,11 @@ constexpr int64_t kNoDartCallbackOwnerEngineId = -1; // Protected by dart_callback_invocation_mutex. int64_t dartCallbackOwnerEngineId = kNoDartCallbackOwnerEngineId; + +// Advances on every prepareEngineInit(). A teardown queued when a FlutterEngine +// detached captures this and aborts if a replacement engine initialized while +// the worker was waiting, so a late teardown can never kill a live engine. +std::atomic engineInitGeneration{0}; } #ifdef __cplusplus @@ -219,17 +225,61 @@ setDartEventCallback(dartVoiceEndedCallback_t voice_ended_callback, dartCallbackOwnerEngineId = owner_engine_id; } -static void clearDartCallbackRegistrationsLocked() { +/// Make the three process-global Dart bridges inert. +/// +/// The caller must hold dart_callback_invocation_mutex. This performs no +/// blocking work, so it stays safe to run on a platform/UI thread. +static void clearDartCallbackPointersLocked() { dartVoiceEndedCallback.store(nullptr, std::memory_order_release); dartFileLoadedCallback.store(nullptr, std::memory_order_release); dartStateChangedCallback.store(nullptr, std::memory_order_release); dartCallbackOwnerEngineId = kNoDartCallbackOwnerEngineId; +} +/// Additionally clear the per-BufferStream Dart callbacks. +/// +/// The caller must hold init_deinit_mutex, which owns the `player` unique_ptr +/// that dispose() resets. BufferStream::clearDartCallbacks() is a pair of +/// atomic stores, so it does not need dart_callback_invocation_mutex. +static void clearPlayerDartCallbackRegistrationsLocked() { if (player.get() != nullptr) { player.get()->clearDartCallbackRegistrations(); } } +static void clearDartCallbackRegistrationsLocked() { + clearDartCallbackPointersLocked(); + clearPlayerDartCallbackRegistrationsLocked(); +} + +/// Clear the per-BufferStream Dart callbacks once init_deinit_mutex becomes +/// available, without making the caller wait for it. +/// +/// Used when the caller runs on a thread that must not block (the Android +/// platform thread) and the mutex is currently held by an unrelated operation +/// such as loadFile() or initEngine(). The worker captures the initialization +/// generation and gives up if a new engine has initialized meanwhile, so it can +/// never erase callbacks that a replacement engine has just registered. +static void queuePlayerDartCallbackClear() { + const uint64_t generation = + engineInitGeneration.load(std::memory_order_acquire); + + try { + std::thread([generation]() { + std::lock_guard guard(init_deinit_mutex); + + if (engineInitGeneration.load(std::memory_order_acquire) != generation) + return; + + clearPlayerDartCallbackRegistrationsLocked(); + }).detach(); + } catch (...) { + // Best effort. The global bridges are already inert, which is what stops + // native code invoking a dead callable; a teardown or a later init() + // still clears the BufferStream callbacks. + } +} + FFI_PLUGIN_EXPORT void clearDartCallbackRegistrations() { std::lock_guard guard_init(init_deinit_mutex); std::lock_guard guard_load(loadMutex); @@ -238,16 +288,49 @@ FFI_PLUGIN_EXPORT void clearDartCallbackRegistrations() { clearDartCallbackRegistrationsLocked(); } +/// Invalidate the Dart bridges owned by [engine_id] when its FlutterEngine is +/// detached. Returns false when a different engine owns the current +/// registration, so a detaching engine never clears another one's callbacks. +/// +/// This runs on the Android platform (UI) thread, so unlike +/// clearDartCallbackRegistrations() it must never wait behind a device +/// operation. init_deinit_mutex is held for the whole of dispose(), which joins +/// the lifecycle scheduler and can therefore inherit a stalled +/// ma_device_stop(); blocking on it here would ANR the app at engine teardown. +/// Only dart_callback_invocation_mutex is taken unconditionally — it is never +/// held across a device operation. FFI_PLUGIN_EXPORT bool clearDartCallbackRegistrationsForEngine(int64_t engine_id) { - std::lock_guard guard_init(init_deinit_mutex); - std::lock_guard guard_load(loadMutex); - std::lock_guard callbackGuard(dart_callback_invocation_mutex); + { + std::lock_guard callbackGuard(dart_callback_invocation_mutex); - if (dartCallbackOwnerEngineId != engine_id) - return false; + if (dartCallbackOwnerEngineId != engine_id) + return false; - clearDartCallbackRegistrationsLocked(); + clearDartCallbackPointersLocked(); + } + + // The BufferStream callbacks live inside Player, so reaching them needs + // init_deinit_mutex, which owns the `player` unique_ptr that dispose() + // resets. Doing this outside dart_callback_invocation_mutex also keeps the + // lock order consistent with disposeSound(), which holds sounds_mutex across + // soloud.stop() and reaches voiceEndedCallback(). + // + // Take the fast path when the mutex happens to be free, but never treat a + // failed try_lock as "someone else will handle it": init_deinit_mutex is held + // by loadFile(), loadMem(), initEngine(), changeDevice(), the device + // start/stop calls and even isInited() — none of which clear these + // callbacks. Hand the work to a worker that can afford to wait instead. + { + std::unique_lock guard_init(init_deinit_mutex, + std::try_to_lock); + if (guard_init.owns_lock()) { + clearPlayerDartCallbackRegistrationsLocked(); + return true; + } + } + + queuePlayerDartCallbackClear(); return true; } @@ -278,6 +361,9 @@ FFI_PLUGIN_EXPORT bool areXiphLibsAvailable() { /// Returns [PlayerErrors.noError] if success. FFI_PLUGIN_EXPORT void prepareEngineInit() { engine_shutdown_requested.store(false, std::memory_order_release); + // Invalidate any teardown queued by a previous engine's detach so it cannot + // dispose the engine this initialization is about to create. + engineInitGeneration.fetch_add(1, std::memory_order_acq_rel); } FFI_PLUGIN_EXPORT void requestEngineShutdown() { @@ -375,7 +461,6 @@ FFI_PLUGIN_EXPORT enum AudioDeviceState getAudioDeviceState() { // query never waits behind an initialization or lifecycle API call. return (AudioDeviceState)SoLoud::miniaudio_getAudioDeviceState(); } - /// Test-only hook that sends an interruption through miniaudio's normal /// notification callback. This is intentionally absent from the public API. FFI_PLUGIN_EXPORT void debugTriggerAudioInterruption(unsigned int began) { @@ -442,13 +527,8 @@ FFI_PLUGIN_EXPORT void freeListPlaybackDevices(char **devicesName, /// Must be called when there is no more need of the player or when closing the /// app /// -FFI_PLUGIN_EXPORT void dispose() { - // Preserve request ordering for asynchronous Dart init/deinit workers. - engine_shutdown_requested.store(true, std::memory_order_release); - - std::lock_guard guard(init_deinit_mutex); - std::lock_guard guard_load(loadMutex); - +/// Teardown body. The caller must hold init_deinit_mutex and loadMutex. +static void disposeLocked() { // Wait for any Dart callback currently executing, then make every bridge // inert. Do not retain the callback mutex while stopping devices, joining // threads, destroying sources, or resetting Player. @@ -469,6 +549,65 @@ FFI_PLUGIN_EXPORT void dispose() { analyzer = std::make_unique(256); } +FFI_PLUGIN_EXPORT void dispose() { + // Preserve request ordering for asynchronous Dart init/deinit workers. + engine_shutdown_requested.store(true, std::memory_order_release); + + std::lock_guard guard(init_deinit_mutex); + std::lock_guard guard_load(loadMutex); + + disposeLocked(); +} + +/// Tear the engine down because its owning FlutterEngine is being destroyed +/// while the process keeps running (the audio_service / add-to-app case). +/// +/// Without this the native engine stays initialized with a live output device +/// and a running scheduler after the last Dart code that could drive it is +/// gone. Returns false when a different engine owns the current registration. +/// +/// The blocking teardown is handed to a detached worker: this is invoked from +/// the Android platform thread, which must never wait on a device operation. +FFI_PLUGIN_EXPORT bool requestEngineTeardownForEngine(int64_t engine_id) { + { + std::lock_guard callbackGuard(dart_callback_invocation_mutex); + + // An unowned registration is accepted: Dart may already have deinited + // cleanly, or this engine never registered callbacks at all. A registration + // owned by a *different* engine must be left alone. + if (dartCallbackOwnerEngineId != engine_id && + dartCallbackOwnerEngineId != kNoDartCallbackOwnerEngineId) + return false; + + clearDartCallbackPointersLocked(); + } + + // Reject an initialization worker that has not entered native code yet. + engine_shutdown_requested.store(true, std::memory_order_release); + const uint64_t generation = + engineInitGeneration.load(std::memory_order_acquire); + + try { + std::thread([generation]() { + std::lock_guard guard(init_deinit_mutex); + std::lock_guard guard_load(loadMutex); + + // A replacement engine initialized while this worker was waiting for the + // mutex. Its engine is live and must not be torn down. + if (engineInitGeneration.load(std::memory_order_acquire) != generation) + return; + + disposeLocked(); + }).detach(); + } catch (...) { + // Thread creation failed. The bridges are already inert, and the next + // init() still recovers by deiniting the stale engine itself. + return false; + } + + return true; +} + #if defined(__ANDROID__) extern "C" JNIEXPORT jboolean JNICALL Java_flutter_soloud_flutter_1soloud_FlutterSoloudPlugin_nativeClearDartCallbackRegistrationsForEngine( @@ -477,6 +616,14 @@ Java_flutter_soloud_flutter_1soloud_FlutterSoloudPlugin_nativeClearDartCallbackR ? JNI_TRUE : JNI_FALSE; } + +extern "C" JNIEXPORT jboolean JNICALL +Java_flutter_soloud_flutter_1soloud_FlutterSoloudPlugin_nativeRequestEngineTeardownForEngine( + JNIEnv *, jclass, jlong engine_id) { + return requestEngineTeardownForEngine(static_cast(engine_id)) + ? JNI_TRUE + : JNI_FALSE; +} #endif FFI_PLUGIN_EXPORT int isInited() { From 230439994d14f3d7ef23cae44c69c92c6436333f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Jul 2026 02:47:06 +0000 Subject: [PATCH 3/8] Gate engine teardown on lifecycle ownership, not callback ownership Callback ownership is the wrong signal for deciding whether a detaching FlutterEngine may dispose the native engine, and it fails in both directions. dartCallbackOwnerEngineId is unset for the whole of an initialization: Dart registers callbacks only after initEngine() returns, and initEngine() opens the audio device, which takes seconds on Android. An engine destroyed in that window could not tear down the Player, scheduler and device it had just built -- exactly the state this teardown path exists to prevent. It is also still set to the *previous* engine once a replacement has called prepareEngineInit(). A detaching engine was therefore accepted, captured the replacement's already-bumped generation, and went on to dispose a live engine -- or raised engine_shutdown_requested after the replacement lowered it, so the replacement's initEngine() refused to initialize. The generation counter cannot separate these: it is bumped when an initialization starts, which is before the teardown reads it. prepareEngineInit() now takes the owning engine id and claims the native engine under engine_lifecycle_mutex, alongside the generation and the shutdown flag. A teardown is accepted only while that claim still names the detaching engine, and its worker re-checks the whole claim before disposing. Verifying the claim and raising the shutdown flag in one critical section is what stops a detach cancelling a replacement's initialization. dartCallbackOwnerEngineId now decides only whose callable pointers may be cleared, which is all it was ever able to answer. Also fixes the Java teardown flag, which was set before the library load and the JNI call, making a failure of either terminal: onDetachedFromEngine() found teardownRequested already set and returned, so the retry the lazy loader is documented to allow could never happen. Verified with a harness that #includes the claim helpers verbatim from bindings.cpp so it cannot drift, over five scenarios plus a 400-trial race: gate gap overlap ordinary deinit race callback-owner-or-unowned ok BROKEN ok BROKEN 95/400 strict callback owner BROKEN BROKEN ok ok 400/400 lifecycle ownership ok ok ok ok 400/400 Race column under ThreadSanitizer, which surfaces the interleaving far more often than default timing (1/400). No TSan race reports. bindings.cpp passes g++ -Wall -Wextra, the plugin compiles clean under -Xlint:all against embedding stubs, flutter analyze reports only the 6 pre-existing info lints, and flutter test passes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PWTMdp5kHauqc4Jjwp7b5t --- CHANGELOG.md | 2 +- .../flutter_soloud/FlutterSoloudPlugin.java | 12 +- lib/src/bindings/bindings_player_ffi.dart | 15 +- src/bindings.cpp | 165 +++++++++++++----- 4 files changed, 147 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7725a0cd..6e02d22e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ #### 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 -- Android: destroying a `FlutterEngine` while the process keeps running (foreground-service apps such as `audio_service`) now tears the native engine down instead of leaving an initialized engine and a running output device with no Dart left to drive them. The blocking part runs on a native worker thread, and a teardown is abandoned if a replacement engine initializes first +- Android: destroying a `FlutterEngine` while the process keeps running (foreground-service apps such as `audio_service`) now tears the native engine down instead of leaving an initialized engine and a running output device with no Dart left to drive them. The blocking part runs on a native worker thread. A `FlutterEngine` claims the native engine for the whole of its initialization, so an engine destroyed mid-init still tears down what it built, and a detaching engine can neither dispose nor cancel the initialization of a replacement that has already claimed it - fix: detaching a `FlutterEngine` on Android no longer blocks the platform thread on the engine-teardown mutex, which could ANR if a device operation was in flight - fix: Android hot restart now clears stale Dart callback registrations. Hot restart replaces the isolate without detaching plugins, so the registered `NativeCallable`s silently went stale - Android: the plugin now does no native work at app startup. Plugin registration and `onAttachedToEngine` are pure Java bookkeeping; the native library is loaded lazily, only when an engine-lifecycle hook actually has to call into it. Previously a static initializer pulled the whole library onto the main thread during app launch even for apps that never played a sound, and a load failure there crashed plugin registration diff --git a/android/src/main/java/flutter/soloud/flutter_soloud/FlutterSoloudPlugin.java b/android/src/main/java/flutter/soloud/flutter_soloud/FlutterSoloudPlugin.java index ca4a62a1..fb3ebf4b 100644 --- a/android/src/main/java/flutter/soloud/flutter_soloud/FlutterSoloudPlugin.java +++ b/android/src/main/java/flutter/soloud/flutter_soloud/FlutterSoloudPlugin.java @@ -158,14 +158,22 @@ private void requestEngineTeardown() { if (id == null || teardownRequested) { return; } - teardownRequested = true; + // Marked as requested only once native code has accepted it. Setting it + // up front would make a failed library load, a failed JNI call, or a + // native worker that could not be spawned terminal: onDetachedFromEngine() + // is the retry for a hook that ran too early or hit a load that can still + // succeed later, and it would have found the flag already set. + // + // A native `false` is not always retryable -- it also means another + // engine owns the native engine, or nothing is claimed -- but retrying + // those costs one rejected call and keeps the recoverable cases working. if (!ensureNativeLibraryLoaded()) { return; } try { - nativeRequestEngineTeardownForEngine(id); + teardownRequested = nativeRequestEngineTeardownForEngine(id); } catch (UnsatisfiedLinkError error) { Log.w( TAG, diff --git a/lib/src/bindings/bindings_player_ffi.dart b/lib/src/bindings/bindings_player_ffi.dart index 20117046..9cbf7660 100644 --- a/lib/src/bindings/bindings_player_ffi.dart +++ b/lib/src/bindings/bindings_player_ffi.dart @@ -395,12 +395,21 @@ class FlutterSoLoudFfi extends FlutterSoLoud { >('initEngine'); @override - void prepareEngineInit() => _prepareEngineInit(); + void prepareEngineInit() { + // Claims the native engine for this FlutterEngine for the whole of the + // initialization that follows, so an engine destroyed mid-init can still + // tear down what it built. Read the same way as in + // [setDartEventCallbacks]; -1 on platforms that expose no engine id, which + // are also the platforms with no engine-lifecycle hooks to act on it. + _prepareEngineInit(ui.PlatformDispatcher.instance.engineId ?? -1); + } late final _prepareEngineInitPtr = - _lookup>('prepareEngineInit'); + _lookup>( + 'prepareEngineInit', + ); late final _prepareEngineInit = _prepareEngineInitPtr - .asFunction(); + .asFunction(); @override void requestEngineShutdown() => _requestEngineShutdown(); diff --git a/src/bindings.cpp b/src/bindings.cpp index 5c8ca38b..fcdeb05a 100644 --- a/src/bindings.cpp +++ b/src/bindings.cpp @@ -34,16 +34,75 @@ std::mutex dart_callback_invocation_mutex; // never be held while joining the lifecycle scheduler, starting or stopping // the audio device, disposing Player, or destroying native audio sources. +// Defined below with the other C-linkage globals. Declared here so the engine +// lifecycle helpers can raise it while holding engine_lifecycle_mutex. +extern "C" std::atomic engine_shutdown_requested; + namespace { -constexpr int64_t kNoDartCallbackOwnerEngineId = -1; +constexpr int64_t kNoEngineId = -1; + +// Protected by dart_callback_invocation_mutex. Decides only whose *callable +// pointers* may be cleared -- never whether the engine may be torn down. It is +// unset for the whole of an initialization, because Dart registers callbacks +// only after initEngine() has returned. +int64_t dartCallbackOwnerEngineId = kNoEngineId; + +// Which FlutterEngine currently claims the native engine, and a counter +// advanced by every prepareEngineInit(). Claimed at the *start* of an +// initialization rather than when callbacks register, so the engine has a +// lifecycle owner during the whole init -- including the long window where +// initEngine() has opened the device but Dart has not registered callbacks yet. +// +// Both fields are read and written together, so they are guarded by a mutex +// rather than made individually atomic: a teardown must observe a consistent +// (owner, generation) pair, and no interleaving of two atomic loads gives that. +// This is a leaf lock -- never held while acquiring another lock or performing +// any blocking work. +std::mutex engine_lifecycle_mutex; +int64_t nativeInitOwnerEngineId = kNoEngineId; +uint64_t engineInitGeneration = 0; + +struct EngineLifecycleClaim { + int64_t ownerEngineId; + uint64_t generation; +}; + +EngineLifecycleClaim currentEngineLifecycleClaim() { + std::lock_guard guard(engine_lifecycle_mutex); + return EngineLifecycleClaim{nativeInitOwnerEngineId, engineInitGeneration}; +} + +/// Accept a teardown for [engine_id] and capture the claim it is tearing down. +/// +/// Verifying the claim and raising engine_shutdown_requested in one critical +/// section is what stops a detaching engine cancelling a *replacement* engine's +/// initialization. prepareEngineInit() lowers that flag and re-claims under the +/// same mutex, so the two can no longer interleave as: teardown reads the old +/// claim, replacement re-claims and lowers the flag, teardown raises it again, +/// and the replacement's initEngine() then refuses to initialize. +bool tryBeginEngineTeardown(int64_t engine_id, EngineLifecycleClaim *out) { + std::lock_guard guard(engine_lifecycle_mutex); + + if (nativeInitOwnerEngineId != engine_id) + return false; + + *out = EngineLifecycleClaim{nativeInitOwnerEngineId, engineInitGeneration}; + // Rejects an initialization worker of this same engine that has not entered + // native code yet. + engine_shutdown_requested.store(true, std::memory_order_release); + return true; +} -// Protected by dart_callback_invocation_mutex. -int64_t dartCallbackOwnerEngineId = kNoDartCallbackOwnerEngineId; +bool engineLifecycleClaimIsCurrent(const EngineLifecycleClaim &claim) { + std::lock_guard guard(engine_lifecycle_mutex); + return nativeInitOwnerEngineId == claim.ownerEngineId && + engineInitGeneration == claim.generation; +} -// Advances on every prepareEngineInit(). A teardown queued when a FlutterEngine -// detached captures this and aborts if a replacement engine initialized while -// the worker was waiting, so a late teardown can never kill a live engine. -std::atomic engineInitGeneration{0}; +void releaseEngineLifecycleClaim() { + std::lock_guard guard(engine_lifecycle_mutex); + nativeInitOwnerEngineId = kNoEngineId; +} } #ifdef __cplusplus @@ -233,7 +292,7 @@ static void clearDartCallbackPointersLocked() { dartVoiceEndedCallback.store(nullptr, std::memory_order_release); dartFileLoadedCallback.store(nullptr, std::memory_order_release); dartStateChangedCallback.store(nullptr, std::memory_order_release); - dartCallbackOwnerEngineId = kNoDartCallbackOwnerEngineId; + dartCallbackOwnerEngineId = kNoEngineId; } /// Additionally clear the per-BufferStream Dart callbacks. @@ -257,18 +316,18 @@ static void clearDartCallbackRegistrationsLocked() { /// /// Used when the caller runs on a thread that must not block (the Android /// platform thread) and the mutex is currently held by an unrelated operation -/// such as loadFile() or initEngine(). The worker captures the initialization -/// generation and gives up if a new engine has initialized meanwhile, so it can -/// never erase callbacks that a replacement engine has just registered. +/// such as loadFile() or initEngine(). The worker captures the engine lifecycle +/// claim and gives up if a new engine has claimed the native engine meanwhile, +/// so it can never erase callbacks that a replacement engine has just +/// registered. static void queuePlayerDartCallbackClear() { - const uint64_t generation = - engineInitGeneration.load(std::memory_order_acquire); + const EngineLifecycleClaim claim = currentEngineLifecycleClaim(); try { - std::thread([generation]() { + std::thread([claim]() { std::lock_guard guard(init_deinit_mutex); - if (engineInitGeneration.load(std::memory_order_acquire) != generation) + if (!engineLifecycleClaimIsCurrent(claim)) return; clearPlayerDartCallbackRegistrationsLocked(); @@ -359,11 +418,23 @@ FFI_PLUGIN_EXPORT bool areXiphLibsAvailable() { /// 2=stereo, 4=quad, 6=5.1, 8=7.1. /// /// Returns [PlayerErrors.noError] if success. -FFI_PLUGIN_EXPORT void prepareEngineInit() { +/// [owner_engine_id] is the FlutterEngine that will own the engine this +/// initialization creates, or -1 on platforms with no engine-lifecycle hooks. +FFI_PLUGIN_EXPORT void prepareEngineInit(int64_t owner_engine_id) { + std::lock_guard guard(engine_lifecycle_mutex); + // Lowered under the same mutex that publishes the claim, so a teardown for + // the previous engine cannot raise it again after this point. See + // tryBeginEngineTeardown(). engine_shutdown_requested.store(false, std::memory_order_release); + // Claim the native engine for this FlutterEngine now, before initEngine() + // runs. Ownership must not wait for callback registration: initEngine() + // opens the audio device and can take seconds on Android, and Dart registers + // callbacks only after it returns. An engine destroyed during that window + // still has to be able to tear down what it just built. + nativeInitOwnerEngineId = owner_engine_id; // Invalidate any teardown queued by a previous engine's detach so it cannot // dispose the engine this initialization is about to create. - engineInitGeneration.fetch_add(1, std::memory_order_acq_rel); + ++engineInitGeneration; } FFI_PLUGIN_EXPORT void requestEngineShutdown() { @@ -538,6 +609,11 @@ static void disposeLocked() { clearDartCallbackRegistrationsLocked(); } + // Nothing is left for a FlutterEngine to own. A detach arriving after this + // finds no claim and correctly declines to tear anything down; the next + // prepareEngineInit() takes a fresh claim. + releaseEngineLifecycleClaim(); + if (player.get() == nullptr) return; @@ -564,43 +640,50 @@ FFI_PLUGIN_EXPORT void dispose() { /// /// Without this the native engine stays initialized with a live output device /// and a running scheduler after the last Dart code that could drive it is -/// gone. Returns false unless [engine_id] owns the current registration. +/// gone. Returns false unless [engine_id] still owns the native engine. +/// +/// Ownership here is the *lifecycle* claim taken by prepareEngineInit(), not +/// dartCallbackOwnerEngineId. Gating on callback ownership would be wrong in +/// both directions: it is unset for the whole of an initialization -- so an +/// engine destroyed after initEngine() opened the device but before Dart +/// registered callbacks could not tear down what it had just built -- and it +/// still names the *previous* engine once a replacement has called +/// prepareEngineInit(), so a detaching engine would be accepted and would then +/// capture the replacement's already-bumped generation and dispose a live +/// engine. The generation cannot rescue either case: it is bumped when an +/// initialization starts, which is before this teardown reads it. /// /// The blocking teardown is handed to a detached worker: this is invoked from /// the Android platform thread, which must never wait on a device operation. FFI_PLUGIN_EXPORT bool requestEngineTeardownForEngine(int64_t engine_id) { + if (engine_id == kNoEngineId) + return false; + + // A different engine has claimed the native engine, so it is live and owns + // its own teardown. An unclaimed engine means Dart already deinited cleanly, + // leaving nothing to dispose. + EngineLifecycleClaim claim; + if (!tryBeginEngineTeardown(engine_id, &claim)) + return false; + { std::lock_guard callbackGuard(dart_callback_invocation_mutex); - // Only the owning engine may tear the engine down. An unowned registration - // is deliberately rejected: it means either that Dart already deinited - // cleanly -- leaving nothing to dispose -- or that a *replacement* engine - // has initialized but has not registered its callbacks yet, which is a live - // engine this detach must not touch. - // - // engineInitGeneration alone cannot separate those two cases. A replacement - // that called prepareEngineInit() before this detach began has already - // bumped the generation, so the worker below would observe the value it - // captured and proceed to dispose an engine that is very much in use. - if (dartCallbackOwnerEngineId != engine_id) - return false; - - clearDartCallbackPointersLocked(); + // Clearing the callable pointers stays gated on callback ownership: this + // engine owns the lifecycle, but only the engine that registered the + // callables may null them. They are already null when unregistered. + if (dartCallbackOwnerEngineId == engine_id) + clearDartCallbackPointersLocked(); } - // Reject an initialization worker that has not entered native code yet. - engine_shutdown_requested.store(true, std::memory_order_release); - const uint64_t generation = - engineInitGeneration.load(std::memory_order_acquire); - try { - std::thread([generation]() { + std::thread([claim]() { std::lock_guard guard(init_deinit_mutex); std::lock_guard guard_load(loadMutex); - // A replacement engine initialized while this worker was waiting for the - // mutex. Its engine is live and must not be torn down. - if (engineInitGeneration.load(std::memory_order_acquire) != generation) + // A replacement engine claimed the native engine while this worker was + // waiting for the mutex. Its engine is live and must not be torn down. + if (!engineLifecycleClaimIsCurrent(claim)) return; disposeLocked(); From a02149f0fdeb193bca4b22dcac1c8895b8d12c0d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Jul 2026 03:13:26 +0000 Subject: [PATCH 4/8] Retire a detaching engine's callables even when its teardown is refused Since the teardown hook replaced the plain callback clear on the Java detach path, requestEngineTeardownForEngine() is the only thing that retires Dart callables when a FlutterEngine is destroyed. Gating that clear on the lifecycle claim therefore left a hole: an engine can own the callables without owning the claim. initEngine() carries no ownership token, so the order in which two engines call prepareEngineInit() does not determine the order their initialization workers acquire init_deinit_mutex. Engine A can claim, engine B can claim after it, and A's worker can still be the one that initializes the Player. A then registers its callables -- registration only happens after initEngine() returns -- leaving callback owner A with lifecycle owner B. Destroying A was correctly refused the engine teardown, but returned before clearing anything, so native code kept invoking callables belonging to a dead isolate. That is the undefined behaviour this path exists to prevent. The clear now runs before the lifecycle gate and stays scoped by callback ownership, which is the question it was always able to answer. It cannot touch another engine's callables: clearDartCallbackRegistrationsForEngine() returns early unless the caller owns the registration. Harness scenario 6 drives that interleaving directly and asserts the callables are retired on a refused teardown: it fails with the clear after the gate and passes with it before. Scenarios 1-5 and the 400-trial race are unchanged, clean under ThreadSanitizer. This does not close the underlying ownership gap -- initEngine(), setDartEventCallback(), requestEngineShutdown() and dispose() still take no ownership token, so a stale initialization can still run under a newer engine's claim, and a stale deinit can still dispose a live engine. Fixing that needs the initialization lease threaded through those calls, which changes exported signatures the prebuilt web wasm depends on. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PWTMdp5kHauqc4Jjwp7b5t --- src/bindings.cpp | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/bindings.cpp b/src/bindings.cpp index fcdeb05a..b9ffadd7 100644 --- a/src/bindings.cpp +++ b/src/bindings.cpp @@ -659,6 +659,21 @@ FFI_PLUGIN_EXPORT bool requestEngineTeardownForEngine(int64_t engine_id) { if (engine_id == kNoEngineId) return false; + // Retire this engine's callables first, whatever the lifecycle decision + // below turns out to be, and gated on callback ownership rather than the + // lifecycle claim. The isolate that created them is going away and invoking + // one afterwards is undefined behaviour, so this must not be conditional on + // also being allowed to dispose the engine. + // + // It is load-bearing that this runs before the early return: since the + // teardown hook replaced the plain clear on the Java detach path, this is the + // only thing that clears callables when a FlutterEngine is destroyed. An + // engine can legitimately own the callables without owning the lifecycle + // claim -- its initialization worker can win the mutex after a later engine + // has already claimed -- and gating the clear on the claim left that engine's + // callables live after its isolate died. + clearDartCallbackRegistrationsForEngine(engine_id); + // A different engine has claimed the native engine, so it is live and owns // its own teardown. An unclaimed engine means Dart already deinited cleanly, // leaving nothing to dispose. @@ -666,16 +681,6 @@ FFI_PLUGIN_EXPORT bool requestEngineTeardownForEngine(int64_t engine_id) { if (!tryBeginEngineTeardown(engine_id, &claim)) return false; - { - std::lock_guard callbackGuard(dart_callback_invocation_mutex); - - // Clearing the callable pointers stays gated on callback ownership: this - // engine owns the lifecycle, but only the engine that registered the - // callables may null them. They are already null when unregistered. - if (dartCallbackOwnerEngineId == engine_id) - clearDartCallbackPointersLocked(); - } - try { std::thread([claim]() { std::lock_guard guard(init_deinit_mutex); From 69cdd8551872c5154819d9f531b232cb7512e956 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Jul 2026 03:26:26 +0000 Subject: [PATCH 5/8] Scope engine init, callback registration and teardown to a lease prepareEngineInit() claimed the native engine, but nothing downstream carried that claim. initEngine() took only the audio configuration and checked a process-global shutdown flag, so calling prepareEngineInit() first did not determine which initialization worker reached init_deinit_mutex first. Three consequences, all reachable with two FlutterEngines: - A superseded initialization could build over the engine that replaced it. It then registered its callables, leaving callback owner A with lifecycle owner B -- and A's later teardown was refused, so its callables outlived its isolate. - setDartEventCallback() validated nothing, so an engine returning from Dart after being superseded overwrote the live engine's callables, silently, with the replacement still believing its registration was current. - requestEngineShutdown() and dispose() were unscoped, so a queued deinit worker could dispose a live engine. disposeLocked() then released whatever claim was current, leaving the replacement initialized but unowned -- and an unowned engine can never be torn down when its FlutterEngine is destroyed. prepareEngineInit() now returns a lease. initEngineOwned(), setDartEventCallback(), requestEngineShutdown() and disposeForEngine() take (engineId, lease) and refuse to act once it is no longer current. initEngineOwned() revalidates after Player::init() returns, since the device open blocks for seconds and ownership can change underneath it; a stale initialization disposes the Player it just built before releasing the mutex, rather than leaving an orphaned device and scheduler running. disposeLocked() takes the claim it is entitled to retire and leaves a claim that has moved on alone. initEngine() and dispose() keep their signatures and become ownership-unaware wrappers. The web build binds those two exported symbols directly from the committed prebuilt wasm, which cannot be regenerated here, and web has no FlutterEngine lifecycle for a lease to describe. Dart holds the claim in the FFI binding rather than threading it through SoLoud: the binding is already one-per-isolate and an isolate belongs to exactly one FlutterEngine, so those fields are that engine's claim. Adds four test-only native barriers (before the init lock, after init succeeded, before callback registration, before the dispose lock) following the existing debugTriggerAudioInterruption precedent. The interleavings that matter are inside the initialization worker while it holds init_deinit_mutex, which no Dart-side sequencing can reach. Harness now covers 10 scenarios including a stale init refused by its lease, a claim lost during the device open (orphan disposed), a stale deinit refused with the live claim intact, and a superseded engine's late registration refused. All pass, clean under ThreadSanitizer. Scenario 6 was rewritten: the lease makes its original premise unreachable, so it now builds the callback/lifecycle owner split the way that is still reachable. flutter analyze unchanged at the 6 pre-existing info lints; flutter test passes; bindings.cpp clean under -Wall -Wextra; plugin clean under -Xlint:all. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PWTMdp5kHauqc4Jjwp7b5t --- CHANGELOG.md | 1 + lib/src/bindings/bindings_player.dart | 21 ++ lib/src/bindings/bindings_player_ffi.dart | 137 +++++++++--- lib/src/bindings/bindings_player_web.dart | 6 + src/bindings.cpp | 242 ++++++++++++++++++++-- 5 files changed, 355 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e02d22e..1d15bbf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ #### 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 - Android: destroying a `FlutterEngine` while the process keeps running (foreground-service apps such as `audio_service`) now tears the native engine down instead of leaving an initialized engine and a running output device with no Dart left to drive them. The blocking part runs on a native worker thread. A `FlutterEngine` claims the native engine for the whole of its initialization, so an engine destroyed mid-init still tears down what it built, and a detaching engine can neither dispose nor cancel the initialization of a replacement that has already claimed it +- fix: engine initialization, Dart callback registration and teardown are now scoped to an initialization lease taken by the owning `FlutterEngine`. Calling `prepareEngineInit()` first does not win the race to the native init mutex, so without it a superseded initialization could build over a newer engine, a superseded isolate could overwrite the live engine's callbacks, and a queued teardown could dispose a live engine or strip its ownership. An initialization that loses the claim while the audio device is opening now disposes what it built instead of leaving an orphaned device running - fix: detaching a `FlutterEngine` on Android no longer blocks the platform thread on the engine-teardown mutex, which could ANR if a device operation was in flight - fix: Android hot restart now clears stale Dart callback registrations. Hot restart replaces the isolate without detaching plugins, so the registered `NativeCallable`s silently went stale - Android: the plugin now does no native work at app startup. Plugin registration and `onAttachedToEngine` are pure Java bookkeeping; the native library is loaded lazily, only when an engine-lifecycle hook actually has to call into it. Previously a static initializer pulled the whole library onto the main thread during app launch even for apps that never played a sound, and a load failure there crashed plugin registration diff --git a/lib/src/bindings/bindings_player.dart b/lib/src/bindings/bindings_player.dart index 2a13414b..daeec344 100644 --- a/lib/src/bindings/bindings_player.dart +++ b/lib/src/bindings/bindings_player.dart @@ -11,6 +11,27 @@ import 'package:flutter_soloud/src/sound_handle.dart'; import 'package:flutter_soloud/src/sound_hash.dart'; import 'package:meta/meta.dart'; +/// Test-only parking points on the native engine-lifecycle path. +/// +/// Must stay in the same order as `EngineLifecycleBarrier` in `bindings.cpp`: +/// the index is what crosses the FFI boundary. +enum EngineLifecycleBarrier { + /// Nothing armed. Arming this releases anything currently parked. + none, + + /// Before the initialization worker takes the native init mutex. + beforeInitLock, + + /// After native initialization succeeded, before its ownership recheck. + afterInitSucceeded, + + /// Before Dart callback registration validates its lease. + beforeCallbackRegistration, + + /// Before an ownership-scoped teardown takes the native init mutex. + beforeDisposeLock, +} + /// Callback set in `setBufferStream` for the `onBuffering` closure. typedef OnBufferingCallbackTFunction = void Function(bool isBuffering, int handle, double time); diff --git a/lib/src/bindings/bindings_player_ffi.dart b/lib/src/bindings/bindings_player_ffi.dart index 9cbf7660..253f42a3 100644 --- a/lib/src/bindings/bindings_player_ffi.dart +++ b/lib/src/bindings/bindings_player_ffi.dart @@ -68,6 +68,8 @@ int _invokeChangeDevice(int address, int deviceId) { /// same process-global engine is initialized. int _invokeInitEngine( int address, + int engineId, + int lease, int deviceId, int sampleRate, int bufferSize, @@ -78,6 +80,8 @@ int _invokeInitEngine( ffi.Pointer< ffi.NativeFunction< ffi.Int32 Function( + ffi.Int64, + ffi.Uint64, ffi.Int, ffi.UnsignedInt, ffi.UnsignedInt, @@ -86,20 +90,24 @@ int _invokeInitEngine( ) > >.fromAddress(address) - .asFunction(); - return fn(deviceId, sampleRate, bufferSize, channels, lowLatency); + .asFunction(); + return fn( + engineId, + lease, + deviceId, + sampleRate, + bufferSize, + channels, + lowLatency, + ); } -/// Rebuilds a `void Function()` native function from its raw pointer [address] -/// and invokes it. -/// -/// Top-level so it can run inside an [Isolate.run] worker: the blocking native -/// teardown (device uninit) then executes off the UI isolate instead of -/// stalling it. Only [address] (a sendable int) crosses the isolate boundary. -void _invokeVoidNative(int address) { - ffi.Pointer>.fromAddress( - address, - ).asFunction()(); +/// Calls `disposeForEngine(engineId, lease)` from a worker isolate. +bool _invokeDisposeForEngine(int address, int engineId, int lease) { + return ffi + .Pointer> + .fromAddress(address) + .asFunction()(engineId, lease); } typedef DartVoiceEndedCallbackT = @@ -286,11 +294,15 @@ class FlutterSoLoudFfi extends FlutterSoLoud { _stateChangedCallback, ); + // The lease is validated natively: registration is refused if this + // initialization was superseded while Dart was away, so a stale engine + // cannot overwrite the callables of the engine that replaced it. _setDartEventCallback( nativeVoiceEndedCallable!.nativeFunction, nativeFileLoadedCallable!.nativeFunction, nativeStateChangedCallable!.nativeFunction, engineId ?? -1, + _claimedLease, ); } @@ -302,6 +314,7 @@ class FlutterSoLoudFfi extends FlutterSoLoud { DartFileLoadedCallbackT, DartStateChangedCallbackT, ffi.Int64, + ffi.Uint64, ) > >('setDartEventCallback'); @@ -312,6 +325,7 @@ class FlutterSoLoudFfi extends FlutterSoLoud { DartFileLoadedCallbackT, DartStateChangedCallbackT, int, + int, ) >(); @@ -365,12 +379,19 @@ class FlutterSoLoudFfi extends FlutterSoLoud { // tripping the ANR watchdog — see #481). Only the raw function pointer // address and the primitive arguments (all sendable ints) are captured; the // pointer is rebuilt and called inside the worker. - final address = _initEnginePtr.address; + final address = _initEngineOwnedPtr.address; final channelCount = channels.count; final lowLatencyInt = lowLatency ? 1 : 0; + // Captured before the worker is dispatched. The worker validates them + // against the current claim, so an initialization superseded while it was + // queued is refused instead of building over the engine that replaced it. + final engineId = _claimedEngineId; + final lease = _claimedLease; final ret = await Isolate.run( () => _invokeInitEngine( address, + engineId, + lease, deviceId, sampleRate, bufferSize, @@ -381,10 +402,12 @@ class FlutterSoLoudFfi extends FlutterSoLoud { return PlayerErrors.values[ret]; } - late final _initEnginePtr = + late final _initEngineOwnedPtr = _lookup< ffi.NativeFunction< ffi.Int32 Function( + ffi.Int64, + ffi.Uint64, ffi.Int, ffi.UnsignedInt, ffi.UnsignedInt, @@ -392,7 +415,17 @@ class FlutterSoLoudFfi extends FlutterSoLoud { ffi.UnsignedInt, ) > - >('initEngine'); + >('initEngineOwned'); + + /// The engine id and lease returned by the most recent [prepareEngineInit]. + /// + /// Held here rather than threaded through `SoLoud` because this binding is + /// already one-per-isolate, and an isolate belongs to exactly one + /// FlutterEngine — so these *are* this engine's claim. Every later call in + /// the initialization hands them back to native code, which refuses to act on + /// a lease that is no longer current. + int _claimedEngineId = -1; + int _claimedLease = 0; @override void prepareEngineInit() { @@ -401,23 +434,27 @@ class FlutterSoLoudFfi extends FlutterSoLoud { // tear down what it built. Read the same way as in // [setDartEventCallbacks]; -1 on platforms that expose no engine id, which // are also the platforms with no engine-lifecycle hooks to act on it. - _prepareEngineInit(ui.PlatformDispatcher.instance.engineId ?? -1); + _claimedEngineId = ui.PlatformDispatcher.instance.engineId ?? -1; + _claimedLease = _prepareEngineInit(_claimedEngineId); } late final _prepareEngineInitPtr = - _lookup>( + _lookup>( 'prepareEngineInit', ); late final _prepareEngineInit = _prepareEngineInitPtr - .asFunction(); + .asFunction(); @override - void requestEngineShutdown() => _requestEngineShutdown(); + void requestEngineShutdown() => + _requestEngineShutdown(_claimedEngineId, _claimedLease); late final _requestEngineShutdownPtr = - _lookup>('requestEngineShutdown'); + _lookup>( + 'requestEngineShutdown', + ); late final _requestEngineShutdown = _requestEngineShutdownPtr - .asFunction(); + .asFunction(); @override void setAndroidAAudioAttributes(bool managed) { @@ -509,6 +546,34 @@ class FlutterSoLoudFfi extends FlutterSoLoud { late final _debugTriggerAudioInterruption = _debugTriggerAudioInterruptionPtr .asFunction(); + /// Test-only: park the next thread reaching [barrier] until it is disarmed. + /// + /// The engine-lifecycle interleavings that matter happen inside the + /// initialization worker while it holds the native init mutex, which no + /// amount of Dart-side sequencing can reach. Pass + /// [EngineLifecycleBarrier.none] to release whatever is parked. + void debugArmEngineLifecycleBarrier(EngineLifecycleBarrier barrier) { + _debugArmEngineLifecycleBarrier(barrier.index); + } + + late final _debugArmEngineLifecycleBarrierPtr = + _lookup>( + 'debugArmEngineLifecycleBarrier', + ); + late final _debugArmEngineLifecycleBarrier = + _debugArmEngineLifecycleBarrierPtr.asFunction(); + + /// Test-only: whether something has parked on the armed barrier. + bool debugEngineLifecycleBarrierReached() => + _debugEngineLifecycleBarrierReached(); + + late final _debugEngineLifecycleBarrierReachedPtr = + _lookup>( + 'debugEngineLifecycleBarrierReached', + ); + late final _debugEngineLifecycleBarrierReached = + _debugEngineLifecycleBarrierReachedPtr.asFunction(); + @override Future changeDevice(int deviceId) async { final address = _changeDevicePtr.address; @@ -616,23 +681,33 @@ class FlutterSoLoudFfi extends FlutterSoLoud { @override void deinit() { - return _dispose(); + _disposeForEngine(_claimedEngineId, _claimedLease); } @override Future deinitAsync() async { // Run the blocking native teardown (device uninit) off the UI isolate so - // it does not freeze the app. Only the raw function pointer address (a - // sendable int) is captured; the pointer is rebuilt and called in the - // worker. - final address = _disposePtr.address; - await Isolate.run(() => _invokeVoidNative(address)); + // it does not freeze the app. Only the raw function pointer address and the + // claim (all sendable ints) are captured; the pointer is rebuilt and called + // in the worker. + // + // The claim is what stops this worker disposing somebody else's engine: it + // can reach init_deinit_mutex after a replacement engine has initialized, + // and native code refuses a teardown whose lease has moved on. + final address = _disposeForEnginePtr.address; + final engineId = _claimedEngineId; + final lease = _claimedLease; + await Isolate.run( + () => _invokeDisposeForEngine(address, engineId, lease), + ); } - late final _disposePtr = _lookup>( - 'dispose', - ); - late final _dispose = _disposePtr.asFunction(); + late final _disposeForEnginePtr = + _lookup>( + 'disposeForEngine', + ); + late final _disposeForEngine = _disposeForEnginePtr + .asFunction(); @override bool isInited() { diff --git a/lib/src/bindings/bindings_player_web.dart b/lib/src/bindings/bindings_player_web.dart index 74af5db4..88ef5f36 100644 --- a/lib/src/bindings/bindings_player_web.dart +++ b/lib/src/bindings/bindings_player_web.dart @@ -179,6 +179,12 @@ class FlutterSoLoudWeb extends FlutterSoLoud { /// miniaudio notifications. void debugTriggerAudioInterruption({required bool began}) {} + /// No-op: web has no FlutterEngine lifecycle to interleave. + void debugArmEngineLifecycleBarrier(EngineLifecycleBarrier barrier) {} + + /// No-op: web has no FlutterEngine lifecycle to interleave. + bool debugEngineLifecycleBarrierReached() => false; + @override Future changeDevice(int deviceId) async { final ret = wasmChangeDevice(deviceId); diff --git a/src/bindings.cpp b/src/bindings.cpp index b9ffadd7..8d2e7d06 100644 --- a/src/bindings.cpp +++ b/src/bindings.cpp @@ -20,6 +20,7 @@ #endif #include +#include #include #include #include @@ -99,10 +100,66 @@ bool engineLifecycleClaimIsCurrent(const EngineLifecycleClaim &claim) { engineInitGeneration == claim.generation; } +bool engineLifecycleClaimIsCurrent(int64_t engine_id, uint64_t lease) { + return engineLifecycleClaimIsCurrent( + EngineLifecycleClaim{engine_id, lease}); +} + void releaseEngineLifecycleClaim() { std::lock_guard guard(engine_lifecycle_mutex); nativeInitOwnerEngineId = kNoEngineId; } + +/// Test-only barriers on the engine-lifecycle path. Intentionally absent from +/// the public API, and inert unless a test arms one. +/// +/// The interleavings that matter here are between an engine's *initialization +/// worker* and another engine's lifecycle calls, and they are not reachable +/// from Dart: the worker is inside Isolate.run and the window that matters is +/// while it holds init_deinit_mutex. Arming a barrier parks the worker at a +/// chosen point so a test can drive the other engine deterministically instead +/// of hoping the scheduler cooperates. +enum EngineLifecycleBarrier { + barrierNone = 0, + /// Before initEngineOwned() takes init_deinit_mutex. + barrierBeforeInitLock = 1, + /// After Player::init() succeeded, before the post-init claim revalidation. + barrierAfterInitSucceeded = 2, + /// Before setDartEventCallback() validates its lease. + barrierBeforeCallbackRegistration = 3, + /// Before disposeForEngine() takes init_deinit_mutex. + barrierBeforeDisposeLock = 4, +}; + +std::mutex engine_barrier_mutex; +std::condition_variable engine_barrier_cv; +int armedEngineBarrier = barrierNone; +bool engineBarrierReached = false; + +/// Blocks while [barrier] is the armed one, until a test releases it. +void engineLifecycleBarrier(int barrier) { + std::unique_lock lock(engine_barrier_mutex); + if (armedEngineBarrier != barrier) + return; + + engineBarrierReached = true; + engine_barrier_cv.notify_all(); + engine_barrier_cv.wait( + lock, [barrier] { return armedEngineBarrier != barrier; }); +} + +/// Release the claim only when it is still the one being torn down. +/// +/// An unconditional release lets a stale operation strip a live engine's +/// ownership: a queued deinit worker that disposes an engine which has since +/// been replaced would leave the replacement initialized but unowned, and an +/// unowned engine can never be torn down when *its* FlutterEngine is destroyed. +void releaseEngineLifecycleClaimIf(const EngineLifecycleClaim &claim) { + std::lock_guard guard(engine_lifecycle_mutex); + if (nativeInitOwnerEngineId == claim.ownerEngineId && + engineInitGeneration == claim.generation) + nativeInitOwnerEngineId = kNoEngineId; +} } #ifdef __cplusplus @@ -276,7 +333,18 @@ FFI_PLUGIN_EXPORT void setDartEventCallback(dartVoiceEndedCallback_t voice_ended_callback, dartFileLoadedCallback_t file_loaded_callback, dartStateChangedCallback_t state_changed_callback, - int64_t owner_engine_id) { + int64_t owner_engine_id, uint64_t lease) { + engineLifecycleBarrier(barrierBeforeCallbackRegistration); + + // Registration is refused once the lease has moved on. These pointers are + // process-global, so an engine whose initialization was superseded while it + // was away in Dart would otherwise overwrite the callables of the engine that + // replaced it -- silently, and with the replacement still believing its own + // registration is live. + if (owner_engine_id != kNoEngineId && + !engineLifecycleClaimIsCurrent(owner_engine_id, lease)) + return; + std::lock_guard callbackGuard(dart_callback_invocation_mutex); dartVoiceEndedCallback.store(voice_ended_callback, std::memory_order_release); dartFileLoadedCallback.store(file_loaded_callback, std::memory_order_release); @@ -410,17 +478,18 @@ FFI_PLUGIN_EXPORT bool areXiphLibsAvailable() { #endif } -/// Initialize the player. Must be called before any other player functions. +/// Claim the native engine for a FlutterEngine ahead of initializing it. /// -/// [sampleRate] the sample rate. Usually is 22050, 44100 (CD quality) or 48000. -/// [bufferSize] the audio buffer size. Usually is 2048, but can be also 512 -/// when low latency is needed for example in games. [channels] 1=mono, -/// 2=stereo, 4=quad, 6=5.1, 8=7.1. -/// -/// Returns [PlayerErrors.noError] if success. /// [owner_engine_id] is the FlutterEngine that will own the engine this /// initialization creates, or -1 on platforms with no engine-lifecycle hooks. -FFI_PLUGIN_EXPORT void prepareEngineInit(int64_t owner_engine_id) { +/// +/// Returns a lease for the claim it takes. The caller must pass that lease back +/// to initEngineOwned(), setDartEventCallback() and disposeForEngine(), each of +/// which refuses to act once the lease is no longer current. Ordering between +/// prepareEngineInit() calls does not order the initialization workers that +/// follow them, so without the lease a worker cannot tell whether it is still +/// the initialization the engine is waiting for. +FFI_PLUGIN_EXPORT uint64_t prepareEngineInit(int64_t owner_engine_id) { std::lock_guard guard(engine_lifecycle_mutex); // Lowered under the same mutex that publishes the claim, so a teardown for // the previous engine cannot raise it again after this point. See @@ -435,17 +504,62 @@ FFI_PLUGIN_EXPORT void prepareEngineInit(int64_t owner_engine_id) { // Invalidate any teardown queued by a previous engine's detach so it cannot // dispose the engine this initialization is about to create. ++engineInitGeneration; + return engineInitGeneration; } -FFI_PLUGIN_EXPORT void requestEngineShutdown() { +/// Ask an initialization worker that has not entered native code yet to abort. +/// +/// Scoped to a lease so a stale request cannot cancel a newer engine's +/// initialization. [engine_id] of -1 skips the check, for callers with no +/// engine lifecycle (web, and the legacy dispose() entry point). +FFI_PLUGIN_EXPORT void requestEngineShutdown(int64_t engine_id, + uint64_t lease) { + if (engine_id != kNoEngineId && + !engineLifecycleClaimIsCurrent(engine_id, lease)) + return; + engine_shutdown_requested.store(true, std::memory_order_release); } -FFI_PLUGIN_EXPORT enum PlayerErrors initEngine(int deviceID, - unsigned int sampleRate, - unsigned int bufferSize, - unsigned int channels, - unsigned int lowLatency) { +/// Tear down a Player built by an initialization that turned out to be stale. +/// +/// Deliberately touches neither the lifecycle claim nor callback ownership: +/// both now belong to whichever engine superseded this one, and the only thing +/// this operation created is the Player itself. +static void disposeStalePlayerLocked() { + if (player.get() == nullptr) + return; + + player->dispose(); + player.reset(); + player = std::make_unique(); + analyzer.reset(); + analyzer = std::make_unique(256); +} + +/// Initialize the player. Must be called before any other player functions. +/// +/// [sampleRate] the sample rate. Usually is 22050, 44100 (CD quality) or 48000. +/// [bufferSize] the audio buffer size. Usually is 2048, but can be also 512 +/// when low latency is needed for example in games. [channels] 1=mono, +/// 2=stereo, 4=quad, 6=5.1, 8=7.1. +/// +/// [engine_id] and [lease] are the claim taken by prepareEngineInit(). +/// [engine_id] of -1 skips ownership validation entirely, which is what the +/// legacy initEngine() entry point below passes. +/// +/// Returns [PlayerErrors.noError] if success. +FFI_PLUGIN_EXPORT enum PlayerErrors initEngineOwned(int64_t engine_id, + uint64_t lease, + int deviceID, + unsigned int sampleRate, + unsigned int bufferSize, + unsigned int channels, + unsigned int lowLatency) { + const bool validateOwnership = engine_id != kNoEngineId; + + engineLifecycleBarrier(barrierBeforeInitLock); + std::lock_guard guard(init_deinit_mutex); std::lock_guard guard_load(loadMutex); @@ -455,6 +569,13 @@ FFI_PLUGIN_EXPORT enum PlayerErrors initEngine(int deviceID, if (engine_shutdown_requested.load(std::memory_order_acquire)) return backendNotInited; + // Calling prepareEngineInit() first does not win the race to this mutex. + // Another engine can have claimed in the meantime, in which case this + // initialization is stale and must not build over the top of it. + if (validateOwnership && + !engineLifecycleClaimIsCurrent(engine_id, lease)) + return backendNotInited; + if (player.get() == nullptr) player = std::make_unique(); @@ -465,6 +586,20 @@ FFI_PLUGIN_EXPORT enum PlayerErrors initEngine(int deviceID, if (res != noError) return res; + engineLifecycleBarrier(barrierAfterInitSucceeded); + + // Re-checked after the fact: Player::init() opens the audio device, which + // blocks for seconds on Android, and nothing stops another engine claiming + // while it runs. Dispose what this stale initialization just built rather + // than leaving an orphaned device and scheduler running with no owner. This + // happens before init_deinit_mutex is released, so the replacement engine's + // own initialization cannot observe the half-built state. + if (validateOwnership && + !engineLifecycleClaimIsCurrent(engine_id, lease)) { + disposeStalePlayerLocked(); + return backendNotInited; + } + // Set window size for filters const int windowSize = (player.get()->soloud.getBackendBufferSize() / player.get()->soloud.getBackendChannels()) - @@ -475,8 +610,20 @@ FFI_PLUGIN_EXPORT enum PlayerErrors initEngine(int deviceID, player.get()->setVoiceEndedCallback(voiceEndedCallback); player.get()->setVoiceInactiveCallback(voiceInactiveCallback); - return PlayerErrors::noError; - } + return PlayerErrors::noError; +} + +/// Ownership-unaware initialization, kept for callers with no FlutterEngine +/// lifecycle. The web build binds this exported symbol directly from the +/// prebuilt wasm, so its signature must not change. +FFI_PLUGIN_EXPORT enum PlayerErrors initEngine(int deviceID, + unsigned int sampleRate, + unsigned int bufferSize, + unsigned int channels, + unsigned int lowLatency) { + return initEngineOwned(kNoEngineId, 0, deviceID, sampleRate, bufferSize, + channels, lowLatency); +} /// Android only: choose whether SoLoud tags the AAudio stream's /// usage/contentType (media/music) or leaves them unset so the app can manage @@ -532,6 +679,22 @@ FFI_PLUGIN_EXPORT enum AudioDeviceState getAudioDeviceState() { // query never waits behind an initialization or lifecycle API call. return (AudioDeviceState)SoLoud::miniaudio_getAudioDeviceState(); } +/// Arms [barrier], or disarms with `barrierNone`, releasing anything parked. +FFI_PLUGIN_EXPORT void debugArmEngineLifecycleBarrier(int barrier) { + { + std::lock_guard lock(engine_barrier_mutex); + armedEngineBarrier = barrier; + engineBarrierReached = false; + } + engine_barrier_cv.notify_all(); +} + +/// True once something has parked on the armed barrier. +FFI_PLUGIN_EXPORT bool debugEngineLifecycleBarrierReached() { + std::lock_guard lock(engine_barrier_mutex); + return engineBarrierReached; +} + /// Test-only hook that sends an interruption through miniaudio's normal /// notification callback. This is intentionally absent from the public API. FFI_PLUGIN_EXPORT void debugTriggerAudioInterruption(unsigned int began) { @@ -599,7 +762,13 @@ FFI_PLUGIN_EXPORT void freeListPlaybackDevices(char **devicesName, /// app /// /// Teardown body. The caller must hold init_deinit_mutex and loadMutex. -static void disposeLocked() { +/// +/// [ownedClaim] is the claim this teardown is entitled to retire, or nullptr +/// for an unscoped teardown that retires whatever claim is current. A scoped +/// caller must not strip a claim that has moved on: doing so would leave the +/// engine that took it initialized but unowned, and an unowned engine can never +/// be torn down when its own FlutterEngine is destroyed. +static void disposeLocked(const EngineLifecycleClaim *ownedClaim) { // Wait for any Dart callback currently executing, then make every bridge // inert. Do not retain the callback mutex while stopping devices, joining // threads, destroying sources, or resetting Player. @@ -612,7 +781,10 @@ static void disposeLocked() { // Nothing is left for a FlutterEngine to own. A detach arriving after this // finds no claim and correctly declines to tear anything down; the next // prepareEngineInit() takes a fresh claim. - releaseEngineLifecycleClaim(); + if (ownedClaim == nullptr) + releaseEngineLifecycleClaim(); + else + releaseEngineLifecycleClaimIf(*ownedClaim); if (player.get() == nullptr) return; @@ -625,6 +797,9 @@ static void disposeLocked() { analyzer = std::make_unique(256); } +/// Ownership-unaware teardown, kept for callers with no FlutterEngine +/// lifecycle. The web build binds this exported symbol directly from the +/// prebuilt wasm, so its signature must not change. FFI_PLUGIN_EXPORT void dispose() { // Preserve request ordering for asynchronous Dart init/deinit workers. engine_shutdown_requested.store(true, std::memory_order_release); @@ -632,7 +807,32 @@ FFI_PLUGIN_EXPORT void dispose() { std::lock_guard guard(init_deinit_mutex); std::lock_guard guard_load(loadMutex); - disposeLocked(); + disposeLocked(nullptr); +} + +/// Ordinary Dart teardown, scoped to the claim taken by prepareEngineInit(). +/// +/// Returns false when the lease has moved on, leaving the newer engine alone. +/// Without this an engine's queued deinit worker could reach init_deinit_mutex +/// after a replacement had initialized and dispose the replacement's engine. +FFI_PLUGIN_EXPORT bool disposeForEngine(int64_t engine_id, uint64_t lease) { + if (engine_id == kNoEngineId) { + dispose(); + return true; + } + + const EngineLifecycleClaim claim{engine_id, lease}; + + engineLifecycleBarrier(barrierBeforeDisposeLock); + + std::lock_guard guard(init_deinit_mutex); + std::lock_guard guard_load(loadMutex); + + if (!engineLifecycleClaimIsCurrent(claim)) + return false; + + disposeLocked(&claim); + return true; } /// Tear the engine down because its owning FlutterEngine is being destroyed @@ -691,7 +891,7 @@ FFI_PLUGIN_EXPORT bool requestEngineTeardownForEngine(int64_t engine_id) { if (!engineLifecycleClaimIsCurrent(claim)) return; - disposeLocked(); + disposeLocked(&claim); }).detach(); } catch (...) { // Thread creation failed. The bridges are already inert, and the next From 69e2dcf176df65ee54970fefd99e4b1c01d1df44 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Jul 2026 03:53:46 +0000 Subject: [PATCH 6/8] Make lease validation atomic, and let Dart see a refused registration Two lifecycle entry points validated the lease through a helper that took and released engine_lifecycle_mutex, then acted after releasing it. That is the same check-then-act window already closed in tryBeginEngineTeardown(), left open in the two places that were converted to the lease later. requestEngineShutdown(): a replacement engine's prepareEngineInit() could claim and lower the shutdown flag between the lease check and the store, after which the store raised it again and the replacement's initEngine() returned backendNotInited. The replacement-cancellation race, relocated into the ordinary Dart shutdown path. setDartEventCallback(): a replacement could claim between the lease check and the pointer publication, so the superseded engine's callables overwrote the live engine's anyway -- exactly what validating the lease was meant to stop. Both now validate and act inside one engine_lifecycle_mutex critical section. That means engine_lifecycle_mutex is held while acquiring dart_callback_invocation_mutex, so it is no longer a leaf lock; nothing takes those two in the opposite order, and the ordering rule is now stated where the mutex is declared. setDartEventCallback() also returned void, so a refusal was invisible to Dart. The isolate created three NativeCallables, called a void function, and carried on to initialize the loader and publish _nativeCallbacksInitialized = true -- believing it was fully initialized with no callbacks registered against a native engine belonging to another FlutterEngine. It now returns whether the registration was accepted; on refusal Dart closes the callables it just created and throws SoLoudInitializationSupersededException, which the existing handler in init() turns into a failed initialization. The teardown that handler attempts is scoped to the same stale lease and is correctly refused, so it cannot dispose the replacement's engine either. Harness scenarios 11 and 12 drive both windows from a second thread, since the atomic form shuts the replacement out by holding the mutex and an inline interleave would only self-deadlock. Built both ways: the non-atomic form fails both (B's initialization cancelled; callables and lifecycle owner disagree), the shipping form passes. Scenario 12 asserts the invariant that holds whichever engine wins the race -- the live callables belong to the lifecycle owner -- rather than a fixed winner. All 12 pass, clean under ThreadSanitizer. flutter analyze unchanged at the 6 pre-existing info lints; flutter test passes; bindings.cpp clean under -Wall -Wextra. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PWTMdp5kHauqc4Jjwp7b5t --- lib/src/bindings/bindings_player_ffi.dart | 28 +++++++++--- lib/src/exceptions/exceptions_from_dart.dart | 18 ++++++++ src/bindings.cpp | 46 ++++++++++++++++---- 3 files changed, 77 insertions(+), 15 deletions(-) diff --git a/lib/src/bindings/bindings_player_ffi.dart b/lib/src/bindings/bindings_player_ffi.dart index 253f42a3..e2d01b0c 100644 --- a/lib/src/bindings/bindings_player_ffi.dart +++ b/lib/src/bindings/bindings_player_ffi.dart @@ -297,19 +297,29 @@ class FlutterSoLoudFfi extends FlutterSoLoud { // The lease is validated natively: registration is refused if this // initialization was superseded while Dart was away, so a stale engine // cannot overwrite the callables of the engine that replaced it. - _setDartEventCallback( + final accepted = _setDartEventCallback( nativeVoiceEndedCallable!.nativeFunction, nativeFileLoadedCallable!.nativeFunction, nativeStateChangedCallable!.nativeFunction, engineId ?? -1, _claimedLease, ); + + if (!accepted) { + // Native code holds no reference to these, so close them here rather than + // leaking three NativeCallables per superseded initialization. Throwing + // is what stops the caller reporting itself as initialized: the + // process-global engine now belongs to another FlutterEngine, and this + // isolate has no callbacks registered against it. + disposeNativeCallables(); + throw const SoLoudInitializationSupersededException(); + } } late final _setDartEventCallbackPtr = _lookup< ffi.NativeFunction< - ffi.Void Function( + ffi.Bool Function( DartVoiceEndedCallbackT, DartFileLoadedCallbackT, DartStateChangedCallbackT, @@ -320,7 +330,7 @@ class FlutterSoLoudFfi extends FlutterSoLoud { >('setDartEventCallback'); late final _setDartEventCallback = _setDartEventCallbackPtr .asFunction< - void Function( + bool Function( DartVoiceEndedCallbackT, DartFileLoadedCallbackT, DartStateChangedCallbackT, @@ -446,15 +456,19 @@ class FlutterSoLoudFfi extends FlutterSoLoud { .asFunction(); @override - void requestEngineShutdown() => - _requestEngineShutdown(_claimedEngineId, _claimedLease); + void requestEngineShutdown() { + // A refusal means another FlutterEngine has claimed the native engine, so + // there is nothing of ours left to cancel. The teardown that follows is + // scoped to the same lease and will be refused for the same reason. + _requestEngineShutdown(_claimedEngineId, _claimedLease); + } late final _requestEngineShutdownPtr = - _lookup>( + _lookup>( 'requestEngineShutdown', ); late final _requestEngineShutdown = _requestEngineShutdownPtr - .asFunction(); + .asFunction(); @override void setAndroidAAudioAttributes(bool managed) { diff --git a/lib/src/exceptions/exceptions_from_dart.dart b/lib/src/exceptions/exceptions_from_dart.dart index 54c71177..b4eadc46 100644 --- a/lib/src/exceptions/exceptions_from_dart.dart +++ b/lib/src/exceptions/exceptions_from_dart.dart @@ -48,6 +48,24 @@ class SoLoudInitializationStoppedByDeinitException extends SoLoudDartException { String get description => 'SoLoud.deinit() was called during initialization.'; } +/// An exception that is thrown when another `FlutterEngine` claimed the native +/// engine while this initialization was in progress. +/// +/// The native engine is process-global while the isolate driving it belongs to +/// a single `FlutterEngine`. When a second engine starts initializing, the +/// first one's initialization is superseded: native code refuses its callback +/// registration rather than let it overwrite the live engine's callables, and +/// this is thrown so the superseded isolate does not go on believing it is +/// initialized. Only reachable in multi-engine setups such as add-to-app. +class SoLoudInitializationSupersededException extends SoLoudDartException { + /// Creates a new [SoLoudInitializationSupersededException]. + const SoLoudInitializationSupersededException([super.message]); + + @override + String get description => + 'Another FlutterEngine claimed the native engine during initialization.'; +} + /// An exception that is thrown when the temporary folder fails to be created /// or opened. class SoLoudTemporaryFolderFailedException extends SoLoudDartException { diff --git a/src/bindings.cpp b/src/bindings.cpp index 8d2e7d06..3b804b8a 100644 --- a/src/bindings.cpp +++ b/src/bindings.cpp @@ -57,8 +57,13 @@ int64_t dartCallbackOwnerEngineId = kNoEngineId; // Both fields are read and written together, so they are guarded by a mutex // rather than made individually atomic: a teardown must observe a consistent // (owner, generation) pair, and no interleaving of two atomic loads gives that. -// This is a leaf lock -- never held while acquiring another lock or performing -// any blocking work. +// +// Lock ordering: this may be held while acquiring +// dart_callback_invocation_mutex, and never the reverse -- setDartEventCallback() +// is the only place that nests them, because validating the lease and +// publishing the callable pointers has to be one critical section. It is never +// held while acquiring init_deinit_mutex or loadMutex, and never across a +// device operation, a thread join, or any other blocking work. std::mutex engine_lifecycle_mutex; int64_t nativeInitOwnerEngineId = kNoEngineId; uint64_t engineInitGeneration = 0; @@ -329,7 +334,13 @@ void stateChangedCallback(unsigned int state) { /// Set a Dart functions to call when an event occurs. /// -FFI_PLUGIN_EXPORT void +/// Returns false when [lease] is no longer the current claim, meaning this +/// initialization has been superseded and the callables were not published. +/// The caller must treat that as a failed initialization: the pointers it +/// created are unreferenced by native code and must be closed, and it must not +/// report itself as initialized -- the process-global engine now belongs to +/// somebody else. +FFI_PLUGIN_EXPORT bool setDartEventCallback(dartVoiceEndedCallback_t voice_ended_callback, dartFileLoadedCallback_t file_loaded_callback, dartStateChangedCallback_t state_changed_callback, @@ -341,15 +352,24 @@ setDartEventCallback(dartVoiceEndedCallback_t voice_ended_callback, // was away in Dart would otherwise overwrite the callables of the engine that // replaced it -- silently, and with the replacement still believing its own // registration is live. + // + // Validation and publication are one critical section. Releasing the + // lifecycle mutex in between leaves the same race the check exists to close: + // a replacement can claim after the lease is found current but before these + // pointers are stored, and the stale engine wins anyway. + std::lock_guard lifecycleGuard(engine_lifecycle_mutex); + if (owner_engine_id != kNoEngineId && - !engineLifecycleClaimIsCurrent(owner_engine_id, lease)) - return; + (nativeInitOwnerEngineId != owner_engine_id || + engineInitGeneration != lease)) + return false; std::lock_guard callbackGuard(dart_callback_invocation_mutex); dartVoiceEndedCallback.store(voice_ended_callback, std::memory_order_release); dartFileLoadedCallback.store(file_loaded_callback, std::memory_order_release); dartStateChangedCallback.store(state_changed_callback, std::memory_order_release); dartCallbackOwnerEngineId = owner_engine_id; + return true; } /// Make the three process-global Dart bridges inert. @@ -512,13 +532,23 @@ FFI_PLUGIN_EXPORT uint64_t prepareEngineInit(int64_t owner_engine_id) { /// Scoped to a lease so a stale request cannot cancel a newer engine's /// initialization. [engine_id] of -1 skips the check, for callers with no /// engine lifecycle (web, and the legacy dispose() entry point). -FFI_PLUGIN_EXPORT void requestEngineShutdown(int64_t engine_id, +/// +/// Validating and raising the flag must be one critical section, for the same +/// reason as in tryBeginEngineTeardown(): a replacement engine's +/// prepareEngineInit() lowers the flag and re-claims under this mutex, so a +/// check that releases it first can be overtaken and then cancel the +/// replacement's initialization. Returns whether the request was accepted. +FFI_PLUGIN_EXPORT bool requestEngineShutdown(int64_t engine_id, uint64_t lease) { + std::lock_guard guard(engine_lifecycle_mutex); + if (engine_id != kNoEngineId && - !engineLifecycleClaimIsCurrent(engine_id, lease)) - return; + (nativeInitOwnerEngineId != engine_id || + engineInitGeneration != lease)) + return false; engine_shutdown_requested.store(true, std::memory_order_release); + return true; } /// Tear down a Player built by an initialization that turned out to be stale. From 348e6d7a16487fb68613aea1b2d456c130f5057e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Jul 2026 04:00:56 +0000 Subject: [PATCH 7/8] Revert the multi-engine lease work Reverts 69cdd85 and 69e2dcf. Both are correct, but every scenario they address requires two concurrent FlutterEngines, and this plugin's consumer runs exactly one: AudioServiceActivity provides a process-global cached engine via AudioServicePlugin.getFlutterEngine() with shouldDestroyEngineWithHost() = false, so the engine deliberately outlives the Activity and is never replaced by a second one. What that leaves in place is what a single-engine foreground-service app actually needs: tearing the native engine down when its FlutterEngine is destroyed rather than leaving a live output device and lifecycle scheduler with no Dart to drive them, and gating that on the lifecycle claim rather than callback ownership -- which matters single-engine too, since the window where initEngine() has opened the audio device but Dart has not yet registered callbacks is seconds on Android/AAudio. Preserved on claude/engine-lifecycle-lease, including the lease plumbing, the test-only lifecycle barriers, SoLoudInitializationSupersededException and the 12-scenario harness. Restore it if a second engine is ever introduced. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PWTMdp5kHauqc4Jjwp7b5t --- CHANGELOG.md | 1 - lib/src/bindings/bindings_player.dart | 21 -- lib/src/bindings/bindings_player_ffi.dart | 157 +++-------- lib/src/bindings/bindings_player_web.dart | 6 - lib/src/exceptions/exceptions_from_dart.dart | 18 -- src/bindings.cpp | 278 ++----------------- 6 files changed, 58 insertions(+), 423 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d15bbf5..6e02d22e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,6 @@ #### 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 - Android: destroying a `FlutterEngine` while the process keeps running (foreground-service apps such as `audio_service`) now tears the native engine down instead of leaving an initialized engine and a running output device with no Dart left to drive them. The blocking part runs on a native worker thread. A `FlutterEngine` claims the native engine for the whole of its initialization, so an engine destroyed mid-init still tears down what it built, and a detaching engine can neither dispose nor cancel the initialization of a replacement that has already claimed it -- fix: engine initialization, Dart callback registration and teardown are now scoped to an initialization lease taken by the owning `FlutterEngine`. Calling `prepareEngineInit()` first does not win the race to the native init mutex, so without it a superseded initialization could build over a newer engine, a superseded isolate could overwrite the live engine's callbacks, and a queued teardown could dispose a live engine or strip its ownership. An initialization that loses the claim while the audio device is opening now disposes what it built instead of leaving an orphaned device running - fix: detaching a `FlutterEngine` on Android no longer blocks the platform thread on the engine-teardown mutex, which could ANR if a device operation was in flight - fix: Android hot restart now clears stale Dart callback registrations. Hot restart replaces the isolate without detaching plugins, so the registered `NativeCallable`s silently went stale - Android: the plugin now does no native work at app startup. Plugin registration and `onAttachedToEngine` are pure Java bookkeeping; the native library is loaded lazily, only when an engine-lifecycle hook actually has to call into it. Previously a static initializer pulled the whole library onto the main thread during app launch even for apps that never played a sound, and a load failure there crashed plugin registration diff --git a/lib/src/bindings/bindings_player.dart b/lib/src/bindings/bindings_player.dart index daeec344..2a13414b 100644 --- a/lib/src/bindings/bindings_player.dart +++ b/lib/src/bindings/bindings_player.dart @@ -11,27 +11,6 @@ import 'package:flutter_soloud/src/sound_handle.dart'; import 'package:flutter_soloud/src/sound_hash.dart'; import 'package:meta/meta.dart'; -/// Test-only parking points on the native engine-lifecycle path. -/// -/// Must stay in the same order as `EngineLifecycleBarrier` in `bindings.cpp`: -/// the index is what crosses the FFI boundary. -enum EngineLifecycleBarrier { - /// Nothing armed. Arming this releases anything currently parked. - none, - - /// Before the initialization worker takes the native init mutex. - beforeInitLock, - - /// After native initialization succeeded, before its ownership recheck. - afterInitSucceeded, - - /// Before Dart callback registration validates its lease. - beforeCallbackRegistration, - - /// Before an ownership-scoped teardown takes the native init mutex. - beforeDisposeLock, -} - /// Callback set in `setBufferStream` for the `onBuffering` closure. typedef OnBufferingCallbackTFunction = void Function(bool isBuffering, int handle, double time); diff --git a/lib/src/bindings/bindings_player_ffi.dart b/lib/src/bindings/bindings_player_ffi.dart index e2d01b0c..9cbf7660 100644 --- a/lib/src/bindings/bindings_player_ffi.dart +++ b/lib/src/bindings/bindings_player_ffi.dart @@ -68,8 +68,6 @@ int _invokeChangeDevice(int address, int deviceId) { /// same process-global engine is initialized. int _invokeInitEngine( int address, - int engineId, - int lease, int deviceId, int sampleRate, int bufferSize, @@ -80,8 +78,6 @@ int _invokeInitEngine( ffi.Pointer< ffi.NativeFunction< ffi.Int32 Function( - ffi.Int64, - ffi.Uint64, ffi.Int, ffi.UnsignedInt, ffi.UnsignedInt, @@ -90,24 +86,20 @@ int _invokeInitEngine( ) > >.fromAddress(address) - .asFunction(); - return fn( - engineId, - lease, - deviceId, - sampleRate, - bufferSize, - channels, - lowLatency, - ); + .asFunction(); + return fn(deviceId, sampleRate, bufferSize, channels, lowLatency); } -/// Calls `disposeForEngine(engineId, lease)` from a worker isolate. -bool _invokeDisposeForEngine(int address, int engineId, int lease) { - return ffi - .Pointer> - .fromAddress(address) - .asFunction()(engineId, lease); +/// Rebuilds a `void Function()` native function from its raw pointer [address] +/// and invokes it. +/// +/// Top-level so it can run inside an [Isolate.run] worker: the blocking native +/// teardown (device uninit) then executes off the UI isolate instead of +/// stalling it. Only [address] (a sendable int) crosses the isolate boundary. +void _invokeVoidNative(int address) { + ffi.Pointer>.fromAddress( + address, + ).asFunction()(); } typedef DartVoiceEndedCallbackT = @@ -294,48 +286,32 @@ class FlutterSoLoudFfi extends FlutterSoLoud { _stateChangedCallback, ); - // The lease is validated natively: registration is refused if this - // initialization was superseded while Dart was away, so a stale engine - // cannot overwrite the callables of the engine that replaced it. - final accepted = _setDartEventCallback( + _setDartEventCallback( nativeVoiceEndedCallable!.nativeFunction, nativeFileLoadedCallable!.nativeFunction, nativeStateChangedCallable!.nativeFunction, engineId ?? -1, - _claimedLease, ); - - if (!accepted) { - // Native code holds no reference to these, so close them here rather than - // leaking three NativeCallables per superseded initialization. Throwing - // is what stops the caller reporting itself as initialized: the - // process-global engine now belongs to another FlutterEngine, and this - // isolate has no callbacks registered against it. - disposeNativeCallables(); - throw const SoLoudInitializationSupersededException(); - } } late final _setDartEventCallbackPtr = _lookup< ffi.NativeFunction< - ffi.Bool Function( + ffi.Void Function( DartVoiceEndedCallbackT, DartFileLoadedCallbackT, DartStateChangedCallbackT, ffi.Int64, - ffi.Uint64, ) > >('setDartEventCallback'); late final _setDartEventCallback = _setDartEventCallbackPtr .asFunction< - bool Function( + void Function( DartVoiceEndedCallbackT, DartFileLoadedCallbackT, DartStateChangedCallbackT, int, - int, ) >(); @@ -389,19 +365,12 @@ class FlutterSoLoudFfi extends FlutterSoLoud { // tripping the ANR watchdog — see #481). Only the raw function pointer // address and the primitive arguments (all sendable ints) are captured; the // pointer is rebuilt and called inside the worker. - final address = _initEngineOwnedPtr.address; + final address = _initEnginePtr.address; final channelCount = channels.count; final lowLatencyInt = lowLatency ? 1 : 0; - // Captured before the worker is dispatched. The worker validates them - // against the current claim, so an initialization superseded while it was - // queued is refused instead of building over the engine that replaced it. - final engineId = _claimedEngineId; - final lease = _claimedLease; final ret = await Isolate.run( () => _invokeInitEngine( address, - engineId, - lease, deviceId, sampleRate, bufferSize, @@ -412,12 +381,10 @@ class FlutterSoLoudFfi extends FlutterSoLoud { return PlayerErrors.values[ret]; } - late final _initEngineOwnedPtr = + late final _initEnginePtr = _lookup< ffi.NativeFunction< ffi.Int32 Function( - ffi.Int64, - ffi.Uint64, ffi.Int, ffi.UnsignedInt, ffi.UnsignedInt, @@ -425,17 +392,7 @@ class FlutterSoLoudFfi extends FlutterSoLoud { ffi.UnsignedInt, ) > - >('initEngineOwned'); - - /// The engine id and lease returned by the most recent [prepareEngineInit]. - /// - /// Held here rather than threaded through `SoLoud` because this binding is - /// already one-per-isolate, and an isolate belongs to exactly one - /// FlutterEngine — so these *are* this engine's claim. Every later call in - /// the initialization hands them back to native code, which refuses to act on - /// a lease that is no longer current. - int _claimedEngineId = -1; - int _claimedLease = 0; + >('initEngine'); @override void prepareEngineInit() { @@ -444,31 +401,23 @@ class FlutterSoLoudFfi extends FlutterSoLoud { // tear down what it built. Read the same way as in // [setDartEventCallbacks]; -1 on platforms that expose no engine id, which // are also the platforms with no engine-lifecycle hooks to act on it. - _claimedEngineId = ui.PlatformDispatcher.instance.engineId ?? -1; - _claimedLease = _prepareEngineInit(_claimedEngineId); + _prepareEngineInit(ui.PlatformDispatcher.instance.engineId ?? -1); } late final _prepareEngineInitPtr = - _lookup>( + _lookup>( 'prepareEngineInit', ); late final _prepareEngineInit = _prepareEngineInitPtr - .asFunction(); + .asFunction(); @override - void requestEngineShutdown() { - // A refusal means another FlutterEngine has claimed the native engine, so - // there is nothing of ours left to cancel. The teardown that follows is - // scoped to the same lease and will be refused for the same reason. - _requestEngineShutdown(_claimedEngineId, _claimedLease); - } + void requestEngineShutdown() => _requestEngineShutdown(); late final _requestEngineShutdownPtr = - _lookup>( - 'requestEngineShutdown', - ); + _lookup>('requestEngineShutdown'); late final _requestEngineShutdown = _requestEngineShutdownPtr - .asFunction(); + .asFunction(); @override void setAndroidAAudioAttributes(bool managed) { @@ -560,34 +509,6 @@ class FlutterSoLoudFfi extends FlutterSoLoud { late final _debugTriggerAudioInterruption = _debugTriggerAudioInterruptionPtr .asFunction(); - /// Test-only: park the next thread reaching [barrier] until it is disarmed. - /// - /// The engine-lifecycle interleavings that matter happen inside the - /// initialization worker while it holds the native init mutex, which no - /// amount of Dart-side sequencing can reach. Pass - /// [EngineLifecycleBarrier.none] to release whatever is parked. - void debugArmEngineLifecycleBarrier(EngineLifecycleBarrier barrier) { - _debugArmEngineLifecycleBarrier(barrier.index); - } - - late final _debugArmEngineLifecycleBarrierPtr = - _lookup>( - 'debugArmEngineLifecycleBarrier', - ); - late final _debugArmEngineLifecycleBarrier = - _debugArmEngineLifecycleBarrierPtr.asFunction(); - - /// Test-only: whether something has parked on the armed barrier. - bool debugEngineLifecycleBarrierReached() => - _debugEngineLifecycleBarrierReached(); - - late final _debugEngineLifecycleBarrierReachedPtr = - _lookup>( - 'debugEngineLifecycleBarrierReached', - ); - late final _debugEngineLifecycleBarrierReached = - _debugEngineLifecycleBarrierReachedPtr.asFunction(); - @override Future changeDevice(int deviceId) async { final address = _changeDevicePtr.address; @@ -695,33 +616,23 @@ class FlutterSoLoudFfi extends FlutterSoLoud { @override void deinit() { - _disposeForEngine(_claimedEngineId, _claimedLease); + return _dispose(); } @override Future deinitAsync() async { // Run the blocking native teardown (device uninit) off the UI isolate so - // it does not freeze the app. Only the raw function pointer address and the - // claim (all sendable ints) are captured; the pointer is rebuilt and called - // in the worker. - // - // The claim is what stops this worker disposing somebody else's engine: it - // can reach init_deinit_mutex after a replacement engine has initialized, - // and native code refuses a teardown whose lease has moved on. - final address = _disposeForEnginePtr.address; - final engineId = _claimedEngineId; - final lease = _claimedLease; - await Isolate.run( - () => _invokeDisposeForEngine(address, engineId, lease), - ); + // it does not freeze the app. Only the raw function pointer address (a + // sendable int) is captured; the pointer is rebuilt and called in the + // worker. + final address = _disposePtr.address; + await Isolate.run(() => _invokeVoidNative(address)); } - late final _disposeForEnginePtr = - _lookup>( - 'disposeForEngine', - ); - late final _disposeForEngine = _disposeForEnginePtr - .asFunction(); + late final _disposePtr = _lookup>( + 'dispose', + ); + late final _dispose = _disposePtr.asFunction(); @override bool isInited() { diff --git a/lib/src/bindings/bindings_player_web.dart b/lib/src/bindings/bindings_player_web.dart index 88ef5f36..74af5db4 100644 --- a/lib/src/bindings/bindings_player_web.dart +++ b/lib/src/bindings/bindings_player_web.dart @@ -179,12 +179,6 @@ class FlutterSoLoudWeb extends FlutterSoLoud { /// miniaudio notifications. void debugTriggerAudioInterruption({required bool began}) {} - /// No-op: web has no FlutterEngine lifecycle to interleave. - void debugArmEngineLifecycleBarrier(EngineLifecycleBarrier barrier) {} - - /// No-op: web has no FlutterEngine lifecycle to interleave. - bool debugEngineLifecycleBarrierReached() => false; - @override Future changeDevice(int deviceId) async { final ret = wasmChangeDevice(deviceId); diff --git a/lib/src/exceptions/exceptions_from_dart.dart b/lib/src/exceptions/exceptions_from_dart.dart index b4eadc46..54c71177 100644 --- a/lib/src/exceptions/exceptions_from_dart.dart +++ b/lib/src/exceptions/exceptions_from_dart.dart @@ -48,24 +48,6 @@ class SoLoudInitializationStoppedByDeinitException extends SoLoudDartException { String get description => 'SoLoud.deinit() was called during initialization.'; } -/// An exception that is thrown when another `FlutterEngine` claimed the native -/// engine while this initialization was in progress. -/// -/// The native engine is process-global while the isolate driving it belongs to -/// a single `FlutterEngine`. When a second engine starts initializing, the -/// first one's initialization is superseded: native code refuses its callback -/// registration rather than let it overwrite the live engine's callables, and -/// this is thrown so the superseded isolate does not go on believing it is -/// initialized. Only reachable in multi-engine setups such as add-to-app. -class SoLoudInitializationSupersededException extends SoLoudDartException { - /// Creates a new [SoLoudInitializationSupersededException]. - const SoLoudInitializationSupersededException([super.message]); - - @override - String get description => - 'Another FlutterEngine claimed the native engine during initialization.'; -} - /// An exception that is thrown when the temporary folder fails to be created /// or opened. class SoLoudTemporaryFolderFailedException extends SoLoudDartException { diff --git a/src/bindings.cpp b/src/bindings.cpp index 3b804b8a..b9ffadd7 100644 --- a/src/bindings.cpp +++ b/src/bindings.cpp @@ -20,7 +20,6 @@ #endif #include -#include #include #include #include @@ -57,13 +56,8 @@ int64_t dartCallbackOwnerEngineId = kNoEngineId; // Both fields are read and written together, so they are guarded by a mutex // rather than made individually atomic: a teardown must observe a consistent // (owner, generation) pair, and no interleaving of two atomic loads gives that. -// -// Lock ordering: this may be held while acquiring -// dart_callback_invocation_mutex, and never the reverse -- setDartEventCallback() -// is the only place that nests them, because validating the lease and -// publishing the callable pointers has to be one critical section. It is never -// held while acquiring init_deinit_mutex or loadMutex, and never across a -// device operation, a thread join, or any other blocking work. +// This is a leaf lock -- never held while acquiring another lock or performing +// any blocking work. std::mutex engine_lifecycle_mutex; int64_t nativeInitOwnerEngineId = kNoEngineId; uint64_t engineInitGeneration = 0; @@ -105,66 +99,10 @@ bool engineLifecycleClaimIsCurrent(const EngineLifecycleClaim &claim) { engineInitGeneration == claim.generation; } -bool engineLifecycleClaimIsCurrent(int64_t engine_id, uint64_t lease) { - return engineLifecycleClaimIsCurrent( - EngineLifecycleClaim{engine_id, lease}); -} - void releaseEngineLifecycleClaim() { std::lock_guard guard(engine_lifecycle_mutex); nativeInitOwnerEngineId = kNoEngineId; } - -/// Test-only barriers on the engine-lifecycle path. Intentionally absent from -/// the public API, and inert unless a test arms one. -/// -/// The interleavings that matter here are between an engine's *initialization -/// worker* and another engine's lifecycle calls, and they are not reachable -/// from Dart: the worker is inside Isolate.run and the window that matters is -/// while it holds init_deinit_mutex. Arming a barrier parks the worker at a -/// chosen point so a test can drive the other engine deterministically instead -/// of hoping the scheduler cooperates. -enum EngineLifecycleBarrier { - barrierNone = 0, - /// Before initEngineOwned() takes init_deinit_mutex. - barrierBeforeInitLock = 1, - /// After Player::init() succeeded, before the post-init claim revalidation. - barrierAfterInitSucceeded = 2, - /// Before setDartEventCallback() validates its lease. - barrierBeforeCallbackRegistration = 3, - /// Before disposeForEngine() takes init_deinit_mutex. - barrierBeforeDisposeLock = 4, -}; - -std::mutex engine_barrier_mutex; -std::condition_variable engine_barrier_cv; -int armedEngineBarrier = barrierNone; -bool engineBarrierReached = false; - -/// Blocks while [barrier] is the armed one, until a test releases it. -void engineLifecycleBarrier(int barrier) { - std::unique_lock lock(engine_barrier_mutex); - if (armedEngineBarrier != barrier) - return; - - engineBarrierReached = true; - engine_barrier_cv.notify_all(); - engine_barrier_cv.wait( - lock, [barrier] { return armedEngineBarrier != barrier; }); -} - -/// Release the claim only when it is still the one being torn down. -/// -/// An unconditional release lets a stale operation strip a live engine's -/// ownership: a queued deinit worker that disposes an engine which has since -/// been replaced would leave the replacement initialized but unowned, and an -/// unowned engine can never be torn down when *its* FlutterEngine is destroyed. -void releaseEngineLifecycleClaimIf(const EngineLifecycleClaim &claim) { - std::lock_guard guard(engine_lifecycle_mutex); - if (nativeInitOwnerEngineId == claim.ownerEngineId && - engineInitGeneration == claim.generation) - nativeInitOwnerEngineId = kNoEngineId; -} } #ifdef __cplusplus @@ -334,42 +272,16 @@ void stateChangedCallback(unsigned int state) { /// Set a Dart functions to call when an event occurs. /// -/// Returns false when [lease] is no longer the current claim, meaning this -/// initialization has been superseded and the callables were not published. -/// The caller must treat that as a failed initialization: the pointers it -/// created are unreferenced by native code and must be closed, and it must not -/// report itself as initialized -- the process-global engine now belongs to -/// somebody else. -FFI_PLUGIN_EXPORT bool +FFI_PLUGIN_EXPORT void setDartEventCallback(dartVoiceEndedCallback_t voice_ended_callback, dartFileLoadedCallback_t file_loaded_callback, dartStateChangedCallback_t state_changed_callback, - int64_t owner_engine_id, uint64_t lease) { - engineLifecycleBarrier(barrierBeforeCallbackRegistration); - - // Registration is refused once the lease has moved on. These pointers are - // process-global, so an engine whose initialization was superseded while it - // was away in Dart would otherwise overwrite the callables of the engine that - // replaced it -- silently, and with the replacement still believing its own - // registration is live. - // - // Validation and publication are one critical section. Releasing the - // lifecycle mutex in between leaves the same race the check exists to close: - // a replacement can claim after the lease is found current but before these - // pointers are stored, and the stale engine wins anyway. - std::lock_guard lifecycleGuard(engine_lifecycle_mutex); - - if (owner_engine_id != kNoEngineId && - (nativeInitOwnerEngineId != owner_engine_id || - engineInitGeneration != lease)) - return false; - + int64_t owner_engine_id) { std::lock_guard callbackGuard(dart_callback_invocation_mutex); dartVoiceEndedCallback.store(voice_ended_callback, std::memory_order_release); dartFileLoadedCallback.store(file_loaded_callback, std::memory_order_release); dartStateChangedCallback.store(state_changed_callback, std::memory_order_release); dartCallbackOwnerEngineId = owner_engine_id; - return true; } /// Make the three process-global Dart bridges inert. @@ -498,18 +410,17 @@ FFI_PLUGIN_EXPORT bool areXiphLibsAvailable() { #endif } -/// Claim the native engine for a FlutterEngine ahead of initializing it. +/// Initialize the player. Must be called before any other player functions. /// +/// [sampleRate] the sample rate. Usually is 22050, 44100 (CD quality) or 48000. +/// [bufferSize] the audio buffer size. Usually is 2048, but can be also 512 +/// when low latency is needed for example in games. [channels] 1=mono, +/// 2=stereo, 4=quad, 6=5.1, 8=7.1. +/// +/// Returns [PlayerErrors.noError] if success. /// [owner_engine_id] is the FlutterEngine that will own the engine this /// initialization creates, or -1 on platforms with no engine-lifecycle hooks. -/// -/// Returns a lease for the claim it takes. The caller must pass that lease back -/// to initEngineOwned(), setDartEventCallback() and disposeForEngine(), each of -/// which refuses to act once the lease is no longer current. Ordering between -/// prepareEngineInit() calls does not order the initialization workers that -/// follow them, so without the lease a worker cannot tell whether it is still -/// the initialization the engine is waiting for. -FFI_PLUGIN_EXPORT uint64_t prepareEngineInit(int64_t owner_engine_id) { +FFI_PLUGIN_EXPORT void prepareEngineInit(int64_t owner_engine_id) { std::lock_guard guard(engine_lifecycle_mutex); // Lowered under the same mutex that publishes the claim, so a teardown for // the previous engine cannot raise it again after this point. See @@ -524,72 +435,17 @@ FFI_PLUGIN_EXPORT uint64_t prepareEngineInit(int64_t owner_engine_id) { // Invalidate any teardown queued by a previous engine's detach so it cannot // dispose the engine this initialization is about to create. ++engineInitGeneration; - return engineInitGeneration; } -/// Ask an initialization worker that has not entered native code yet to abort. -/// -/// Scoped to a lease so a stale request cannot cancel a newer engine's -/// initialization. [engine_id] of -1 skips the check, for callers with no -/// engine lifecycle (web, and the legacy dispose() entry point). -/// -/// Validating and raising the flag must be one critical section, for the same -/// reason as in tryBeginEngineTeardown(): a replacement engine's -/// prepareEngineInit() lowers the flag and re-claims under this mutex, so a -/// check that releases it first can be overtaken and then cancel the -/// replacement's initialization. Returns whether the request was accepted. -FFI_PLUGIN_EXPORT bool requestEngineShutdown(int64_t engine_id, - uint64_t lease) { - std::lock_guard guard(engine_lifecycle_mutex); - - if (engine_id != kNoEngineId && - (nativeInitOwnerEngineId != engine_id || - engineInitGeneration != lease)) - return false; - +FFI_PLUGIN_EXPORT void requestEngineShutdown() { engine_shutdown_requested.store(true, std::memory_order_release); - return true; -} - -/// Tear down a Player built by an initialization that turned out to be stale. -/// -/// Deliberately touches neither the lifecycle claim nor callback ownership: -/// both now belong to whichever engine superseded this one, and the only thing -/// this operation created is the Player itself. -static void disposeStalePlayerLocked() { - if (player.get() == nullptr) - return; - - player->dispose(); - player.reset(); - player = std::make_unique(); - analyzer.reset(); - analyzer = std::make_unique(256); } -/// Initialize the player. Must be called before any other player functions. -/// -/// [sampleRate] the sample rate. Usually is 22050, 44100 (CD quality) or 48000. -/// [bufferSize] the audio buffer size. Usually is 2048, but can be also 512 -/// when low latency is needed for example in games. [channels] 1=mono, -/// 2=stereo, 4=quad, 6=5.1, 8=7.1. -/// -/// [engine_id] and [lease] are the claim taken by prepareEngineInit(). -/// [engine_id] of -1 skips ownership validation entirely, which is what the -/// legacy initEngine() entry point below passes. -/// -/// Returns [PlayerErrors.noError] if success. -FFI_PLUGIN_EXPORT enum PlayerErrors initEngineOwned(int64_t engine_id, - uint64_t lease, - int deviceID, - unsigned int sampleRate, - unsigned int bufferSize, - unsigned int channels, - unsigned int lowLatency) { - const bool validateOwnership = engine_id != kNoEngineId; - - engineLifecycleBarrier(barrierBeforeInitLock); - +FFI_PLUGIN_EXPORT enum PlayerErrors initEngine(int deviceID, + unsigned int sampleRate, + unsigned int bufferSize, + unsigned int channels, + unsigned int lowLatency) { std::lock_guard guard(init_deinit_mutex); std::lock_guard guard_load(loadMutex); @@ -599,13 +455,6 @@ FFI_PLUGIN_EXPORT enum PlayerErrors initEngineOwned(int64_t engine_id, if (engine_shutdown_requested.load(std::memory_order_acquire)) return backendNotInited; - // Calling prepareEngineInit() first does not win the race to this mutex. - // Another engine can have claimed in the meantime, in which case this - // initialization is stale and must not build over the top of it. - if (validateOwnership && - !engineLifecycleClaimIsCurrent(engine_id, lease)) - return backendNotInited; - if (player.get() == nullptr) player = std::make_unique(); @@ -616,20 +465,6 @@ FFI_PLUGIN_EXPORT enum PlayerErrors initEngineOwned(int64_t engine_id, if (res != noError) return res; - engineLifecycleBarrier(barrierAfterInitSucceeded); - - // Re-checked after the fact: Player::init() opens the audio device, which - // blocks for seconds on Android, and nothing stops another engine claiming - // while it runs. Dispose what this stale initialization just built rather - // than leaving an orphaned device and scheduler running with no owner. This - // happens before init_deinit_mutex is released, so the replacement engine's - // own initialization cannot observe the half-built state. - if (validateOwnership && - !engineLifecycleClaimIsCurrent(engine_id, lease)) { - disposeStalePlayerLocked(); - return backendNotInited; - } - // Set window size for filters const int windowSize = (player.get()->soloud.getBackendBufferSize() / player.get()->soloud.getBackendChannels()) - @@ -640,20 +475,8 @@ FFI_PLUGIN_EXPORT enum PlayerErrors initEngineOwned(int64_t engine_id, player.get()->setVoiceEndedCallback(voiceEndedCallback); player.get()->setVoiceInactiveCallback(voiceInactiveCallback); - return PlayerErrors::noError; -} - -/// Ownership-unaware initialization, kept for callers with no FlutterEngine -/// lifecycle. The web build binds this exported symbol directly from the -/// prebuilt wasm, so its signature must not change. -FFI_PLUGIN_EXPORT enum PlayerErrors initEngine(int deviceID, - unsigned int sampleRate, - unsigned int bufferSize, - unsigned int channels, - unsigned int lowLatency) { - return initEngineOwned(kNoEngineId, 0, deviceID, sampleRate, bufferSize, - channels, lowLatency); -} + return PlayerErrors::noError; + } /// Android only: choose whether SoLoud tags the AAudio stream's /// usage/contentType (media/music) or leaves them unset so the app can manage @@ -709,22 +532,6 @@ FFI_PLUGIN_EXPORT enum AudioDeviceState getAudioDeviceState() { // query never waits behind an initialization or lifecycle API call. return (AudioDeviceState)SoLoud::miniaudio_getAudioDeviceState(); } -/// Arms [barrier], or disarms with `barrierNone`, releasing anything parked. -FFI_PLUGIN_EXPORT void debugArmEngineLifecycleBarrier(int barrier) { - { - std::lock_guard lock(engine_barrier_mutex); - armedEngineBarrier = barrier; - engineBarrierReached = false; - } - engine_barrier_cv.notify_all(); -} - -/// True once something has parked on the armed barrier. -FFI_PLUGIN_EXPORT bool debugEngineLifecycleBarrierReached() { - std::lock_guard lock(engine_barrier_mutex); - return engineBarrierReached; -} - /// Test-only hook that sends an interruption through miniaudio's normal /// notification callback. This is intentionally absent from the public API. FFI_PLUGIN_EXPORT void debugTriggerAudioInterruption(unsigned int began) { @@ -792,13 +599,7 @@ FFI_PLUGIN_EXPORT void freeListPlaybackDevices(char **devicesName, /// app /// /// Teardown body. The caller must hold init_deinit_mutex and loadMutex. -/// -/// [ownedClaim] is the claim this teardown is entitled to retire, or nullptr -/// for an unscoped teardown that retires whatever claim is current. A scoped -/// caller must not strip a claim that has moved on: doing so would leave the -/// engine that took it initialized but unowned, and an unowned engine can never -/// be torn down when its own FlutterEngine is destroyed. -static void disposeLocked(const EngineLifecycleClaim *ownedClaim) { +static void disposeLocked() { // Wait for any Dart callback currently executing, then make every bridge // inert. Do not retain the callback mutex while stopping devices, joining // threads, destroying sources, or resetting Player. @@ -811,10 +612,7 @@ static void disposeLocked(const EngineLifecycleClaim *ownedClaim) { // Nothing is left for a FlutterEngine to own. A detach arriving after this // finds no claim and correctly declines to tear anything down; the next // prepareEngineInit() takes a fresh claim. - if (ownedClaim == nullptr) - releaseEngineLifecycleClaim(); - else - releaseEngineLifecycleClaimIf(*ownedClaim); + releaseEngineLifecycleClaim(); if (player.get() == nullptr) return; @@ -827,9 +625,6 @@ static void disposeLocked(const EngineLifecycleClaim *ownedClaim) { analyzer = std::make_unique(256); } -/// Ownership-unaware teardown, kept for callers with no FlutterEngine -/// lifecycle. The web build binds this exported symbol directly from the -/// prebuilt wasm, so its signature must not change. FFI_PLUGIN_EXPORT void dispose() { // Preserve request ordering for asynchronous Dart init/deinit workers. engine_shutdown_requested.store(true, std::memory_order_release); @@ -837,32 +632,7 @@ FFI_PLUGIN_EXPORT void dispose() { std::lock_guard guard(init_deinit_mutex); std::lock_guard guard_load(loadMutex); - disposeLocked(nullptr); -} - -/// Ordinary Dart teardown, scoped to the claim taken by prepareEngineInit(). -/// -/// Returns false when the lease has moved on, leaving the newer engine alone. -/// Without this an engine's queued deinit worker could reach init_deinit_mutex -/// after a replacement had initialized and dispose the replacement's engine. -FFI_PLUGIN_EXPORT bool disposeForEngine(int64_t engine_id, uint64_t lease) { - if (engine_id == kNoEngineId) { - dispose(); - return true; - } - - const EngineLifecycleClaim claim{engine_id, lease}; - - engineLifecycleBarrier(barrierBeforeDisposeLock); - - std::lock_guard guard(init_deinit_mutex); - std::lock_guard guard_load(loadMutex); - - if (!engineLifecycleClaimIsCurrent(claim)) - return false; - - disposeLocked(&claim); - return true; + disposeLocked(); } /// Tear the engine down because its owning FlutterEngine is being destroyed @@ -921,7 +691,7 @@ FFI_PLUGIN_EXPORT bool requestEngineTeardownForEngine(int64_t engine_id) { if (!engineLifecycleClaimIsCurrent(claim)) return; - disposeLocked(&claim); + disposeLocked(); }).detach(); } catch (...) { // Thread creation failed. The bridges are already inert, and the next From 3c4b8a015382bc72937497bfa9c83a7ae2ef0b46 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Jul 2026 04:03:48 +0000 Subject: [PATCH 8/8] Recover from a failed device start, and from a missed interruption end Fixes RELIEFMIX-3MP, which is two distinct failures reaching the same symptom: the app believes it is playing and produces silence. Android -- SoLoudUnknownErrorException from startAudioDevice(). Every miniaudio failure is flattened to UNKNOWN_ERROR by soloud_miniaudio_resume(), so "an unknown error on the C++ side" means precisely that ma_device_start() failed and the reason was discarded. There was no retry and no recovery anywhere on that path, so one failure left the device stopped indefinitely. The stream is kept open across an idle stop, so a device stopped for a while can be holding a stream the OS has since invalidated. AAudio only reports a disconnect through the error callback of a *running* stream, so a stream torn down while stopped -- a route change, or the framework reclaiming resources from a backgrounded app -- is never rerouted by miniaudio, and the staleness surfaces only here. That matches the field reports: both Android users are on Android 16, and one had been backgrounded 16 hours with repeated low-memory warnings before pressing play. It also explains the first-seen date: 4.0.13 is the release that started stopping the Android device when idle, so before it a stale restart was not reachable. performAudioDeviceStart() now rebuilds the device and retries once. Callers hold mDeviceLifecycleOperationMutex so this cannot interleave with another device operation, and voices, sources and filters live in SoLoud rather than in the device, so playback resumes where it left off. One retry only: a freshly built device that will not start is not failing from staleness. iOS -- the same issue collected events where startAudioDevice() returned success and the device stayed stopped. That is Player::startAudioDevice() returning noError under mInterruptionActive without touching the device. The flag is set from miniaudio's AVAudioSession observer and cleared only by init()/deinit(), and iOS does not reliably deliver AVAudioSessionInterruptionTypeEnded -- notably when the interruption ends while the app is backgrounded. One missed notification latched it true and made every later start a silent no-op for the life of the engine. The event timing agrees: five failures within twenty seconds, identical each time, which is a latched state rather than a transient race. An explicit start is an authoritative request from the app, so it now clears the latch and performs the start. If an interruption really is still in force the OS refuses to activate the session and the start fails, surfacing a real error instead of a false success; a genuine new interruption sets the flag again through the normal callback. Adds a regression test for the iOS half: interrupt, drop the matching "ended" notification, and assert an explicit start still reaches started. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PWTMdp5kHauqc4Jjwp7b5t --- CHANGELOG.md | 2 + .../tests/audio_device_lifecycle_races.dart | 28 ++++++++++++ src/player.cpp | 45 ++++++++++++++++++- 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e02d22e..a9324764 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,6 @@ #### 4.0.13 (20 Jul 2026) +- fix: a failed audio device start now rebuilds the device and retries once instead of returning an unrecoverable error. The backend keeps the output stream open across an idle stop, so a device stopped for a while can hold a stream the OS has since invalidated; on Android AAudio only reports a disconnect through the error callback of a *running* stream, so a stream torn down while stopped is never rerouted and the staleness surfaces as `startAudioDevice()` failing with an unknown error. Voices and loaded sources live in SoLoud rather than the device, so playback resumes where it left off +- fix: `startAudioDevice()` no longer reports success while leaving the device stopped when an OS interruption is still flagged as active. iOS does not reliably deliver `AVAudioSessionInterruptionTypeEnded` -- notably when the interruption ends while the app is backgrounded -- and the flag was otherwise cleared only by `init()`/`deinit()`, so one missed notification made every later start a silent no-op for the life of the engine. An explicit start is now authoritative: it clears the flag and performs the start, surfacing a real error if the OS still refuses - 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 - Android: destroying a `FlutterEngine` while the process keeps running (foreground-service apps such as `audio_service`) now tears the native engine down instead of leaving an initialized engine and a running output device with no Dart left to drive them. The blocking part runs on a native worker thread. A `FlutterEngine` claims the native engine for the whole of its initialization, so an engine destroyed mid-init still tears down what it built, and a detaching engine can neither dispose nor cancel the initialization of a replacement that has already claimed it - fix: detaching a `FlutterEngine` on Android no longer blocks the platform thread on the engine-teardown mutex, which could ANR if a device operation was in flight diff --git a/example/tests/tests/audio_device_lifecycle_races.dart b/example/tests/tests/audio_device_lifecycle_races.dart index ef3a804d..9d822d0c 100644 --- a/example/tests/tests/audio_device_lifecycle_races.dart +++ b/example/tests/tests/audio_device_lifecycle_races.dart @@ -333,6 +333,34 @@ Future testAudioDeviceLifecycleRaces() async { ); output.writeln('Idle/keep-alive interruption policies: OK'); + // An interruption whose "ended" notification never arrives must not wedge + // the device permanently. iOS does not reliably deliver + // AVAudioSessionInterruptionTypeEnded -- notably when the interruption ends + // while the app is backgrounded -- and the interruption flag is otherwise + // only cleared by init()/deinit(). While it was latched, startAudioDevice() + // returned success without touching the device, so the app saw a healthy + // API and silence. An explicit start is authoritative and must recover. + SoLoud.instance.setAudioDeviceIdleTimeout(null); + state = await _waitForDeviceState(AudioDeviceState.started); + assert( + state == AudioDeviceState.started, + 'Keep-alive did not start device before the missed-end check.', + ); + SoLoudController().soLoudFFI.debugTriggerAudioInterruption(began: true); + state = await _waitForDeviceState(AudioDeviceState.stopped); + assert( + state == AudioDeviceState.stopped, + 'Interruption did not stop device before the missed-end check.', + ); + // Deliberately no matching `began: false`: that is the dropped one. + await SoLoud.instance.startAudioDevice(); + state = await _waitForDeviceState(AudioDeviceState.started); + assert( + state == AudioDeviceState.started, + 'Explicit start did not recover from a missed interruption end.', + ); + output.writeln('Explicit start recovers a missed interruption end: OK'); + await SoLoud.instance.stop(handle); await SoLoud.instance.disposeSource(waveform); diff --git a/src/player.cpp b/src/player.cpp index a6f03927..c3c38ffc 100644 --- a/src/player.cpp +++ b/src/player.cpp @@ -1078,6 +1078,31 @@ PlayerErrors Player::performAudioDeviceStart() // Use the normal resume hook so iOS reactivates AVAudioSession before the // Audio Unit is restarted. SoLoud::result result = soloud.resume(); + if (result == SoLoud::SO_NO_ERROR) + return noError; + + // The start failed. Rebuild the device and try once more before giving up. + // + // The backend keeps the stream open across an idle stop, so a device that + // has been stopped for a while can be holding a stream the OS has since + // invalidated. On Android this is the common case: AAudio only reports a + // disconnect through the error callback of a *running* stream, so a stream + // that is torn down while stopped -- a route change, or the framework + // reclaiming resources from a backgrounded app -- is never rerouted, and + // the staleness only surfaces here, as AAudioStream_requestStart failing. + // + // Replacing the device recreates the stream against the current default + // output. Voices, loaded sources and filters all live in SoLoud rather than + // in the device, so playback resumes where it left off. + // + // Callers hold mDeviceLifecycleOperationMutex, so this cannot interleave + // with another device operation. One retry only: if a freshly built device + // will not start either, the failure is not staleness. + const SoLoud::result rebuilt = soloud.miniaudio_changeDevice(nullptr); + if (rebuilt != SoLoud::SO_NO_ERROR) + return unknownError; + + result = soloud.resume(); if (result != SoLoud::SO_NO_ERROR) return unknownError; return noError; @@ -1139,8 +1164,24 @@ PlayerErrors Player::startAudioDevice() { std::lock_guard operationLock( mDeviceLifecycleOperationMutex); - if (mInterruptionActive.load(std::memory_order_acquire)) - return noError; + + // An explicit start is an authoritative request from the app, so it + // clears the interruption latch rather than being suppressed by it. + // + // Returning noError here without touching the device made this API + // able to report success while leaving the device stopped, and the + // latch can be stuck: iOS does not reliably deliver + // AVAudioSessionInterruptionTypeEnded -- notably when the interruption + // ends while the app is backgrounded or suspended -- and the flag is + // otherwise only cleared by init()/deinit(). Once missed, every later + // start was a silent no-op for the lifetime of the engine. + // + // If an interruption really is still in force the OS refuses to + // activate the session and the start below fails, which surfaces a real + // error instead of a false success. A genuine new interruption arriving + // afterwards sets the flag again through the normal callback. + mInterruptionActive.store(false, std::memory_order_release); + // Cancel a stale delayed idle stop before prewarming the device. invalidatePendingDeviceRequest(); result = performAudioDeviceStart();