Bug 1532253 - Add threadsafe BlobURLProtocolHandler::CreateNewURI r=baku
☠☠ backed out by 949fbfb190a2 ☠ ☠
authorValentin Gosu <valentin.gosu@gmail.com>
Tue, 05 Mar 2019 18:06:16 +0100
changeset 521354 8e10880ff30b14d588329924f59b7e5ade671bfa
parent 521353 0491d225285c9ed0228228fb785b99a3fddc8f36
child 521355 498d0bcc8ce8b5cc8c5f7b9af8ebda9136718e12
push id10866
push usernerli@mozilla.com
push dateTue, 12 Mar 2019 18:59:09 +0000
treeherdermozilla-beta@445c24a51727 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1532253
milestone67.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 1532253 - Add threadsafe BlobURLProtocolHandler::CreateNewURI r=baku Differential Revision: https://phabricator.services.mozilla.com/D22135
dom/file/uri/BlobURLProtocolHandler.cpp
dom/file/uri/BlobURLProtocolHandler.h
netwerk/base/nsNetUtil.cpp
--- a/dom/file/uri/BlobURLProtocolHandler.cpp
+++ b/dom/file/uri/BlobURLProtocolHandler.cpp
@@ -68,16 +68,24 @@ struct DataInfo {
   nsCString mStack;
 
   // When a blobURL is revoked, we keep it alive for RELEASING_TIMER
   // milliseconds in order to support pending operations such as navigation,
   // download and so on.
   bool mRevoked;
 };
 
+// The mutex is locked whenever gDataTable is changed, or if gDataTable
+// is accessed off-main-thread.
+static StaticMutex sMutex;
+
+// All changes to gDataTable must happen on the main thread, while locking
+// sMutex. Reading from gDataTable on the main thread may happen without
+// locking, since no changes are possible. Reading it from another thread
+// must also lock sMutex to prevent data races.
 static nsClassHashtable<nsCStringHashKey, DataInfo>* gDataTable;
 
 static DataInfo* GetDataInfo(const nsACString& aUri,
                              bool aAlsoIfRevoked = false) {
   if (!gDataTable) {
     return nullptr;
   }
 
@@ -161,16 +169,18 @@ void BroadcastBlobURLUnregistration(cons
 }
 
 class BlobURLsReporter final : public nsIMemoryReporter {
  public:
   NS_DECL_ISUPPORTS
 
   NS_IMETHOD CollectReports(nsIHandleReportCallback* aCallback,
                             nsISupports* aData, bool aAnonymize) override {
+    MOZ_ASSERT(NS_IsMainThread(),
+             "without locking gDataTable is main-thread only");
     if (!gDataTable) {
       return NS_OK;
     }
 
     nsDataHashtable<nsPtrHashKey<BlobImpl>, uint32_t> refCounts;
 
     // Determine number of URLs per BlobImpl, to handle the case where it's > 1.
     for (auto iter = gDataTable->Iter(); !iter.Done(); iter.Next()) {
@@ -456,25 +466,28 @@ class ReleasingTimerHolder final : publi
 
   void RevokeURI() {
     // Remove the shutting down blocker
     nsCOMPtr<nsIAsyncShutdownClient> phase = GetShutdownPhase();
     if (phase) {
       phase->RemoveBlocker(this);
     }
 
+    MOZ_ASSERT(NS_IsMainThread(),
+               "without locking gDataTable is main-thread only");
     DataInfo* info =
         GetDataInfo(mURI, true /* We care about revoked dataInfo */);
     if (!info) {
       // Already gone!
       return;
     }
 
     MOZ_ASSERT(info->mRevoked);
 
+    StaticMutexAutoLock lock(sMutex);
     gDataTable->Remove(mURI);
     if (gDataTable->Count() == 0) {
       delete gDataTable;
       gDataTable = nullptr;
     }
   }
 
   void CancelTimerAndRevokeURI() {
@@ -502,16 +515,18 @@ class ReleasingTimerHolder final : publi
 };
 
 NS_IMPL_ISUPPORTS_INHERITED(ReleasingTimerHolder, Runnable, nsITimerCallback,
                             nsIAsyncShutdownBlocker)
 
 template <typename T>
 static nsresult AddDataEntryInternal(const nsACString& aURI, T aObject,
                                      nsIPrincipal* aPrincipal) {
+  MOZ_ASSERT(NS_IsMainThread(), "changing gDataTable is main-thread only");
+  StaticMutexAutoLock lock(sMutex);
   if (!gDataTable) {
     gDataTable = new nsClassHashtable<nsCStringHashKey, DataInfo>;
   }
 
   DataInfo* info = new DataInfo(aObject, aPrincipal);
   BlobURLsReporter::GetJSStackForBlob(info);
 
   gDataTable->Put(aURI, info);
@@ -577,16 +592,18 @@ nsresult BlobURLProtocolHandler::AddData
 
   return AddDataEntryInternal(aURI, aBlobImpl, aPrincipal);
 }
 
 /* static */
 bool BlobURLProtocolHandler::GetAllBlobURLEntries(
     nsTArray<BlobURLRegistrationData>& aRegistrations, ContentParent* aCP) {
   MOZ_ASSERT(aCP);
+  MOZ_ASSERT(NS_IsMainThread(),
+             "without locking gDataTable is main-thread only");
 
   if (!gDataTable) {
     return true;
   }
 
   for (auto iter = gDataTable->ConstIter(); !iter.Done(); iter.Next()) {
     DataInfo* info = iter.UserData();
     MOZ_ASSERT(info);
@@ -609,50 +626,57 @@ bool BlobURLProtocolHandler::GetAllBlobU
   }
 
   return true;
 }
 
 /*static */
 void BlobURLProtocolHandler::RemoveDataEntry(const nsACString& aUri,
                                              bool aBroadcastToOtherProcesses) {
+  MOZ_ASSERT(NS_IsMainThread(), "changing gDataTable is main-thread only");
   if (!gDataTable) {
     return;
   }
-
   DataInfo* info = GetDataInfo(aUri);
   if (!info) {
     return;
   }
 
-  info->mRevoked = true;
+  {
+    StaticMutexAutoLock lock(sMutex);
+    info->mRevoked = true;
+  }
 
   if (aBroadcastToOtherProcesses && info->mObjectType == DataInfo::eBlobImpl) {
     BroadcastBlobURLUnregistration(nsCString(aUri));
   }
 
   // The timer will take care of removing the entry for real after
   // RELEASING_TIMER milliseconds. In the meantime, the DataInfo, marked as
   // revoked, will not be exposed.
   ReleasingTimerHolder::Create(aUri);
 }
 
 /* static */
 void BlobURLProtocolHandler::RemoveDataEntries() {
+  MOZ_ASSERT(NS_IsMainThread(), "changing gDataTable is main-thread only");
+  StaticMutexAutoLock lock(sMutex);
   if (!gDataTable) {
     return;
   }
 
   gDataTable->Clear();
   delete gDataTable;
   gDataTable = nullptr;
 }
 
 /* static */
 bool BlobURLProtocolHandler::HasDataEntry(const nsACString& aUri) {
+  MOZ_ASSERT(NS_IsMainThread(),
+             "without locking gDataTable is main-thread only");
   return !!GetDataInfo(aUri);
 }
 
 /* static */
 nsresult BlobURLProtocolHandler::GenerateURIString(nsIPrincipal* aPrincipal,
                                                    nsACString& aUri) {
   nsresult rv;
   nsCOMPtr<nsIUUIDGenerator> uuidgen =
@@ -683,32 +707,36 @@ nsresult BlobURLProtocolHandler::Generat
   aUri += Substring(chars + 1, chars + NSID_LENGTH - 2);
 
   return NS_OK;
 }
 
 /* static */
 nsIPrincipal* BlobURLProtocolHandler::GetDataEntryPrincipal(
     const nsACString& aUri) {
+  MOZ_ASSERT(NS_IsMainThread(),
+             "without locking gDataTable is main-thread only");
   if (!gDataTable) {
     return nullptr;
   }
 
   DataInfo* res = GetDataInfo(aUri);
 
   if (!res) {
     return nullptr;
   }
 
   return res->mPrincipal;
 }
 
 /* static */
 void BlobURLProtocolHandler::Traverse(
     const nsACString& aUri, nsCycleCollectionTraversalCallback& aCallback) {
+  MOZ_ASSERT(NS_IsMainThread(),
+             "without locking gDataTable is main-thread only");
   if (!gDataTable) {
     return;
   }
 
   DataInfo* res;
   gDataTable->Get(aUri, &res);
   if (!res) {
     return;
@@ -747,22 +775,34 @@ BlobURLProtocolHandler::GetFlagsForURI(n
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BlobURLProtocolHandler::NewURI(const nsACString& aSpec, const char* aCharset,
                                nsIURI* aBaseURI, nsIURI** aResult) {
+  return BlobURLProtocolHandler::CreateNewURI(aSpec, aCharset, aBaseURI,
+                                              aResult);
+}
+
+/* static */ nsresult BlobURLProtocolHandler::CreateNewURI(
+    const nsACString& aSpec, const char* aCharset, nsIURI* aBaseURI,
+    nsIURI** aResult) {
   *aResult = nullptr;
 
+  // This method can be called on any thread, which is why we lock the mutex
+  // for read access to gDataTable.
   bool revoked = true;
-  DataInfo* info = GetDataInfo(aSpec);
-  if (info && info->mObjectType == DataInfo::eBlobImpl) {
-    revoked = info->mRevoked;
+  {
+    StaticMutexAutoLock lock(sMutex);
+    DataInfo* info = GetDataInfo(aSpec);
+    if (info && info->mObjectType == DataInfo::eBlobImpl) {
+      revoked = info->mRevoked;
+    }
   }
 
   return NS_MutateURI(new BlobURL::Mutator())
       .SetSpec(aSpec)
       .Apply(NS_MutatorMethod(&nsIBlobURLMutator::SetRevoked, revoked))
       .Finalize(aResult);
 }
 
@@ -778,16 +818,18 @@ BlobURLProtocolHandler::NewChannel(nsIUR
 
   RefPtr<BlobURL> blobURL;
   nsresult rv =
       aURI->QueryInterface(kHOSTOBJECTURICID, getter_AddRefs(blobURL));
   if (NS_FAILED(rv) || !blobURL) {
     return NS_OK;
   }
 
+  MOZ_ASSERT(NS_IsMainThread(),
+             "without locking gDataTable is main-thread only");
   DataInfo* info = GetDataInfoFromURI(aURI, true /*aAlsoIfRevoked */);
   if (!info || info->mObjectType != DataInfo::eBlobImpl || !info->mBlobImpl) {
     return NS_OK;
   }
 
   if (blobURL->Revoked()) {
     return NS_OK;
   }
@@ -838,16 +880,18 @@ bool BlobURLProtocolHandler::GetBlobURLP
 
   RefPtr<BlobURL> blobURL;
   nsresult rv =
       aURI->QueryInterface(kHOSTOBJECTURICID, getter_AddRefs(blobURL));
   if (NS_FAILED(rv) || !blobURL) {
     return false;
   }
 
+  MOZ_ASSERT(NS_IsMainThread(),
+             "without locking gDataTable is main-thread only");
   DataInfo* info = GetDataInfoFromURI(aURI, true /*aAlsoIfRevoked */);
   if (!info || info->mObjectType != DataInfo::eBlobImpl || !info->mBlobImpl) {
     return false;
   }
 
   nsCOMPtr<nsIPrincipal> principal;
 
   if (blobURL->Revoked()) {
@@ -861,57 +905,64 @@ bool BlobURLProtocolHandler::GetBlobURLP
   return true;
 }
 
 }  // namespace dom
 }  // namespace mozilla
 
 nsresult NS_GetBlobForBlobURI(nsIURI* aURI, BlobImpl** aBlob) {
   *aBlob = nullptr;
-
+  MOZ_ASSERT(NS_IsMainThread(),
+             "without locking gDataTable is main-thread only");
   DataInfo* info = GetDataInfoFromURI(aURI, false /* aAlsoIfRevoked */);
   if (!info || info->mObjectType != DataInfo::eBlobImpl) {
     return NS_ERROR_DOM_BAD_URI;
   }
 
   RefPtr<BlobImpl> blob = info->mBlobImpl;
   blob.forget(aBlob);
   return NS_OK;
 }
 
 nsresult NS_GetBlobForBlobURISpec(const nsACString& aSpec, BlobImpl** aBlob) {
   *aBlob = nullptr;
+  MOZ_ASSERT(NS_IsMainThread(),
+             "without locking gDataTable is main-thread only");
 
   DataInfo* info = GetDataInfo(aSpec);
   if (!info || info->mObjectType != DataInfo::eBlobImpl) {
     return NS_ERROR_DOM_BAD_URI;
   }
 
   RefPtr<BlobImpl> blob = info->mBlobImpl;
   blob.forget(aBlob);
   return NS_OK;
 }
 
 nsresult NS_GetSourceForMediaSourceURI(nsIURI* aURI, MediaSource** aSource) {
   *aSource = nullptr;
 
+  MOZ_ASSERT(NS_IsMainThread(),
+             "without locking gDataTable is main-thread only");
   DataInfo* info = GetDataInfoFromURI(aURI);
   if (!info || info->mObjectType != DataInfo::eMediaSource) {
     return NS_ERROR_DOM_BAD_URI;
   }
 
   RefPtr<MediaSource> mediaSource = info->mMediaSource;
   mediaSource.forget(aSource);
   return NS_OK;
 }
 
 namespace mozilla {
 namespace dom {
 
 bool IsType(nsIURI* aUri, DataInfo::ObjectType aType) {
+  MOZ_ASSERT(NS_IsMainThread(),
+             "without locking gDataTable is main-thread only");
   DataInfo* info = GetDataInfoFromURI(aUri);
   if (!info) {
     return false;
   }
 
   return info->mObjectType == aType;
 }
 
--- a/dom/file/uri/BlobURLProtocolHandler.h
+++ b/dom/file/uri/BlobURLProtocolHandler.h
@@ -34,16 +34,19 @@ class BlobURLProtocolHandler final : pub
                                      public nsSupportsWeakReference {
  public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIPROTOCOLHANDLER
   NS_DECL_NSIPROTOCOLHANDLERWITHDYNAMICFLAGS
 
   BlobURLProtocolHandler();
 
+  static nsresult CreateNewURI(const nsACString& aSpec, const char* aCharset,
+                               nsIURI* aBaseURI, nsIURI** result);
+
   // Methods for managing uri->object mapping
   // AddDataEntry creates the URI with the given scheme and returns it in aUri
   static nsresult AddDataEntry(mozilla::dom::BlobImpl* aBlobImpl,
                                nsIPrincipal* aPrincipal, nsACString& aUri);
   static nsresult AddDataEntry(mozilla::dom::MediaSource* aMediaSource,
                                nsIPrincipal* aPrincipal, nsACString& aUri);
   // IPC only
   static nsresult AddDataEntry(const nsACString& aURI, nsIPrincipal* aPrincipal,
--- a/netwerk/base/nsNetUtil.cpp
+++ b/netwerk/base/nsNetUtil.cpp
@@ -84,16 +84,17 @@
 #include "nsICertOverrideService.h"
 #include "nsQueryObject.h"
 #include "mozIThirdPartyUtil.h"
 #include "../mime/nsMIMEHeaderParamImpl.h"
 #include "nsStandardURL.h"
 #include "nsChromeProtocolHandler.h"
 #include "nsJSProtocolHandler.h"
 #include "nsDataHandler.h"
+#include "mozilla/dom/BlobURLProtocolHandler.h"
 
 #include <limits>
 
 using namespace mozilla;
 using namespace mozilla::net;
 using mozilla::dom::BlobURLProtocolHandler;
 using mozilla::dom::ClientInfo;
 using mozilla::dom::PerformanceStorage;
@@ -1786,16 +1787,21 @@ nsresult NS_NewURIOnAnyThread(nsIURI **a
     return nsChromeProtocolHandler::CreateNewURI(aSpec, aCharset, aBaseURI,
                                                  aURI);
   }
 
   if (scheme.EqualsLiteral("javascript")) {
     return nsJSProtocolHandler::CreateNewURI(aSpec, aCharset, aBaseURI, aURI);
   }
 
+  if (scheme.EqualsLiteral("blob")) {
+    return BlobURLProtocolHandler::CreateNewURI(aSpec, aCharset, aBaseURI,
+                                                aURI);
+  }
+
   if (NS_IsMainThread()) {
     // XXX (valentin): this fallback should be removed once we get rid of
     // nsIProtocolHandler.newURI
     return NS_NewURI(aURI, aSpec, aCharset, aBaseURI, aIOService);
   }
 
   return NS_ERROR_UNKNOWN_PROTOCOL;
 }