Bug 1552687 - guard access to gfxDWriteFontFileStream with mutex. r=j…

…fkthame a=jcristau Differential Revision: https://phabricator.services.mozilla.com/D32214 --HG-- extra : rebase_source : 6d05d135ff113076b958faaa4a370a1bb84e7c37 extra : source : 57626db4615868cfa10834a2bf6529457c334684
trman · May 23, 2019 · 098dc3f · 098dc3f
1 parent e04c537
commit 098dc3f
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 9 deletions.
diff --git a/gfx/2d/NativeFontResourceDWrite.cpp b/gfx/2d/NativeFontResourceDWrite.cpp
@@ -101,6 +101,12 @@ class DWriteFontFileStream final : public IDWriteFontFileStream {
   IFACEMETHOD_(ULONG, Release)() {
     uint32_t count = --mRefCnt;
     if (count == 0) {
+      // Avoid locking unless necessary. Verify the refcount hasn't changed
+      // while locked. Delete within the scope of the lock when zero.
+      StaticMutexAutoLock lock(sFontFileStreamsMutex);
+      if (0 != mRefCnt) {
+        return mRefCnt;
+      }
       delete this;
     }
     return count;
@@ -151,7 +157,6 @@ DWriteFontFileStream::DWriteFontFileStream(uint64_t aFontFileKey)
     : mRefCnt(0), mFontFileKey(aFontFileKey) {}
 
 DWriteFontFileStream::~DWriteFontFileStream() {
-  StaticMutexAutoLock lock(sFontFileStreamsMutex);
   sFontFileStreams.erase(mFontFileKey);
 }
 

diff --git a/gfx/thebes/gfxDWriteCommon.cpp b/gfx/thebes/gfxDWriteCommon.cpp
@@ -8,11 +8,13 @@
 #include <unordered_map>
 
 #include "mozilla/Atomics.h"
+#include "mozilla/StaticMutex.h"
 #include "mozilla/gfx/Logging.h"
 
 class gfxDWriteFontFileStream;
 
-static mozilla::Atomic<uint64_t> sNextFontFileKey;
+static mozilla::StaticMutex sFontFileStreamsMutex;
+static uint64_t sNextFontFileKey = 0;
 static std::unordered_map<uint64_t, gfxDWriteFontFileStream*> sFontFileStreams;
 
 IDWriteFontFileLoader* gfxDWriteFontFileLoader::mInstance = nullptr;
@@ -46,18 +48,22 @@ class gfxDWriteFontFileStream final : public IDWriteFontFileStream {
 
   IFACEMETHOD_(ULONG, AddRef)() {
     MOZ_ASSERT(int32_t(mRefCnt) >= 0, "illegal refcnt");
-    ++mRefCnt;
-    return mRefCnt;
+    return ++mRefCnt;
   }
 
   IFACEMETHOD_(ULONG, Release)() {
     MOZ_ASSERT(0 != mRefCnt, "dup release");
-    --mRefCnt;
-    if (mRefCnt == 0) {
+    uint32_t count = --mRefCnt;
+    if (count == 0) {
+      // Avoid locking unless necessary. Verify the refcount hasn't changed
+      // while locked. Delete within the scope of the lock when zero.
+      mozilla::StaticMutexAutoLock lock(sFontFileStreamsMutex);
+      if (0 != mRefCnt) {
+        return mRefCnt;
+      }
       delete this;
-      return 0;
     }
-    return mRefCnt;
+    return count;
   }
 
   // IDWriteFontFileStream methods
@@ -81,7 +87,7 @@ class gfxDWriteFontFileStream final : public IDWriteFontFileStream {
 
  private:
   FallibleTArray<uint8_t> mData;
-  nsAutoRefCnt mRefCnt;
+  mozilla::Atomic<uint32_t> mRefCnt;
   uint64_t mFontFileKey;
 };
 
@@ -134,6 +140,7 @@ HRESULT STDMETHODCALLTYPE gfxDWriteFontFileLoader::CreateStreamFromKey(
     return E_POINTER;
   }
 
+  mozilla::StaticMutexAutoLock lock(sFontFileStreamsMutex);
   uint64_t fontFileKey = *static_cast<const uint64_t*>(fontFileReferenceKey);
   auto found = sFontFileStreams.find(fontFileKey);
   if (found == sFontFileStreams.end()) {
@@ -161,10 +168,12 @@ gfxDWriteFontFileLoader::CreateCustomFontFile(
     return E_FAIL;
   }
 
+  sFontFileStreamsMutex.Lock();
   uint64_t fontFileKey = sNextFontFileKey++;
   RefPtr<gfxDWriteFontFileStream> ffsRef =
       new gfxDWriteFontFileStream(aFontData, aLength, fontFileKey);
   sFontFileStreams[fontFileKey] = ffsRef;
+  sFontFileStreamsMutex.Unlock();
 
   RefPtr<IDWriteFontFile> fontFile;
   HRESULT hr = factory->CreateCustomFontFileReference(