We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4169

Summary: Crash due to audio session observer race condition
Product: SDL Reporter: Jona <jona>
Component: audioAssignee: Ryan C. Gordon <icculus>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: major    
Priority: P2    
Version: 2.0.8   
Hardware: iPhone/iPod touch   
OS: macOS 10.13   

Description Jona 2018-05-15 15:26:07 UTC
There is a race condition crash when trying to remove the audio session observer. Basically we are setting the listeners device to NULL inside a "@synchronized" call but right after the synchronized call we are removing the observer.

if (this->hidden->interruption_listener != NULL) 
{
    SDLInterruptionListener *listener = nil;
    listener = (SDLInterruptionListener *) CFBridgingRelease(this->hidden->interruption_listener);
    @synchronized (listener) {
        listener.device = NULL;
    }
    [center removeObserver:listener];
}

This leads to the problem where the audioSessionInterruption is called and it no longer has a valid "self.device" value. This NULL value is not handled and passed directly to the "interruption_begin" method where things crash.

- (void)audioSessionInterruption:(NSNotification *)note
{
    @synchronized (self) {
        NSNumber *type = note.userInfo[AVAudioSessionInterruptionTypeKey];
        if (type.unsignedIntegerValue == AVAudioSessionInterruptionTypeBegan) {
            interruption_begin(self.device);
        } else {
            interruption_end(self.device);
        }
    }
}

The solution could likely be to call "[center removeObserver:listener];" before setting the "listener.device" to NULL.
Comment 1 Sam Lantinga 2018-05-21 03:02:21 UTC
That sounds right to me. Can you provide a tested patch?

Thanks!
Comment 2 Jona 2018-05-22 14:14:19 UTC
After further exploration and allowing our app to be beta tested by various users we noticed that the fix mentioned here did not resolve the problem.

Further testing and experimentation we discovered the issue and a fix!

The following explains why this bug was happening:
This crash was caused because the audio session was being set as active [session setActive:YES error:&err] when the audio device was actually being CLOSED. Certain cases the audio session being set to active would fail and the method would return right away. Because of the way the error was handled we never removed the SDLInterruptionListener thus leaking it. Later when an interruption was received the THIS_ object would contain a pointer to an already released device causing the crash.

The fix:
When only one device remained open and it was being closed we needed to set the audio session as NOT active and completely ignore the returned error to successfully release the SDLInterruptionListener. I think the user assumed that the open_playback_devices and open_capture_devices would equal 0 when all of them where closed but the truth is that at the end of the closing process that the open devices count is decremented.

Here is our fix in github:
https://github.com/jonameson/SDL-mirror/commit/1d96bd6fe77c2d43d609061e4e86db59d3455bba

I apologize that I'm unable to provide a patch. I'm not too familiar with the process. I hope this helps.
Comment 3 Sam Lantinga 2018-05-24 14:26:43 UTC
It looks like the latest SDL in Mercurial may have already resolved this, as it contains a similar change to that patch. Can you take a look?
http://www.libsdl.org/tmp/SDL-2.0.zip

Thanks!
Comment 4 Jona 2018-05-24 20:33:21 UTC
That fix is good! It's a little clearer than my fix.
Comment 5 Sam Lantinga 2018-05-25 19:27:03 UTC
Great, thanks!