From c22288fbdbcd7a48c66612db70168fa5cdf715dc Mon Sep 17 00:00:00 2001 From: Ben Wagner Date: Mon, 2 Nov 2020 12:15:20 -0500 Subject: [PATCH] Declare FontConfig not threadsafe. Some time ago FontConfig stopped being thread antagonistic and documented that it was thread safe (or at least any thread un-safety was a bug). However, until https://gitlab.freedesktop.org/fontconfig/fontconfig/-/commit/447b9ccc7d03bf953d1f1c4708ca16c56c1511ec at least one of the basic atomic primitives was implemented incorrectly. Resume using a mutex to serialize access to FontConfig until 2.13.93. Bug: cl/339089311 Change-Id: I632f03bc575a37b5391390a0868aef256e3aacda Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331339 Reviewed-by: Mike Klein Commit-Queue: Ben Wagner --- src/ports/SkFontConfigInterface_direct.cpp | 14 ++++++++------ src/ports/SkFontMgr_fontconfig.cpp | 13 ++++++++----- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/ports/SkFontConfigInterface_direct.cpp b/src/ports/SkFontConfigInterface_direct.cpp index 54d2a4d01b..555dd75fbd 100644 --- a/src/ports/SkFontConfigInterface_direct.cpp +++ b/src/ports/SkFontConfigInterface_direct.cpp @@ -25,31 +25,33 @@ namespace { -// Fontconfig is not threadsafe before 2.10.91. Before that, we lock with a global mutex. -// See https://bug.skia.org/1497 for background. +// FontConfig was thread antagonistic until 2.10.91 with known thread safety issues until 2.13.93. +// Before that, lock with a global mutex. +// See https://bug.skia.org/1497 and cl/339089311 for background. static SkMutex& f_c_mutex() { static SkMutex& mutex = *(new SkMutex); return mutex; } struct FCLocker { - // Assume FcGetVersion() has always been thread safe. + static constexpr int FontConfigThreadSafeVersion = 21393; + // Assume FcGetVersion() has always been thread safe. FCLocker() { - if (FcGetVersion() < 21091) { + if (FcGetVersion() < FontConfigThreadSafeVersion) { f_c_mutex().acquire(); } } ~FCLocker() { AssertHeld(); - if (FcGetVersion() < 21091) { + if (FcGetVersion() < FontConfigThreadSafeVersion) { f_c_mutex().release(); } } static void AssertHeld() { SkDEBUGCODE( - if (FcGetVersion() < 21091) { + if (FcGetVersion() < FontConfigThreadSafeVersion) { f_c_mutex().assertHeld(); } ) } diff --git a/src/ports/SkFontMgr_fontconfig.cpp b/src/ports/SkFontMgr_fontconfig.cpp index aa261e8174..43537a4985 100644 --- a/src/ports/SkFontMgr_fontconfig.cpp +++ b/src/ports/SkFontMgr_fontconfig.cpp @@ -57,23 +57,26 @@ class SkData; namespace { -// Fontconfig is not threadsafe before 2.10.91. Before that, we lock with a global mutex. -// See https://bug.skia.org/1497 for background. +// FontConfig was thread antagonistic until 2.10.91 with known thread safety issues until 2.13.93. +// Before that, lock with a global mutex. +// See https://bug.skia.org/1497 and cl/339089311 for background. static SkMutex& f_c_mutex() { static SkMutex& mutex = *(new SkMutex); return mutex; } class FCLocker { + static constexpr int FontConfigThreadSafeVersion = 21393; + // Assume FcGetVersion() has always been thread safe. static void lock() SK_NO_THREAD_SAFETY_ANALYSIS { - if (FcGetVersion() < 21091) { + if (FcGetVersion() < FontConfigThreadSafeVersion) { f_c_mutex().acquire(); } } static void unlock() SK_NO_THREAD_SAFETY_ANALYSIS { AssertHeld(); - if (FcGetVersion() < 21091) { + if (FcGetVersion() < FontConfigThreadSafeVersion) { f_c_mutex().release(); } } @@ -93,7 +96,7 @@ public: }; static void AssertHeld() { SkDEBUGCODE( - if (FcGetVersion() < 21091) { + if (FcGetVersion() < FontConfigThreadSafeVersion) { f_c_mutex().assertHeld(); } ) }