fix(audio): break permanent main-thread deadlock on default device change while idle#543
Open
devzahirul wants to merge 1 commit into
Open
Conversation
…altic-dev#542) A default audio device change while the app is idle could freeze it permanently: retiring the prewarmed AVAudioEngine dropped the final engine reference on the main thread, so -[AVAudioEngine dealloc] blocked waiting on the engine's internal serial queue, while that same queue was blocked posting AVAudioEngineConfigurationChange to a main-queue observer. Neither side could proceed; only force quit recovered. Fix both legs of the cycle: - Register the AVAudioEngineConfigurationChange observer with queue: nil so delivery runs synchronously on the posting thread and the engine's internal queue never waits on the main thread. The handler already only spawns a MainActor task, which is safe from any thread. - Release the retired engine's final reference deterministically on the background queue. The previous hand-off captured the engine in an async block, but the caller's locals raced it: when the block finished first, the last release - and dealloc - landed back on the main thread, which is exactly what the sample in the issue shows. A dedicated holder keeps the retired engine out of main-thread locals entirely, so the drain on the utility queue is always the final release. Fixes altic-dev#542
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes the permanent main-thread deadlock reported in #542: a system default audio device change while FluidVoice is idle (with "sync audio devices with system" enabled) could beachball the app forever at 0% CPU, recoverable only by force quit.
The
sampleattached to the issue shows a circular wait between two threads, and both legs live inASRService:scheduleAudioRouteRecovery→retireAudioEngine(reason: "idle_route_change:..."), and the final strong reference to the oldAVAudioEnginegets released on main.-[AVAudioEngine dealloc]then does a synchronous hop onto the engine's internal serial queue and blocks.IOUnitConfigurationChanged(), which postsAVAudioEngineConfigurationChange. Because our observer was registered withqueue: .main, NotificationCenter wraps delivery in anNSOperationand blocks the posting thread inwaitUntilFinisheduntil the main thread runs it — which it never can.This PR fixes both legs independently:
registerEngineConfigurationChangeObserver): register withqueue: nilso delivery is synchronous on the posting thread and the engine's queue never waits on main. The handler body only spawns aTask { @MainActor ... }, which is safe from any thread, so behavior is unchanged. This eliminates the whole deadlock class — any main-thread call that synchronizes with the engine queue (dealloc,stop(),removeTap) can no longer wait forever on a blocked post.retireAudioEngine): the existing hand-off (DispatchQueue.global(qos: .utility).async { _ = oldEngine }) answers the question left open in the issue ("some other strong reference is being dropped synchronously"). There is no other reference — the hand-off itself is racy. Dispatching the block only guarantees the utility queue holds a reference, not the last one: when the trivial block finishes beforeretireAudioEnginereturns (likely, since main still runs the DebugLogger call), the caller's own local performs the final release and dealloc lands back on main, directly under theretireAudioEngineframe — exactly what all 2,557 sample snapshots show. The fix moves the retired engine into a smallRetiredAudioEngineReferenceholder so no main-thread local ever owns it past the dispatch; every main-side release happens-before the enqueue, so the drain on the utility queue is provably the final release.Type of Change
Related Issue or Discussion
Fixes #542
Testing
swiftlint --strict --config .swiftlint.yml(0 violations)swiftformat --config .swiftformat Sourcesxcodebuild testsuite passes onplatform=macOS,arch=arm64(all suites green)The deadlock itself is a scheduling race between engine retirement and the engine's own configuration-change handling, so it cannot be reproduced on demand (the reporter hit it via Bluetooth headset connect/disconnect). The fix was validated by matching both changed code paths against the two blocked stacks in the issue's
sampleoutput, plus a clean build and full test run.Screenshots / Video
Notes
queue: nilis intentionally load-bearing and now documented inline: block-based observers on an explicit queue makepostNotificationwait (-[NSOperation waitUntilFinished]in the issue's sample), so the engine's internal queue must never target main. Semantics are preserved because the handler already hopped to the main actor viaTask; delivery order relative to the recovery guards (isRunning/isRecoveringAudioRoute) is unaffected.RetiredAudioEngineReferencefollows the existing pattern in this file for cross-thread helpers (private final nonisolated class ... : @unchecked Sendable, cf.AudioCapturePipeline). It is created on main and touched exactly once by the draining block; the dispatch enqueue provides the ordering.queue: .mainand filtering byobject:(the notification legitimately comes from our own engine, so the wait would remain), and movingengine.stop()off main (unnecessary once the posting thread can no longer block on main, and it would change engine state timing during recovery).IOUnitConfigurationChanged()to post at the exact moment of retirement requires real device-route changes and cannot be scripted deterministically in CI without flakiness.