[cairo] Re: Thread-specific linked-list for locked FT_Face objects

[David Turner]

On Tue, 13 Feb 2007 14:22:47 -0500, "Owen Taylor" <otaylor at redhat.com> said:
> My worry about this approach is deadlocks:
> 
>  cairo_scaled_font_create()
> 
> for example, does:
> 
>  _cairo_scaled_font_map_lock();
>  [... call into backend ]
>  _cairo_scaled_font_map_unlock();
> 
> With an exposed cairo_ft_lock(), this would create a potential deadlock
> if anybody called cairo_scaled_font_create() after locking the FT lock,
> since you then have both lock orders:
> 
>  - Lock the font map; Lock the FT lock
>  - Lock the FT lock; lock the font map
> 
> A complicated set of rules about what functions you could call with 
> FT locked would be a invitation to all sorts of subtle hard-to-reproduce
> problems.
> 

I think that this is solvable by a different approach. Basically,
you need to synchronize access/usage to the FreeType API and data
structures, but the locking doesn't need to be pushed back to the
cairo font structures themselves, only to their operations when
they act on a FT_Face or FT_Size.

In other words, you need to separate the ownership of FT_Library,
FT_Face and FT_Size objects from the rest of Cairo data structures.
Consider this example:

 /* an opaque structure that holds a FT_Library, a small
  * MRU-ordered list of FT_Face and FT_Size objects, all
  * protected by a mutex/lock
  */
  typedef struct cairo_ft_state     cairo_ft_state_t;

 /* grab the FreeType state from Cairo, this is a potentially blocking
  * call that locks the state's mutex. this must be done by any routine
  * that uses a FT_Face or FT_Size object within Cairo
  */
  cairo_public  cairo_ft_state_t*
  cairo_ft_state_lock(void);

 /* get FT_Library from FreeType state
  */
  cairo_public FT_Library
  cairo_ft_state_get_library (cairo_ft_state_t  *ft_state);

 /* retrieve a FT_Face for a given ft_font_face, you shouldn't
  * assume anything about its mutable state.
  */
  cairo_public FT_Face
  cairo_ft_state_get_face (cairo_ft_state_t      *ft_state,
                           cairo_ft_font_face_t  *ft_font_face);

 /* retrieve a FT_Size for a given ft_scaled_font
  * the corresponding FT_Face can be accessed directly as size->face.
  * the returned FT_Size is already activated, so there is no need
  * to call FT_Size_Activate.
  */
  cairo_public void
  cairo_ft_state_get_size (cairo_ft_state_t        *ft_state,
                           cairo_ft_scaled_font_t  *ft_scaled_font,
                           void                   **psize_ref);

 /* release a FreeType state, release/recycle the corresponding
  * FT_Face and FT_Size that were extracted with cairo_ft_state_get_face
  * and cairo_ft_state_get_size, then unlock the state's mutex
  */
  cairo_public  void
  cairo_ft_state_unlock (cairo_ft_state_t  *ft_state);

the important point is that cairo_ft_font_face_t should hold *no* pointer to
a FT_Face, similarly, ft_scaled_font shouldn't hold any pointer to a FT_Face
or FT_Size object.

the same is true of glyph caching, i.e. cairo_unscaled_font_t must be changed
to not hold a FT_Face, instead one should be obtained through ft_state_lock()
and ft_state_get_face/size() when a glyph needs to be loaded.

there is an additionnal detail to consider, which is that a cairo_font_face
can be created from cairo_font_face_create_for_ft_face() where the caller
provides its own FT_Face to Cairo. I assume that in this case, the FT_Face
is owned by a different FT_Library, and can be stored directly in the
cairo_font_face_t object, as a special exception.

cairo_ft_scaled_font_lock_face() and cairo_ft_scaled_font_unlock_face() cannot
be used in a thread-safe way and will probably need to be deprecated. I wonder
how many Cairo client are using them though... every code that uses them is
already broken anyway.

similarly, all operations that need to access FT_Face/FT_Size objects would
lock the cairo_ft_state_t, get the objects from it, perform access/calls on
them, then release the state.

> > An alternative would be to wrap more of the FreeType API in cairo
> > cloathing and perform the necessary locking within the cairo library;
> > this doesn't seem commensurate with the current pango implementation
> > though, which shares FreeType usage across several output modules.
> 
> I'm not sure what you mean by this. Right now, Pango exposes the ability
> to get a FT_Face in the public API. (Actually, not fully public API, but
> rather reduced-stability engine-and-backend API.) No matter what
> approach we take, that can't be made thread-safe as long as FreeType
> is not thread-safe.
>

The FreeType API cannot be made thread-safe, consider the following sequence
of operations:

  FT_Size_Activate(size);
  FT_Set_Transform(size->face, some_transform);
  FT_Load_Glyph(size->face, some_glyph, some_flags);

if two threads are trying to do the same operations on two distinct FT_Size
objects related to the same parent FT_Face, you have pretty nice race conditions
everywhere unless you can wrap these around a lock that synchronizes access to
the corresponding FT_Library

> But almost anything that Pango does with FreeType can (theoretically) be
> reduced to atomic operations that could be individually wrapped by
> cairo.
> 
I don't see why cairo-pango couldn't use the cairo_ft_state_t itself. I suppose
that it holds references to FT_Face and maybe FT_Size objects internally, but
it could be modified not to.

On the other hand, I'm more worried about the API that provides a FT_Face, who
owns it after the call ? what about the corresponding FT_Library ? I am right to
assure that Pango uses its own FT_Library, independent from the Cairo one ?

> The most problematical part might be harfbuzz, which needs access to
> entire raw tables from the FreeType font. There's no problem with doing
> that with an API that simply copies the data out of the font into a
> malloc'ed segment, but if you wanted to try and share the mmap's that
> FreeType creates, then things get trickier.
> 
the tables are read-only, I don't see exactly what the problem would be.
At worse, you could simply re-map part of the font files into your own
memory segments and read directly from them, independent of the lifetime
of the FT_Face objects themselves...


Hope this helps,

- David Turner
- The FreeType Project  (www.freetype.org)