From 0abc869e3e4b2ec6be387c3298062477f51fc338 Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Sun, 18 Mar 2007 17:57:48 +0000 Subject: [PATCH] ext/jack/gstjackaudioclient.c: Don't need to take the connection lock, it will not be used and could cause deadlocks. Original commit message from CVS: Based on patch by: Paul Davis * ext/jack/gstjackaudioclient.c: (gst_jack_audio_unref_connection): Don't need to take the connection lock, it will not be used and could cause deadlocks. --- ChangeLog | 8 ++++++++ ext/jack/gstjackaudioclient.c | 26 +++++++++++++++++++------- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index dd5c3d7..f0e15a1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2007-03-18 Wim Taymans + + Based on patch by: Paul Davis + + * ext/jack/gstjackaudioclient.c: (gst_jack_audio_unref_connection): + Don't need to take the connection lock, it will not be used and could + cause deadlocks. + 2007-03-16 Edward Hervey * sys/osxvideo/osxvideosink.m: diff --git a/ext/jack/gstjackaudioclient.c b/ext/jack/gstjackaudioclient.c index c811259..9777cd9 100644 --- a/ext/jack/gstjackaudioclient.c +++ b/ext/jack/gstjackaudioclient.c @@ -283,18 +283,32 @@ static void gst_jack_audio_unref_connection (GstJackAudioConnection * conn) { gint res; + gboolean zero; - GST_DEBUG ("unref connection %p", conn); + GST_DEBUG ("unref connection %p refcnt %d", conn, conn->refcount); G_LOCK (connections_lock); conn->refcount--; - if (conn->refcount == 0) { + if ((zero = (conn->refcount == 0))) { GST_DEBUG ("closing connection %p", conn); - /* remove from list */ + /* remove from list, we can release the mutex after removing the connection + * from the list because after that, nobody can access the connection anymore. */ connections = g_list_remove (connections, conn); + } + G_UNLOCK (connections_lock); - /* grab lock to be sure that we are not in one of the callbacks */ - g_mutex_lock (conn->lock); + /* if we are zero, close and cleanup the connection */ + if (zero) { + /* don't use conn->lock here. two reasons: + * + * 1) its not necessary: jack_deactivate() will not return until the JACK thread + * associated with this connection is cleaned up by a thread join, hence + * no more callbacks can occur or be in progress. + * + * 2) it would deadlock anyway, because jack_deactivate() will sleep + * waiting for the JACK thread, and can thus cause deadlock in + * jack_process_cb() + */ if ((res = jack_deactivate (conn->client))) { /* we only warn, this means the server is probably shut down and the client * is gone anyway. */ @@ -305,7 +319,6 @@ gst_jack_audio_unref_connection (GstJackAudioConnection * conn) /* we assume the client is gone. */ GST_WARNING ("close failed (%d)", res); } - g_mutex_unlock (conn->lock); /* free resources */ g_mutex_free (conn->lock); @@ -313,7 +326,6 @@ gst_jack_audio_unref_connection (GstJackAudioConnection * conn) g_free (conn->server); g_free (conn); } - G_UNLOCK (connections_lock); } static void -- 2.7.4