Fix deadlock when signalling the thread which freed a thread pool

2006-02-15  Sebastian Wilhelmi  <seppi@seppi.de>

	* glib/gthreadpool.c: Fix deadlock when signalling the thread
	which freed a thread pool (#331110, Chris Wilson).

diff --git a/ChangeLog b/ChangeLog
index ea6b4b5..e5437cb 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2006-02-15  Sebastian Wilhelmi  <seppi@seppi.de>
+
+       * glib/gthreadpool.c: Fix deadlock when signalling the thread
+       which freed a thread pool (#331110, Chris Wilson).
+
 Tue Feb 14 17:00:43 2006  Tim Janik  <timj@imendio.com>
 
        * glib/gslice.c: only define _XOPEN_SOURCE to 600 to get at 

diff --git a/ChangeLog.pre-2-10 b/ChangeLog.pre-2-10
index ea6b4b5..e5437cb 100644 (file)
--- a/ChangeLog.pre-2-10
+++ b/ChangeLog.pre-2-10
@@ -1,3 +1,8 @@
+2006-02-15  Sebastian Wilhelmi  <seppi@seppi.de>
+
+       * glib/gthreadpool.c: Fix deadlock when signalling the thread
+       which freed a thread pool (#331110, Chris Wilson).
+
 Tue Feb 14 17:00:43 2006  Tim Janik  <timj@imendio.com>
 
        * glib/gslice.c: only define _XOPEN_SOURCE to 600 to get at 

diff --git a/ChangeLog.pre-2-12 b/ChangeLog.pre-2-12
index ea6b4b5..e5437cb 100644 (file)
--- a/ChangeLog.pre-2-12
+++ b/ChangeLog.pre-2-12
@@ -1,3 +1,8 @@
+2006-02-15  Sebastian Wilhelmi  <seppi@seppi.de>
+
+       * glib/gthreadpool.c: Fix deadlock when signalling the thread
+       which freed a thread pool (#331110, Chris Wilson).
+
 Tue Feb 14 17:00:43 2006  Tim Janik  <timj@imendio.com>
 
        * glib/gslice.c: only define _XOPEN_SOURCE to 600 to get at 

diff --git a/glib/gthreadpool.c b/glib/gthreadpool.c
index 1ccc73b..db8181f 100644 (file)
--- a/glib/gthreadpool.c
+++ b/glib/gthreadpool.c
@@ -201,7 +201,9 @@ g_thread_pool_thread_proxy (gpointer data)
                   * are either no tasks left or the pool shall stop
                   * immediatly, inform the waiting thread of a change
                   * of the thread pool state. */
+                 g_mutex_lock (inform_mutex);
                  g_cond_broadcast (inform_cond);
+                 g_mutex_unlock (inform_mutex);
                }
            }
          g_async_queue_unlock (pool->queue);
@@ -640,7 +642,7 @@ g_thread_pool_free (GThreadPool     *pool,
 
   g_return_if_fail (real);
   g_return_if_fail (real->running);
-  /* It there's no thread allowed here, there is not much sense in
+  /* If there's no thread allowed here, there is not much sense in
    * not stopping this pool immediately, when it's not empty */
   g_return_if_fail (immediate || real->max_threads != 0 || 
                    g_async_queue_length (real->queue) == 0);
@@ -653,15 +655,21 @@ g_thread_pool_free (GThreadPool     *pool,
 
   if (wait)
     {
-      g_mutex_lock (inform_mutex);
       while (g_async_queue_length_unlocked (real->queue) != -real->num_threads &&
             !(immediate && real->num_threads == 0))
        {
+         /* This locking is a bit delicate to avoid the broadcast of
+          * inform_cond to overtake the wait for it.  Therefore the
+          * inform_mutex is locked before the queue is unlocked. To
+          * avoid deadlocks however, the queue _must_ always be
+          * locked before locking the inform_mutex, if both are to be
+          * locked at the same time. */
+         g_mutex_lock (inform_mutex);
          g_async_queue_unlock (real->queue); 
          g_cond_wait (inform_cond, inform_mutex); 
+         g_mutex_unlock (inform_mutex); 
          g_async_queue_lock (real->queue); 
        }
-      g_mutex_unlock (inform_mutex); 
     }
 
   if (immediate ||