From eec5cb0850b966ccc89a738a4d7e7e0ece1fdd5c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 04:30:15 +0000 Subject: [PATCH] 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() {