[cairo-commit] 6 commits - src/cairo-font.c src/cairo-ft-font.c
test/pthread-show-text.c
Carl Worth
cworth at kemper.freedesktop.org
Tue Feb 13 15:59:16 PST 2007
src/cairo-font.c | 40 +++++++++++++++-----
src/cairo-ft-font.c | 90 ++++++++++++++++++++++++++++++-----------------
test/pthread-show-text.c | 5 ++
3 files changed, 92 insertions(+), 43 deletions(-)
New commits:
diff-tree aad1374caab3a05893c9f1f2605131edd9f62409 (from e107b70b4d282d78ebe32f4d2381fcf98df324f6)
Author: Carl Worth <cworth at cworth.org>
Date: Sat Feb 10 09:39:24 2007 -0800
Rename cairo_toy_font_face_hash_table_mutex to cairo_font_face_mutex
The new name more accurately reflects its recently expanded role.
diff --git a/src/cairo-font.c b/src/cairo-font.c
index cf32cf1..a310542 100644
--- a/src/cairo-font.c
+++ b/src/cairo-font.c
@@ -65,7 +65,9 @@ _cairo_font_face_init (cairo_font_face_t
_cairo_user_data_array_init (&font_face->user_data);
}
-CAIRO_MUTEX_DECLARE (cairo_toy_font_face_hash_table_mutex);
+/* This mutex protects both cairo_toy_font_hash_table as well as
+ reference count manipulations for all cairo_font_face_t. */
+CAIRO_MUTEX_DECLARE (cairo_font_face_mutex);
/**
* cairo_font_face_reference:
@@ -87,7 +89,7 @@ cairo_font_face_reference (cairo_font_fa
if (font_face->ref_count == CAIRO_REF_COUNT_INVALID)
return font_face;
- CAIRO_MUTEX_LOCK (cairo_toy_font_face_hash_table_mutex);
+ CAIRO_MUTEX_LOCK (cairo_font_face_mutex);
/* We would normally assert (font_face->ref_count >0) here but we
* can't get away with that due to the zombie case as documented
@@ -95,7 +97,7 @@ cairo_font_face_reference (cairo_font_fa
font_face->ref_count++;
- CAIRO_MUTEX_UNLOCK (cairo_toy_font_face_hash_table_mutex);
+ CAIRO_MUTEX_UNLOCK (cairo_font_face_mutex);
return font_face;
}
@@ -118,16 +120,16 @@ cairo_font_face_destroy (cairo_font_face
if (font_face->ref_count == CAIRO_REF_COUNT_INVALID)
return;
- CAIRO_MUTEX_LOCK (cairo_toy_font_face_hash_table_mutex);
+ CAIRO_MUTEX_LOCK (cairo_font_face_mutex);
assert (font_face->ref_count > 0);
if (--(font_face->ref_count) > 0) {
- CAIRO_MUTEX_UNLOCK (cairo_toy_font_face_hash_table_mutex);
+ CAIRO_MUTEX_UNLOCK (cairo_font_face_mutex);
return;
}
- CAIRO_MUTEX_UNLOCK (cairo_toy_font_face_hash_table_mutex);
+ CAIRO_MUTEX_UNLOCK (cairo_font_face_mutex);
font_face->backend->destroy (font_face);
@@ -238,14 +240,16 @@ _cairo_toy_font_face_keys_equal (const v
* our cache and mapping from cairo_font_face_t => cairo_scaled_font_t
* works. Once the corresponding cairo_font_face_t objects fall out of
* downstream caches, we don't need them in this hash table anymore.
+ *
+ * Modifications to this hash table are protected by
+ * cairo_font_face_mutex.
*/
-
static cairo_hash_table_t *cairo_toy_font_face_hash_table = NULL;
static cairo_hash_table_t *
_cairo_toy_font_face_hash_table_lock (void)
{
- CAIRO_MUTEX_LOCK (cairo_toy_font_face_hash_table_mutex);
+ CAIRO_MUTEX_LOCK (cairo_font_face_mutex);
if (cairo_toy_font_face_hash_table == NULL)
{
@@ -253,7 +257,7 @@ _cairo_toy_font_face_hash_table_lock (vo
_cairo_hash_table_create (_cairo_toy_font_face_keys_equal);
if (cairo_toy_font_face_hash_table == NULL) {
- CAIRO_MUTEX_UNLOCK (cairo_toy_font_face_hash_table_mutex);
+ CAIRO_MUTEX_UNLOCK (cairo_font_face_mutex);
return NULL;
}
}
@@ -264,7 +268,7 @@ _cairo_toy_font_face_hash_table_lock (vo
static void
_cairo_toy_font_face_hash_table_unlock (void)
{
- CAIRO_MUTEX_UNLOCK (cairo_toy_font_face_hash_table_mutex);
+ CAIRO_MUTEX_UNLOCK (cairo_font_face_mutex);
}
/**
@@ -484,8 +488,11 @@ _cairo_font_reset_static_data (void)
{
_cairo_scaled_font_map_destroy ();
- CAIRO_MUTEX_LOCK (cairo_toy_font_face_hash_table_mutex);
+ /* We manually acquire the lock rather than calling
+ * cairo_toy_font_face_hash_table_lock simply to avoid
+ * creating the table only to destroy it again. */
+ CAIRO_MUTEX_LOCK (cairo_font_face_mutex);
_cairo_hash_table_destroy (cairo_toy_font_face_hash_table);
cairo_toy_font_face_hash_table = NULL;
- CAIRO_MUTEX_UNLOCK (cairo_toy_font_face_hash_table_mutex);
+ CAIRO_MUTEX_UNLOCK (cairo_font_face_mutex);
}
diff-tree e107b70b4d282d78ebe32f4d2381fcf98df324f6 (from d6d1767f9a89bf7e16288d0a90e5fafc5ad2d9d6)
Author: Carl Worth <cworth at cworth.org>
Date: Sat Feb 10 09:39:09 2007 -0800
Add locking to cairo_font_face_reference/destroy
The reference count of cairo_font_face_t is rather intimately tied,
(for toy font faces), with the cairo_font_face_hash_table, so we
expand the existing cairo_toy_font_face_hash_table_mutex to cover
the manipulation of font_face->ref_count as well.
This commit eliminates an assertion failure that is (occasionally)
exposed by the pthread-show-text test case:
lt-pthread-show-text: cairo-hash.c:196: _cairo_hash_table_destroy: Assertion `hash_table->live_entries == 0' failed.
diff --git a/src/cairo-font.c b/src/cairo-font.c
index 3730686..cf32cf1 100644
--- a/src/cairo-font.c
+++ b/src/cairo-font.c
@@ -65,6 +65,8 @@ _cairo_font_face_init (cairo_font_face_t
_cairo_user_data_array_init (&font_face->user_data);
}
+CAIRO_MUTEX_DECLARE (cairo_toy_font_face_hash_table_mutex);
+
/**
* cairo_font_face_reference:
* @font_face: a #cairo_font_face_t, (may be NULL in which case this
@@ -85,12 +87,16 @@ cairo_font_face_reference (cairo_font_fa
if (font_face->ref_count == CAIRO_REF_COUNT_INVALID)
return font_face;
+ CAIRO_MUTEX_LOCK (cairo_toy_font_face_hash_table_mutex);
+
/* We would normally assert (font_face->ref_count >0) here but we
* can't get away with that due to the zombie case as documented
* in _cairo_ft_font_face_destroy. */
font_face->ref_count++;
+ CAIRO_MUTEX_UNLOCK (cairo_toy_font_face_hash_table_mutex);
+
return font_face;
}
slim_hidden_def (cairo_font_face_reference);
@@ -112,10 +118,16 @@ cairo_font_face_destroy (cairo_font_face
if (font_face->ref_count == CAIRO_REF_COUNT_INVALID)
return;
+ CAIRO_MUTEX_LOCK (cairo_toy_font_face_hash_table_mutex);
+
assert (font_face->ref_count > 0);
- if (--(font_face->ref_count) > 0)
+ if (--(font_face->ref_count) > 0) {
+ CAIRO_MUTEX_UNLOCK (cairo_toy_font_face_hash_table_mutex);
return;
+ }
+
+ CAIRO_MUTEX_UNLOCK (cairo_toy_font_face_hash_table_mutex);
font_face->backend->destroy (font_face);
@@ -230,8 +242,6 @@ _cairo_toy_font_face_keys_equal (const v
static cairo_hash_table_t *cairo_toy_font_face_hash_table = NULL;
-CAIRO_MUTEX_DECLARE (cairo_toy_font_face_hash_table_mutex);
-
static cairo_hash_table_t *
_cairo_toy_font_face_hash_table_lock (void)
{
@@ -363,8 +373,11 @@ _cairo_toy_font_face_create (const char
&key.base.hash_entry,
(cairo_hash_entry_t **) &font_face))
{
+ /* We increment the reference count here manually to avoid
+ double-locking. */
+ font_face->base.ref_count++;
_cairo_toy_font_face_hash_table_unlock ();
- return cairo_font_face_reference (&font_face->base);
+ return &font_face->base;
}
/* Otherwise create it and insert into hash table. */
diff-tree d6d1767f9a89bf7e16288d0a90e5fafc5ad2d9d6 (from fdffde8b9e7a2308b822ddef7b02a8e85cc8ba1e)
Author: Carl Worth <cworth at cworth.org>
Date: Sat Feb 10 09:38:13 2007 -0800
Increase pthread-show-text thread count and add cairo_select_font_face to expose more bugs.
diff --git a/test/pthread-show-text.c b/test/pthread-show-text.c
index 94724c7..b674e37 100644
--- a/test/pthread-show-text.c
+++ b/test/pthread-show-text.c
@@ -39,7 +39,7 @@
#include <fontconfig/fontconfig.h>
#endif
-#define NUM_THREADS_DEFAULT 20
+#define NUM_THREADS_DEFAULT 50
#define NUM_ITERATIONS 50
static void *
@@ -62,6 +62,9 @@ start (void *closure)
cairo_move_to (cr, 1, 1);
for (i=0; i < NUM_ITERATIONS; i++) {
+ cairo_select_font_face (cr, "serif",
+ CAIRO_FONT_SLANT_NORMAL,
+ CAIRO_FONT_WEIGHT_NORMAL);
cairo_set_font_size (cr, 8 + i);
cairo_show_text (cr, "Hello world.\n");
}
diff-tree fdffde8b9e7a2308b822ddef7b02a8e85cc8ba1e (from 25a370d7992d7f70d8cf79f1b328e40437b40ac4)
Author: Carl Worth <cworth at cworth.org>
Date: Sat Feb 10 09:14:07 2007 -0800
Add mutex to implement _cairo_ft_unscaled_font_lock_face and _cairo_ft_unscaled_font_unlock_face
Previously we just had an integer counter here, but that is not
sufficient as multiple cairo_scaled_font_t objects, (which are
implicitly shared through the font caches), can reference the
same cairo_ft_unscaled_font_t so real locking is needed here.
This commit eliminates an assertion failure exposed by the
pthread-show-text test case:
lt-pthread-show-text: cairo-ft-font.c:562: _cairo_ft_unscaled_font_unlock_face: Assertion `unscaled->lock > 0' failed.
diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c
index 6f648a4..d6ee6de 100644
--- a/src/cairo-ft-font.c
+++ b/src/cairo-ft-font.c
@@ -100,7 +100,8 @@ struct _cairo_ft_unscaled_font {
cairo_matrix_t current_shape;
FT_Matrix Current_Shape;
- int lock; /* count of how many times this font has been locked */
+ cairo_mutex_t mutex;
+ cairo_bool_t is_locked;
cairo_ft_font_face_t *faces; /* Linked list of faces for this font */
};
@@ -333,7 +334,8 @@ _cairo_ft_unscaled_font_init (cairo_ft_u
}
unscaled->have_scale = FALSE;
- unscaled->lock = 0;
+ CAIRO_MUTEX_INIT (&unscaled->mutex);
+ unscaled->is_locked = FALSE;
unscaled->faces = NULL;
@@ -366,6 +368,8 @@ _cairo_ft_unscaled_font_fini (cairo_ft_u
free (unscaled->filename);
unscaled->filename = NULL;
}
+
+ CAIRO_MUTEX_FINI (&unscaled->mutex);
}
static int
@@ -497,7 +501,7 @@ _has_unlocked_face (void *entry)
{
cairo_ft_unscaled_font_t *unscaled = entry;
- return (unscaled->lock == 0 && unscaled->face);
+ return (! unscaled->is_locked && unscaled->face);
}
/* Ensures that an unscaled font has a face object. If we exceed
@@ -512,44 +516,47 @@ _cairo_ft_unscaled_font_lock_face (cairo
cairo_ft_unscaled_font_map_t *font_map;
FT_Face face = NULL;
- if (unscaled->face) {
- unscaled->lock++;
+ CAIRO_MUTEX_LOCK (unscaled->mutex);
+ unscaled->is_locked = TRUE;
+
+ if (unscaled->face)
return unscaled->face;
- }
/* If this unscaled font was created from an FT_Face then we just
* returned it above. */
assert (!unscaled->from_face);
font_map = _cairo_ft_unscaled_font_map_lock ();
- assert (font_map != NULL);
-
- while (font_map->num_open_faces >= MAX_OPEN_FACES)
{
- cairo_ft_unscaled_font_t *entry;
+ assert (font_map != NULL);
- entry = _cairo_hash_table_random_entry (font_map->hash_table,
- _has_unlocked_face);
- if (entry == NULL)
- break;
+ while (font_map->num_open_faces >= MAX_OPEN_FACES)
+ {
+ cairo_ft_unscaled_font_t *entry;
+
+ entry = _cairo_hash_table_random_entry (font_map->hash_table,
+ _has_unlocked_face);
+ if (entry == NULL)
+ break;
- _font_map_release_face_lock_held (font_map, entry);
+ _font_map_release_face_lock_held (font_map, entry);
+ }
}
+ _cairo_ft_unscaled_font_map_unlock ();
if (FT_New_Face (font_map->ft_library,
unscaled->filename,
unscaled->id,
&face) != FT_Err_Ok)
- goto FAIL;
+ {
+ CAIRO_MUTEX_UNLOCK (unscaled->mutex);
+ return NULL;
+ }
unscaled->face = face;
- unscaled->lock++;
font_map->num_open_faces++;
- FAIL:
- _cairo_ft_unscaled_font_map_unlock ();
-
return face;
}
slim_hidden_def (cairo_ft_scaled_font_lock_face);
@@ -559,9 +566,11 @@ slim_hidden_def (cairo_ft_scaled_font_lo
void
_cairo_ft_unscaled_font_unlock_face (cairo_ft_unscaled_font_t *unscaled)
{
- assert (unscaled->lock > 0);
+ assert (unscaled->is_locked);
+
+ unscaled->is_locked = FALSE;
- unscaled->lock--;
+ CAIRO_MUTEX_UNLOCK (unscaled->mutex);
}
slim_hidden_def (cairo_ft_scaled_font_unlock_face);
@@ -2414,13 +2423,14 @@ cairo_ft_font_face_create_for_ft_face (F
* of times.
*
* You must be careful when using this function in a library or in a
- * threaded application, because other threads may lock faces that
- * share the same #FT_Face object. For this reason, you must call
- * cairo_ft_lock() before locking any face objects, and
- * cairo_ft_unlock() after you are done. (These functions are not yet
- * implemented, so this function cannot be currently safely used in a
- * threaded application.)
-
+ * threaded application, because freetype's design makes it unsafe to
+ * call freetype functions simultaneously from multiple threads, (even
+ * if using distinct FT_Face objects). Because of this, application
+ * code that acquires an FT_Face object with this call must add it's
+ * own locking to protect any use of that object, (and which also must
+ * protect any other calls into cairo as almost any cairo function
+ * might result in a call into the freetype library).
+ *
* Return value: The #FT_Face object for @font, scaled appropriately,
* or %NULL if @scaled_font is in an error state (see
* cairo_scaled_font_status()) or there is insufficient memory.
@@ -2443,6 +2453,14 @@ cairo_ft_scaled_font_lock_face (cairo_sc
_cairo_ft_unscaled_font_set_scale (scaled_font->unscaled,
&scaled_font->base.scale);
+ /* NOTE: We deliberately release the unscaled font's mutex here,
+ * so that we are not holding a lock across two separate calls to
+ * cairo function, (which would give the application some
+ * opportunity for creating deadlock. This is obviously unsafe,
+ * but as documented, the user must add manual locking when using
+ * this function. */
+ CAIRO_MUTEX_UNLOCK (scaled_font->unscaled->mutex);
+
return face;
}
@@ -2463,6 +2481,12 @@ cairo_ft_scaled_font_unlock_face (cairo_
if (scaled_font->base.status)
return;
+ /* NOTE: We released the unscaled font's mutex at the end of
+ * cairo_ft_scaled_font_lock_face, so we have to acquire it again
+ * as _cairo_ft_unscaled_font_unlock_face expects it to be held
+ * when we call into it. */
+ CAIRO_MUTEX_UNLOCK (scaled_font->unscaled->mutex);
+
_cairo_ft_unscaled_font_unlock_face (scaled_font->unscaled);
}
diff-tree 25a370d7992d7f70d8cf79f1b328e40437b40ac4 (from 7e1301ffb066b04d95dc71144d86e660f0155bba)
Author: Carl Worth <cworth at cworth.org>
Date: Tue Feb 13 10:40:53 2007 -0800
Avoid public cairo_ft_scaled_font_lock_face for internal use
We're planning to change the implementation of the public function,
(which will remove some locking safety), so use the safe, locked
_cairo_ft_unscaled_font_lock_face for internal use instead.
diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c
index 0d10dce..6f648a4 100644
--- a/src/cairo-ft-font.c
+++ b/src/cairo-ft-font.c
@@ -1769,7 +1769,7 @@ _cairo_ft_scaled_glyph_init (void *abs
cairo_bool_t vertical_layout = FALSE;
cairo_status_t status = CAIRO_STATUS_SUCCESS;
- face = cairo_ft_scaled_font_lock_face (abstract_font);
+ face = _cairo_ft_unscaled_font_lock_face (unscaled);
if (!face)
return CAIRO_STATUS_NO_MEMORY;
@@ -1935,7 +1935,7 @@ _cairo_ft_scaled_glyph_init (void *abs
load_flags | FT_LOAD_NO_BITMAP);
if (error) {
- cairo_ft_scaled_font_unlock_face (abstract_font);
+ _cairo_ft_unscaled_font_unlock_face (unscaled);
return CAIRO_STATUS_NO_MEMORY;
}
#if HAVE_FT_GLYPHSLOT_EMBOLDEN
@@ -1963,7 +1963,7 @@ _cairo_ft_scaled_glyph_init (void *abs
path);
}
FAIL:
- cairo_ft_scaled_font_unlock_face (abstract_font);
+ _cairo_ft_unscaled_font_unlock_face (unscaled);
return status;
}
diff-tree 7e1301ffb066b04d95dc71144d86e660f0155bba (from 6f7cfdf5c79c3c09a0241ae25ff540fb0d893d31)
Author: Carl Worth <cworth at cworth.org>
Date: Sat Feb 10 09:12:38 2007 -0800
Add missing _cairo_ft_unscaled_font_unlock_face to _cairo_ft_scaled_font_create
diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c
index 4d66274..0d10dce 100644
--- a/src/cairo-ft-font.c
+++ b/src/cairo-ft-font.c
@@ -1488,6 +1488,8 @@ _cairo_ft_scaled_font_create (cairo_ft_u
_cairo_scaled_font_set_metrics (&scaled_font->base, &fs_metrics);
+ _cairo_ft_unscaled_font_unlock_face (unscaled);
+
return &scaled_font->base;
}
More information about the cairo-commit
mailing list