diff options
author | Chris Wilson <chris@chris-wilson.co.uk> | 2013-09-17 16:28:19 +0100 |
---|---|---|
committer | Chris Wilson <chris@chris-wilson.co.uk> | 2013-09-17 16:37:47 +0100 |
commit | 337ab1f8d9e29086bfb4001508b28835b41c6390 (patch) | |
tree | 7187ccfda4d59b754b1919e97be7811db9f07eb8 /src/cairo-font-face.c | |
parent | 0ac81988c199df1a6652dc0ea72627122bf95c6c (diff) | |
download | cairo-337ab1f8d9e29086bfb4001508b28835b41c6390.tar.gz |
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@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=69470
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Diffstat (limited to 'src/cairo-font-face.c')
-rw-r--r-- | src/cairo-font-face.c | 43 |
1 files changed, 31 insertions, 12 deletions
diff --git a/src/cairo-font-face.c b/src/cairo-font-face.c index b93bd8caa..e32a9bb75 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); } |