Bug 988698: Ensure threads can't be started during nsThreadManager::Shutdown() r=nfroyd a=abillings
authorRandell Jesup <rjesup@jesup.org>
Mon, 23 Mar 2015 16:49:09 -0400
changeset 248388 1915368f79fe9f1bc8b1dec80211adca29da324f
parent 248387 52d27124efe7df8c3f39aa9526847552648b2b30
child 248389 9b7464221b8ace296ce545aeb36fc9a2a8eb6cde
push id7830
push userrjesup@wgate.com
push dateWed, 25 Mar 2015 19:42:57 +0000
treeherdermozilla-aurora@1915368f79fe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd, abillings
bugs988698
milestone38.0a2
Bug 988698: Ensure threads can't be started during nsThreadManager::Shutdown() r=nfroyd a=abillings
xpcom/threads/nsThreadManager.cpp
xpcom/threads/nsThreadManager.h
--- a/xpcom/threads/nsThreadManager.cpp
+++ b/xpcom/threads/nsThreadManager.cpp
@@ -157,17 +157,16 @@ nsThreadManager::Init()
 #ifdef MOZ_NUWA_PROCESS
   if (PR_NewThreadPrivateIndex(
       &mThreadStatusInfoIndex,
       nsThreadManager::DeleteThreadStatusInfo) == PR_FAILURE) {
     return NS_ERROR_FAILURE;
   }
 #endif // MOZ_NUWA_PROCESS
 
-  mLock = new Mutex("nsThreadManager.mLock");
 #ifdef MOZ_NUWA_PROCESS
   mMonitor = MakeUnique<ReentrantMonitor>("nsThreadManager.mMonitor");
 #endif // MOZ_NUWA_PROCESS
 
 #ifdef MOZ_CANARY
   const int flags = O_WRONLY | O_APPEND | O_CREAT | O_NONBLOCK;
   const mode_t mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH;
   char* env_var_flag = getenv("MOZ_KILL_CANARIES");
@@ -202,29 +201,31 @@ nsThreadManager::Init()
 
 void
 nsThreadManager::Shutdown()
 {
   MOZ_ASSERT(NS_IsMainThread(), "shutdown not called from main thread");
 
   // Prevent further access to the thread manager (no more new threads!)
   //
-  // XXX What happens if shutdown happens before NewThread completes?
-  //     Fortunately, NewThread is only called on the main thread for now.
+  // What happens if shutdown happens before NewThread completes?
+  // We Shutdown() the new thread, and return error if we've started Shutdown
+  // between when NewThread started, and when the thread finished initializing
+  // and registering with ThreadManager.
   //
   mInitialized = false;
 
   // Empty the main thread event queue before we begin shutting down threads.
   NS_ProcessPendingEvents(mMainThread);
 
   // We gather the threads from the hashtable into a list, so that we avoid
   // holding the hashtable lock while calling nsIThread::Shutdown.
   nsThreadArray threads;
   {
-    MutexAutoLock lock(*mLock);
+    OffTheBooksMutexAutoLock lock(mLock);
     mThreadsByPRThread.Enumerate(AppendAndRemoveThread, &threads);
   }
 
   // It's tempting to walk the list of threads here and tell them each to stop
   // accepting new events, but that could lead to badness if one of those
   // threads is stuck waiting for a response from another thread.  To do it
   // right, we'd need some way to interrupt the threads.
   //
@@ -242,43 +243,42 @@ nsThreadManager::Shutdown()
 
   // In case there are any more events somehow...
   NS_ProcessPendingEvents(mMainThread);
 
   // There are no more background threads at this point.
 
   // Clear the table of threads.
   {
-    MutexAutoLock lock(*mLock);
+    OffTheBooksMutexAutoLock lock(mLock);
     mThreadsByPRThread.Clear();
   }
 
   // Normally thread shutdown clears the observer for the thread, but since the
   // main thread is special we do it manually here after we're sure all events
   // have been processed.
   mMainThread->SetObserver(nullptr);
   mMainThread->ClearObservers();
 
   // Release main thread object.
   mMainThread = nullptr;
-  mLock = nullptr;
 
   // Remove the TLS entry for the main thread.
   PR_SetThreadPrivate(mCurThreadIndex, nullptr);
 #ifdef MOZ_NUWA_PROCESS
   PR_SetThreadPrivate(mThreadStatusInfoIndex, nullptr);
 #endif
 }
 
 void
 nsThreadManager::RegisterCurrentThread(nsThread* aThread)
 {
   MOZ_ASSERT(aThread->GetPRThread() == PR_GetCurrentThread(), "bad aThread");
 
-  MutexAutoLock lock(*mLock);
+  OffTheBooksMutexAutoLock lock(mLock);
 
   ++mCurrentNumberOfThreads;
   if (mCurrentNumberOfThreads > mHighestNumberOfThreads) {
     mHighestNumberOfThreads = mCurrentNumberOfThreads;
   }
 
   mThreadsByPRThread.Put(aThread->GetPRThread(), aThread);  // XXX check OOM?
 
@@ -286,17 +286,17 @@ nsThreadManager::RegisterCurrentThread(n
   PR_SetThreadPrivate(mCurThreadIndex, aThread);
 }
 
 void
 nsThreadManager::UnregisterCurrentThread(nsThread* aThread)
 {
   MOZ_ASSERT(aThread->GetPRThread() == PR_GetCurrentThread(), "bad aThread");
 
-  MutexAutoLock lock(*mLock);
+  OffTheBooksMutexAutoLock lock(mLock);
 
   --mCurrentNumberOfThreads;
   mThreadsByPRThread.Remove(aThread->GetPRThread());
 
   PR_SetThreadPrivate(mCurThreadIndex, nullptr);
   // Ref-count balanced via ReleaseObject
 #ifdef MOZ_NUWA_PROCESS
   PR_SetThreadPrivate(mThreadStatusInfoIndex, nullptr);
@@ -346,55 +346,59 @@ nsThreadManager::GetCurrentThreadStatusI
 }
 #endif
 
 NS_IMETHODIMP
 nsThreadManager::NewThread(uint32_t aCreationFlags,
                            uint32_t aStackSize,
                            nsIThread** aResult)
 {
+  // Note: can be called from arbitrary threads
+  
   // No new threads during Shutdown
   if (NS_WARN_IF(!mInitialized)) {
     return NS_ERROR_NOT_INITIALIZED;
   }
 
-  nsThread* thr = new nsThread(nsThread::NOT_MAIN_THREAD, aStackSize);
-  if (!thr) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-  NS_ADDREF(thr);
-
-  nsresult rv = thr->Init();
+  nsRefPtr<nsThread> thr = new nsThread(nsThread::NOT_MAIN_THREAD, aStackSize);
+  nsresult rv = thr->Init();  // Note: blocks until the new thread has been set up
   if (NS_FAILED(rv)) {
-    NS_RELEASE(thr);
     return rv;
   }
 
-  // At this point, we expect that the thread has been registered in mThread;
+  // At this point, we expect that the thread has been registered in mThreadByPRThread;
   // however, it is possible that it could have also been replaced by now, so
-  // we cannot really assert that it was added.
+  // we cannot really assert that it was added.  Instead, kill it if we entered
+  // Shutdown() during/before Init()
 
-  *aResult = thr;
+  if (NS_WARN_IF(!mInitialized)) {
+    if (thr->ShutdownRequired()) {
+      thr->Shutdown(); // ok if it happens multiple times
+    }
+    return NS_ERROR_NOT_INITIALIZED;
+  }
+
+  thr.forget(aResult);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThreadManager::GetThreadFromPRThread(PRThread* aThread, nsIThread** aResult)
 {
   // Keep this functioning during Shutdown
   if (NS_WARN_IF(!mMainThread)) {
     return NS_ERROR_NOT_INITIALIZED;
   }
   if (NS_WARN_IF(!aThread)) {
     return NS_ERROR_INVALID_ARG;
   }
 
   nsRefPtr<nsThread> temp;
   {
-    MutexAutoLock lock(*mLock);
+    OffTheBooksMutexAutoLock lock(mLock);
     mThreadsByPRThread.Get(aThread, getter_AddRefs(temp));
   }
 
   NS_IF_ADDREF(*aResult = temp);
   return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -430,17 +434,17 @@ nsThreadManager::GetIsMainThread(bool* a
 
   *aResult = (PR_GetCurrentThread() == mMainPRThread);
   return NS_OK;
 }
 
 uint32_t
 nsThreadManager::GetHighestNumberOfThreads()
 {
-  MutexAutoLock lock(*mLock);
+  OffTheBooksMutexAutoLock lock(mLock);
   return mHighestNumberOfThreads;
 }
 
 #ifdef MOZ_NUWA_PROCESS
 void
 nsThreadManager::SetIgnoreThreadStatus()
 {
   GetCurrentThreadStatusInfo()->mIgnored = true;
--- a/xpcom/threads/nsThreadManager.h
+++ b/xpcom/threads/nsThreadManager.h
@@ -95,36 +95,34 @@ public:
   void RemoveAllThreadsWereIdleListener(AllThreadsWereIdleListener *listener);
   ThreadStatusInfo* GetCurrentThreadStatusInfo();
 #endif // MOZ_NUWA_PROCESS
 
 private:
   nsThreadManager()
     : mCurThreadIndex(0)
     , mMainPRThread(nullptr)
-    , mLock(nullptr)
+    , mLock("nsThreadManager.mLock")
     , mInitialized(false)
     , mCurrentNumberOfThreads(1)
     , mHighestNumberOfThreads(1)
 #ifdef MOZ_NUWA_PROCESS
     , mMonitor(nullptr)
     , mMainThreadStatusInfo(nullptr)
     , mDispatchingToMainThread(nullptr)
 #endif
   {
   }
 
   nsRefPtrHashtable<nsPtrHashKey<PRThread>, nsThread> mThreadsByPRThread;
-  unsigned             mCurThreadIndex;  // thread-local-storage index
+  unsigned            mCurThreadIndex;  // thread-local-storage index
   nsRefPtr<nsThread>  mMainThread;
   PRThread*           mMainPRThread;
-  // This is a pointer in order to allow creating nsThreadManager from
-  // the static context in debug builds.
-  nsAutoPtr<mozilla::Mutex> mLock;  // protects tables
-  bool                mInitialized;
+  mozilla::OffTheBooksMutex mLock;  // protects tables
+  mozilla::Atomic<bool> mInitialized;
 
   // The current number of threads
   uint32_t            mCurrentNumberOfThreads;
   // The highest number of threads encountered so far during the session
   uint32_t            mHighestNumberOfThreads;
 
 #ifdef MOZ_NUWA_PROCESS
   static void DeleteThreadStatusInfo(void *aData);