search

libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.72k stars 1.8k forks source link

SDL_CreateAudioStream/SDL_Quit: destroy locked mutex #9379

Closed madebr closed 6 months ago

madebr commented 6 months ago

Build the following reproducer with -fsanitize=thread:

#include <SDL3/SDL.h>

int main() {
    SDL_SetHint(SDL_HINT_AUDIO_DRIVER, "dummy");
    if (SDL_Init(SDL_INIT_AUDIO) < 0) {
        SDL_Log("SDL_Init failed (%s)", SDL_GetError());
        return 1;
    }
    SDL_AudioStream *stream = SDL_CreateAudioStream(NULL, NULL);
    (void)stream;
    SDL_Quit();
    return 0;
}

error message:

WARNING: ThreadSanitizer: destroy of a locked mutex (pid=73749)
    #0 pthread_mutex_destroy <null> (libtsan.so.2+0x3e6ad) (BuildId: 66cad9ec271b9ca10171b0eb35058d43560b9843)
    #1 SDL_DestroyMutex_REAL /home/maarten/projects/SDL/src/thread/pthread/SDL_sysmutex.c:56 (libSDL3.so.0+0x3c9d50) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #2 SDL_DestroyAudioStream_REAL /home/maarten/projects/SDL/src/audio/SDL_audiocvt.c:1199 (libSDL3.so.0+0x49434) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #3 SDL_QuitAudio /home/maarten/projects/SDL/src/audio/SDL_audio.c:970 (libSDL3.so.0+0x39f14) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #4 SDL_QuitSubSystem_REAL /home/maarten/projects/SDL/src/SDL.c:471 (libSDL3.so.0+0x2ebaa) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #5 SDL_Quit_REAL /home/maarten/projects/SDL/src/SDL.c:543 (libSDL3.so.0+0x2edfd) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #6 SDL_Quit /home/maarten/projects/SDL/src/dynapi/SDL_dynapi_procs.h:656 (libSDL3.so.0+0x7f36a) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #7 main /home/maarten/projects/SDL/test/a.c:11 (a+0x40120d) (BuildId: 00013f7c458b385f6c2dd231a32e3af0c3a698b2)

  and:
    #0 pthread_mutex_lock <null> (libtsan.so.2+0x4d441) (BuildId: 66cad9ec271b9ca10171b0eb35058d43560b9843)
    #1 SDL_LockMutex_REAL /home/maarten/projects/SDL/src/thread/pthread/SDL_sysmutex.c:79 (libSDL3.so.0+0x3c9d8f) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #2 SDL_UnbindAudioStreams_REAL /home/maarten/projects/SDL/src/audio/SDL_audio.c:1871 (libSDL3.so.0+0x3df39) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #3 SDL_UnbindAudioStream_REAL /home/maarten/projects/SDL/src/audio/SDL_audio.c:1920 (libSDL3.so.0+0x3e3e9) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #4 SDL_DestroyAudioStream_REAL /home/maarten/projects/SDL/src/audio/SDL_audiocvt.c:1193 (libSDL3.so.0+0x493b4) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #5 SDL_QuitAudio /home/maarten/projects/SDL/src/audio/SDL_audio.c:970 (libSDL3.so.0+0x39f14) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #6 SDL_QuitSubSystem_REAL /home/maarten/projects/SDL/src/SDL.c:471 (libSDL3.so.0+0x2ebaa) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #7 SDL_Quit_REAL /home/maarten/projects/SDL/src/SDL.c:543 (libSDL3.so.0+0x2edfd) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #8 SDL_Quit /home/maarten/projects/SDL/src/dynapi/SDL_dynapi_procs.h:656 (libSDL3.so.0+0x7f36a) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #9 main /home/maarten/projects/SDL/test/a.c:11 (a+0x40120d) (BuildId: 00013f7c458b385f6c2dd231a32e3af0c3a698b2)

  Location is heap block of size 40 at 0x7b0c00000cc0 allocated by main thread:
    #0 calloc <null> (libtsan.so.2+0x3d575) (BuildId: 66cad9ec271b9ca10171b0eb35058d43560b9843)
    #1 real_calloc /home/maarten/projects/SDL/src/stdlib/SDL_malloc.c:5189 (libSDL3.so.0+0x17541e) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #2 SDL_calloc_REAL /home/maarten/projects/SDL/src/stdlib/SDL_malloc.c:5306 (libSDL3.so.0+0x1758a1) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #3 SDL_CreateMutex_REAL /home/maarten/projects/SDL/src/thread/pthread/SDL_sysmutex.c:34 (libSDL3.so.0+0x3c9cac) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #4 SDL_CreateAudioStream_REAL /home/maarten/projects/SDL/src/audio/SDL_audiocvt.c:423 (libSDL3.so.0+0x46a8b) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #5 SDL_CreateAudioStream /home/maarten/projects/SDL/src/dynapi/SDL_dynapi_procs.h:129 (libSDL3.so.0+0x75ac5) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #6 main /home/maarten/projects/SDL/test/a.c:9 (a+0x401204) (BuildId: 00013f7c458b385f6c2dd231a32e3af0c3a698b2)

  Mutex M0 (0x7b0c00000cc0) created at:
    #0 pthread_mutex_init <null> (libtsan.so.2+0x3e5b8) (BuildId: 66cad9ec271b9ca10171b0eb35058d43560b9843)
    #1 SDL_CreateMutex_REAL /home/maarten/projects/SDL/src/thread/pthread/SDL_sysmutex.c:44 (libSDL3.so.0+0x3c9ce7) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #2 SDL_CreateAudioStream_REAL /home/maarten/projects/SDL/src/audio/SDL_audiocvt.c:423 (libSDL3.so.0+0x46a8b) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #3 SDL_CreateAudioStream /home/maarten/projects/SDL/src/dynapi/SDL_dynapi_procs.h:129 (libSDL3.so.0+0x75ac5) (BuildId: 03422b188ec49b2b90d5cff002e620cc9e71b0c1)
    #4 main /home/maarten/projects/SDL/test/a.c:9 (a+0x401204) (BuildId: 00013f7c458b385f6c2dd231a32e3af0c3a698b2)

SUMMARY: ThreadSanitizer: destroy of a locked mutex (/lib64/libtsan.so.2+0x3e6ad) (BuildId: 66cad9ec271b9ca10171b0eb35058d43560b9843) in pthread_mutex_destroy

@icculus (moved from #9365)

0x1F9F1 commented 6 months ago

I think this would fix it, but do we need SDL_UnbindAudioStreams? It only seems to be called by SDL_UnbindAudioStream internally, and those extra loops make it more confusing than it needs to be.

diff --git a/src/audio/SDL_audio.c b/src/audio/SDL_audio.c
index 60fdde704..a2468e8c2 100644
--- a/src/audio/SDL_audio.c
+++ b/src/audio/SDL_audio.c
@@ -1903,7 +1903,7 @@ void SDL_UnbindAudioStreams(SDL_AudioStream **streams, int num_streams)
     // Finalize and unlock everything.
     for (int i = 0; i < num_streams; i++) {
         SDL_AudioStream *stream = streams[i];
-        if (stream && stream->bound_device) {
+        if (stream) {
             SDL_LogicalAudioDevice *logdev = stream->bound_device;
             stream->bound_device = NULL;
             SDL_UnlockMutex(stream->lock);
icculus commented 6 months ago

but do we need SDL_UnbindAudioStreams?

This removes multiple streams from an audio device atomically, so they all stop playing at the same time. If you unbind them one at a time, you race against the device playing the last streams unbound a little longer than the first streams.

(as many things won't need this, there's the simpler wrapper function to unbind a single stream.)

icculus commented 6 months ago

I do believe @0x1F9F1's patch is correct, though, so I'm going to apply that.