[cairo-commit] src/cairoint.h src/cairo-scaled-font.c src/cairo-scaled-font-private.h src/cairo-user-font.c

Behdad Esfahbod behdad at kemper.freedesktop.org
Sat May 24 22:58:09 PDT 2008


 src/cairo-scaled-font-private.h |    2 
 src/cairo-scaled-font.c         |  141 +++++++++++++++++++++-------------------
 src/cairo-user-font.c           |    4 -
 src/cairoint.h                  |    4 -
 4 files changed, 81 insertions(+), 70 deletions(-)

New commits:
commit 238a3117f191c927abcce6b0f5c555d8f34af59c
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Sun May 25 01:51:05 2008 -0400

    [cairo-scaled-font] Clean up recent locking changes
    
    Based on feedback from Keith.

diff --git a/src/cairo-scaled-font-private.h b/src/cairo-scaled-font-private.h
index 6571d06..c1d87ae 100644
--- a/src/cairo-scaled-font-private.h
+++ b/src/cairo-scaled-font-private.h
@@ -89,7 +89,7 @@ struct _cairo_scaled_font {
     cairo_matrix_t ctm;	          /* user space => device space */
     cairo_font_options_t options;
 
-    cairo_bool_t created; /*  protected by fontmap mutex */
+    cairo_bool_t placeholder; /*  protected by fontmap mutex */
 
     cairo_bool_t finished;
 
diff --git a/src/cairo-scaled-font.c b/src/cairo-scaled-font.c
index 6521fe8..66d7a0e 100644
--- a/src/cairo-scaled-font.c
+++ b/src/cairo-scaled-font.c
@@ -192,7 +192,7 @@ static const cairo_scaled_font_t _cairo_scaled_font_nil = {
       CAIRO_SUBPIXEL_ORDER_DEFAULT,
       CAIRO_HINT_STYLE_DEFAULT,
       CAIRO_HINT_METRICS_DEFAULT} ,
-    TRUE,			/* created */
+    FALSE,			/* placeholder */
     TRUE,			/* finished */
     { 1., 0., 0., 1., 0, 0},	/* scale */
     { 1., 0., 0., 1., 0, 0},	/* scale_inverse */
@@ -392,90 +392,107 @@ _cairo_scaled_font_map_destroy (void)
  * created (needed for user-fonts), we need to take extra care not
  * ending up with multiple identical scaled fonts being created.
  *
- * What we do is, we create a dummy identical scaled font, also marked
- * as being created, lock its mutex, and insert that in the fontmap
+ * What we do is, we create a fake identical scaled font, and mark
+ * it as placeholder, lock its mutex, and insert that in the fontmap
  * hash table.  This makes other code trying to create an identical
  * scaled font to just wait and retry.
  *
- * The reason we have to create a new key instead of just using
+ * The reason we have to create a fake scaled font instead of just using
  * scaled_font is for lifecycle management: we need to (or rather,
- * other code needs to) reference the key.  We can't do that on the
- * input scaled_font as it may be freed by font backend upon error.
+ * other code needs to) reference the scaked_font in the hash.
+ * We can't do that on the input scaled_font as it may be freed by
+ * font backend upon error.
  */
 
 void
-_cairo_scaled_font_unlock_font_map (cairo_scaled_font_t *scaled_font)
+_cairo_scaled_font_register_placeholder_and_unlock_font_map (cairo_scaled_font_t *scaled_font)
 {
-    if (!scaled_font->created) {
-        cairo_status_t status = CAIRO_STATUS_SUCCESS;
-	cairo_scaled_font_t *key;
-
-	key = malloc (sizeof (cairo_scaled_font_t));
-	if (!key) {
-	    status = CAIRO_STATUS_NO_MEMORY;
-	    goto FREE;
-	}
+    cairo_status_t status = CAIRO_STATUS_SUCCESS;
+    cairo_scaled_font_t *placeholder_scaled_font;
 
-	/* full initialization is wasteful, but who cares... */
-	status = _cairo_scaled_font_init (key,
-					  scaled_font->font_face,
-					  &scaled_font->font_matrix,
-					  &scaled_font->ctm,
-					  &scaled_font->options,
-					  NULL);
-	if (status)
-	    goto FINI;
+    placeholder_scaled_font = malloc (sizeof (cairo_scaled_font_t));
+    if (!placeholder_scaled_font) {
+	status = CAIRO_STATUS_NO_MEMORY;
+	goto FREE;
+    }
 
-	CAIRO_MUTEX_LOCK (key->mutex);
+    /* full initialization is wasteful, but who cares... */
+    status = _cairo_scaled_font_init (placeholder_scaled_font,
+				      scaled_font->font_face,
+				      &scaled_font->font_matrix,
+				      &scaled_font->ctm,
+				      &scaled_font->options,
+				      NULL);
+    if (status)
+	goto FINI;
 
-	status = _cairo_hash_table_insert (cairo_scaled_font_map->hash_table,
-					   &key->hash_entry);
-	if (status)
-	    goto UNLOCK_KEY;
+    placeholder_scaled_font->placeholder = TRUE;
 
-	goto UNLOCK;
+    CAIRO_MUTEX_LOCK (placeholder_scaled_font->mutex);
 
-       UNLOCK_KEY:
-	CAIRO_MUTEX_UNLOCK (key->mutex);
+    status = _cairo_hash_table_insert (cairo_scaled_font_map->hash_table,
+				       &placeholder_scaled_font->hash_entry);
+    if (status)
+	goto UNLOCK_KEY;
 
-       FINI:
-	_cairo_scaled_font_fini (key);
+    goto UNLOCK;
 
-       FREE:
-	free (key);
+   UNLOCK_KEY:
+    CAIRO_MUTEX_UNLOCK (placeholder_scaled_font->mutex);
 
-	status = _cairo_scaled_font_set_error (scaled_font, status);
-    }
+   FINI:
+    _cairo_scaled_font_fini (placeholder_scaled_font);
+
+   FREE:
+    free (placeholder_scaled_font);
+
+    status = _cairo_scaled_font_set_error (scaled_font, status);
 
  UNLOCK:
     CAIRO_MUTEX_UNLOCK (_cairo_scaled_font_map_mutex);
 }
 
 void
-_cairo_scaled_font_lock_font_map (cairo_scaled_font_t *scaled_font)
+_cairo_scaled_font_unregister_placeholder_and_lock_font_map (cairo_scaled_font_t *scaled_font)
 {
-    CAIRO_MUTEX_LOCK (_cairo_scaled_font_map_mutex);
+    cairo_scaled_font_t *placeholder_scaled_font;
+    cairo_bool_t found;
 
-    if (!scaled_font->created) {
-	cairo_scaled_font_t *key;
-	cairo_bool_t found;
+    CAIRO_MUTEX_LOCK (_cairo_scaled_font_map_mutex);
 
-	found = _cairo_hash_table_lookup (cairo_scaled_font_map->hash_table,
-					  &scaled_font->hash_entry,
-					  (cairo_hash_entry_t**) &key);
-	assert (found);
+    found = _cairo_hash_table_lookup (cairo_scaled_font_map->hash_table,
+				      &scaled_font->hash_entry,
+				      (cairo_hash_entry_t**) &placeholder_scaled_font);
+    assert (found);
+    assert (placeholder_scaled_font->placeholder);
 
-	_cairo_hash_table_remove (cairo_scaled_font_map->hash_table,
-				  &scaled_font->hash_entry);
+    _cairo_hash_table_remove (cairo_scaled_font_map->hash_table,
+			      &scaled_font->hash_entry);
 
-	CAIRO_MUTEX_UNLOCK (scaled_font->mutex);
+    CAIRO_MUTEX_UNLOCK (scaled_font->mutex);
 
-	CAIRO_MUTEX_UNLOCK (_cairo_scaled_font_map_mutex);
-	cairo_scaled_font_destroy (key);
-	CAIRO_MUTEX_LOCK (_cairo_scaled_font_map_mutex);
-    }
+    CAIRO_MUTEX_UNLOCK (_cairo_scaled_font_map_mutex);
+    cairo_scaled_font_destroy (placeholder_scaled_font);
+    CAIRO_MUTEX_LOCK (_cairo_scaled_font_map_mutex);
 }
 
+static void
+_cairo_scaled_font_placeholder_wait_for_creation_to_finish (cairo_scaled_font_t *scaled_font)
+{
+    /* reference the place holder so it doesn't go away */
+    cairo_scaled_font_reference (scaled_font);
+
+    /* now unlock the fontmap mutex so creation has a chance to finish */
+    CAIRO_MUTEX_UNLOCK (_cairo_scaled_font_map_mutex);
+
+    /* wait on placeholde mutex until we are awaken */
+    CAIRO_MUTEX_LOCK (scaled_font->mutex);
+
+    /* ok, creation done.  just clean up and back out */
+    CAIRO_MUTEX_UNLOCK (scaled_font->mutex);
+    CAIRO_MUTEX_LOCK (_cairo_scaled_font_map_mutex);
+    cairo_scaled_font_destroy (scaled_font);
+}
 
 /* Fowler / Noll / Vo (FNV) Hash (http://www.isthe.com/chongo/tech/comp/fnv/)
  *
@@ -509,7 +526,7 @@ _cairo_scaled_font_init_key (cairo_scaled_font_t        *scaled_font,
     uint32_t hash = FNV1_32_INIT;
 
     scaled_font->status = CAIRO_STATUS_SUCCESS;
-    scaled_font->created = FALSE;
+    scaled_font->placeholder = FALSE;
     scaled_font->font_face = font_face;
     scaled_font->font_matrix = *font_matrix;
     scaled_font->ctm = *ctm;
@@ -751,17 +768,12 @@ cairo_scaled_font_create (cairo_font_face_t          *font_face,
     while (_cairo_hash_table_lookup (font_map->hash_table, &key.hash_entry,
 				     (cairo_hash_entry_t**) &scaled_font))
     {
-	if (scaled_font->created)
+	if (!scaled_font->placeholder)
 	    break;
 
 	/* If the scaled font is being created (happens for user-font),
 	 * just wait until it's done, then retry */
-	cairo_scaled_font_reference (scaled_font);
-	CAIRO_MUTEX_UNLOCK (_cairo_scaled_font_map_mutex);
-	CAIRO_MUTEX_LOCK (scaled_font->mutex);
-	CAIRO_MUTEX_UNLOCK (scaled_font->mutex);
-	CAIRO_MUTEX_LOCK (_cairo_scaled_font_map_mutex);
-	cairo_scaled_font_destroy (scaled_font);
+	_cairo_scaled_font_placeholder_wait_for_creation_to_finish (scaled_font);
     }
 
     /* Return existing scaled_font if it exists in the hash table. */
@@ -812,7 +824,6 @@ cairo_scaled_font_create (cairo_font_face_t          *font_face,
 	return _cairo_scaled_font_create_in_error (status);
     }
 
-    scaled_font->created = TRUE;
     status = _cairo_hash_table_insert (font_map->hash_table,
 				       &scaled_font->hash_entry);
     _cairo_scaled_font_map_unlock ();
@@ -937,7 +948,7 @@ cairo_scaled_font_destroy (cairo_scaled_font_t *scaled_font)
 	CAIRO_MUTEX_LOCK (_cairo_scaled_font_map_mutex);
 	font_map = cairo_scaled_font_map;
 
-	if (scaled_font->created && font_map && scaled_font->hash_entry.hash != ZOMBIE) {
+	if (!scaled_font->placeholder && font_map && scaled_font->hash_entry.hash != ZOMBIE) {
 	    /* Rather than immediately destroying this object, we put it into
 	     * the font_map->holdovers array in case it will get used again
 	     * soon (and is why we must hold the lock over the atomic op on
diff --git a/src/cairo-user-font.c b/src/cairo-user-font.c
index e9b1d7c..75f0a75 100644
--- a/src/cairo-user-font.c
+++ b/src/cairo-user-font.c
@@ -333,12 +333,12 @@ _cairo_user_font_face_scaled_font_create (void                        *abstract_
 	CAIRO_MUTEX_LOCK (user_scaled_font->base.mutex);
 
 	/* Give away fontmap lock such that user-font can use other fonts */
-	_cairo_scaled_font_unlock_font_map (&user_scaled_font->base);
+	_cairo_scaled_font_register_placeholder_and_unlock_font_map (&user_scaled_font->base);
 
 	status = font_face->scaled_font_methods.init (&user_scaled_font->base,
 						      &font_extents);
 
-	_cairo_scaled_font_lock_font_map (&user_scaled_font->base);
+	_cairo_scaled_font_unregister_placeholder_and_lock_font_map (&user_scaled_font->base);
 
 	CAIRO_MUTEX_UNLOCK (user_scaled_font->base.mutex);
     }
diff --git a/src/cairoint.h b/src/cairoint.h
index cc97cb2..bb60151 100755
--- a/src/cairoint.h
+++ b/src/cairoint.h
@@ -1472,10 +1472,10 @@ cairo_private void
 _cairo_scaled_font_reset_static_data (void);
 
 cairo_private void
-_cairo_scaled_font_unlock_font_map (cairo_scaled_font_t *scaled_font);
+_cairo_scaled_font_register_placeholder_and_unlock_font_map (cairo_scaled_font_t *scaled_font);
 
 cairo_private void
-_cairo_scaled_font_lock_font_map (cairo_scaled_font_t *scaled_font);
+_cairo_scaled_font_unregister_placeholder_and_lock_font_map (cairo_scaled_font_t *scaled_font);
 
 cairo_private cairo_status_t
 _cairo_scaled_font_init (cairo_scaled_font_t               *scaled_font,


More information about the cairo-commit mailing list