[cairo] [PATCH] Do not hold mutexes when calling destructors.

Jeff Muizelaar jeff at infidigm.net
Fri May 9 15:15:21 PDT 2008


cairo_surface_destroy and _cairo_scaled_font_fini will call destroy closures
which may call functions that attempt to acquire the mutex resulting in a
deadlock. We fix this by releasing the lock for the call to
cairo_surface_destroy or _cairo_scaled_font_fini.
---

 src/cairo-pattern.c     |   17 ++++++++++++-----
 src/cairo-scaled-font.c |   14 +++++++++++---
 2 files changed, 23 insertions(+), 8 deletions(-)


diff --git a/src/cairo-pattern.c b/src/cairo-pattern.c
index 0f2eb3e..903d81f 100644
--- a/src/cairo-pattern.c
+++ b/src/cairo-pattern.c
@@ -1515,13 +1515,20 @@ UNLOCK:
 static void
 _cairo_pattern_reset_solid_surface_cache (void)
 {
-    int i;
-
     CAIRO_MUTEX_LOCK (_cairo_pattern_solid_surface_cache_lock);
 
-    for (i = 0; i < solid_surface_cache.size; i++)
-	cairo_surface_destroy (solid_surface_cache.cache[i].surface);
-    solid_surface_cache.size = 0;
+    /* remove surfaces starting from the end so that solid_surface_cache.cache
+     * is always in a consistent state when we release the mutex. */
+    while (solid_surface_cache.size) {
+	cairo_surface_t *surface = solid_surface_cache.cache[solid_surface_cache.size-1].surface;
+	solid_surface_cache.size--;
+
+	/* release the lock to avoid the possibility of a recursive
+	 * deadlock when the scaled font destroy closure gets called */
+	CAIRO_MUTEX_UNLOCK (_cairo_pattern_solid_surface_cache_lock);
+	cairo_surface_destroy (surface);
+	CAIRO_MUTEX_LOCK (_cairo_pattern_solid_surface_cache_lock);
+    }
 
     CAIRO_MUTEX_UNLOCK (_cairo_pattern_solid_surface_cache_lock);
 }
diff --git a/src/cairo-scaled-font.c b/src/cairo-scaled-font.c
index e12446c..d216001 100644
--- a/src/cairo-scaled-font.c
+++ b/src/cairo-scaled-font.c
@@ -344,7 +344,6 @@ _cairo_scaled_font_map_unlock (void)
 void
 _cairo_scaled_font_map_destroy (void)
 {
-    int i;
     cairo_scaled_font_map_t *font_map;
     cairo_scaled_font_t *scaled_font;
 
@@ -355,15 +354,24 @@ _cairo_scaled_font_map_destroy (void)
         goto CLEANUP_MUTEX_LOCK;
     }
 
-    for (i = 0; i < font_map->num_holdovers; i++) {
-	scaled_font = font_map->holdovers[i];
+    /* remove scaled_fonts starting from the end so that font_map->holdovers
+     * is always in a consistent state when we release the mutex. */
+    while (font_map->num_holdovers) {
+	scaled_font = font_map->holdovers[font_map->num_holdovers-1];
 	/* We should only get here through the reset_static_data path
 	 * and there had better not be any active references at that
 	 * point. */
 	assert (! CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&scaled_font->ref_count));
 	_cairo_hash_table_remove (font_map->hash_table,
 				  &scaled_font->hash_entry);
+	font_map->num_holdovers--;
+
+	/* release the lock to avoid the possibility of a recursive
+	 * deadlock when the scaled font destroy closure gets called */
+	CAIRO_MUTEX_UNLOCK(_cairo_scaled_font_map_mutex);
 	_cairo_scaled_font_fini (scaled_font);
+	CAIRO_MUTEX_LOCK(_cairo_scaled_font_map_mutex);
+
 	free (scaled_font);
     }
 



More information about the cairo mailing list