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

[David Turner]

ok, just to inform you that I've started coding something like what
I've described in my previous e-mail. I'll try to send a patch in a couple
of days when it's finished.

Regards,

- David

On Tue, 13 Feb 2007 23:19:49 +0100, "David Turner" <david at freetype.org> said:
> 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)
> _______________________________________________
> cairo mailing list
> cairo at cairographics.org
> http://cairographics.org/cgi-bin/mailman/listinfo/cairo