coreaudio: unregister interruption observer unconditionally on close

[jonameson]

Summary

Fix a long-standing CoreAudio observer-lifetime bug where SDLInterruptionListener outlives its backing SDL_AudioDevice, leading to an EXC_BAD_ACCESS on the next app foreground.

update_audio_session() registers the listener on open and unregisters it on close — but the close-path unregister sits at the bottom of the function, after [AVAudioSession setCategory:…error:&err] and [setActive:…error:&err] calls that can return NO early. If any of those AVAudioSession calls fails during close (which happens in the wild: phone calls, Siri, CarPlay / AirPlay handoff, session owned by another app), the listener is never removed from NSNotificationCenter. Its device pointer continues to reference memory that is freed by SDL_free(this->hidden) in COREAUDIO_CloseDevice right after update_audio_session returns. A later UIApplicationWillEnterForegroundNotification or UIApplicationDidBecomeActiveNotification is then delivered to the now-dangling listener, which calls interruption_end(self.device) and crashes.

This is the same class of bug as #2900 (2018) — except that fix only closed the setActive:YES early-return path on last-device close. The setCategory: early-return paths have remained since. We're seeing it in production from a real-world app with ~40 crashes over 7 days on a single app version, all with the exact stack pattern (SDL_SemPost / interruption_end-adjacent → __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ → -[UIApplication _sendWillEnterForegroundCallbacks]). The same root cause is still reported against SDL3 in #12660.

Change

• Move the close-path observer unregister to the top of update_audio_session(), so it runs unconditionally before any setCategory: / setActive: early-return. Close is a teardown path; session-config failures should not be able to leak resources.
• Remove the now-dead close-side cleanup from the bottom else branch.
• Add if (self.device == NULL) return; guards at the top of both SDLInterruptionListener callbacks (audioSessionInterruption: and applicationBecameActive:). These are defense-in-depth — with the top-of-function unregister in place, the callbacks should never fire post-close, but the guards prevent any future edit to the cleanup flow from silently re-introducing the crash.

No API surface changes, no behavioral change on the open path, no behavioral change on the close path when all AVAudioSession calls succeed.

Testing

• testautomation suite passes locally on macOS (no new failures vs. the baseline).
• Integration-verified on-device against the real production crash: with the patch applied to an FcLib-consuming iOS app (which calls SDL_OpenAudioDevice / SDL_CloseAudioDevice inside an Fc2AudioMixer wrapper), the willEnterForeground-after-background crash stops reproducing. Without the patch, injecting a simulated setCategory: failure at close reproduces the crash reliably.

Notes

• Mirror fix for SDL3 main (file renamed to UpdateAudioSession, same structural bug) can be done as a follow-up PR if the project prefers it separated.
• Related: Crash due to audio session observer race condition #2900 (closed), InterruptionBegin (in SDL3) (SDL_coreaudio.m:344) #12660 (open, SDL3, same root cause).

[slouken]

This looks good. Once you fix the CI errors I'll go ahead and merged this. Can you please also make a PR for main?

[icculus]

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

[icculus]

The bigger question is: why are we trying to set up the audio session when closing the audio device?

I do think Claude's patch will stop the crash, but I think the actual bug here is that update_audio_session is doing way more than it should during COREAUDIO_CloseDevice.

[icculus]

Tossing this into 3.4.6 to try my idea (and will backport to SDL2).

[jonameson]

The bigger question is: why are we trying to set up the audio session when closing the audio device?

I do think Claude's patch will stop the crash, but I think the actual bug here is that update_audio_session is doing way more than it should during COREAUDIO_CloseDevice.

Yeah, update_audio_session does do a lot... but we aren't trying to setup audio session when closing but we are reusing and cleaning up the audio session on closing the audio device. Just looks like a large method that a refactor would eventually be nice.

So about the AI use for patch, I just couldn't not share it... we had it review our app for top crashes and this came up and the suggested patch that made sense. 😅

[jonameson]

Any ideas why we're failing here? This seems like existing failing checks. I didn't change any declarations:

/Users/runner/work/SDL/SDL/src/audio/coreaudio/SDL_coreaudio.m:358:19: error: mixing declarations and code is incompatible with standards before C99 [-Werror,-Wdeclaration-after-statement]
  358 |         NSNumber *type = note.userInfo[AVAudioSessionInterruptionTypeKey];
      |                   ^
/Users/runner/work/SDL/SDL/src/audio/coreaudio/SDL_coreaudio.m:402:19: error: mixing declarations and code is incompatible with standards before C99 [-Werror,-Wdeclaration-after-statement]
  402 |         NSString *category = AVAudioSessionCategoryPlayback;

[sezero]

Any ideas why we're failing here? This seems like existing failing checks. I didn't change any declarations:

/Users/runner/work/SDL/SDL/src/audio/coreaudio/SDL_coreaudio.m:358:19: error: mixing declarations and code is incompatible with standards before C99 [-Werror,-Wdeclaration-after-statement]
  358 |         NSNumber *type = note.userInfo[AVAudioSessionInterruptionTypeKey];
      |                   ^
/Users/runner/work/SDL/SDL/src/audio/coreaudio/SDL_coreaudio.m:402:19: error: mixing declarations and code is incompatible with standards before C99 [-Werror,-Wdeclaration-after-statement]
  402 |         NSString *category = AVAudioSessionCategoryPlayback;

Unlike SDL3 (main), SDL2 builds with -Wdeclaration-after-statement, so
something like the following should fix it:

diff --git a/src/audio/coreaudio/SDL_coreaudio.m b/src/audio/coreaudio/SDL_coreaudio.m
index 95251b2..c33f3db 100644
--- a/src/audio/coreaudio/SDL_coreaudio.m
+++ b/src/audio/coreaudio/SDL_coreaudio.m
@@ -351,11 +351,12 @@ static void interruption_end(_THIS)
 - (void)audioSessionInterruption:(NSNotification *)note
 {
     @synchronized(self) {
+        NSNumber *type;
         /* Defensive: skip if the device was already torn down. */
         if (self.device == NULL) {
             return;
         }
-        NSNumber *type = note.userInfo[AVAudioSessionInterruptionTypeKey];
+        type = note.userInfo[AVAudioSessionInterruptionTypeKey];
         if (type.unsignedIntegerValue == AVAudioSessionInterruptionTypeBegan) {
             interruption_begin(self.device);
         } else {
@@ -382,6 +383,12 @@ static BOOL update_audio_session(_THIS, SDL_bool open, SDL_bool allow_playandrec
         AVAudioSession *session = [AVAudioSession sharedInstance];
         NSNotificationCenter *center = [NSNotificationCenter defaultCenter];
 
+        NSString *category;
+        NSString *mode;
+        NSUInteger options;
+        NSError *err = nil;
+        const char *hint;
+
         /* Close path: unregister the interruption observer FIRST, before any
            code that can early-return. [AVAudioSession setCategory:] failures
            (phone call, Siri, CarPlay / AirPlay handoff, etc.) would otherwise
@@ -399,11 +406,9 @@ static BOOL update_audio_session(_THIS, SDL_bool open, SDL_bool allow_playandrec
             }
         }
 
-        NSString *category = AVAudioSessionCategoryPlayback;
-        NSString *mode = AVAudioSessionModeDefault;
-        NSUInteger options = AVAudioSessionCategoryOptionMixWithOthers;
-        NSError *err = nil;
-        const char *hint;
+        category = AVAudioSessionCategoryPlayback;
+        mode = AVAudioSessionModeDefault;
+        options = AVAudioSessionCategoryOptionMixWithOthers;
 
         hint = SDL_GetHint(SDL_HINT_AUDIO_CATEGORY);
         if (hint) {