Fix main-actor isolation crash on AVAudioSession route changes - #86
Conversation
Three AudioPlayerManager sinks observed AVAudioSession notifications
without hopping to the main queue first:
NotificationCenter.default.publisher(for: .routeChangeNotification)
.sink { [weak self] note in self?.handleRouteChange(note) }
Under Swift 6 with SWIFT_DEFAULT_ACTOR_ISOLATION=MainActor these sink
closures inherit @mainactor, so Combine delivering them on the posting
thread traps instead of hopping. AVAudioSession posts routeChange on a
secondary thread, which means a headphone unplug, Bluetooth connect or
AirPlay switch could crash the app. mediaServicesWereReset has no
documented delivery thread, so it gets the same treatment;
interruption is documented as main-thread but is guarded for
consistency and to stop it silently regressing.
Verified with a standalone probe built as
`swiftc -swift-version 6 -default-isolation MainActor`: reproducing this
pattern and posting from a background thread dies with SIGILL, and
adding .receive(on: DispatchQueue.main) makes the sink run on main.
The watch and TV AudioManagers already guard the identical
handleInterruption/handleRouteChange handlers this way; the iOS target
was the only one missing it.
The deinit wrapped its body in MainActor.assumeIsolated, which asserts that the last release happened on the main thread rather than making it so. The body invalidates a RunLoop.main timer, removes an AVPlayer time observer and pauses the radio player, all of which need the main actor. `isolated deinit` runs the body on the main actor instead. It also lets the endBackgroundTask call happen directly rather than being deferred to a Task that could outlive the deinit. This restores the pattern used by AVEnginePlayback, AuthManager and the watch/TV AudioManagers. It was removed in 0d7308f because isolated deinit needs a Swift 6.1+ compiler and the project was still in Swift 5 language mode; every target is on Swift 6 now, so that no longer applies. Also marks the class final with a private init. The deinit's reasoning depends on `shared` being the only instance, and it already is -- this makes the invariant enforced rather than assumed.
Reviewer's GuideThis PR hardens AudioPlayerManager’s Swift 6 main-actor isolation by ensuring AVAudioSession-related Combine subscriptions hop to the main queue and by converting teardown to an isolated deinit on a single-instance, final class. Sequence diagram for AVAudioSession route change handling on the main actorsequenceDiagram
participant AVAudioSession
participant NotificationCenter
participant DispatchQueue_main
participant AudioPlayerManager
AVAudioSession->>NotificationCenter: post routeChangeNotification
NotificationCenter->>NotificationCenter: publisher(for: AVAudioSession.routeChangeNotification)
NotificationCenter->>DispatchQueue_main: receive(on: DispatchQueue.main)
DispatchQueue_main->>AudioPlayerManager: sink handleRouteChange(note)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesAudioPlayerManager updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Fixes a Swift 6 main-actor isolation crash in
AudioPlayerManager, plus a related teardown cleanup.1. AVAudioSession notifications were delivered off the main queue
Three sinks observed
AVAudioSessionnotifications without hopping to main first:With
SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor, these sink closures inherit@MainActor. Combine delivers on the posting thread, so when that thread isn't main the closure traps rather than hopping.AVAudioSessionpostsrouteChangeNotificationon a secondary thread, so unplugging headphones, connecting Bluetooth, or switching to AirPlay could crash the app.mediaServicesWereResethas no documented delivery thread and gets the same treatment.interruptionNotificationis documented as main-thread, but is guarded too so it can't silently regress.The watch and TV
AudioManagers already guard the identicalhandleInterruption/handleRouteChangehandlers this way — the iOS target was the only one missing it.Verification. A standalone probe built with
swiftc -swift-version 6 -default-isolation MainActor:.sinkwithout.receive(on:), posted from a background thread.receive(on: DispatchQueue.main)2.
isolated deinitfor teardownThe deinit wrapped its body in
MainActor.assumeIsolated, which asserts that the last release happened on the main thread rather than ensuring it. The body invalidates aRunLoop.maintimer, removes anAVPlayertime observer, and pauses the radio player — all main-actor work.isolated deinitruns the body on the main actor instead, and letsendBackgroundTaskbe called directly rather than deferred to aTaskthat could outlive the deinit.This restores the pattern already used by
AVEnginePlayback,AuthManager, and the watch/TVAudioManagers. It was removed in0d7308fbecauseisolated deinitneeds a Swift 6.1+ compiler while the project was still in Swift 5 language mode; every target is on Swift 6 now, so that constraint is gone.The class is also marked
finalwith aprivate init. The deinit's reasoning depends onsharedbeing the only instance, and it already is — this makes the invariant enforced rather than assumed.Testing
Notes
The route-change path is only reachable on a real device, so the crash isn't covered by the test suites. It was latent until the Swift 6 migration, which is what armed the isolation checks.
Summary by Sourcery
Guard AudioPlayerManager’s AVAudioSession notification handling and teardown against Swift 6 main-actor isolation crashes while enforcing its singleton usage.
New Features:
Bug Fixes:
Enhancements:
Summary by CodeRabbit