[cairo-commit] 2 commits - src/cairo-scaled-font.c test/pthread-show-text.c

Carl Worth cworth at kemper.freedesktop.org
Tue Feb 6 20:47:41 PST 2007


 src/cairo-scaled-font.c  |    9 +++------
 test/pthread-show-text.c |    7 +++++--
 2 files changed, 8 insertions(+), 8 deletions(-)

New commits:
diff-tree 1503a41c7f115033b10470574027cffab0730687 (from fc3ce1e80a89aab4f1ec75946771f6c2a79f0b90)
Author: Carl Worth <cworth at cworth.org>
Date:   Tue Feb 6 20:32:08 2007 -0800

    Expand font_map locking to cover call to backend->scaled_font_create
    
    It seemed like a good idea to avoid holding the lock over the call
    into the backend. But the procedure in place was quite broken:
    
    	LOCK
    	Fail to find font in hash table
    	UNLOCK
    	Create font
    	LOCK
    	Insert font into hash table
    	UNLOCK
    
    since while we're busy creating the font and unlocked, another thread
    can insert the font. Our paranoid hash table implementation noted
    the problem quite readily.
    
    This patch simply removes the internal UNLOCK/LOCK to hold the mutex
    over the whole procedure.

diff --git a/src/cairo-scaled-font.c b/src/cairo-scaled-font.c
index cdabb62..1ae7bfc 100755
--- a/src/cairo-scaled-font.c
+++ b/src/cairo-scaled-font.c
@@ -497,17 +497,14 @@ cairo_scaled_font_create (cairo_font_fac
 	scaled_font->ref_count++;
 	_cairo_scaled_font_map_unlock ();
     } else {
-	/* We don't want any mutex held while calling into the backend
-	 * function. */
-	_cairo_scaled_font_map_unlock ();
-
 	/* Otherwise create it and insert it into the hash table. */
 	status = font_face->backend->scaled_font_create (font_face, font_matrix,
 							 ctm, options, &scaled_font);
-	if (status)
+	if (status) {
+	    _cairo_scaled_font_map_unlock ();
 	    return NULL;
+	}
 
-	font_map = _cairo_scaled_font_map_lock ();
 	status = _cairo_hash_table_insert (font_map->hash_table,
 					   &scaled_font->hash_entry);
 	_cairo_scaled_font_map_unlock ();
diff-tree fc3ce1e80a89aab4f1ec75946771f6c2a79f0b90 (from d07827ba2a225f880f4bbf6980872f5ae8898288)
Author: Carl Worth <cworth at cworth.org>
Date:   Tue Feb 6 20:41:58 2007 -0800

    test/pthread-show-text: Increease iterations to expose locking bug
    
    With 50 iterations I'm seeing the following assertion failure:
    
    cairo-hash.c:477: _cairo_hash_table_insert: Assertion `NOT_REACHED' failed.
    
    Thanks to Jan Slupski <jslupski at juljas.net> for pointing out this bug.

diff --git a/test/pthread-show-text.c b/test/pthread-show-text.c
index d04de8b..94724c7 100644
--- a/test/pthread-show-text.c
+++ b/test/pthread-show-text.c
@@ -39,6 +39,9 @@
 #include <fontconfig/fontconfig.h>
 #endif
 
+#define NUM_THREADS_DEFAULT 20
+#define NUM_ITERATIONS 50
+
 static void *
 start (void *closure)
 {
@@ -58,7 +61,7 @@ start (void *closure)
 
     cairo_move_to (cr, 1, 1);
 
-    for (i=0; i < 10; i++) {
+    for (i=0; i < NUM_ITERATIONS; i++) {
 	cairo_set_font_size (cr, 8 + i);
 	cairo_show_text (cr, "Hello world.\n");
     }
@@ -79,7 +82,7 @@ main (int argc, char *argv[])
     if (argc > 1) {
 	num_threads = atoi (argv[1]);
     } else {
-	num_threads = 20;
+	num_threads = NUM_THREADS_DEFAULT;
     }
 
     cairo_test_init ("pthread-show-text");


More information about the cairo-commit mailing list