Bug 1299389 - Replace some raw pointers in nsThreadManager. r=froydnj.
authorNicholas Nethercote <nnethercote@mozilla.com>
Fri, 10 Jun 2016 16:04:49 +1000
changeset 312376 88dd53397d30e1853122f1284de94f103f6e4436
parent 312375 c212e496d0ce0dcdd52828c8beaa592f29d240cb
child 312377 5ad925dd2e4e9f6943b228f0173d01278a74c2a8
push id20447
push userkwierso@gmail.com
push dateFri, 02 Sep 2016 20:36:44 +0000
treeherderfx-team@969397f22187 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1299389
milestone51.0a1
Bug 1299389 - Replace some raw pointers in nsThreadManager. r=froydnj. nsThreadManager::get() can return a reference. This lets us remove some redundant assertions. nsThreadArray elements can be NotNull<>s.
dom/ipc/ContentChild.cpp
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerThread.cpp
image/DecodePool.cpp
toolkit/components/telemetry/Telemetry.cpp
xpcom/build/XPCOMInit.cpp
xpcom/glue/nsThreadUtils.cpp
xpcom/threads/nsThread.cpp
xpcom/threads/nsThreadManager.cpp
xpcom/threads/nsThreadManager.h
xpcom/threads/nsThreadPool.cpp
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -567,17 +567,17 @@ ContentChild::Init(MessageLoop* aIOLoop,
   XRE_InstallX11ErrorHandler();
 #endif
 
   NS_ASSERTION(!sSingleton, "only one ContentChild per child");
 
   // Once we start sending IPC messages, we need the thread manager to be
   // initialized so we can deal with the responses. Do that here before we
   // try to construct the crash reporter.
-  nsresult rv = nsThreadManager::get()->Init();
+  nsresult rv = nsThreadManager::get().Init();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return false;
   }
 
   if (!Open(aChannel, aParentPid, aIOLoop)) {
     return false;
   }
   sSingleton = this;
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -6428,22 +6428,20 @@ void
 WorkerPrivate::AssertIsOnWorkerThread() const
 {
   // This is much more complicated than it needs to be but we can't use mThread
   // because it must be protected by mMutex and sometimes this method is called
   // when mMutex is already locked. This method should always work.
   MOZ_ASSERT(mPRThread,
              "AssertIsOnWorkerThread() called before a thread was assigned!");
 
-  MOZ_ASSERT(nsThreadManager::get());
-
   nsCOMPtr<nsIThread> thread;
   nsresult rv =
-    nsThreadManager::get()->GetThreadFromPRThread(mPRThread,
-                                                  getter_AddRefs(thread));
+    nsThreadManager::get().GetThreadFromPRThread(mPRThread,
+                                                 getter_AddRefs(thread));
   MOZ_ASSERT(NS_SUCCEEDED(rv));
   MOZ_ASSERT(thread);
 
   bool current;
   rv = thread->IsOnCurrentThread(&current);
   MOZ_ASSERT(NS_SUCCEEDED(rv));
   MOZ_ASSERT(current, "Wrong thread!");
 }
--- a/dom/workers/WorkerThread.cpp
+++ b/dom/workers/WorkerThread.cpp
@@ -81,18 +81,16 @@ WorkerThread::~WorkerThread()
   MOZ_ASSERT(!mOtherThreadsDispatchingViaEventTarget);
   MOZ_ASSERT(mAcceptingNonWorkerRunnables);
 }
 
 // static
 already_AddRefed<WorkerThread>
 WorkerThread::Create(const WorkerThreadFriendKey& /* aKey */)
 {
-  MOZ_ASSERT(nsThreadManager::get());
-
   RefPtr<WorkerThread> thread = new WorkerThread();
   if (NS_FAILED(thread->Init())) {
     NS_WARNING("Failed to create new thread!");
     return nullptr;
   }
 
   return thread.forget();
 }
--- a/image/DecodePool.cpp
+++ b/image/DecodePool.cpp
@@ -163,17 +163,17 @@ public:
 
   NS_IMETHOD Run() override
   {
     MOZ_ASSERT(!NS_IsMainThread());
 
     mImpl->InitCurrentThread();
 
     nsCOMPtr<nsIThread> thisThread;
-    nsThreadManager::get()->GetCurrentThread(getter_AddRefs(thisThread));
+    nsThreadManager::get().GetCurrentThread(getter_AddRefs(thisThread));
 
     do {
       Work work = mImpl->PopWork();
       switch (work.mType) {
         case Work::Type::TASK:
           work.mTask->Run();
           break;
 
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -1218,17 +1218,17 @@ TelemetryImpl::GetWebrtcStats(JSContext 
   if (mWebrtcTelemetry.GetWebrtcStats(cx, ret))
     return NS_OK;
   return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 TelemetryImpl::GetMaximalNumberOfConcurrentThreads(uint32_t *ret)
 {
-  *ret = nsThreadManager::get()->GetHighestNumberOfThreads();
+  *ret = nsThreadManager::get().GetHighestNumberOfThreads();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 TelemetryImpl::GetChromeHangs(JSContext *cx, JS::MutableHandle<JS::Value> ret)
 {
   MutexAutoLock hangReportMutex(mHangReportsMutex);
 
--- a/xpcom/build/XPCOMInit.cpp
+++ b/xpcom/build/XPCOMInit.cpp
@@ -243,17 +243,17 @@ nsThreadManagerGetSingleton(nsISupports*
                             const nsIID& aIID,
                             void** aInstancePtr)
 {
   NS_ASSERTION(aInstancePtr, "null outptr");
   if (NS_WARN_IF(aOuter)) {
     return NS_ERROR_NO_AGGREGATION;
   }
 
-  return nsThreadManager::get()->QueryInterface(aIID, aInstancePtr);
+  return nsThreadManager::get().QueryInterface(aIID, aInstancePtr);
 }
 
 NS_GENERIC_FACTORY_CONSTRUCTOR(nsThreadPool)
 
 static nsresult
 nsXPTIInterfaceInfoManagerGetSingleton(nsISupports* aOuter,
                                        const nsIID& aIID,
                                        void** aInstancePtr)
@@ -537,17 +537,17 @@ NS_InitXPCOM2(nsIServiceManager** aResul
     if (NS_WARN_IF(!ioThread->StartWithOptions(options))) {
       return NS_ERROR_FAILURE;
     }
 
     sIOThread = ioThread.release();
   }
 
   // Establish the main thread here.
-  rv = nsThreadManager::get()->Init();
+  rv = nsThreadManager::get().Init();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   // Set up the timer globals/timer thread
   rv = nsTimerImpl::Startup();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
@@ -770,17 +770,17 @@ NS_InitMinimalXPCOM()
   mozilla::TimeStamp::Startup();
   NS_LogInit();
   NS_InitAtomTable();
   mozilla::LogModule::Init();
 
   char aLocal;
   profiler_init(&aLocal);
 
-  nsresult rv = nsThreadManager::get()->Init();
+  nsresult rv = nsThreadManager::get().Init();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   // Set up the timer globals/timer thread.
   rv = nsTimerImpl::Startup();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
@@ -909,17 +909,17 @@ ShutdownXPCOM(nsIServiceManager* aServMg
     // shutting down the component manager
     nsTimerImpl::Shutdown();
 
     NS_ProcessPendingEvents(thread);
 
     // Shutdown all remaining threads.  This method does not return until
     // all threads created using the thread manager (with the exception of
     // the main thread) have exited.
-    nsThreadManager::get()->Shutdown();
+    nsThreadManager::get().Shutdown();
 
     NS_ProcessPendingEvents(thread);
 
     HangMonitor::NotifyActivity();
 
     // Late-write checks needs to find the profile directory, so it has to
     // be initialized before mozilla::services::Shutdown or (because of
     // xpcshell tests replacing the service) modules being unloaded.
--- a/xpcom/glue/nsThreadUtils.cpp
+++ b/xpcom/glue/nsThreadUtils.cpp
@@ -56,18 +56,18 @@ CancelableRunnable::Cancel()
 //-----------------------------------------------------------------------------
 
 nsresult
 NS_NewThread(nsIThread** aResult, nsIRunnable* aEvent, uint32_t aStackSize)
 {
   nsCOMPtr<nsIThread> thread;
 #ifdef MOZILLA_INTERNAL_API
   nsresult rv =
-    nsThreadManager::get()->nsThreadManager::NewThread(0, aStackSize,
-                                                       getter_AddRefs(thread));
+    nsThreadManager::get().nsThreadManager::NewThread(0, aStackSize,
+                                                      getter_AddRefs(thread));
 #else
   nsresult rv;
   nsCOMPtr<nsIThreadManager> mgr =
     do_GetService(NS_THREADMANAGER_CONTRACTID, &rv);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
@@ -88,33 +88,33 @@ NS_NewThread(nsIThread** aResult, nsIRun
   thread.swap(*aResult);
   return NS_OK;
 }
 
 nsresult
 NS_GetCurrentThread(nsIThread** aResult)
 {
 #ifdef MOZILLA_INTERNAL_API
-  return nsThreadManager::get()->nsThreadManager::GetCurrentThread(aResult);
+  return nsThreadManager::get().nsThreadManager::GetCurrentThread(aResult);
 #else
   nsresult rv;
   nsCOMPtr<nsIThreadManager> mgr =
     do_GetService(NS_THREADMANAGER_CONTRACTID, &rv);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   return mgr->GetCurrentThread(aResult);
 #endif
 }
 
 nsresult
 NS_GetMainThread(nsIThread** aResult)
 {
 #ifdef MOZILLA_INTERNAL_API
-  return nsThreadManager::get()->nsThreadManager::GetMainThread(aResult);
+  return nsThreadManager::get().nsThreadManager::GetMainThread(aResult);
 #else
   nsresult rv;
   nsCOMPtr<nsIThreadManager> mgr =
     do_GetService(NS_THREADMANAGER_CONTRACTID, &rv);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   return mgr->GetMainThread(aResult);
@@ -339,17 +339,17 @@ NS_SetThreadName(nsIThread* aThread, con
 }
 
 #endif
 
 #ifdef MOZILLA_INTERNAL_API
 nsIThread*
 NS_GetCurrentThread()
 {
-  return nsThreadManager::get()->GetCurrentThread();
+  return nsThreadManager::get().GetCurrentThread();
 }
 #endif
 
 // nsThreadPoolNaming
 void
 nsThreadPoolNaming::SetThreadPoolName(const nsACString& aPoolName,
                                       nsIThread* aThread)
 {
--- a/xpcom/threads/nsThread.cpp
+++ b/xpcom/threads/nsThread.cpp
@@ -429,17 +429,17 @@ nsThread::ThreadFunc(void* aArg)
 {
   using mozilla::ipc::BackgroundChild;
 
   nsThread* self = static_cast<nsThread*>(aArg);  // strong reference
   self->mThread = PR_GetCurrentThread();
   SetupCurrentThreadForChaosMode();
 
   // Inform the ThreadManager
-  nsThreadManager::get()->RegisterCurrentThread(self);
+  nsThreadManager::get().RegisterCurrentThread(*self);
 
   mozilla::IOInterposer::RegisterCurrentThread();
 
   // Wait for and process startup event
   nsCOMPtr<nsIRunnable> event;
   {
     MutexAutoLock lock(self->mLock);
     if (!self->mEvents->GetEvent(true, getter_AddRefs(event), lock)) {
@@ -486,17 +486,17 @@ nsThread::ThreadFunc(void* aArg)
       }
       NS_ProcessPendingEvents(self);
     }
   }
 
   mozilla::IOInterposer::UnregisterCurrentThread();
 
   // Inform the threadmanager that this thread is going away
-  nsThreadManager::get()->UnregisterCurrentThread(self);
+  nsThreadManager::get().UnregisterCurrentThread(*self);
 
   // Dispatch shutdown ACK
   NotNull<nsThreadShutdownContext*> context =
     WrapNotNull(self->mShutdownContext);
   MOZ_ASSERT(context->mTerminatingThread == self);
   event = do_QueryObject(new nsThreadShutdownAckEvent(context));
   context->mJoiningThread->Dispatch(event, NS_DISPATCH_NORMAL);
 
@@ -649,17 +649,17 @@ nsThread::Init()
 }
 
 nsresult
 nsThread::InitCurrentThread()
 {
   mThread = PR_GetCurrentThread();
   SetupCurrentThreadForChaosMode();
 
-  nsThreadManager::get()->RegisterCurrentThread(this);
+  nsThreadManager::get().RegisterCurrentThread(*this);
   return NS_OK;
 }
 
 nsresult
 nsThread::PutEvent(nsIRunnable* aEvent, nsNestedEventTarget* aTarget)
 {
   nsCOMPtr<nsIRunnable> event(aEvent);
   return PutEvent(event.forget(), aTarget);
@@ -715,17 +715,17 @@ nsThread::DispatchInternal(already_AddRe
 #ifdef MOZ_TASK_TRACER
   nsCOMPtr<nsIRunnable> tracedRunnable = CreateTracedRunnable(event.take());
   (static_cast<TracedRunnable*>(tracedRunnable.get()))->DispatchTask();
   // XXX tracedRunnable will always leaked when we fail to disptch.
   event = tracedRunnable.forget();
 #endif
 
   if (aFlags & DISPATCH_SYNC) {
-    nsThread* thread = nsThreadManager::get()->GetCurrentThread();
+    nsThread* thread = nsThreadManager::get().GetCurrentThread();
     if (NS_WARN_IF(!thread)) {
       return NS_ERROR_NOT_AVAILABLE;
     }
 
     // XXX we should be able to do something better here... we should
     //     be able to monitor the slot occupied by this event and use
     //     that to tell us when the event has been processed.
 
@@ -828,17 +828,17 @@ nsThread::ShutdownInternal(bool aSync)
     MutexAutoLock lock(mLock);
     if (!mShutdownRequired) {
       return nullptr;
     }
     mShutdownRequired = false;
   }
 
   NotNull<nsThread*> currentThread =
-    WrapNotNull(nsThreadManager::get()->GetCurrentThread());
+    WrapNotNull(nsThreadManager::get().GetCurrentThread());
 
   nsAutoPtr<nsThreadShutdownContext>& context =
     *currentThread->mRequestedShutdownContexts.AppendElement();
   context = new nsThreadShutdownContext(WrapNotNull(this), currentThread, aSync);
 
   // Set mShutdownContext and wake up the thread in case it is waiting for
   // events to process.
   nsCOMPtr<nsIRunnable> event =
--- a/xpcom/threads/nsThreadManager.cpp
+++ b/xpcom/threads/nsThreadManager.cpp
@@ -31,17 +31,17 @@ NS_SetMainThread()
 {
   if (!sTLSIsMainThread.init()) {
     MOZ_CRASH();
   }
   sTLSIsMainThread.set(true);
   MOZ_ASSERT(NS_IsMainThread());
 }
 
-typedef nsTArray<RefPtr<nsThread>> nsThreadArray;
+typedef nsTArray<NotNull<RefPtr<nsThread>>> nsThreadArray;
 
 //-----------------------------------------------------------------------------
 
 static void
 ReleaseObject(void* aData)
 {
   static_cast<nsISupports*>(aData)->Release();
 }
@@ -126,33 +126,33 @@ nsThreadManager::Shutdown()
 
   // We gather the threads from the hashtable into a list, so that we avoid
   // holding the hashtable lock while calling nsIThread::Shutdown.
   nsThreadArray threads;
   {
     OffTheBooksMutexAutoLock lock(mLock);
     for (auto iter = mThreadsByPRThread.Iter(); !iter.Done(); iter.Next()) {
       RefPtr<nsThread>& thread = iter.Data();
-      threads.AppendElement(thread);
+      threads.AppendElement(WrapNotNull(thread));
       iter.Remove();
     }
   }
 
   // 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.
   //
   // Instead, we process events on the current thread while waiting for threads
   // to shutdown.  This means that we have to preserve a mostly functioning
   // world until such time as the threads exit.
 
   // Shutdown all threads that require it (join with threads that we created).
   for (uint32_t i = 0; i < threads.Length(); ++i) {
-    nsThread* thread = threads[i];
+    NotNull<nsThread*> thread = threads[i];
     if (thread->ShutdownRequired()) {
       thread->Shutdown();
     }
   }
 
   // NB: It's possible that there are events in the queue that want to *start*
   // an asynchronous shutdown. But we have already shutdown the threads above,
   // so there's no need to worry about them. We only have to wait for all
@@ -179,42 +179,42 @@ nsThreadManager::Shutdown()
   // Release main thread object.
   mMainThread = nullptr;
 
   // Remove the TLS entry for the main thread.
   PR_SetThreadPrivate(mCurThreadIndex, nullptr);
 }
 
 void
-nsThreadManager::RegisterCurrentThread(nsThread* aThread)
+nsThreadManager::RegisterCurrentThread(nsThread& aThread)
 {
-  MOZ_ASSERT(aThread->GetPRThread() == PR_GetCurrentThread(), "bad aThread");
+  MOZ_ASSERT(aThread.GetPRThread() == PR_GetCurrentThread(), "bad aThread");
 
   OffTheBooksMutexAutoLock lock(mLock);
 
   ++mCurrentNumberOfThreads;
   if (mCurrentNumberOfThreads > mHighestNumberOfThreads) {
     mHighestNumberOfThreads = mCurrentNumberOfThreads;
   }
 
-  mThreadsByPRThread.Put(aThread->GetPRThread(), aThread);  // XXX check OOM?
+  mThreadsByPRThread.Put(aThread.GetPRThread(), &aThread);  // XXX check OOM?
 
-  NS_ADDREF(aThread);  // for TLS entry
-  PR_SetThreadPrivate(mCurThreadIndex, aThread);
+  aThread.AddRef();  // for TLS entry
+  PR_SetThreadPrivate(mCurThreadIndex, &aThread);
 }
 
 void
-nsThreadManager::UnregisterCurrentThread(nsThread* aThread)
+nsThreadManager::UnregisterCurrentThread(nsThread& aThread)
 {
-  MOZ_ASSERT(aThread->GetPRThread() == PR_GetCurrentThread(), "bad aThread");
+  MOZ_ASSERT(aThread.GetPRThread() == PR_GetCurrentThread(), "bad aThread");
 
   OffTheBooksMutexAutoLock lock(mLock);
 
   --mCurrentNumberOfThreads;
-  mThreadsByPRThread.Remove(aThread->GetPRThread());
+  mThreadsByPRThread.Remove(aThread.GetPRThread());
 
   PR_SetThreadPrivate(mCurThreadIndex, nullptr);
   // Ref-count balanced via ReleaseObject
 }
 
 nsThread*
 nsThreadManager::GetCurrentThread()
 {
--- a/xpcom/threads/nsThreadManager.h
+++ b/xpcom/threads/nsThreadManager.h
@@ -15,35 +15,35 @@
 class nsIRunnable;
 
 class nsThreadManager : public nsIThreadManager
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSITHREADMANAGER
 
-  static nsThreadManager* get()
+  static nsThreadManager& get()
   {
     static nsThreadManager sInstance;
-    return &sInstance;
+    return sInstance;
   }
 
   nsresult Init();
 
   // Shutdown all threads.  This function should only be called on the main
   // thread of the application process.
   void Shutdown();
 
   // Called by nsThread to inform the ThreadManager it exists.  This method
   // must be called when the given thread is the current thread.
-  void RegisterCurrentThread(nsThread* aThread);
+  void RegisterCurrentThread(nsThread& aThread);
 
   // Called by nsThread to inform the ThreadManager it is going away.  This
   // method must be called when the given thread is the current thread.
-  void UnregisterCurrentThread(nsThread* aThread);
+  void UnregisterCurrentThread(nsThread& aThread);
 
   // Returns the current thread.  Returns null if OOM or if ThreadManager isn't
   // initialized.
   nsThread* GetCurrentThread();
 
   // Returns the maximal number of threads that have been in existence
   // simultaneously during the execution of the thread manager.
   uint32_t GetHighestNumberOfThreads();
@@ -63,17 +63,17 @@ private:
     , mCurrentNumberOfThreads(1)
     , mHighestNumberOfThreads(1)
   {
   }
 
   nsRefPtrHashtable<nsPtrHashKey<PRThread>, nsThread> mThreadsByPRThread;
   unsigned            mCurThreadIndex;  // thread-local-storage index
   RefPtr<nsThread>  mMainThread;
-  PRThread*           mMainPRThread;
+  PRThread*         mMainPRThread;
   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;
 };
--- a/xpcom/threads/nsThreadPool.cpp
+++ b/xpcom/threads/nsThreadPool.cpp
@@ -98,19 +98,17 @@ nsThreadPool::PutEvent(already_AddRefed<
   }
 
   LOG(("THRD-P(%p) put [spawn=%d]\n", this, spawnThread));
   if (!spawnThread) {
     return NS_OK;
   }
 
   nsCOMPtr<nsIThread> thread;
-  nsThreadManager::get()->NewThread(0,
-                                    stackSize,
-                                    getter_AddRefs(thread));
+  nsThreadManager::get().NewThread(0, stackSize, getter_AddRefs(thread));
   if (NS_WARN_IF(!thread)) {
     return NS_ERROR_UNEXPECTED;
   }
 
   bool killThread = false;
   {
     MutexAutoLock lock(mMutex);
     if (mThreads.Count() < (int32_t)mThreadLimit) {
@@ -153,17 +151,17 @@ nsThreadPool::ShutdownThread(nsIThread* 
 NS_IMETHODIMP
 nsThreadPool::Run()
 {
   mThreadNaming.SetThreadPoolName(mName);
 
   LOG(("THRD-P(%p) enter %s\n", this, mName.BeginReading()));
 
   nsCOMPtr<nsIThread> current;
-  nsThreadManager::get()->GetCurrentThread(getter_AddRefs(current));
+  nsThreadManager::get().GetCurrentThread(getter_AddRefs(current));
 
   bool shutdownThreadOnExit = false;
   bool exitThread = false;
   bool wasIdle = false;
   PRIntervalTime idleSince;
 
   nsCOMPtr<nsIThreadPoolListener> listener;
   {
@@ -253,17 +251,17 @@ nsThreadPool::Dispatch(already_AddRefed<
   LOG(("THRD-P(%p) dispatch [%p %x]\n", this, /* XXX aEvent*/ nullptr, aFlags));
 
   if (NS_WARN_IF(mShutdown)) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   if (aFlags & DISPATCH_SYNC) {
     nsCOMPtr<nsIThread> thread;
-    nsThreadManager::get()->GetCurrentThread(getter_AddRefs(thread));
+    nsThreadManager::get().GetCurrentThread(getter_AddRefs(thread));
     if (NS_WARN_IF(!thread)) {
       return NS_ERROR_NOT_AVAILABLE;
     }
 
     RefPtr<nsThreadSyncDispatch> wrapper =
       new nsThreadSyncDispatch(thread, Move(aEvent));
     PutEvent(wrapper);