Bug 1473732 - Avoid race condition when accessing and creating SkCairoFTTypeface. r=rhunt
☠☠ backed out by 1161111168cf ☠ ☠
authorLee Salzman <lsalzman@mozilla.com>
Mon, 30 Jul 2018 12:11:46 -0400
changeset 429267 3354b6d032f3a436e6b2d11987b1c315ecf44c5c
parent 429266 46cf39b31988ad011da0bb61274798b7a51ccc96
child 429268 7742f4ea046bee56dd0141e7038ea8a1fefe6139
push id67094
push userccoroiu@mozilla.com
push dateMon, 30 Jul 2018 22:02:32 +0000
treeherderautoland@397b4d841690 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrhunt
bugs1473732
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1473732 - Avoid race condition when accessing and creating SkCairoFTTypeface. r=rhunt
gfx/skia/skia/src/ports/SkFontHost_cairo.cpp
--- a/gfx/skia/skia/src/ports/SkFontHost_cairo.cpp
+++ b/gfx/skia/skia/src/ports/SkFontHost_cairo.cpp
@@ -8,16 +8,17 @@
 
 #include "cairo.h"
 #include "cairo-ft.h"
 
 #include "SkFontHost_FreeType_common.h"
 
 #include "SkAdvancedTypefaceMetrics.h"
 #include "SkFDot6.h"
+#include "SkMutex.h"
 #include "SkPath.h"
 #include "SkScalerContext.h"
 #include "SkTypefaceCache.h"
 
 #include <cmath>
 
 #include <ft2build.h>
 #include FT_FREETYPE_H
@@ -164,16 +165,18 @@ static bool bothZero(SkScalar a, SkScala
 
 // returns false if there is any non-90-rotation or skew
 static bool isAxisAligned(const SkScalerContextRec& rec) {
     return 0 == rec.fPreSkewX &&
            (bothZero(rec.fPost2x2[0][1], rec.fPost2x2[1][0]) ||
             bothZero(rec.fPost2x2[0][0], rec.fPost2x2[1][1]));
 }
 
+SK_DECLARE_STATIC_MUTEX(gTypefaceMutex);
+
 class SkCairoFTTypeface : public SkTypeface {
 public:
     virtual SkStreamAsset* onOpenStream(int*) const override { return nullptr; }
 
     virtual std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override
     {
         SkDEBUGCODE(SkDebugf("SkCairoFTTypeface::onGetAdvancedMetrics unimplemented\n"));
         return nullptr;
@@ -264,19 +267,29 @@ public:
 #ifdef CAIRO_HAS_FC_FONT
         if (fPattern) {
             FcPatternReference(fPattern);
         }
 #endif
     }
 
 private:
+
+    void internal_dispose() const override
+    {
+        SkAutoMutexAcquire lock(gTypefaceMutex);
+        internal_dispose_restore_refcnt_to_1();
+        delete this;
+    }
+
     ~SkCairoFTTypeface()
     {
-        cairo_font_face_set_user_data(fFontFace, &kSkTypefaceKey, nullptr, nullptr);
+        if (cairo_font_face_get_user_data(fFontFace, &kSkTypefaceKey) == this) {
+            cairo_font_face_set_user_data(fFontFace, &kSkTypefaceKey, nullptr, nullptr);
+        }
         cairo_font_face_destroy(fFontFace);
 #ifdef CAIRO_HAS_FC_FONT
         if (fPattern) {
             FcPatternDestroy(fPattern);
         }
 #endif
     }
 
@@ -285,22 +298,23 @@ private:
 };
 
 SkTypeface* SkCreateTypefaceFromCairoFTFontWithFontconfig(cairo_scaled_font_t* scaledFont, FcPattern* pattern)
 {
     cairo_font_face_t* fontFace = cairo_scaled_font_get_font_face(scaledFont);
     SkASSERT(cairo_font_face_status(fontFace) == CAIRO_STATUS_SUCCESS);
     SkASSERT(cairo_font_face_get_type(fontFace) == CAIRO_FONT_TYPE_FT);
 
+    SkAutoMutexAcquire lock(gTypefaceMutex);
+
     SkTypeface* typeface = reinterpret_cast<SkTypeface*>(cairo_font_face_get_user_data(fontFace, &kSkTypefaceKey));
-    if (typeface) {
+    if (typeface && typeface->getRefCnt() > 0) {
         typeface->ref();
     } else {
         typeface = new SkCairoFTTypeface(fontFace, pattern);
-        SkTypefaceCache::Add(typeface);
     }
 
     return typeface;
 }
 
 SkTypeface* SkCreateTypefaceFromCairoFTFont(cairo_scaled_font_t* scaledFont)
 {
     return SkCreateTypefaceFromCairoFTFontWithFontconfig(scaledFont, nullptr);