[ft] Resolve mutual referencing problems with zombie faces

Bug 21706 -- zombie ft_font_face / ft_unscaled_font mutual
             referencing problems
[http://bugs.freedesktop.org/show_bug.cgi?id=21706]

There can be more than one zombie font_face belonging to an unscaled_font,
but only the first is destroyed.  This leaks the client's FT_Face
(and associated font data) as release of the FT_Face depends on release
of the font_face.

(The reason why Firefox ends up with two different font_faces for one
unscaled_font is that load_flags for faces with artificial oblique have
FT_LOAD_NO_BITMAP set.
https://bugzilla.mozilla.org/show_bug.cgi?id=486974)

Also it's possible for _cairo_ft_font_face_create to pull out a zombie
font_face from the unscaled_font, which would crash
_cairo_ft_font_face_scaled_font_create, as that expects non-null
font_face->unscaled (if !font-face->pattern).

2 files changed, 31 insertions, 9 deletions

diff --git a/AUTHORS b/AUTHORS
index 289fecba..8c061743 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -86,7 +86,7 @@ Travis Spencer <tspencer@cs.pdx.edu> XCB backend fix
 Bill Spitzak <spitzak@d2.com> Build fix to find Xrender.h without xrender.pc
 Zhe Su <james.su@gmail.com> Add support for fontconfig's embeddedbitmap option
 Owen Taylor <otaylor@redhat.com> Font rewrite, documentation, win32 backend
-Karl Tomlinson <karlt+@karlt.net>
+Karl Tomlinson <karlt+@karlt.net> Optimisation and obscure bug fixes (mozilla)
 Alp Toker <alp@atoker.com> Fix several code/comment typos
 Malcolm Tredinnick <malcolm@commsecure.com.au> Documentation fixes
 David Turner <david@freetype.org> Optimize gradient calculations
diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c
index 1e2a18ee..f9ff0b10 100644
--- a/src/cairo-ft-font.c
+++ b/src/cairo-ft-font.c
@@ -543,8 +543,10 @@ _cairo_ft_unscaled_font_destroy (void *abstract_font)
 	/* See comments in _ft_font_face_destroy about the "zombie" state
 	 * for a _ft_font_face.
 	 */
-	if (unscaled->faces && !unscaled->faces->unscaled)
+	if (unscaled->faces && unscaled->faces->unscaled == NULL) {
+	    assert (unscaled->faces->next == NULL);
 	    cairo_font_face_destroy (&unscaled->faces->base);
+	}
     } else {
 	_font_map_release_face_lock_held (font_map, unscaled);
     }
@@ -2233,9 +2235,10 @@ _cairo_ft_font_face_destroy (void *abstract_face)
     if (font_face == NULL)
 	return;
 
-    /* When destroying the face created by cairo_ft_font_face_create_for_ft_face,
+    /* When destroying a face created by cairo_ft_font_face_create_for_ft_face,
      * we have a special "zombie" state for the face when the unscaled font
-     * is still alive but there are no public references to the font face.
+     * is still alive but there are no other references to a font face with
+     * the same FT_Face.
      *
      * We go from:
      *
@@ -2249,6 +2252,8 @@ _cairo_ft_font_face_destroy (void *abstract_face)
 
     if (font_face->unscaled &&
 	font_face->unscaled->from_face &&
+	font_face->next == NULL &&
+	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);
@@ -2394,12 +2399,21 @@ _cairo_ft_font_face_create (cairo_ft_unscaled_font_t *unscaled,
 	    font_face->ft_options.extra_flags == ft_options->extra_flags &&
 	    cairo_font_options_equal (&font_face->ft_options.base, &ft_options->base))
 	{
-	    if (font_face->base.status == CAIRO_STATUS_SUCCESS)
-		return cairo_font_face_reference (&font_face->base);
+	    if (font_face->base.status) {
+		/* The font_face has been left in an error state, abandon it. */
+		*prev_font_face = font_face->next;
+		break;
+	    }
 
-	    /* The font_face has been left in an error state, abandon it. */
-	    *prev_font_face = font_face->next;
-	    break;
+	    if (font_face->unscaled == NULL) {
+		/* Resurrect this "zombie" font_face (from
+		 * _cairo_ft_font_face_destroy), switching its unscaled_font
+		 * from owner to ownee. */
+		font_face->unscaled = unscaled;
+		_cairo_unscaled_font_reference (&unscaled->base);
+		return &font_face->base;
+	    } else
+		return cairo_font_face_reference (&font_face->base);
 	}
     }
 
@@ -2415,6 +2429,14 @@ _cairo_ft_font_face_create (cairo_ft_unscaled_font_t *unscaled,
 
     font_face->ft_options = *ft_options;
 
+    if (unscaled->faces && unscaled->faces->unscaled == NULL) {
+	/* This "zombie" font_face (from _cairo_ft_font_face_destroy)
+	 * is no longer needed. */
+	assert (unscaled->from_face && unscaled->faces->next == NULL);
+	cairo_font_face_destroy (&unscaled->faces->base);
+	unscaled->faces = NULL;
+    }
+
     font_face->next = unscaled->faces;
     unscaled->faces = font_face;