diff --git a/src/cairo-scaled-font-private.h b/src/cairo-scaled-font-private.h index 86fbec614..6571d0608 100644 --- a/src/cairo-scaled-font-private.h +++ b/src/cairo-scaled-font-private.h @@ -89,6 +89,8 @@ 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 finished; /* "live" scaled_font members */ diff --git a/src/cairo-scaled-font.c b/src/cairo-scaled-font.c index 01bf169b2..6521fe88e 100644 --- a/src/cairo-scaled-font.c +++ b/src/cairo-scaled-font.c @@ -192,6 +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 */ TRUE, /* finished */ { 1., 0., 0., 1., 0, 0}, /* scale */ { 1., 0., 0., 1., 0, 0}, /* scale_inverse */ @@ -386,6 +387,96 @@ _cairo_scaled_font_map_destroy (void) CAIRO_MUTEX_UNLOCK (_cairo_scaled_font_map_mutex); } + +/* If a scaled font wants to unlock the font map while still being + * 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 + * 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 + * 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. + */ + +void +_cairo_scaled_font_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; + } + + /* 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; + + CAIRO_MUTEX_LOCK (key->mutex); + + status = _cairo_hash_table_insert (cairo_scaled_font_map->hash_table, + &key->hash_entry); + if (status) + goto UNLOCK_KEY; + + goto UNLOCK; + + UNLOCK_KEY: + CAIRO_MUTEX_UNLOCK (key->mutex); + + FINI: + _cairo_scaled_font_fini (key); + + FREE: + free (key); + + 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_MUTEX_LOCK (_cairo_scaled_font_map_mutex); + + if (!scaled_font->created) { + cairo_scaled_font_t *key; + cairo_bool_t found; + + found = _cairo_hash_table_lookup (cairo_scaled_font_map->hash_table, + &scaled_font->hash_entry, + (cairo_hash_entry_t**) &key); + assert (found); + + _cairo_hash_table_remove (cairo_scaled_font_map->hash_table, + &scaled_font->hash_entry); + + 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); + } +} + + /* Fowler / Noll / Vo (FNV) Hash (http://www.isthe.com/chongo/tech/comp/fnv/) * * Not necessarily better than a lot of other hashes, but should be OK, and @@ -418,6 +509,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->font_face = font_face; scaled_font->font_matrix = *font_matrix; scaled_font->ctm = *ctm; @@ -601,7 +693,7 @@ _cairo_scaled_font_fini (cairo_scaled_font_t *scaled_font) scaled_font->surface_backend->scaled_font_fini != NULL) scaled_font->surface_backend->scaled_font_fini (scaled_font); - if (scaled_font->backend->fini != NULL) + if (scaled_font->backend != NULL && scaled_font->backend->fini != NULL) scaled_font->backend->fini (scaled_font); _cairo_user_data_array_fini (&scaled_font->user_data); @@ -655,9 +747,25 @@ cairo_scaled_font_create (cairo_font_face_t *font_face, _cairo_scaled_font_init_key (&key, font_face, font_matrix, ctm, options); + + while (_cairo_hash_table_lookup (font_map->hash_table, &key.hash_entry, + (cairo_hash_entry_t**) &scaled_font)) + { + if (scaled_font->created) + 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); + } + /* Return existing scaled_font if it exists in the hash table. */ - if (_cairo_hash_table_lookup (font_map->hash_table, &key.hash_entry, - (cairo_hash_entry_t**) &scaled_font)) + if (scaled_font) { /* If the original reference count is 0, then this font must have * been found in font_map->holdovers, (which means this caching is @@ -704,6 +812,7 @@ 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 (); @@ -828,7 +937,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 (font_map && scaled_font->hash_entry.hash != ZOMBIE) { + if (scaled_font->created && 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 b33bf1238..e9b1d7c22 100644 --- a/src/cairo-user-font.c +++ b/src/cairo-user-font.c @@ -327,21 +327,19 @@ _cairo_user_font_face_scaled_font_create (void *abstract_ } if (font_face->scaled_font_methods.init != NULL) { - /* XXX by releasing fontmap lock here, we open up the case for having - * two scaled fonts with the same key to be inserted in the fontmap. - * - * To fix that, we should make _cairo_scaled_font_init() insert - * scaled_font in the fontmap, then lock the scaled_font lock here and - * release the fontmap's. - */ + /* Lock the scaled_font mutex such that user doesn't accidentally try - * to use it just yet. - */ + * to use it just yet. */ CAIRO_MUTEX_LOCK (user_scaled_font->base.mutex); - CAIRO_MUTEX_UNLOCK (_cairo_scaled_font_map_mutex); + + /* Give away fontmap lock such that user-font can use other fonts */ + _cairo_scaled_font_unlock_font_map (&user_scaled_font->base); + status = font_face->scaled_font_methods.init (&user_scaled_font->base, &font_extents); - CAIRO_MUTEX_LOCK (_cairo_scaled_font_map_mutex); + + _cairo_scaled_font_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 54cd0d8a3..cc97cb23f 100755 --- a/src/cairoint.h +++ b/src/cairoint.h @@ -1471,6 +1471,11 @@ _cairo_scaled_font_create_in_error (cairo_status_t status); 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_private void +_cairo_scaled_font_lock_font_map (cairo_scaled_font_t *scaled_font); cairo_private cairo_status_t _cairo_scaled_font_init (cairo_scaled_font_t *scaled_font,