From 13537a913fff1168789f55e17625d854d3929534 Mon Sep 17 00:00:00 2001 From: Pavel Sountsov Date: Mon, 27 Apr 2015 21:37:22 -0700 Subject: [PATCH] Fix a race condition between the acodec and audio addons. If you try destroy an audio stream before the feeder thread starts, the feeder thread will never get the quit event and will deadlock forever. --- addons/acodec/flac.c | 3 +-- addons/acodec/helper.c | 18 ++++++++++++++++++ addons/acodec/helper.h | 1 + addons/acodec/modaudio.c | 3 +-- addons/acodec/ogg.c | 3 +-- addons/acodec/wav.c | 3 +-- addons/audio/allegro5/internal/aintern_audio.h | 3 +++ addons/audio/kcm_stream.c | 5 +++++ 8 files changed, 31 insertions(+), 8 deletions(-) diff --git a/addons/acodec/flac.c b/addons/acodec/flac.c index d398f9e..5408c63 100644 --- a/addons/acodec/flac.c +++ b/addons/acodec/flac.c @@ -615,7 +615,6 @@ ALLEGRO_AUDIO_STREAM *_al_load_flac_audio_stream_f(ALLEGRO_FILE* f, stream->extra = ff; ff->loop_start = 0; ff->loop_end = ff->total_samples; - stream->feed_thread = al_create_thread(_al_kcm_feed_stream, stream); stream->feeder = flac_stream_update; stream->unload_feeder = flac_stream_close; stream->rewind_feeder = flac_stream_rewind; @@ -623,7 +622,7 @@ ALLEGRO_AUDIO_STREAM *_al_load_flac_audio_stream_f(ALLEGRO_FILE* f, stream->get_feeder_position = flac_stream_get_position; stream->get_feeder_length = flac_stream_get_length; stream->set_feeder_loop = flac_stream_set_loop; - al_start_thread(stream->feed_thread); + _al_acodec_start_feed_thread(stream); } else { al_fclose(ff->fh); diff --git a/addons/acodec/helper.c b/addons/acodec/helper.c index 8619298..26194ca 100644 --- a/addons/acodec/helper.c +++ b/addons/acodec/helper.c @@ -5,14 +5,32 @@ #include "allegro5/internal/aintern_system.h" #include "helper.h" +void _al_acodec_start_feed_thread(ALLEGRO_AUDIO_STREAM *stream) +{ + stream->feed_thread = al_create_thread(_al_kcm_feed_stream, stream); + stream->feed_thread_started_cond = al_create_cond(); + stream->feed_thread_started_mutex = al_create_mutex(); + al_start_thread(stream->feed_thread); +} + void _al_acodec_stop_feed_thread(ALLEGRO_AUDIO_STREAM *stream) { ALLEGRO_EVENT quit_event; + /* Need to wait for the thread to start, otherwise the quit event may be + * sent before the event source is registered with the queue. */ + al_lock_mutex(stream->feed_thread_started_mutex); + while (!stream->feed_thread_started) { + al_wait_cond(stream->feed_thread_started_cond, stream->feed_thread_started_mutex); + } + al_unlock_mutex(stream->feed_thread_started_mutex); + quit_event.type = _KCM_STREAM_FEEDER_QUIT_EVENT_TYPE; al_emit_user_event(al_get_audio_stream_event_source(stream), &quit_event, NULL); al_join_thread(stream->feed_thread, NULL); al_destroy_thread(stream->feed_thread); + al_destroy_cond(stream->feed_thread_started_cond); + al_destroy_mutex(stream->feed_thread_started_mutex); stream->feed_thread = NULL; } diff --git a/addons/acodec/helper.h b/addons/acodec/helper.h index 9d4836f..91a018c 100644 --- a/addons/acodec/helper.h +++ b/addons/acodec/helper.h @@ -1,6 +1,7 @@ #ifndef __al_included_acodec_helper_h #define __al_included_acodec_helper_h +void _al_acodec_start_feed_thread(ALLEGRO_AUDIO_STREAM *stream); void _al_acodec_stop_feed_thread(ALLEGRO_AUDIO_STREAM *stream); #endif diff --git a/addons/acodec/modaudio.c b/addons/acodec/modaudio.c index b1ce849..5b73b98 100644 --- a/addons/acodec/modaudio.c +++ b/addons/acodec/modaudio.c @@ -241,7 +241,6 @@ static ALLEGRO_AUDIO_STREAM *mod_stream_init(ALLEGRO_FILE* f, mf->loop_end = -1; stream->extra = mf; - stream->feed_thread = al_create_thread(_al_kcm_feed_stream, stream); stream->feeder = modaudio_stream_update; stream->unload_feeder = modaudio_stream_close; stream->rewind_feeder = modaudio_stream_rewind; @@ -249,7 +248,7 @@ static ALLEGRO_AUDIO_STREAM *mod_stream_init(ALLEGRO_FILE* f, stream->get_feeder_position = modaudio_stream_get_position; stream->get_feeder_length = modaudio_stream_get_length; stream->set_feeder_loop = modaudio_stream_set_loop; - al_start_thread(stream->feed_thread); + _al_acodec_start_feed_thread(stream); } else { goto Error; diff --git a/addons/acodec/ogg.c b/addons/acodec/ogg.c index 57f13e1..8415b77 100644 --- a/addons/acodec/ogg.c +++ b/addons/acodec/ogg.c @@ -518,7 +518,6 @@ ALLEGRO_AUDIO_STREAM *_al_load_ogg_vorbis_audio_stream_f(ALLEGRO_FILE *file, extra->loop_start = 0.0; extra->loop_end = ogg_stream_get_length(stream); - stream->feed_thread = al_create_thread(_al_kcm_feed_stream, stream); stream->quit_feed_thread = false; stream->feeder = ogg_stream_update; stream->rewind_feeder = ogg_stream_rewind; @@ -527,7 +526,7 @@ ALLEGRO_AUDIO_STREAM *_al_load_ogg_vorbis_audio_stream_f(ALLEGRO_FILE *file, stream->get_feeder_length = ogg_stream_get_length; stream->set_feeder_loop = ogg_stream_set_loop; stream->unload_feeder = ogg_stream_close; - al_start_thread(stream->feed_thread); + _al_acodec_start_feed_thread(stream); return stream; } diff --git a/addons/acodec/wav.c b/addons/acodec/wav.c index 2cfff36..71d37e4 100644 --- a/addons/acodec/wav.c +++ b/addons/acodec/wav.c @@ -368,7 +368,6 @@ ALLEGRO_AUDIO_STREAM *_al_load_wav_audio_stream_f(ALLEGRO_FILE* f, stream->extra = wavfile; wavfile->loop_start = 0.0; wavfile->loop_end = wav_stream_get_length(stream); - stream->feed_thread = al_create_thread(_al_kcm_feed_stream, stream); stream->feeder = wav_stream_update; stream->unload_feeder = wav_stream_close; stream->rewind_feeder = wav_stream_rewind; @@ -376,7 +375,7 @@ ALLEGRO_AUDIO_STREAM *_al_load_wav_audio_stream_f(ALLEGRO_FILE* f, stream->get_feeder_position = wav_stream_get_position; stream->get_feeder_length = wav_stream_get_length; stream->set_feeder_loop = wav_stream_set_loop; - al_start_thread(stream->feed_thread); + _al_acodec_start_feed_thread(stream); } else { wav_close(wavfile); diff --git a/addons/audio/allegro5/internal/aintern_audio.h b/addons/audio/allegro5/internal/aintern_audio.h index 6209d20..6c6948f 100644 --- a/addons/audio/allegro5/internal/aintern_audio.h +++ b/addons/audio/allegro5/internal/aintern_audio.h @@ -255,6 +255,9 @@ struct ALLEGRO_AUDIO_STREAM { */ ALLEGRO_THREAD *feed_thread; + ALLEGRO_MUTEX *feed_thread_started_mutex; + ALLEGRO_COND *feed_thread_started_cond; + bool feed_thread_started; volatile bool quit_feed_thread; unload_feeder_t unload_feeder; rewind_feeder_t rewind_feeder; diff --git a/addons/audio/kcm_stream.c b/addons/audio/kcm_stream.c index b256de5..3cdd725 100644 --- a/addons/audio/kcm_stream.c +++ b/addons/audio/kcm_stream.c @@ -673,6 +673,11 @@ void *_al_kcm_feed_stream(ALLEGRO_THREAD *self, void *vstream) queue = al_create_event_queue(); al_register_event_source(queue, &stream->spl.es); + al_lock_mutex(stream->feed_thread_started_mutex); + stream->feed_thread_started = true; + al_broadcast_cond(stream->feed_thread_started_cond); + al_unlock_mutex(stream->feed_thread_started_mutex); + stream->quit_feed_thread = false; while (!stream->quit_feed_thread) { -- 1.9.1