[poppler] [PATCH] Poppler Crash Fix: Do not call FT_Done_Freetype in CairoOutputDev

[Michael Vrable]

I came across a rather interesting crash in Evince recently, which I 
think was due to FT_Done_FreeType being called in CairoOutputDev before 
all font faces loaded by that instance of FreeType were done being used 
by Cairo.  The bug was non-deterministic, but usually showed up as a 
segmentation fault after reloading a document in Evince some number of 
times (sometimes one reload, sometimes many were needed).

The sequence of events I think went something like this:
 1. Poppler rendered a page, in the process calling on FreeType to load
    a font.
 2. After rendering was done, the CairoOutputDev object was deleted.
    Cairo still maintains a reference to some of the loaded fonts, but,
    the FT_Library object associated with the CairoOutputDev is deleted.
    My guess is that doing so causes the FT_Face objects to be freed.
 3. In a later page render, poppler loads calls on FreeType to load
    another font.  By chance, this ends up being allocated at the same
    address in memory as one of the fonts from step 1 (since the memory
    has been freed).
 4. When asked to create a Cairo font for this FreeType font, Cairo
    believes that the font face is already in its cache--Cairo still has
    a reference to the font from step 1, and the underlying FreeType
    font is at the same address in memory as the font in the new
    request.  Cairo returns a copy of its old font face.
 5. CairoFont::create registers a destructor with the font face returned
    by Cairo (using cairo_font_face_set_user_data).  However, since
    cairo has returned an already-created font, there is already a
    destructor, using the same key, so before replacing it, cairo calls
    the old destructor.  This in turn calls FT_Done_Face on the new font
    which we were just loading.
 6. Havoc ensues.

The simple fix is to never call FT_Done_FreeType; cairo might hold on to 
a reference to the fonts until the application exits in any case.  To 
avoid unbounded resource leaks, however, I switched to using a single 
static FT_Library instance instead of one per CairoOutputDev.  A patch 
is attached.

A downside to this approach is that it may not be thread-safe.  It's not 
clear to me that the poppler library as a whole is--can anyone comment 
on what the goals for poppler are here, and on the status of the 
libraries it uses?  An alternate approach is to keep the one FT_Library 
per CairoOutputDev approach, but do some reference counting on it so as 
to avoid calling FT_Done_FreeType until FT_Done_Face has been called for 
each font face loaded with it.  This is a bit more complex to implement, 
but if it would be the preferred approach I can work on a patch to do 
that.

--Michael Vrable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cairo-use-global-ft_library.patch
Type: text/x-diff
Size: 3104 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/poppler/attachments/20080620/7f060965/attachment.patch