From fdffde8b9e7a2308b822ddef7b02a8e85cc8ba1e Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Sat, 10 Feb 2007 09:14:07 -0800 Subject: [PATCH] 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. --- src/cairo-ft-font.c | 82 +++++++++++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 29 deletions(-) diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c index 6f648a4cd..d6ee6de98 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_unscaled_font_t *unscaled, } 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_unscaled_font_t *unscaled) 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_ft_unscaled_font_t *unscaled) 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; - _font_map_release_face_lock_held (font_map, 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); + } } + _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_lock_face); void _cairo_ft_unscaled_font_unlock_face (cairo_ft_unscaled_font_t *unscaled) { - assert (unscaled->lock > 0); + assert (unscaled->is_locked); - unscaled->lock--; + unscaled->is_locked = FALSE; + + 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 (FT_Face face, * 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_scaled_font_t *abstract_font) _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_scaled_font_t *abstract_font) 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); }