Bug 988698: Ensure threads can't be started during nsThreadManager::Shutdown() r=nfroyd
authorRandell Jesup <rjesup@jesup.org>
Mon, 23 Mar 2015 16:49:09 -0400
changeset 265562 5627301b95de8f603d8a1624d1a9f826474cef20
parent 265561 e609a7d24a153a916ac261527b1d9b35ebb2170d
child 265563 5329cda711c8f3ca57768a3f37e6a4f17996acd9
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd
bugs988698
milestone39.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 988698: Ensure threads can't be started during nsThreadManager::Shutdown() r=nfroyd
xpcom/threads/nsThreadManager.cpp
xpcom/threads/nsThreadManager.h
--- a/xpcom/threads/nsThreadManager.cpp
+++ b/xpcom/threads/nsThreadManager.cpp
@@ -150,17 +150,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");
@@ -189,29 +188,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.
   //
@@ -229,43 +230,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?
 
@@ -273,17 +273,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);
@@ -333,55 +333,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
@@ -417,17 +421,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);