Skip to content

Commit

Permalink
Fix sporadic deadlocks in the audio addon.
Browse files Browse the repository at this point in the history 
Occasionally the stream mutex would get set to NULL in between lock and unlock,
thus causing the mutex to never get unlocked.

(cherry picked from commit b65b53d)
  • Loading branch information
@SiegeLordEx @SiegeLord
SiegeLordEx authored and SiegeLord committed Jan 24, 2016
1 parent 6aaab2b commit 6db0bd2
Showing 1 changed file with 39 additions and 32 deletions.
71 changes: 39 additions & 32 deletions addons/audio/kcm_stream.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,16 @@ ALLEGRO_DEBUG_CHANNEL("audio")
#define MAX_LAG (3)


static void maybe_lock_mutex(ALLEGRO_MUTEX *mutex)
/*
* To avoid deadlocks, unlock the mutex returned by this function, rather than
* whatever you passed as the argument.
*/
static ALLEGRO_MUTEX *maybe_lock_mutex(ALLEGRO_MUTEX *mutex)
{
if (mutex) {
al_lock_mutex(mutex);
}
return mutex;
}


Expand Down Expand Up @@ -307,17 +312,18 @@ bool al_get_audio_stream_attached(const ALLEGRO_AUDIO_STREAM *stream)
uint64_t al_get_audio_stream_played_samples(const ALLEGRO_AUDIO_STREAM *stream)
{
uint64_t result;
ALLEGRO_MUTEX *stream_mutex;
ASSERT(stream);

maybe_lock_mutex(stream->spl.mutex);
stream_mutex = maybe_lock_mutex(stream->spl.mutex);
if (stream->spl.spl_data.buffer.ptr) {
result = stream->consumed_fragments * stream->spl.spl_data.len +
stream->spl.pos;
}
else {
result = 0;
}
maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);

return result;
}
Expand All @@ -328,9 +334,10 @@ void *al_get_audio_stream_fragment(const ALLEGRO_AUDIO_STREAM *stream)
{
size_t i;
void *fragment;
ALLEGRO_MUTEX *stream_mutex;
ASSERT(stream);

maybe_lock_mutex(stream->spl.mutex);
stream_mutex = maybe_lock_mutex(stream->spl.mutex);

if (!stream->used_bufs[0]) {
/* No free fragments are available. */
Expand All @@ -344,7 +351,7 @@ void *al_get_audio_stream_fragment(const ALLEGRO_AUDIO_STREAM *stream)
stream->used_bufs[i] = NULL;
}

maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);

return fragment;
}
Expand All @@ -371,8 +378,7 @@ bool al_set_audio_stream_speed(ALLEGRO_AUDIO_STREAM *stream, float val)
stream->spl.speed = val;
if (stream->spl.parent.u.mixer) {
ALLEGRO_MIXER *mixer = stream->spl.parent.u.mixer;

maybe_lock_mutex(stream->spl.mutex);
ALLEGRO_MUTEX *stream_mutex = maybe_lock_mutex(stream->spl.mutex);

stream->spl.step = (stream->spl.spl_data.frequency) * stream->spl.speed;
stream->spl.step_denom = mixer->ss.spl_data.frequency;
Expand All @@ -381,7 +387,7 @@ bool al_set_audio_stream_speed(ALLEGRO_AUDIO_STREAM *stream, float val)
stream->spl.step = 1;
}

maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);
}

return true;
Expand All @@ -408,10 +414,9 @@ bool al_set_audio_stream_gain(ALLEGRO_AUDIO_STREAM *stream, float val)
*/
if (stream->spl.parent.u.mixer) {
ALLEGRO_MIXER *mixer = stream->spl.parent.u.mixer;

maybe_lock_mutex(stream->spl.mutex);
ALLEGRO_MUTEX *stream_mutex = maybe_lock_mutex(stream->spl.mutex);
_al_kcm_mixer_rejig_sample_matrix(mixer, &stream->spl);
maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);
}
}

Expand Down Expand Up @@ -443,10 +448,9 @@ bool al_set_audio_stream_pan(ALLEGRO_AUDIO_STREAM *stream, float val)
*/
if (stream->spl.parent.u.mixer) {
ALLEGRO_MIXER *mixer = stream->spl.parent.u.mixer;

maybe_lock_mutex(stream->spl.mutex);
ALLEGRO_MUTEX *stream_mutex = maybe_lock_mutex(stream->spl.mutex);
_al_kcm_mixer_rejig_sample_matrix(mixer, &stream->spl);
maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);
}
}

Expand Down Expand Up @@ -530,6 +534,7 @@ static void reset_stopped_stream(ALLEGRO_AUDIO_STREAM *stream)
bool al_set_audio_stream_playing(ALLEGRO_AUDIO_STREAM *stream, bool val)
{
bool rc = true;
ALLEGRO_MUTEX *stream_mutex;
ASSERT(stream);

if (stream->spl.parent.u.ptr && stream->spl.parent.is_voice) {
Expand All @@ -539,7 +544,7 @@ bool al_set_audio_stream_playing(ALLEGRO_AUDIO_STREAM *stream, bool val)
}
}

maybe_lock_mutex(stream->spl.mutex);
stream_mutex = maybe_lock_mutex(stream->spl.mutex);

stream->spl.is_playing = rc && val;

Expand All @@ -554,7 +559,7 @@ bool al_set_audio_stream_playing(ALLEGRO_AUDIO_STREAM *stream, bool val)
reset_stopped_stream(stream);
}

maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);

return rc;
}
Expand All @@ -578,9 +583,10 @@ bool al_set_audio_stream_fragment(ALLEGRO_AUDIO_STREAM *stream, void *val)
{
size_t i;
bool ret;
ALLEGRO_MUTEX *stream_mutex;
ASSERT(stream);

maybe_lock_mutex(stream->spl.mutex);
stream_mutex = maybe_lock_mutex(stream->spl.mutex);

for (i = 0; i < stream->buf_count && stream->pending_bufs[i] ; i++)
;
Expand All @@ -594,7 +600,7 @@ bool al_set_audio_stream_fragment(ALLEGRO_AUDIO_STREAM *stream, void *val)
ret = false;
}

maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);

return ret;
}
Expand Down Expand Up @@ -690,6 +696,7 @@ void *_al_kcm_feed_stream(ALLEGRO_THREAD *self, void *vstream)
&& !stream->is_draining) {
unsigned long bytes;
unsigned long bytes_written;
ALLEGRO_MUTEX *stream_mutex;

fragment = al_get_audio_stream_fragment(stream);
if (!fragment) {
Expand All @@ -701,21 +708,21 @@ void *_al_kcm_feed_stream(ALLEGRO_THREAD *self, void *vstream)
al_get_channel_count(stream->spl.spl_data.chan_conf) *
al_get_audio_depth_size(stream->spl.spl_data.depth);

maybe_lock_mutex(stream->spl.mutex);
stream_mutex = maybe_lock_mutex(stream->spl.mutex);
bytes_written = stream->feeder(stream, fragment, bytes);
maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);

if (stream->spl.loop == _ALLEGRO_PLAYMODE_STREAM_ONEDIR) {
/* Keep rewinding until the fragment is filled. */
while (bytes_written < bytes &&
stream->spl.loop == _ALLEGRO_PLAYMODE_STREAM_ONEDIR) {
size_t bw;
al_rewind_audio_stream(stream);
maybe_lock_mutex(stream->spl.mutex);
stream_mutex = maybe_lock_mutex(stream->spl.mutex);
bw = stream->feeder(stream, fragment + bytes_written,
bytes - bytes_written);
bytes_written += bw;
maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);
}
}
else if (bytes_written < bytes) {
Expand Down Expand Up @@ -787,9 +794,9 @@ bool al_rewind_audio_stream(ALLEGRO_AUDIO_STREAM *stream)
bool ret;

if (stream->rewind_feeder) {
maybe_lock_mutex(stream->spl.mutex);
ALLEGRO_MUTEX *stream_mutex = maybe_lock_mutex(stream->spl.mutex);
ret = stream->rewind_feeder(stream);
maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);
return ret;
}

Expand All @@ -804,9 +811,9 @@ bool al_seek_audio_stream_secs(ALLEGRO_AUDIO_STREAM *stream, double time)
bool ret;

if (stream->seek_feeder) {
maybe_lock_mutex(stream->spl.mutex);
ALLEGRO_MUTEX *stream_mutex = maybe_lock_mutex(stream->spl.mutex);
ret = stream->seek_feeder(stream, time);
maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);
return ret;
}

Expand All @@ -821,9 +828,9 @@ double al_get_audio_stream_position_secs(ALLEGRO_AUDIO_STREAM *stream)
double ret;

if (stream->get_feeder_position) {
maybe_lock_mutex(stream->spl.mutex);
ALLEGRO_MUTEX *stream_mutex = maybe_lock_mutex(stream->spl.mutex);
ret = stream->get_feeder_position(stream);
maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);
return ret;
}

Expand All @@ -838,9 +845,9 @@ double al_get_audio_stream_length_secs(ALLEGRO_AUDIO_STREAM *stream)
double ret;

if (stream->get_feeder_length) {
maybe_lock_mutex(stream->spl.mutex);
ALLEGRO_MUTEX *stream_mutex = maybe_lock_mutex(stream->spl.mutex);
ret = stream->get_feeder_length(stream);
maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);
return ret;
}

Expand All @@ -859,9 +866,9 @@ bool al_set_audio_stream_loop_secs(ALLEGRO_AUDIO_STREAM *stream,
return false;

if (stream->set_feeder_loop) {
maybe_lock_mutex(stream->spl.mutex);
ALLEGRO_MUTEX *stream_mutex = maybe_lock_mutex(stream->spl.mutex);
ret = stream->set_feeder_loop(stream, start, end);
maybe_unlock_mutex(stream->spl.mutex);
maybe_unlock_mutex(stream_mutex);
return ret;
}

Expand Down

0 comments on commit 6db0bd2

Please sign in to comment.