commit f33dcc2aa20e426f66e5d018d0e8e5689ac67693
author Ben Wagner <bungeman@google.com> Thu Oct 03 17:47:02 2019 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Fri Jan 24 18:54:18 2020 +0000
tree 6a94958362cd148f72119fe15123b43cb9be3653
parent cb93a20d9a961219bad674d6c168bfde03a16246

Preserve FcConfig in SkFontConfigInterface_direct methods.

It is possible that some other thread will change the default FcConfig
out from under one of these methods, which is a terrible race. Prevent
this by taking a reference to the default FcConfig in the method and
keeping it alive while it is being used.

Previously an attempt was made to hold onto the FcConfig for the
lifetime of the SkFontConfigInterface_direct object. Unfortunately, some
users are holing onto SkFontConfigInterface_direct as a global and so it
is not obvious to free all instances before calling FcFini, which will
asert that no instances are still using the cache.

Bug: chromium:1004254
Change-Id: Ia173e90b95c43c40d91b366095eb97c0e466ab86
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/246216
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>

src/ports/SkFontConfigInterface_direct.cpp

diff --git a/src/ports/SkFontConfigInterface_direct.cpp b/src/ports/SkFontConfigInterface_direct.cpp
index 84fd11b..085509b 100644
--- a/src/ports/SkFontConfigInterface_direct.cpp
+++ b/src/ports/SkFontConfigInterface_direct.cpp
@@ -74,6 +74,8 @@
     ) }
 };
 
+using UniqueFCConfig = std::unique_ptr<FcConfig, SkFunctionWrapper<decltype(FcConfigDestroy), FcConfigDestroy>>;
+
 } // namespace
 
 size_t SkFontConfigInterface::FontIdentity::writeToMemory(void* addr) const {
@@ -509,10 +511,6 @@
 #endif
 
 SkFontConfigInterfaceDirect::SkFontConfigInterfaceDirect() {
-    FCLocker lock;
-
-    FcInit();
-
     SkDEBUGCODE(fontconfiginterface_unittest();)
 }
 
@@ -542,7 +540,8 @@
     if (!c_filename) {
         return false;
     }
-    const char* sysroot = (const char*)FcConfigGetSysRoot(nullptr);
+    UniqueFCConfig fcConfig(FcConfigReference(nullptr));
+    const char* sysroot = (const char*)FcConfigGetSysRoot(fcConfig.get());
     SkString resolvedFilename;
     if (sysroot) {
         resolvedFilename = sysroot;
@@ -603,7 +602,7 @@
     }
 
     FCLocker lock;
-
+    UniqueFCConfig fcConfig(FcConfigReference(nullptr));
     FcPattern* pattern = FcPatternCreate();
 
     if (familyName) {
@@ -613,7 +612,7 @@
 
     FcPatternAddBool(pattern, FC_SCALABLE, FcTrue);
 
-    FcConfigSubstitute(nullptr, pattern, FcMatchPattern);
+    FcConfigSubstitute(fcConfig.get(), pattern, FcMatchPattern);
     FcDefaultSubstitute(pattern);
 
     // Font matching:
@@ -652,7 +651,7 @@
     }
 
     FcResult result;
-    FcFontSet* font_set = FcFontSort(nullptr, pattern, 0, nullptr, &result);
+    FcFontSet* font_set = FcFontSort(fcConfig.get(), pattern, 0, nullptr, &result);
     if (!font_set) {
         FcPatternDestroy(pattern);
         return false;
@@ -680,7 +679,7 @@
         FcFontSetDestroy(font_set);
         return false;
     }
-    const char* sysroot = (const char*)FcConfigGetSysRoot(nullptr);
+    const char* sysroot = (const char*)FcConfigGetSysRoot(fcConfig.get());
     SkString resolvedFilename;
     if (sysroot) {
         resolvedFilename = sysroot;