Bug 69034 (gray)

Summary:	Make cairo_ft thread safe
Product:	freetype	Reporter:	Marek Kasik <mkasik>
Component:	general	Assignee:	Werner Lemberg <wl>
Status:	RESOLVED FIXED	QA Contact:
Severity:	critical
Priority:	highest	CC:	chris, freedesktop, mail, psychon
Version:	unspecified
Hardware:	All
OS:	All
URL:	https://bugs.launchpad.net/libcairo/+bug/1199571
Whiteboard:
i915 platform:		i915 features:
Attachments:	Make usage of freetype thread safe

Description Marek Kasik 2013-09-06 13:19:58 UTC

Created attachment 85352 [details]
Make usage of freetype thread safe

Current freetype backend is not thread safe. It can cause segmentation fault when threads are not properly handled by applications which use cairo (see https://bugzilla.redhat.com/show_bug.cgi?id=678397 and its duplicates).

Attached patch tries to fix this problem by creating FT_Library object for each thread. This is recommended solution by freetype (http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/include/freetype/freetype.h#n1684).

It adds a hash table to which FT_Library is added if a new thread needs one. Thread IDs are used as keys for this table.

Marek

Comment 1 Behdad Esfahbod 2013-09-06 17:58:01 UTC

Unfortunately that FreeType "recommendation" is red herring.  First, people are free to pass around FT_Face among threads, and second, we will be leaking stuff this way, unless we add proper TLS support.

At any rate.  I suggest we use cairo for all glyph rasterizer instead.

Comment 2 Behdad Esfahbod 2013-12-16 06:16:19 UTC

Chris, any chance you have some time to think about this?

Comment 3 Behdad Esfahbod 2014-07-31 16:45:44 UTC

*** Bug 81874 has been marked as a duplicate of this bug. ***

Comment 4 Behdad Esfahbod 2014-07-31 16:59:16 UTC

Looks like copying thread-local storage macros from pixman-compiler.h and using them to use per-thread FT_Library might easily get most of this resolved.  I can take a look if no one else steps in.

Comment 5 Marek Kasik 2014-11-18 12:05:14 UTC

(In reply to Behdad Esfahbod from comment #4)
> Looks like copying thread-local storage macros from pixman-compiler.h and
> using them to use per-thread FT_Library might easily get most of this
> resolved.  I can take a look if no one else steps in.

Hi Behdad,

had you time to look at this already?

Regards

Marek

Comment 6 Behdad Esfahbod 2014-12-30 01:38:24 UTC

So, I ended up trying to fix this in FreeType.  Have made huge progress so far.  Here's a tree:

  https://github.com/behdad/freetype/commits/ftthread

And here's a standalone test:

  https://github.com/behdad/ftthread

There's still some more work to do.  I can't yet understand this crash for example:

https://bugzilla.redhat.com/show_bug.cgi?id=1165471

Comment 7 Behdad Esfahbod 2014-12-30 23:29:58 UTC

I now have a complete patchset up here:

  https://github.com/behdad/freetype/commits/ftthread


Will be sending upstream soon.

Comment 8 Behdad Esfahbod 2014-12-31 00:14:35 UTC

Done.  See:
http://www.mail-archive.com/freetype-devel@nongnu.org/msg06758.html

Comment 9 Behdad Esfahbod 2015-01-22 07:58:58 UTC

Patchset is now upstream in FreeType.

Comment 10 Marek Kasik 2015-01-22 09:42:42 UTC

(In reply to Behdad Esfahbod from comment #9)
> Patchset is now upstream in FreeType.

Thank you very much for all the effort you've put into this!

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.