Bug 1479035: Part 1 - Don't create event queues for stub nsThread wrappers. r?froydnj draft
authorKris Maglione <maglione.k@gmail.com>
Fri, 27 Jul 2018 15:13:12 -0700
changeset 823810 063cfc1af22ff9822f5f7d3c561708594b090914
parent 823464 ad1674e9152da31151ab9f9f099f83ca4ff2d832
child 823811 0cbd46770f37a269bd7bd0be1431dbb9c9d3b9d8
push id117794
push usermaglione.k@gmail.com
push dateSat, 28 Jul 2018 18:51:57 +0000
reviewersfroydnj
bugs1479035
milestone63.0a1
Bug 1479035: Part 1 - Don't create event queues for stub nsThread wrappers. r?froydnj Most of the times when we automatically create nsThread wrappers for threads that don't already have them, we don't actually need the event targets, since those threads don't run XPCOM event loops. Aside from wasting memory, actually creating these event loops can lead to leaks if a thread tries to dispatch a runnable to the queue which creates a reference cycle with the thread. Not creating the event queues for threads that don't actually need them helps avoid those foot guns, and also makes it easier to figure out which treads actually run XPCOM event loops. MozReview-Commit-ID: Arck4VQqdne
dom/storage/StorageDBThread.cpp
netwerk/cache2/CacheIOThread.cpp
xpcom/threads/nsThread.cpp
xpcom/threads/nsThread.h
xpcom/threads/nsThreadManager.cpp
--- a/dom/storage/StorageDBThread.cpp
+++ b/dom/storage/StorageDBThread.cpp
@@ -20,18 +20,22 @@
 #include "mozIStorageService.h"
 #include "mozIStorageBindingParamsArray.h"
 #include "mozIStorageBindingParams.h"
 #include "mozIStorageValueArray.h"
 #include "mozIStorageFunction.h"
 #include "mozilla/BasePrincipal.h"
 #include "mozilla/ipc/BackgroundParent.h"
 #include "nsIObserverService.h"
+#include "nsThread.h"
+#include "nsThreadManager.h"
 #include "nsVariant.h"
+#include "mozilla/EventQueue.h"
 #include "mozilla/IOInterposer.h"
+#include "mozilla/ThreadEventQueue.h"
 #include "mozilla/Services.h"
 #include "mozilla/Tokenizer.h"
 #include "GeckoProfiler.h"
 
 // How long we collect write oprerations
 // before they are flushed to the database
 // In milliseconds.
 #define FLUSHING_INTERVAL_MS 5000
@@ -483,16 +487,22 @@ StorageDBThread::SetDefaultPriority()
   if (--mPriorityCounter <= 0) {
     PR_SetThreadPriority(mThread, PR_PRIORITY_LOW);
   }
 }
 
 void
 StorageDBThread::ThreadFunc(void* aArg)
 {
+  {
+    auto queue = MakeRefPtr<ThreadEventQueue<EventQueue>>(MakeUnique<EventQueue>());
+    nsCOMPtr<nsIThread> thread =
+      nsThreadManager::get().CreateCurrentThread(queue, nsThread::NOT_MAIN_THREAD);
+  }
+
   AUTO_PROFILER_REGISTER_THREAD("localStorage DB");
   NS_SetCurrentThreadName("localStorage DB");
   mozilla::IOInterposer::RegisterCurrentThread();
 
   StorageDBThread* thread = static_cast<StorageDBThread*>(aArg);
   thread->ThreadFunc();
   mozilla::IOInterposer::UnregisterCurrentThread();
 }
--- a/netwerk/cache2/CacheIOThread.cpp
+++ b/netwerk/cache2/CacheIOThread.cpp
@@ -3,18 +3,22 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "CacheIOThread.h"
 #include "CacheFileIOManager.h"
 
 #include "nsIRunnable.h"
 #include "nsISupportsImpl.h"
 #include "nsPrintfCString.h"
+#include "nsThread.h"
+#include "nsThreadManager.h"
 #include "nsThreadUtils.h"
+#include "mozilla/EventQueue.h"
 #include "mozilla/IOInterposer.h"
+#include "mozilla/ThreadEventQueue.h"
 #include "GeckoProfiler.h"
 
 #ifdef XP_WIN
 #include <windows.h>
 #endif
 
 #ifdef MOZ_TASK_TRACER
 #include "GeckoTaskTracer.h"
@@ -456,18 +460,20 @@ void CacheIOThread::ThreadFunc()
   nsCOMPtr<nsIThreadInternal> threadInternal;
 
   {
     MonitorAutoLock lock(mMonitor);
 
     MOZ_ASSERT(mBlockingIOWatcher);
     mBlockingIOWatcher->InitThread();
 
-    // This creates nsThread for this PRThread
-    nsCOMPtr<nsIThread> xpcomThread = NS_GetCurrentThread();
+    auto queue = MakeRefPtr<ThreadEventQueue<mozilla::EventQueue>>(
+      MakeUnique<mozilla::EventQueue>());
+    nsCOMPtr<nsIThread> xpcomThread =
+      nsThreadManager::get().CreateCurrentThread(queue, nsThread::NOT_MAIN_THREAD);
 
     threadInternal = do_QueryInterface(xpcomThread);
     if (threadInternal)
       threadInternal->SetObserver(this);
 
     mXPCOMThread = xpcomThread.forget().take();
 
     lock.NotifyAll();
--- a/xpcom/threads/nsThread.cpp
+++ b/xpcom/threads/nsThread.cpp
@@ -421,16 +421,19 @@ nsThread::Enumerate()
 /*static*/ void
 nsThread::ThreadFunc(void* aArg)
 {
   using mozilla::ipc::BackgroundChild;
 
   ThreadInitData* initData = static_cast<ThreadInitData*>(aArg);
   nsThread* self = initData->thread;  // strong reference
 
+  MOZ_ASSERT(self->mEventTarget);
+  MOZ_ASSERT(self->mEvents);
+
   self->mThread = PR_GetCurrentThread();
   self->mVirtualThread = GetCurrentVirtualThread();
   self->mEventTarget->SetCurrentThread();
   SetupCurrentThreadForChaosMode();
 
   if (!initData->name.IsEmpty()) {
     NS_SetCurrentThreadName(initData->name.BeginReading());
   }
@@ -678,16 +681,37 @@ nsThread::nsThread(NotNull<SynchronizedE
   , mIsMainThread(uint8_t(aMainThread))
   , mCanInvokeJS(false)
   , mCurrentEvent(nullptr)
   , mCurrentEventStart(TimeStamp::Now())
   , mCurrentPerformanceCounter(nullptr)
 {
 }
 
+
+nsThread::nsThread()
+  : mEvents(nullptr)
+  , mEventTarget(nullptr)
+  , mShutdownContext(nullptr)
+  , mScriptObserver(nullptr)
+  , mThread(nullptr)
+  , mStackSize(0)
+  , mNestedEventLoopDepth(0)
+  , mCurrentEventLoopDepth(-1)
+  , mShutdownRequired(false)
+  , mPriority(PRIORITY_NORMAL)
+  , mIsMainThread(NOT_MAIN_THREAD)
+  , mCanInvokeJS(false)
+  , mCurrentEvent(nullptr)
+  , mCurrentEventStart(TimeStamp::Now())
+  , mCurrentPerformanceCounter(nullptr)
+{
+  MOZ_ASSERT(!NS_IsMainThread());
+}
+
 nsThread::~nsThread()
 {
   NS_ASSERTION(mRequestedShutdownContexts.IsEmpty(),
                "shouldn't be waiting on other threads to shutdown");
 
   // We shouldn't need to lock before checking isInList at this point. We're
   // destroying the last reference to this object, so there's no way for anyone
   // else to remove it in the middle of our check. And the not-in-list state is
@@ -709,16 +733,19 @@ nsThread::~nsThread()
     Unused << mRequestedShutdownContexts[i].forget();
   }
 #endif
 }
 
 nsresult
 nsThread::Init(const nsACString& aName)
 {
+  MOZ_ASSERT(mEvents);
+  MOZ_ASSERT(mEventTarget);
+
   // spawn thread and wait until it is fully setup
   RefPtr<nsThreadStartupEvent> startup = new nsThreadStartupEvent();
 
   NS_ADDREF_THIS();
 
   mShutdownRequired = true;
 
   ThreadInitData initData = { this, aName };
@@ -757,38 +784,51 @@ nsThread::InitCurrentThread()
 }
 
 //-----------------------------------------------------------------------------
 // nsIEventTarget
 
 NS_IMETHODIMP
 nsThread::DispatchFromScript(nsIRunnable* aEvent, uint32_t aFlags)
 {
+  MOZ_ASSERT(mEventTarget);
+  NS_ENSURE_TRUE(mEventTarget, NS_ERROR_NOT_IMPLEMENTED);
+
   nsCOMPtr<nsIRunnable> event(aEvent);
   return mEventTarget->Dispatch(event.forget(), aFlags);
 }
 
 NS_IMETHODIMP
 nsThread::Dispatch(already_AddRefed<nsIRunnable> aEvent, uint32_t aFlags)
 {
+  MOZ_ASSERT(mEventTarget);
+  NS_ENSURE_TRUE(mEventTarget, NS_ERROR_NOT_IMPLEMENTED);
+
   LOG(("THRD(%p) Dispatch [%p %x]\n", this, /* XXX aEvent */nullptr, aFlags));
 
   return mEventTarget->Dispatch(std::move(aEvent), aFlags);
 }
 
 NS_IMETHODIMP
 nsThread::DelayedDispatch(already_AddRefed<nsIRunnable> aEvent, uint32_t aDelayMs)
 {
+  MOZ_ASSERT(mEventTarget);
+  NS_ENSURE_TRUE(mEventTarget, NS_ERROR_NOT_IMPLEMENTED);
+
   return mEventTarget->DelayedDispatch(std::move(aEvent), aDelayMs);
 }
 
 NS_IMETHODIMP
 nsThread::IsOnCurrentThread(bool* aResult)
 {
-  return mEventTarget->IsOnCurrentThread(aResult);
+  if (mEventTarget) {
+    return mEventTarget->IsOnCurrentThread(aResult);
+  }
+  *aResult = GetCurrentVirtualThread() == mVirtualThread;
+  return NS_OK;
 }
 
 NS_IMETHODIMP_(bool)
 nsThread::IsOnCurrentThreadInfallible()
 {
   // Rely on mVirtualThread being correct.
   MOZ_CRASH("IsOnCurrentThreadInfallible should never be called on nsIThread");
 }
@@ -830,16 +870,18 @@ nsThread::AsyncShutdown()
   }
 
   return !!ShutdownInternal(/* aSync = */ false) ? NS_OK : NS_ERROR_UNEXPECTED;
 }
 
 nsThreadShutdownContext*
 nsThread::ShutdownInternal(bool aSync)
 {
+  MOZ_ASSERT(mEvents);
+  MOZ_ASSERT(mEventTarget);
   MOZ_ASSERT(mThread);
   MOZ_ASSERT(mThread != PR_GetCurrentThread());
   if (NS_WARN_IF(mThread == PR_GetCurrentThread())) {
     return nullptr;
   }
 
   // Prevent multiple calls to this method
   if (!mShutdownRequired.compareExchange(true, false)) {
@@ -871,16 +913,18 @@ nsThread::ShutdownInternal(bool aSync)
   // task, but that's okay because we process pending events in ThreadFunc
   // after setting mShutdownContext just before exiting.
   return context;
 }
 
 void
 nsThread::ShutdownComplete(NotNull<nsThreadShutdownContext*> aContext)
 {
+  MOZ_ASSERT(mEvents);
+  MOZ_ASSERT(mEventTarget);
   MOZ_ASSERT(mThread);
   MOZ_ASSERT(aContext->mTerminatingThread == this);
 
   {
     OffTheBooksMutexAutoLock mal(ThreadListMutex());
     if (isInList()) {
       removeFrom(ThreadList());
     }
@@ -1094,16 +1138,19 @@ size_t
 nsThread::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
 {
   return ShallowSizeOfIncludingThis(aMallocSizeOf) + SizeOfEventQueues(aMallocSizeOf);
 }
 
 NS_IMETHODIMP
 nsThread::ProcessNextEvent(bool aMayWait, bool* aResult)
 {
+  MOZ_ASSERT(mEvents);
+  NS_ENSURE_TRUE(mEvents, NS_ERROR_NOT_IMPLEMENTED);
+
   LOG(("THRD(%p) ProcessNextEvent [%u %u]\n", this, aMayWait,
        mNestedEventLoopDepth));
 
   if (NS_WARN_IF(PR_GetCurrentThread() != mThread)) {
     return NS_ERROR_NOT_SAME_THREAD;
   }
 
   // When recording or replaying, execute triggers that were activated
@@ -1328,24 +1375,30 @@ nsThread::AdjustPriority(int32_t aDelta)
 }
 
 //-----------------------------------------------------------------------------
 // nsIThreadInternal
 
 NS_IMETHODIMP
 nsThread::GetObserver(nsIThreadObserver** aObs)
 {
+  MOZ_ASSERT(mEvents);
+  NS_ENSURE_TRUE(mEvents, NS_ERROR_NOT_IMPLEMENTED);
+
   nsCOMPtr<nsIThreadObserver> obs = mEvents->GetObserver();
   obs.forget(aObs);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThread::SetObserver(nsIThreadObserver* aObs)
 {
+  MOZ_ASSERT(mEvents);
+  NS_ENSURE_TRUE(mEvents, NS_ERROR_NOT_IMPLEMENTED);
+
   if (NS_WARN_IF(PR_GetCurrentThread() != mThread)) {
     return NS_ERROR_NOT_SAME_THREAD;
   }
 
   mEvents->SetObserver(aObs);
   return NS_OK;
 }
 
@@ -1354,31 +1407,37 @@ nsThread::RecursionDepth() const
 {
   MOZ_ASSERT(PR_GetCurrentThread() == mThread);
   return mNestedEventLoopDepth;
 }
 
 NS_IMETHODIMP
 nsThread::AddObserver(nsIThreadObserver* aObserver)
 {
+  MOZ_ASSERT(mEvents);
+  NS_ENSURE_TRUE(mEvents, NS_ERROR_NOT_IMPLEMENTED);
+
   if (NS_WARN_IF(!aObserver)) {
     return NS_ERROR_INVALID_ARG;
   }
   if (NS_WARN_IF(PR_GetCurrentThread() != mThread)) {
     return NS_ERROR_NOT_SAME_THREAD;
   }
 
   EventQueue()->AddObserver(aObserver);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThread::RemoveObserver(nsIThreadObserver* aObserver)
 {
+  MOZ_ASSERT(mEvents);
+  NS_ENSURE_TRUE(mEvents, NS_ERROR_NOT_IMPLEMENTED);
+
   if (NS_WARN_IF(PR_GetCurrentThread() != mThread)) {
     return NS_ERROR_NOT_SAME_THREAD;
   }
 
   EventQueue()->RemoveObserver(aObserver);
 
   return NS_OK;
 }
--- a/xpcom/threads/nsThread.h
+++ b/xpcom/threads/nsThread.h
@@ -55,16 +55,18 @@ public:
     MAIN_THREAD,
     NOT_MAIN_THREAD
   };
 
   nsThread(NotNull<mozilla::SynchronizedEventQueue*> aQueue,
            MainThreadFlag aMainThread,
            uint32_t aStackSize);
 
+  nsThread();
+
   // Initialize this as a wrapper for a new PRThread, and optionally give it a name.
   nsresult Init(const nsACString& aName = NS_LITERAL_CSTRING(""));
 
   // Initialize this as a wrapper for the current PRThread.
   nsresult InitCurrentThread();
 
 private:
   // Initializes the mThreadId and stack base/size members, and adds the thread
--- a/xpcom/threads/nsThreadManager.cpp
+++ b/xpcom/threads/nsThreadManager.cpp
@@ -442,17 +442,16 @@ nsThreadManager::CreateCurrentThread(Syn
 {
   // Make sure we don't have an nsThread yet.
   MOZ_ASSERT(!PR_GetThreadPrivate(mCurThreadIndex));
 
   if (!mInitialized) {
     return nullptr;
   }
 
-  // OK, that's fine.  We'll dynamically create one :-)
   RefPtr<nsThread> thread = new nsThread(WrapNotNull(aQueue), aMainThread, 0);
   if (!thread || NS_FAILED(thread->InitCurrentThread())) {
     return nullptr;
   }
 
   return thread.get();  // reference held in TLS
 }
 
@@ -465,34 +464,32 @@ nsThreadManager::GetCurrentThread()
     return static_cast<nsThread*>(data);
   }
 
   if (!mInitialized) {
     return nullptr;
   }
 
   // OK, that's fine.  We'll dynamically create one :-)
-  RefPtr<ThreadEventQueue<EventQueue>> queue =
-    new ThreadEventQueue<EventQueue>(MakeUnique<EventQueue>());
-  RefPtr<nsThread> thread = new nsThread(WrapNotNull(queue), nsThread::NOT_MAIN_THREAD, 0);
+  RefPtr<nsThread> thread = new nsThread();
   if (!thread || NS_FAILED(thread->InitCurrentThread())) {
     return nullptr;
   }
 
   return thread.get();  // reference held in TLS
 }
 
 bool
 nsThreadManager::IsNSThread() const
 {
   if (!mInitialized) {
     return false;
   }
   if (auto* thread = (nsThread*)PR_GetThreadPrivate(mCurThreadIndex)) {
-    return thread->mShutdownRequired;
+    return thread->EventQueue();
   }
   return false;
 }
 
 NS_IMETHODIMP
 nsThreadManager::NewThread(uint32_t aCreationFlags,
                            uint32_t aStackSize,
                            nsIThread** aResult)