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 1396

Summary: SDL_Init, SDL_AudioInit, SDL_OpenAudio -> SDL_CloseAudio, SDL_AudioQuit, SDL_Quit crashes
Product: SDL Reporter: Ellie <etc0de>
Component: audioAssignee: Ryan C. Gordon <icculus>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: critical    
Priority: P2 CC: aschiffler, dimitris.zenios
Version: HG 2.0   
Hardware: x86   
OS: Linux   

Description Ellie 2012-01-23 23:36:36 UTC
SDL_Init, SDL_AudioInit, SDL_OpenAudio -> SDL_CloseAuudio, SDL_AudioQuit, SDL_Quit crashes, which is a bit odd (since it seems to be the sane way of cleaning up things to me).

I can reproduce it 100% with the following small test case:

#include "SDL.h"
#include <stdio.h>
#include <unistd.h>

void audiocallback(void *intentionally_unused, Uint8 *stream, int len) {
    //dummy callback
    memset(stream, 0, len);
}

int main(int argc, char** argv) {
    //initialise:
    if (SDL_Init(SDL_INIT_TIMER) < 0) {return -1;}
    if (SDL_AudioInit(NULL) < 0) {return -1;}

    //open audio:
    SDL_AudioSpec fmt;
    memset(&fmt,0,sizeof(fmt));
    fmt.freq = 48000;
    fmt.format = AUDIO_F32SYS;
    fmt.channels = 2;
    fmt.samples = 2048;
    fmt.callback = audiocallback;
    fmt.userdata = NULL;
    if (SDL_OpenAudio(&fmt, NULL) < 0) {return -1;}

    //close audio:
    SDL_CloseAudio();

    //cleanup:
    SDL_AudioQuit();
    printf("Now we expect a crash:\n");fflush(stdout);
    SDL_Quit(); //segfaults
}

gdb backtrace:

(gdb) backtrace
#0  0x00000000 in ?? ()
#1  0x0804c9ae in SDL_AudioQuit () at src/audio/SDL_audio.c:1189
#2  0x08049dbd in SDL_QuitSubSystem (flags=<optimized out>) at src/SDL.c:180
#3  SDL_Quit () at src/SDL.c:214
#4  0x08049acc in main ()
(gdb)

valgrind backtrace:

==7484== Memcheck, a memory error detector
==7484== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==7484== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==7484== Command: ./test
==7484== 
Now we expect a crash:
==7484== Jump to the invalid address stated on the next line
==7484==    at 0x0: ???
==7484==    by 0x40B86B2: (below main) (libc-start.c:226)
==7484==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==7484== 
==7484== 
==7484== Process terminating with default action of signal 11 (SIGSEGV)
==7484==  Bad permissions for mapped region at address 0x0
==7484==    at 0x0: ???
==7484==    by 0x40B86B2: (below main) (libc-start.c:226)
==7484== 
==7484== HEAP SUMMARY:
==7484==     in use at exit: 50,473 bytes in 1,398 blocks
==7484==   total heap usage: 2,146 allocs, 748 frees, 217,058 bytes allocated
==7484== 
==7484== LEAK SUMMARY:
==7484==    definitely lost: 124 bytes in 1 blocks
==7484==    indirectly lost: 0 bytes in 0 blocks
==7484==      possibly lost: 23,284 bytes in 1,233 blocks
==7484==    still reachable: 27,065 bytes in 164 blocks
==7484==         suppressed: 0 bytes in 0 blocks
==7484== Rerun with --leak-check=full to see details of leaked memory
==7484== 
==7484== For counts of detected and suppressed errors, rerun with: -v
==7484== ERROR SUMMARY: 2 errors from 1 contexts (suppressed: 0 from 0)
Killed

Would be nice if this was fixed so a proper cleanup sequence is possible.
Comment 1 Andreas Schiffler 2013-04-19 10:33:40 UTC
Resolving bug as no repro.

The testautomation code has this case covered and I can't repro this issue on Linux (Ubuntu) or Windows (cygwin).

Note that the use of SDL_AudioInit and SDL_AudioQuit is NOT the most common or recommended code path as per SDL_audio.h:

/**
 *  \name Initialization and cleanup
 *
 *  \internal These functions are used internally, and should not be used unless
 *            you have a specific need to specify the audio driver you want to
 *            use.  You should normally use SDL_Init() or SDL_InitSubSystem().
 */
/*@{*/
extern DECLSPEC int SDLCALL SDL_AudioInit(const char *driver_name);
extern DECLSPEC void SDLCALL SDL_AudioQuit(void);


----------
Log of a run on Linux using
./testautomation --filter Audio 

INFO:  04/19/13 07:26:34: ::::: Test Run /w seed 'IX2TEOADU8HIZNI' started
INFO:  04/19/13 07:26:34: Filtering: running only suite 'Audio'
INFO:  04/19/13 07:26:34: ===== Test Suite 1: 'Audio' started
INFO:  04/19/13 07:26:34: ----- Test Case 1.1: 'audio_enumerateAndNameAudioDevices' started
...
INFO:  04/19/13 07:26:34: Total Test runtime: 0.0 sec
INFO:  04/19/13 07:26:34: Total Suite runtime: 0.0 sec
INFO:  04/19/13 07:26:34: Suite Summary: Total=11 Passed=9 Failed=0 Skipped=2
INFO:  04/19/13 07:26:34: >>> Suite 'Audio': Passed
...
INFO:  04/19/13 07:26:34: Total Run runtime: 0.0 sec
INFO:  04/19/13 07:26:34: Run Summary: Total=11 Passed=9 Failed=0 Skipped=2
INFO:  04/19/13 07:26:34: >>> Run /w seed 'IX2TEOADU8HIZNI': Passed

INFO:  04/19/13 07:26:34: Exit code: 0
Comment 2 Ellie 2013-04-19 15:52:33 UTC
I just did a fresh mercurial checkout from SDL2 in a VirtualBox Fedora 18 (x86_64). I did a simple "rm -rf ./.hg && ./configure && make" without any special options. Then I used this to build this little crash test again.

See what happens:

[jonas@localhost ~]$ gcc -o test ./test.c -L. -lSDL2 -ISDL/include -ldl -lm -lpthread
[jonas@localhost ~]$ ./test
Now we expect a crash:
Segmentation fault (core dumped)
[jonas@localhost ~]$ cat test.c


#include "SDL.h"
#include <stdio.h>
#include <unistd.h>

void audiocallback(void *intentionally_unused, Uint8 *stream, int len) {
    //dummy callback
    memset(stream, 0, len);
}

int main(int argc, char** argv) {
    //initialise:
    if (SDL_Init(SDL_INIT_TIMER) < 0) {return -1;}
    if (SDL_AudioInit(NULL) < 0) {return -1;}

    //open audio:
    SDL_AudioSpec fmt;
    memset(&fmt,0,sizeof(fmt));
    fmt.freq = 48000;
    fmt.format = AUDIO_F32SYS;
    fmt.channels = 2;
    fmt.samples = 2048;
    fmt.callback = audiocallback;
    fmt.userdata = NULL;
    if (SDL_OpenAudio(&fmt, NULL) < 0) {return -1;}

    //close audio:
    SDL_CloseAudio();

    //cleanup:
    SDL_AudioQuit();
    printf("Now we expect a crash:\n");fflush(stdout);
    SDL_Quit(); //segfaults
}

[jonas@localhost ~]$

Maybe it is limited to Fedora. If you have trouble reproducing it, I recommend booting a Fedora 18 x86_64 live cd in VirtualBox (or on a real machine) and doing the test there.

(When I filed this bug report, I tested on Fedora 16 or 17, so it's not limited to the current Fedora 18)
Comment 3 Sam Lantinga 2013-04-19 16:46:12 UTC
Can you run under the debugger and get a stack trace so we can see what's happening?
Comment 4 Ellie 2013-04-19 17:12:22 UTC
[jonas@localhost ~]$ gdb ./test
GNU gdb (GDB) Fedora (7.5.1-37.fc18)
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /home/jonas/test...done.
(gdb) run
Starting program: /home/jonas/test 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[New Thread 0x7ffff7fe2700 (LWP 448)]
[New Thread 0x7ffff35d9700 (LWP 449)]
[New Thread 0x7ffff2dd8700 (LWP 452)]
[Thread 0x7ffff2dd8700 (LWP 452) exited]
Now we expect a crash:
[Thread 0x7ffff35d9700 (LWP 449) exited]

Program received signal SIGSEGV, Segmentation fault.
0x0000000000000000 in ?? ()
(gdb) backtrace
#0  0x0000000000000000 in ?? ()
#1  0x0000000000405d56 in SDL_AudioQuit () at src/audio/SDL_audio.c:1181
#2  0x0000000000402b05 in SDL_QuitSubSystem (flags=flags@entry=12849) at src/SDL.c:257
#3  0x0000000000402ba8 in SDL_Quit () at src/SDL.c:316
#4  0x00000000004026f8 in main (argc=1, argv=0x7fffffffe178) at ./test.c:34
(gdb) up
#1  0x0000000000405d56 in SDL_AudioQuit () at src/audio/SDL_audio.c:1181
1181	    current_audio.impl.Deinitialize();
(gdb) print current_audio.impl
$1 = {DetectDevices = 0x0, OpenDevice = 0x0, ThreadInit = 0x0, WaitDevice = 0x0, PlayDevice = 0x0, 
  GetDeviceBuf = 0x0, WaitDone = 0x0, CloseDevice = 0x0, LockDevice = 0x0, UnlockDevice = 0x0, 
  Deinitialize = 0x0, ProvidesOwnCallbackThread = 0, SkipMixerLock = 0, HasCaptureSupport = 0, 
  OnlyHasDefaultOutputDevice = 0, OnlyHasDefaultInputDevice = 0}
(gdb)


If you want me to print out some other variables or do a breakpoint at some earlier stage or something like that, let me know.
Comment 5 Ellie 2013-04-19 17:28:12 UTC
It might be that bug #1343 is caused by this aswell. (I am not familiar with debugging under wine where I could reproduce #1343, so I cannot easily obtain a backtrace for #1343 to confirm/compare right now)
Comment 6 Andreas Schiffler 2013-04-23 11:15:43 UTC
Added a test case to the audio test suite to repro this issue. Simply run:
   ./testautomation --filter audio_initOpenCloseQuitAudio
to get the segfault.

Calling just init/quit (directly or subsystem) without opening the audio device does not cause any issues (added test cases for this as well). So the crash is likely triggered during the audio thread tear-down or after an unsuccessful tear-down. 

Since it segfaults on my cygwin/64bit code, this is not x86/Linux specific.
Comment 7 Dimitris Zenios 2013-04-23 18:18:49 UTC
The problem is the flow of the commands

SDL_AudioInit initialized the audio sub system without letting SDL know that AUDIO was actually initialized

Then SDL_OpenAudio Initializes the audio again since SDL_WasInit(AUDIO) returns false.The difference this time is that audio is initialized by calling SDL_InitSubSystem(AUDIO) so SDL knows that AUDIO subsystem is initialized.

After that SDL_CloseAudio comes which closed the audio but does not inform SDL that audio was closed.

In the end SDL_Quit tried to close the audio again since it thinks that it was reopened. -> Double free -> corruption

I think this is more of a generic problem with SDLs subsystems.Either all the subsystems init function will call SDL_InitSubSystem so SDL will know about it or we will make them private and will leave only SDL_InitSubSystem as a function for initializing subsystems.

In my opinion since you decided to initialize audio by your self without calling SDL_initSubSystem then SDLs subsystems dont have to inform SDL.In order to achieve this the call to SDL_InitSubSystem from internal function has to be removed.There are 2 invocations right now

1.SDL_OpenAudio as i said above
2.SDL_PromptAssertion_cocoa (I think its not called from anywhere)

By replacing SDL_InitSubSystem(AUDIO) with SDL_AudioInit inside SDL_OpenAudio and having an internal integer variable whether audio was initialized or not then this bug will be solved.Is is the correct solution though?
Comment 8 Ellie 2013-04-23 19:00:51 UTC
Whatever the final solution might be, the reason I used SDL_AudioInit (instead of just SDL_Init with SDL_AUDIO) is that I want to specify the audio device. I suppose making SDL_AudioInit private would remove that possiblity, so it would be neat if you considered that possibility carefully before choosing that one as a solution.

So in the light of that, (just my humble opinion though) I would prefer the "sdl isn't informed" approach. (this is also what I expected from reading the api/docs, hence the explicit SDL_AudioQuit())

Also, maybe this is the right moment to ask for a review of bug #1343 which might be a similar issue?
Comment 9 Andreas Schiffler 2013-04-26 00:03:07 UTC
In my view, SDL_AudioInit and SDL_InitSubSystem(AUDIO) should be equivalent as far as state is concerned; that is, SDL_WasInit(AUDIO) should return true after either of these calls. Any other function that relies on audio state such as SDL_OpenAudio, SDL_AudioQuit, etc. should only take a dependency on SDL_WasInit(AUDIO) for discovering state.
Comment 10 Ryan C. Gordon 2013-07-05 00:55:33 UTC
Regardless of the "right" way to use SDL in this case, I've fixed this specific crash, in hg changeset 0021ad840cdd.

Thanks!

--ryan.