[cairo-commit] src/cairo-font-face.c src/cairo-ft-font.c src/cairoint.h src/cairo-quartz-font.c src/cairo-toy-font-face.c src/cairo-user-font.c src/win32
Chris Wilson
ickle at kemper.freedesktop.org
Tue Sep 17 08:52:42 PDT 2013
src/cairo-font-face.c | 43 +++++++++++++++++++++++++++++++------------
src/cairo-ft-font.c | 18 ++++++++----------
src/cairo-quartz-font.c | 3 ++-
src/cairo-toy-font-face.c | 7 ++++---
src/cairo-user-font.c | 2 +-
src/cairoint.h | 7 +++++--
src/win32/cairo-win32-font.c | 7 ++++---
7 files changed, 55 insertions(+), 32 deletions(-)
New commits:
commit 337ab1f8d9e29086bfb4001508b28835b41c6390
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date: Tue Sep 17 16:28:19 2013 +0100
font: Push the last reference dec into the backend->destroy() callback
In order to close a race between locking the backend and resurrecting a
font via the cache, we need to keep the font face alive until after we
take the backend lock. Once we have that lock, we can drop our reference
and test if that was the last. Otherwise we must abort the destroy().
This fixes the double-free exposed by multithreaded applications trying
to create and destroy the same font concurrently.
Reported-by: Weeble <clockworksaint at gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=69470
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
diff --git a/src/cairo-font-face.c b/src/cairo-font-face.c
index b93bd8c..e32a9bb 100644
--- a/src/cairo-font-face.c
+++ b/src/cairo-font-face.c
@@ -115,7 +115,7 @@ cairo_font_face_t *
cairo_font_face_reference (cairo_font_face_t *font_face)
{
if (font_face == NULL ||
- CAIRO_REFERENCE_COUNT_IS_INVALID (&font_face->ref_count))
+ CAIRO_REFERENCE_COUNT_IS_INVALID (&font_face->ref_count))
return font_face;
/* We would normally assert that we have a reference here but we
@@ -128,6 +128,27 @@ cairo_font_face_reference (cairo_font_face_t *font_face)
}
slim_hidden_def (cairo_font_face_reference);
+static inline int __put(cairo_reference_count_t *v)
+{
+ int c, old;
+
+ c = CAIRO_REFERENCE_COUNT_GET_VALUE(v);
+ while (c != 1 && (old = _cairo_atomic_int_cmpxchg_return_old(&v->ref_count, c, c - 1)) != c)
+ c = old;
+
+ return c;
+}
+
+cairo_bool_t
+_cairo_font_face_destroy (void *abstract_face)
+{
+#if 0 /* Nothing needs to be done, we can just drop the last reference */
+ cairo_font_face_t *font_face = abstract_face;
+ return _cairo_reference_count_dec_and_test (&font_face->ref_count);
+#endif
+ return TRUE;
+}
+
/**
* cairo_font_face_destroy:
* @font_face: a #cairo_font_face_t
@@ -142,22 +163,19 @@ void
cairo_font_face_destroy (cairo_font_face_t *font_face)
{
if (font_face == NULL ||
- CAIRO_REFERENCE_COUNT_IS_INVALID (&font_face->ref_count))
+ CAIRO_REFERENCE_COUNT_IS_INVALID (&font_face->ref_count))
return;
assert (CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&font_face->ref_count));
- if (! _cairo_reference_count_dec_and_test (&font_face->ref_count))
- return;
-
- if (font_face->backend->destroy)
- font_face->backend->destroy (font_face);
-
/* We allow resurrection to deal with some memory management for the
* FreeType backend where cairo_ft_font_face_t and cairo_ft_unscaled_font_t
* need to effectively mutually reference each other
*/
- if (CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&font_face->ref_count))
+ if (__put (&font_face->ref_count))
+ return;
+
+ if (! font_face->backend->destroy (font_face))
return;
_cairo_user_data_array_fini (&font_face->user_data);
@@ -201,7 +219,7 @@ unsigned int
cairo_font_face_get_reference_count (cairo_font_face_t *font_face)
{
if (font_face == NULL ||
- CAIRO_REFERENCE_COUNT_IS_INVALID (&font_face->ref_count))
+ CAIRO_REFERENCE_COUNT_IS_INVALID (&font_face->ref_count))
return 0;
return CAIRO_REFERENCE_COUNT_GET_VALUE (&font_face->ref_count);
@@ -309,10 +327,11 @@ _cairo_unscaled_font_destroy (cairo_unscaled_font_t *unscaled_font)
assert (CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&unscaled_font->ref_count));
- if (! _cairo_reference_count_dec_and_test (&unscaled_font->ref_count))
+ if (__put (&unscaled_font->ref_count))
return;
- unscaled_font->backend->destroy (unscaled_font);
+ if (! unscaled_font->backend->destroy (unscaled_font))
+ return;
free (unscaled_font);
}
diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c
index 9f4b524..8c95863 100644
--- a/src/cairo-ft-font.c
+++ b/src/cairo-ft-font.c
@@ -588,23 +588,20 @@ _cairo_ft_unscaled_font_create_from_face (FT_Face face,
return _cairo_ft_unscaled_font_create_internal (TRUE, NULL, 0, face, out);
}
-static void
+static cairo_bool_t
_cairo_ft_unscaled_font_destroy (void *abstract_font)
{
cairo_ft_unscaled_font_t *unscaled = abstract_font;
cairo_ft_unscaled_font_map_t *font_map;
- if (unscaled == NULL)
- return;
-
font_map = _cairo_ft_unscaled_font_map_lock ();
/* All created objects must have been mapped in the font map. */
assert (font_map != NULL);
- if (CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&unscaled->base.ref_count)) {
+ if (! _cairo_reference_count_dec_and_test (&unscaled->base.ref_count)) {
/* somebody recreated the font whilst we waited for the lock */
_cairo_ft_unscaled_font_map_unlock ();
- return;
+ return FALSE;
}
_cairo_hash_table_remove (font_map->hash_table,
@@ -626,6 +623,7 @@ _cairo_ft_unscaled_font_destroy (void *abstract_font)
_cairo_ft_unscaled_font_map_unlock ();
_cairo_ft_unscaled_font_fini (unscaled);
+ return TRUE;
}
static cairo_bool_t
@@ -2771,7 +2769,7 @@ _cairo_ft_font_face_create_for_toy (cairo_toy_font_face_t *toy_face,
}
#endif
-static void
+static cairo_bool_t
_cairo_ft_font_face_destroy (void *abstract_face)
{
cairo_ft_font_face_t *font_face = abstract_face;
@@ -2797,12 +2795,10 @@ _cairo_ft_font_face_destroy (void *abstract_face)
font_face->unscaled->faces == font_face &&
CAIRO_REFERENCE_COUNT_GET_VALUE (&font_face->unscaled->base.ref_count) > 1)
{
- cairo_font_face_reference (&font_face->base);
-
_cairo_unscaled_font_destroy (&font_face->unscaled->base);
font_face->unscaled = NULL;
- return;
+ return FALSE;
}
if (font_face->unscaled) {
@@ -2834,6 +2830,8 @@ _cairo_ft_font_face_destroy (void *abstract_face)
cairo_font_face_destroy (font_face->resolved_font_face);
}
#endif
+
+ return TRUE;
}
static cairo_font_face_t *
diff --git a/src/cairo-quartz-font.c b/src/cairo-quartz-font.c
index a9bbbdc..e6a379a 100644
--- a/src/cairo-quartz-font.c
+++ b/src/cairo-quartz-font.c
@@ -241,12 +241,13 @@ _cairo_quartz_font_face_create_for_toy (cairo_toy_font_face_t *toy_face,
return CAIRO_STATUS_SUCCESS;
}
-static void
+static cairo_bool_t
_cairo_quartz_font_face_destroy (void *abstract_face)
{
cairo_quartz_font_face_t *font_face = (cairo_quartz_font_face_t*) abstract_face;
CGFontRelease (font_face->cgFont);
+ return TRUE;
}
static const cairo_scaled_font_backend_t _cairo_quartz_scaled_font_backend;
diff --git a/src/cairo-toy-font-face.c b/src/cairo-toy-font-face.c
index 363b9a2..4fe94ab 100644
--- a/src/cairo-toy-font-face.c
+++ b/src/cairo-toy-font-face.c
@@ -342,7 +342,7 @@ cairo_toy_font_face_create (const char *family,
}
slim_hidden_def (cairo_toy_font_face_create);
-static void
+static cairo_bool_t
_cairo_toy_font_face_destroy (void *abstract_face)
{
cairo_toy_font_face_t *font_face = abstract_face;
@@ -352,10 +352,10 @@ _cairo_toy_font_face_destroy (void *abstract_face)
/* All created objects must have been mapped in the hash table. */
assert (hash_table != NULL);
- if (CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&font_face->base.ref_count)) {
+ if (! _cairo_reference_count_dec_and_test (&font_face->base.ref_count)) {
/* somebody recreated the font whilst we waited for the lock */
_cairo_toy_font_face_hash_table_unlock ();
- return;
+ return FALSE;
}
/* Font faces in SUCCESS status are guaranteed to be in the
@@ -369,6 +369,7 @@ _cairo_toy_font_face_destroy (void *abstract_face)
_cairo_toy_font_face_hash_table_unlock ();
_cairo_toy_font_face_fini (font_face);
+ return TRUE;
}
static cairo_status_t
diff --git a/src/cairo-user-font.c b/src/cairo-user-font.c
index 297f21c..6d2de20 100644
--- a/src/cairo-user-font.c
+++ b/src/cairo-user-font.c
@@ -507,7 +507,7 @@ _cairo_user_font_face_scaled_font_create (void *abstract_
const cairo_font_face_backend_t _cairo_user_font_face_backend = {
CAIRO_FONT_TYPE_USER,
_cairo_user_font_face_create_for_toy,
- NULL, /* destroy */
+ _cairo_font_face_destroy,
_cairo_user_font_face_scaled_font_create
};
diff --git a/src/cairoint.h b/src/cairoint.h
index d31e07b..44e0ec5 100644
--- a/src/cairoint.h
+++ b/src/cairoint.h
@@ -424,7 +424,7 @@ _cairo_cogl_context_reset_static_data (void);
/* the font backend interface */
struct _cairo_unscaled_font_backend {
- void (*destroy) (void *unscaled_font);
+ cairo_bool_t (*destroy) (void *unscaled_font);
};
/* #cairo_toy_font_face_t - simple family/slant/weight font faces used for
@@ -583,7 +583,7 @@ struct _cairo_font_face_backend {
/* The destroy() function is allowed to resurrect the font face
* by re-referencing. This is needed for the FreeType backend.
*/
- void
+ cairo_bool_t
(*destroy) (void *font_face);
cairo_warn cairo_status_t
@@ -793,6 +793,9 @@ cairo_private void
_cairo_font_face_init (cairo_font_face_t *font_face,
const cairo_font_face_backend_t *backend);
+cairo_private cairo_bool_t
+_cairo_font_face_destroy (void *abstract_face);
+
cairo_private cairo_status_t
_cairo_font_face_set_error (cairo_font_face_t *font_face,
cairo_status_t status);
diff --git a/src/win32/cairo-win32-font.c b/src/win32/cairo-win32-font.c
index a65d81b..5ec2c2c 100644
--- a/src/win32/cairo-win32-font.c
+++ b/src/win32/cairo-win32-font.c
@@ -1950,7 +1950,7 @@ _cairo_win32_font_face_hash_table_unlock (void)
CAIRO_MUTEX_UNLOCK (_cairo_win32_font_face_mutex);
}
-static void
+static cairo_bool_t
_cairo_win32_font_face_destroy (void *abstract_face)
{
cairo_win32_font_face_t *font_face = abstract_face;
@@ -1960,10 +1960,10 @@ _cairo_win32_font_face_destroy (void *abstract_face)
/* All created objects must have been mapped in the hash table. */
assert (hash_table != NULL);
- if (CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&font_face->base.ref_count)) {
+ if (! _cairo_reference_count_dec_and_test (&font_face->base.ref_count)) {
/* somebody recreated the font whilst we waited for the lock */
_cairo_win32_font_face_hash_table_unlock ();
- return;
+ return FALSE;
}
/* Font faces in SUCCESS status are guaranteed to be in the
@@ -1975,6 +1975,7 @@ _cairo_win32_font_face_destroy (void *abstract_face)
_cairo_hash_table_remove (hash_table, &font_face->base.hash_entry);
_cairo_win32_font_face_hash_table_unlock ();
+ return TRUE;
}
static void
More information about the cairo-commit
mailing list