Bug 73012 - Improve thread safety of Cairo Font on Windows
Summary: Improve thread safety of Cairo Font on Windows
Status: RESOLVED MOVED
Alias: None
Product: cairo
Classification: Unclassified
Component: win32 backend (show other bugs)
Version: unspecified
Hardware: Other Windows (All)
: medium normal
Assignee: cairo-bugs mailing list
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-24 09:25 UTC by Fan, Chun-wei
Modified: 2018-08-25 13:46 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Attempt to make cairo-win32-font.c thread-safe (14.75 KB, patch)
2013-12-24 09:35 UTC, Fan, Chun-wei
Details | Splinter Review

Description Fan, Chun-wei 2013-12-24 09:25:37 UTC
Hi,

In its current state, the HDC in cairo-win32-font.c is a static variable that can be compromised if multiple threads are attempting to create HDC's for Windows to render the fonts, making this code thread-unsafe.  This also makes PangoCairo (which also relies on this code) unable to run in a thread-safe manner, which I am also trying to investigate and fix[1].

This bug attempts to track my try, based on someone else's patch, to tackle this issue.

[1]:https://bugzilla.gnome.org/show_bug.cgi?id=695913
Comment 1 Fan, Chun-wei 2013-12-24 09:35:07 UTC
Created attachment 91176 [details] [review]
Attempt to make cairo-win32-font.c thread-safe

Hi,

Lufy D.Monkey's have come up with a patch[1][2] that attempts to make cairo-win32-
font.c thread-safe.  I have adapted his patch (which was dated back to 2010) so that it fits in the current state of the code.  Basically what happens is that the HDC's creations/releases are guarded by mutexes and the created HDC's are cached by thread id's so that they will need to be created and released less often.

This does not yet fix the PangoCairo situation (although it would fare better there by reducing intermittent crashes by a whole lot, say from once every 8 runs to 30 runs or so), but I have used this cairocffi Python test[3] to test this code, and it seems that the fix will allow the test to pass, whereas the original code will fail with a

MemoryError: ('cairo returned CAIRO_STATUS_NO_MEMORY: out of memory', 1L)

[1]: http://lists.cairographics.org/archives/cairo/2010-July/020336.html
[2]: http://lists.cairographics.org/archives/cairo/attachments/20100715/8b3a8f05/attachment-0001.obj
[3]: https://gist.github.com/akx/7307462/raw/13c03a220662928778ab626625cd53251d1ec295/cairocffi-22.py

Merry Christmas, and with blessings, thank you!
Comment 2 Alexandre 2014-07-25 22:19:26 UTC
How about making the static variable a TLS (Thread Local Storage) one?

-    static HDC hdc;
+    static __thread HDC hdc;
Comment 3 GitLab Migration User 2018-08-25 13:46:51 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/cairo/cairo/issues/190.


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.