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

--HG--
extra : rebase_source : 02c5572b92ee48c11697d90941336e10c03d49cf

dom/storage/StorageDBThread.cpp

@@ -25,8 +25,12 @@
 #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"
@@ -488,6 +492,12 @@ StorageDBThread::SetDefaultPriority()
 void
 StorageDBThread::ThreadFunc(void* aArg)
 {
+  {
+    auto queue = MakeRefPtr<ThreadEventQueue<EventQueue>>(MakeUnique<EventQueue>());
+    Unused <<
+      nsThreadManager::get().CreateCurrentThread(queue, nsThread::NOT_MAIN_THREAD);
+  }
+
   AUTO_PROFILER_REGISTER_THREAD("localStorage DB");
   NS_SetCurrentThreadName("localStorage DB");
   mozilla::IOInterposer::RegisterCurrentThread();

ipc/chromium/src/base/platform_thread_posix.cc

@@ -35,10 +35,6 @@ void InitThreading();
 #endif
 
 static void* ThreadFunc(void* closure) {
-  // Create a nsThread wrapper for the current platform thread, and register it
-  // with the thread manager.
-  (void) NS_GetCurrentThread();
-
   PlatformThread::Delegate* delegate =
       static_cast<PlatformThread::Delegate*>(closure);
   delegate->ThreadMain();

ipc/chromium/src/base/platform_thread_win.cc

@@ -25,10 +25,6 @@ typedef struct tagTHREADNAME_INFO {
 } THREADNAME_INFO;
 
 DWORD __stdcall ThreadFunc(void* closure) {
-  // Create a nsThread wrapper for the current platform thread, and register it
-  // with the thread manager.
-  (void) NS_GetCurrentThread();
-
   PlatformThread::Delegate* delegate =
       static_cast<PlatformThread::Delegate*>(closure);
   delegate->ThreadMain();

ipc/chromium/src/base/thread.cc

@@ -10,8 +10,11 @@
 #include "base/thread_local.h"
 #include "base/waitable_event.h"
 #include "GeckoProfiler.h"
+#include "mozilla/EventQueue.h"
 #include "mozilla/IOInterposer.h"
+#include "mozilla/ThreadEventQueue.h"
 #include "nsThreadUtils.h"
+#include "nsThreadManager.h"
 
 #ifdef MOZ_TASK_TRACER
 #include "GeckoTaskTracer.h"
@@ -154,12 +157,24 @@ void Thread::StopSoon() {
 }
 
 void Thread::ThreadMain() {
+  nsCOMPtr<nsIThread> xpcomThread;
+  if (startup_data_->options.message_loop_type == MessageLoop::TYPE_MOZILLA_NONMAINTHREAD) {
+    auto queue = mozilla::MakeRefPtr<mozilla::ThreadEventQueue<mozilla::EventQueue>>(
+      mozilla::MakeUnique<mozilla::EventQueue>());
+    xpcomThread =
+      nsThreadManager::get().CreateCurrentThread(queue, nsThread::NOT_MAIN_THREAD);
+  } else {
+    xpcomThread = NS_GetCurrentThread();
+  }
+
   AUTO_PROFILER_REGISTER_THREAD(name_.c_str());
   mozilla::IOInterposer::RegisterCurrentThread();
 
   // The message loop for this thread.
   MessageLoop message_loop(startup_data_->options.message_loop_type,
-                           NS_GetCurrentThread());
+                           xpcomThread);
+
+  xpcomThread = nullptr;
 
   // Complete the initialization of our Thread object.
   thread_id_ = PlatformThread::CurrentId();

netwerk/cache2/CacheIOThread.cpp

@@ -8,8 +8,12 @@
 #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
@@ -461,8 +465,10 @@ void CacheIOThread::ThreadFunc()
     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)

security/sandbox/linux/broker/SandboxBroker.cpp

@@ -31,6 +31,7 @@
 #include "mozilla/ipc/FileDescriptor.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsAppDirectoryServiceDefs.h"
+#include "nsThreadUtils.h"
 #include "SpecialSystemDirectory.h"
 #include "sandbox/linux/system_headers/linux_syscalls.h"
 
@@ -663,6 +664,10 @@ SandboxBroker::SymlinkPermissions(const char* aPath, const size_t aPathLen)
 void
 SandboxBroker::ThreadMain(void)
 {
+  // Create a nsThread wrapper for the current platform thread, and register it
+  // with the thread manager.
+  (void) NS_GetCurrentThread();
+
   char threadName[16];
   SprintfLiteral(threadName, "FS Broker %d", mChildPid);
   PlatformThread::SetName(threadName);

security/sandbox/linux/reporter/SandboxReporter.cpp

@@ -237,6 +237,10 @@ SandboxReporter::AddOne(const SandboxReport& aReport)
 void
 SandboxReporter::ThreadMain(void)
 {
+  // Create a nsThread wrapper for the current platform thread, and register it
+  // with the thread manager.
+  (void) NS_GetCurrentThread();
+
   for (;;) {
     SandboxReport rep;
     struct iovec iov;

xpcom/threads/nsThread.cpp

@@ -424,6 +424,9 @@ nsThread::ThreadFunc(void* aArg)
   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();
@@ -616,6 +619,27 @@ nsThread::nsThread(NotNull<SynchronizedEventQueue*> aQueue,
 {
 }
 
+
+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(),
@@ -647,6 +671,9 @@ nsThread::~nsThread()
 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();
 
@@ -695,6 +722,9 @@ nsThread::InitCurrentThread()
 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);
 }
@@ -702,6 +732,9 @@ nsThread::DispatchFromScript(nsIRunnable* aEvent, uint32_t 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);
@@ -710,13 +743,20 @@ nsThread::Dispatch(already_AddRefed<nsIRunnable> aEvent, uint32_t 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)
@@ -768,6 +808,8 @@ nsThread::AsyncShutdown()
 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())) {
@@ -809,6 +851,8 @@ nsThread::ShutdownInternal(bool aSync)
 void
 nsThread::ShutdownComplete(NotNull<nsThreadShutdownContext*> aContext)
 {
+  MOZ_ASSERT(mEvents);
+  MOZ_ASSERT(mEventTarget);
   MOZ_ASSERT(mThread);
   MOZ_ASSERT(aContext->mTerminatingThread == this);
 
@@ -1032,6 +1076,9 @@ nsThread::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
 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));
 
@@ -1264,6 +1311,9 @@ nsThread::AdjustPriority(int32_t aDelta)
 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;
@@ -1272,6 +1322,9 @@ nsThread::GetObserver(nsIThreadObserver** aObs)
 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;
   }
@@ -1290,6 +1343,9 @@ nsThread::RecursionDepth() const
 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;
   }
@@ -1305,6 +1361,9 @@ nsThread::AddObserver(nsIThreadObserver* aObserver)
 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;
   }

xpcom/threads/nsThread.h

@@ -60,6 +60,10 @@ public:
            MainThreadFlag aMainThread,
            uint32_t aStackSize);
 
+private:
+  nsThread();
+
+public:
   // Initialize this as a wrapper for a new PRThread, and optionally give it a name.
   nsresult Init(const nsACString& aName = NS_LITERAL_CSTRING(""));
 
@@ -174,6 +178,12 @@ protected:
   static mozilla::LinkedList<nsThread>& ThreadList();
   static void ClearThreadList();
 
+  // Whether or not these members have a value determines whether the nsThread
+  // is treated as a full XPCOM thread or as a thin wrapper.
+  //
+  // For full nsThreads, they will always contain valid pointers. For thin
+  // wrappers around non-XPCOM threads, they will be null, and event dispatch
+  // methods which rely on them will fail (and assert) if called.
   RefPtr<mozilla::SynchronizedEventQueue> mEvents;
   RefPtr<mozilla::ThreadEventTarget> mEventTarget;
 

xpcom/threads/nsThreadManager.cpp

@@ -447,7 +447,6 @@ nsThreadManager::CreateCurrentThread(SynchronizedEventQueue* aQueue,
     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;
@@ -470,9 +469,11 @@ nsThreadManager::GetCurrentThread()
   }
 
   // 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);
+  //
+  // We assume that if we're implicitly creating a thread here that it doesn't
+  // want an event queue. Any thread which wants an event queue should
+  // explicitly create its nsThread wrapper.
+  RefPtr<nsThread> thread = new nsThread();
   if (!thread || NS_FAILED(thread->InitCurrentThread())) {
     return nullptr;
   }
@@ -487,7 +488,7 @@ nsThreadManager::IsNSThread() const
     return false;
   }
   if (auto* thread = (nsThread*)PR_GetThreadPrivate(mCurThreadIndex)) {
-    return thread->mShutdownRequired;
+    return thread->EventQueue();
   }
   return false;
 }