Bug 1472018 - Limit the lock scope in WebCryptoThreadPool::Shutdown. r=bz, a=RyanVM
authorEric Rahm <erahm@mozilla.com>
Thu, 28 Jun 2018 15:34:40 -0700
changeset 473799 d7133ef39f36
parent 473798 2826a36d480d
child 473800 f7ffdfcae3e7
push id1739
push userryanvm@gmail.com
push date2018-07-02 20:22 +0000
treeherdermozilla-release@d7133ef39f36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, RyanVM
bugs1472018, 1364624
milestone61.0.1
Bug 1472018 - Limit the lock scope in WebCryptoThreadPool::Shutdown. r=bz, a=RyanVM In bug 1364624 we switched over to SRWLock on Windows for our internal implementation of mozilla::Mutex. This doesn't allow for re-entrancy. The WebCryptoThreadPool shutdown code has potential for re-entrancy due to the spinning of the main thread event loop while shutting down the worker threads. By limiting the scope of the lock protecting mPool during shutdown we can avoid the re-entrancy. Addtionally we track the shutdown status to avoid dispatching events once shutdown has started.
dom/crypto/WebCryptoThreadPool.cpp
dom/crypto/WebCryptoThreadPool.h
--- a/dom/crypto/WebCryptoThreadPool.cpp
+++ b/dom/crypto/WebCryptoThreadPool.cpp
@@ -61,16 +61,20 @@ WebCryptoThreadPool::Init()
   return obs->AddObserver(this, NS_XPCOM_SHUTDOWN_THREADS_OBSERVER_ID, false);
 }
 
 nsresult
 WebCryptoThreadPool::DispatchInternal(nsIRunnable* aRunnable)
 {
   MutexAutoLock lock(mMutex);
 
+  if (mShutdown) {
+    return NS_ERROR_FAILURE;
+  }
+
   if (!mPool) {
     NS_ENSURE_TRUE(EnsureNSSInitializedChromeOrContent(), NS_ERROR_FAILURE);
 
     nsCOMPtr<nsIThreadPool> pool(do_CreateInstance(NS_THREADPOOL_CONTRACTID));
     NS_ENSURE_TRUE(pool, NS_ERROR_FAILURE);
 
     nsresult rv = pool->SetName(NS_LITERAL_CSTRING("SubtleCrypto"));
     NS_ENSURE_SUCCESS(rv, rv);
@@ -80,20 +84,31 @@ WebCryptoThreadPool::DispatchInternal(ns
 
   return mPool->Dispatch(aRunnable, NS_DISPATCH_NORMAL);
 }
 
 void
 WebCryptoThreadPool::Shutdown()
 {
   MOZ_ASSERT(NS_IsMainThread(), "Wrong thread!");
-  MutexAutoLock lock(mMutex);
 
-  if (mPool) {
-    mPool->Shutdown();
+  // Limit the scope of locking to avoid deadlocking if DispatchInternal ends
+  // up getting called during shutdown event processing.
+  nsCOMPtr<nsIThreadPool> pool;
+  {
+    MutexAutoLock lock(mMutex);
+    if (mShutdown) {
+      return;
+    }
+    pool = mPool;
+    mShutdown = true;
+  }
+
+  if (pool) {
+    pool->Shutdown();
   }
 
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   NS_WARNING_ASSERTION(obs, "Failed to retrieve observer service!");
 
   if (obs) {
     if (NS_FAILED(obs->RemoveObserver(this,
                                       NS_XPCOM_SHUTDOWN_THREADS_OBSERVER_ID))) {
--- a/dom/crypto/WebCryptoThreadPool.h
+++ b/dom/crypto/WebCryptoThreadPool.h
@@ -24,16 +24,17 @@ public:
 
   static nsresult
   Dispatch(nsIRunnable* aRunnable);
 
 private:
   WebCryptoThreadPool()
     : mMutex("WebCryptoThreadPool::mMutex")
     , mPool(nullptr)
+    , mShutdown(false)
   { }
   virtual ~WebCryptoThreadPool()
   { }
 
   nsresult
   Init();
 
   nsresult
@@ -43,14 +44,15 @@ private:
   Shutdown();
 
   NS_IMETHOD Observe(nsISupports* aSubject,
                      const char* aTopic,
                      const char16_t* aData) override;
 
   mozilla::Mutex mMutex;
   nsCOMPtr<nsIThreadPool> mPool;
+  bool mShutdown;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_WebCryptoThreadPool_h