[poppler] poppler/CairoFontEngine.cc

[Carlos Garcia Campos]

 poppler/CairoFontEngine.cc |   19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

New commits:
commit 42db4890e8295aaec5a1be12d1414fc0a9048550
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Thu Mar 27 10:52:22 2008 +0000

    Do not call FT_Done_Face on a live cairo_font_face_t.
    
    Currently CairoFont calls FT_Done_Face on its deletion, but the FT_Face
    is usually still in use within cairo. This causes a failure later when
    cairo tries to create a glyph from its cairo_font_face_t.
    
    From http://bugs.freedesktop.org/show_bug.cgi?id=15216:
    ==13745== Invalid read of size 4
    ==13745==    at 0x51BE572: FT_Load_Glyph (ftobjs.c:549)
    ==13745==    by 0x4A24921: _cairo_ft_scaled_glyph_init (cairo-ft-font.c:1922)
    ==13745==    by 0x4A117AB: _cairo_scaled_glyph_lookup
    (cairo-scaled-font.c:1674)
    ==13745==    by 0x4A12A5A: _cairo_scaled_font_glyph_device_extents
    (cairo-scaled-font.c:1124)
    ==13745==    by 0x4A21ECD: _cairo_analysis_surface_show_glyphs
    (cairo-analysis-surface.c:516)
    ==13745==    by 0x4A144DC: _cairo_surface_show_glyphs (cairo-surface.c:2086)
    ==13745==    by 0x4A1FCC8: _cairo_meta_surface_replay_internal
    (cairo-meta-surface.c:816)
    ==13745==    by 0x4A214B1: _paint_page (cairo-paginated-surface.c:299)
    ==13745==    by 0x4A2171E: _cairo_paginated_surface_show_page
    (cairo-paginated-surface.c:445)
    ==13745==    by 0x4A14BDF: cairo_surface_show_page (cairo-surface.c:1702)
    ==13745==    by 0x49FF661: cairo_show_page (cairo.c:2155)
    ==13745==    by 0xA267D97:
    pdf_document_file_exporter_end_page(_EvFileExporter*) (ev-poppler.cc:1753)
    ==13745==  Address 0x55c5630 is 88 bytes inside a block of size 552 free'd
    ==13745==    at 0x402269C: free (vg_replace_malloc.c:326)
    ==13745==    by 0x51B7ABC: ft_free (ftsystem.c:158)
    ==13745==    by 0x51BB319: ft_mem_free (ftutil.c:171)
    ==13745==    by 0x51BC318: destroy_face (ftobjs.c:856)
    ==13745==    by 0x51BC3B2: FT_Done_Face (ftobjs.c:1972)
    ==13745==    by 0x4363704: CairoFont::~CairoFont() (CairoFontEngine.cc:251)
    ==13745==    by 0x436401D: CairoFontEngine::getFont(GfxFont*, XRef*)
    (CairoFontEngine.cc:335)
    ==13745==    by 0x4366915: CairoOutputDev::updateFont(GfxState*)
    (CairoOutputDev.cc:318)
    ==13745==    by 0x5093BF1: Gfx::opShowText(Object*, int) (Gfx.cc:3073)
    ==13745==    by 0x508F901: Gfx::execOp(Object*, Object*, int) (Gfx.cc:726)
    ==13745==    by 0x50906FF: Gfx::go(int) (Gfx.cc:594)
    ==13745==    by 0x5090C96: Gfx::display(Object*, int) (Gfx.cc:557)
    
    The solution is to release the reference to the cairo_font_face_t upon
    destruction of the CairoFont, and then to release the FT_Face from the
    destroy notify of the cairo_font_face_t.

diff --git a/poppler/CairoFontEngine.cc b/poppler/CairoFontEngine.cc
index 67034f0..5073252 100644
--- a/poppler/CairoFontEngine.cc
+++ b/poppler/CairoFontEngine.cc
@@ -31,11 +31,10 @@ static void fileWrite(void *stream, char *data, int len) {
 // CairoFont
 //------------------------------------------------------------------------
 
-static void cairo_font_face_destroy (void *data)
+static void _ft_done_face (void *data)
 {
-  CairoFont *font = (CairoFont *) data;
-
-  delete font;
+  FT_Face face = (FT_Face) data;
+  FT_Done_Face (face);
 }
 
 CairoFont *CairoFont::create(GfxFont *gfxFont, XRef *xref, FT_Library lib, GBool useCIDs) {
@@ -224,16 +223,16 @@ CairoFont *CairoFont::create(GfxFont *gfxFont, XRef *xref, FT_Library lib, GBool
   cairo_font_face = cairo_ft_font_face_create_for_ft_face (face,
 							   FT_LOAD_NO_HINTING |
 							   FT_LOAD_NO_BITMAP);
-  if (cairo_font_face == NULL) {
-    error(-1, "could not create cairo font\n");
+  if (cairo_font_face_status (cairo_font_face)) {
+    error(-1, "could not create cairo font: %s\n", cairo_status_to_string (cairo_font_face_status (cairo_font_face)));
     goto err2; /* this doesn't do anything, but it looks like we're
 		* handling the error */
   } {
   CairoFont *ret = new CairoFont(ref, cairo_font_face, face, codeToGID, codeToGIDLen, substitute);
-  cairo_font_face_set_user_data (cairo_font_face, 
+  cairo_font_face_set_user_data (cairo_font_face,
 				 &cairo_font_face_key,
-				 ret,
-				 cairo_font_face_destroy);
+				 face,
+				 _ft_done_face);
 
   return ret;
   }
@@ -249,7 +248,7 @@ CairoFont::CairoFont(Ref ref, cairo_font_face_t *cairo_font_face, FT_Face face,
 					    codeToGIDLen(codeToGIDLen), substitute(substitute) { }
 
 CairoFont::~CairoFont() {
-  FT_Done_Face (face);
+  cairo_font_face_destroy (cairo_font_face);
   gfree(codeToGID);
 }