Bug 1222101 - Reorder some thread code (r=jld) a=kwierso
authorBill McCloskey <billm@mozilla.com>
Wed, 30 Mar 2016 12:20:20 -0700
changeset 291102 c91015c43d5556059210a2cffac02e7a5c547ed5
parent 291101 da683f5c0a43b8ee64b604406d784667b474f6d1
child 291103 62f68029c49a1cdab61b2db68dd46828e67bbfce
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjld, kwierso
bugs1222101
milestone48.0a1
Bug 1222101 - Reorder some thread code (r=jld) a=kwierso MozReview-Commit-ID: Alg40mu9aU3
ipc/chromium/src/base/message_loop.cc
ipc/chromium/src/base/message_loop.h
ipc/chromium/src/base/thread.cc
ipc/glue/MessagePump.cpp
ipc/glue/MessagePump.h
xpcom/threads/nsThread.cpp
--- a/ipc/chromium/src/base/message_loop.cc
+++ b/ipc/chromium/src/base/message_loop.cc
@@ -84,17 +84,17 @@ static LPTOP_LEVEL_EXCEPTION_FILTER GetT
 
 // static
 MessageLoop* MessageLoop::current() {
   return get_tls_ptr().Get();
 }
 
 static mozilla::Atomic<int32_t> message_loop_id_seq(0);
 
-MessageLoop::MessageLoop(Type type)
+MessageLoop::MessageLoop(Type type, nsIThread* aThread)
     : type_(type),
       id_(++message_loop_id_seq),
       nestable_tasks_allowed_(true),
       exception_restoration_(false),
       state_(NULL),
       run_depth_base_(1),
 #ifdef OS_WIN
       os_modal_loop_(false),
@@ -102,33 +102,35 @@ MessageLoop::MessageLoop(Type type)
       transient_hang_timeout_(0),
       permanent_hang_timeout_(0),
       next_sequence_num_(0) {
   DCHECK(!current()) << "should only have one message loop per thread";
   get_tls_ptr().Set(this);
 
   switch (type_) {
   case TYPE_MOZILLA_PARENT:
-    pump_ = new mozilla::ipc::MessagePump();
+    MOZ_RELEASE_ASSERT(!aThread);
+    pump_ = new mozilla::ipc::MessagePump(aThread);
     return;
   case TYPE_MOZILLA_CHILD:
+    MOZ_RELEASE_ASSERT(!aThread);
     pump_ = new mozilla::ipc::MessagePumpForChildProcess();
     // There is a MessageLoop Run call from XRE_InitChildProcess
     // and another one from MessagePumpForChildProcess. The one
     // from MessagePumpForChildProcess becomes the base, so we need
     // to set run_depth_base_ to 2 or we'll never be able to process
     // Idle tasks.
     run_depth_base_ = 2;
     return;
   case TYPE_MOZILLA_NONMAINTHREAD:
-    pump_ = new mozilla::ipc::MessagePumpForNonMainThreads();
+    pump_ = new mozilla::ipc::MessagePumpForNonMainThreads(aThread);
     return;
 #if defined(OS_WIN)
   case TYPE_MOZILLA_NONMAINUITHREAD:
-    pump_ = new mozilla::ipc::MessagePumpForNonMainUIThreads();
+    pump_ = new mozilla::ipc::MessagePumpForNonMainUIThreads(aThread);
     return;
 #endif
   default:
     // Create one of Chromium's standard MessageLoop types below.
     break;
   }
 
 #if defined(OS_WIN)
--- a/ipc/chromium/src/base/message_loop.h
+++ b/ipc/chromium/src/base/message_loop.h
@@ -22,16 +22,18 @@
 // really just eliminate.
 #include "base/message_pump_win.h"
 #elif defined(OS_POSIX)
 #include "base/message_pump_libevent.h"
 #endif
 
 #include "nsAutoPtr.h"
 
+class nsIThread;
+
 namespace mozilla {
 namespace ipc {
 
 class DoWorkRunnable;
 
 } /* namespace ipc */
 } /* namespace mozilla */
 
@@ -218,17 +220,17 @@ public:
     TYPE_MOZILLA_CHILD,
     TYPE_MOZILLA_PARENT,
     TYPE_MOZILLA_NONMAINTHREAD,
     TYPE_MOZILLA_NONMAINUITHREAD
   };
 
   // Normally, it is not necessary to instantiate a MessageLoop.  Instead, it
   // is typical to make use of the current thread's MessageLoop instance.
-  explicit MessageLoop(Type type = TYPE_DEFAULT);
+  explicit MessageLoop(Type type = TYPE_DEFAULT, nsIThread* aThread = nullptr);
   ~MessageLoop();
 
   // Returns the type passed to the constructor.
   Type type() const { return type_; }
 
   // Unique, non-repeating ID for this message loop.
   int32_t id() const { return id_; }
 
--- a/ipc/chromium/src/base/thread.cc
+++ b/ipc/chromium/src/base/thread.cc
@@ -4,16 +4,17 @@
 
 #include "base/thread.h"
 
 #include "base/string_util.h"
 #include "base/thread_local.h"
 #include "base/waitable_event.h"
 #include "GeckoProfiler.h"
 #include "mozilla/IOInterposer.h"
+#include "nsThreadUtils.h"
 
 #ifdef MOZ_TASK_TRACER
 #include "GeckoTaskTracer.h"
 #endif
 
 namespace base {
 
 // This task is used to trigger the message loop to exit.
@@ -146,17 +147,18 @@ void Thread::StopSoon() {
 }
 
 void Thread::ThreadMain() {
   char aLocal;
   profiler_register_thread(name_.c_str(), &aLocal);
   mozilla::IOInterposer::RegisterCurrentThread();
 
   // The message loop for this thread.
-  MessageLoop message_loop(startup_data_->options.message_loop_type);
+  MessageLoop message_loop(startup_data_->options.message_loop_type,
+                           NS_GetCurrentThread());
 
   // Complete the initialization of our Thread object.
   thread_id_ = PlatformThread::CurrentId();
   PlatformThread::SetName(name_.c_str());
   message_loop.set_thread_name(name_);
   message_loop.set_hang_timeouts(startup_data_->options.transient_hang_timeout,
                                  startup_data_->options.permanent_hang_timeout);
   message_loop_ = &message_loop;
--- a/ipc/glue/MessagePump.cpp
+++ b/ipc/glue/MessagePump.cpp
@@ -61,45 +61,46 @@ private:
   MessagePump* mPump;
   // DoWorkRunnable is designed as a stateless singleton.  Do not add stateful
   // members here!
 };
 
 } /* namespace ipc */
 } /* namespace mozilla */
 
-MessagePump::MessagePump()
-: mThread(nullptr)
+MessagePump::MessagePump(nsIThread* aThread)
+: mThread(aThread)
 {
   mDoWorkEvent = new DoWorkRunnable(this);
 }
 
 MessagePump::~MessagePump()
 {
 }
 
 void
 MessagePump::Run(MessagePump::Delegate* aDelegate)
 {
   MOZ_ASSERT(keep_running_);
-  MOZ_ASSERT(NS_IsMainThread(),
-             "Use mozilla::ipc::MessagePumpForNonMainThreads instead!");
+  MOZ_RELEASE_ASSERT(NS_IsMainThread(),
+		     "Use mozilla::ipc::MessagePumpForNonMainThreads instead!");
+  MOZ_RELEASE_ASSERT(!mThread);
 
-  mThread = NS_GetCurrentThread();
-  MOZ_ASSERT(mThread);
+  nsIThread* thisThread = NS_GetCurrentThread();
+  MOZ_ASSERT(thisThread);
 
   mDelayedWorkTimer = do_CreateInstance(kNS_TIMER_CID);
   MOZ_ASSERT(mDelayedWorkTimer);
 
   base::ScopedNSAutoreleasePool autoReleasePool;
 
   for (;;) {
     autoReleasePool.Recycle();
 
-    bool did_work = NS_ProcessNextEvent(mThread, false) ? true : false;
+    bool did_work = NS_ProcessNextEvent(thisThread, false) ? true : false;
     if (!keep_running_)
       break;
 
     // NB: it is crucial *not* to directly call |aDelegate->DoWork()|
     // here.  To ensure that MessageLoop tasks and XPCOM events have
     // equal priority, we sensitively rely on processing exactly one
     // Task per DoWorkRunnable XPCOM event.
 
@@ -121,35 +122,34 @@ if (did_work && delayed_work_time_.is_nu
     did_work = aDelegate->DoIdleWork();
     if (!keep_running_)
       break;
 
     if (did_work)
       continue;
 
     // This will either sleep or process an event.
-    NS_ProcessNextEvent(mThread, true);
+    NS_ProcessNextEvent(thisThread, true);
   }
 
 #ifdef MOZ_NUWA_PROCESS
   if (!IsNuwaReady() || !IsNuwaProcess())
 #endif
     mDelayedWorkTimer->Cancel();
 
   keep_running_ = true;
 }
 
 void
 MessagePump::ScheduleWork()
 {
   // Make sure the event loop wakes up.
   if (mThread) {
     mThread->Dispatch(mDoWorkEvent, NS_DISPATCH_NORMAL);
-  }
-  else {
+  } else {
     // Some things (like xpcshell) don't use the app shell and so Run hasn't
     // been called. We still need to wake up the main thread.
     NS_DispatchToMainThread(mDoWorkEvent);
   }
   event_.Signal();
 }
 
 void
@@ -164,16 +164,21 @@ MessagePump::ScheduleWorkForNestedLoop()
 void
 MessagePump::ScheduleDelayedWork(const base::TimeTicks& aDelayedTime)
 {
 #ifdef MOZ_NUWA_PROCESS
   if (IsNuwaReady() && IsNuwaProcess())
     return;
 #endif
 
+  // To avoid racing on mDelayedWorkTimer, we need to be on the same thread as
+  // ::Run().
+  MOZ_RELEASE_ASSERT(NS_GetCurrentThread() == mThread ||
+		     (!mThread && NS_IsMainThread()));
+
   if (!mDelayedWorkTimer) {
     mDelayedWorkTimer = do_CreateInstance(kNS_TIMER_CID);
     if (!mDelayedWorkTimer) {
         // Called before XPCOM has started up? We can't do this correctly.
         NS_WARNING("Delayed task might not run!");
         delayed_work_time_ = aDelayedTime;
         return;
     }
@@ -294,47 +299,47 @@ MessagePumpForChildProcess::Run(base::Me
   // Really run.
   mozilla::ipc::MessagePump::Run(aDelegate);
 }
 
 void
 MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate* aDelegate)
 {
   MOZ_ASSERT(keep_running_);
-  MOZ_ASSERT(!NS_IsMainThread(), "Use mozilla::ipc::MessagePump instead!");
+  MOZ_RELEASE_ASSERT(!NS_IsMainThread(), "Use mozilla::ipc::MessagePump instead!");
 
-  mThread = NS_GetCurrentThread();
-  MOZ_ASSERT(mThread);
+  nsIThread* thread = NS_GetCurrentThread();
+  MOZ_RELEASE_ASSERT(mThread == thread);
 
   mDelayedWorkTimer = do_CreateInstance(kNS_TIMER_CID);
   MOZ_ASSERT(mDelayedWorkTimer);
 
-  if (NS_FAILED(mDelayedWorkTimer->SetTarget(mThread))) {
+  if (NS_FAILED(mDelayedWorkTimer->SetTarget(thread))) {
     MOZ_CRASH("Failed to set timer target!");
   }
 
   // Chromium event notifications to be processed will be received by this
   // event loop as a DoWorkRunnables via ScheduleWork. Chromium events that
-  // were received before our mThread is valid, however, will not generate
+  // were received before our thread is valid, however, will not generate
   // runnable wrappers. We must process any of these before we enter this
   // loop, or we will forever have unprocessed chromium messages in our queue.
   //
   // Note we would like to request a flush of the chromium event queue
   // using a runnable on the xpcom side, but some thread implementations
   // (dom workers) get cranky if we call ScheduleWork here (ScheduleWork
   // calls dispatch on mThread) before the thread processes an event. As
   // such, clear the queue manually.
   while (aDelegate->DoWork()) {
   }
 
   base::ScopedNSAutoreleasePool autoReleasePool;
   for (;;) {
     autoReleasePool.Recycle();
 
-    bool didWork = NS_ProcessNextEvent(mThread, false) ? true : false;
+    bool didWork = NS_ProcessNextEvent(thread, false) ? true : false;
     if (!keep_running_) {
       break;
     }
 
     didWork |= aDelegate->DoDelayedWork(&delayed_work_time_);
 
     if (didWork && delayed_work_time_.is_null()) {
       mDelayedWorkTimer->Cancel();
@@ -353,48 +358,51 @@ MessagePumpForNonMainThreads::Run(base::
       break;
     }
 
     if (didWork) {
       continue;
     }
 
     // This will either sleep or process an event.
-    NS_ProcessNextEvent(mThread, true);
+    NS_ProcessNextEvent(thread, true);
   }
 
   mDelayedWorkTimer->Cancel();
 
   keep_running_ = true;
 }
 
 #if defined(XP_WIN)
 
 NS_IMPL_QUERY_INTERFACE(MessagePumpForNonMainUIThreads, nsIThreadObserver)
 
 #define CHECK_QUIT_STATE { if (state_->should_quit) { break; } }
 
-void MessagePumpForNonMainUIThreads::DoRunLoop()
+void
+MessagePumpForNonMainUIThreads::DoRunLoop()
 {
+  MOZ_RELEASE_ASSERT(!NS_IsMainThread(), "Use mozilla::ipc::MessagePump instead!");
+
   // If this is a chromium thread and no nsThread is associated
   // with it, this call will create a new nsThread.
-  mThread = NS_GetCurrentThread();
-  MOZ_ASSERT(mThread);
+  nsIThread* thread = NS_GetCurrentThread();
+  MOZ_ASSERT(thread);
 
   // Set the main thread observer so we can wake up when
   // xpcom events need to get processed.
-  nsCOMPtr<nsIThreadInternal> ti(do_QueryInterface(mThread));
+  nsCOMPtr<nsIThreadInternal> ti(do_QueryInterface(thread));
   MOZ_ASSERT(ti);
   ti->SetObserver(this);
 
   base::ScopedNSAutoreleasePool autoReleasePool;
   for (;;) {
     autoReleasePool.Recycle();
 
-    bool didWork = NS_ProcessNextEvent(mThread, false);
+    bool didWork = NS_ProcessNextEvent(thread, false);
 
     didWork |= ProcessNextWindowsMessage();
     CHECK_QUIT_STATE
 
     didWork |= state_->delegate->DoWork();
     CHECK_QUIT_STATE
 
     didWork |= state_->delegate->DoDelayedWork(&delayed_work_time_);
@@ -406,17 +414,17 @@ void MessagePumpForNonMainUIThreads::DoR
     if (didWork) {
       continue;
     }
 
     didWork = state_->delegate->DoIdleWork();
     CHECK_QUIT_STATE
 
     SetInWait();
-    bool hasWork = NS_HasPendingEvents(mThread);
+    bool hasWork = NS_HasPendingEvents(thread);
     if (didWork || hasWork) {
       ClearInWait();
       continue;
     }
     WaitForWork(); // Calls MsgWaitForMultipleObjectsEx(QS_ALLINPUT)
     ClearInWait();
   }
 
--- a/ipc/glue/MessagePump.h
+++ b/ipc/glue/MessagePump.h
@@ -25,17 +25,17 @@ namespace ipc {
 
 class DoWorkRunnable;
 
 class MessagePump : public base::MessagePumpDefault
 {
   friend class DoWorkRunnable;
 
 public:
-  MessagePump();
+  explicit MessagePump(nsIThread* aThread);
 
   // From base::MessagePump.
   virtual void
   Run(base::MessagePump::Delegate* aDelegate) override;
 
   // From base::MessagePump.
   virtual void
   ScheduleWork() override;
@@ -51,46 +51,49 @@ public:
 protected:
   virtual ~MessagePump();
 
 private:
   // Only called by DoWorkRunnable.
   void DoDelayedWork(base::MessagePump::Delegate* aDelegate);
 
 protected:
+  nsIThread* mThread;
+
   // mDelayedWorkTimer and mThread are set in Run() by this class or its
   // subclasses.
   nsCOMPtr<nsITimer> mDelayedWorkTimer;
-  nsIThread* mThread;
 
 private:
   // Only accessed by this class.
   RefPtr<DoWorkRunnable> mDoWorkEvent;
 };
 
 class MessagePumpForChildProcess final: public MessagePump
 {
 public:
   MessagePumpForChildProcess()
-  : mFirstRun(true)
+    : MessagePump(nullptr),
+      mFirstRun(true)
   { }
 
   virtual void Run(base::MessagePump::Delegate* aDelegate) override;
 
 private:
   ~MessagePumpForChildProcess()
   { }
 
   bool mFirstRun;
 };
 
 class MessagePumpForNonMainThreads final : public MessagePump
 {
 public:
-  MessagePumpForNonMainThreads()
+  explicit MessagePumpForNonMainThreads(nsIThread* aThread)
+    : MessagePump(aThread)
   { }
 
   virtual void Run(base::MessagePump::Delegate* aDelegate) override;
 
 private:
   ~MessagePumpForNonMainThreads()
   { }
 };
@@ -111,29 +114,26 @@ public:
   NS_IMETHOD_(MozExternalRefCountType) Release(void) override  {
     return 1;
   }
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
 
   NS_DECL_NSITHREADOBSERVER
 
 public:
-  MessagePumpForNonMainUIThreads() :
-    mThread(nullptr),
+  explicit MessagePumpForNonMainUIThreads(nsIThread* aThread) :
     mInWait(false),
     mWaitLock("mInWait")
   {
   }
 
   // The main run loop for this thread.
   virtual void DoRunLoop() override;
 
 protected:
-  nsIThread* mThread;
-
   void SetInWait() {
     MutexAutoLock lock(mWaitLock);
     mInWait = true;
   }
 
   void ClearInWait() {
     MutexAutoLock lock(mWaitLock);
     mInWait = false;
--- a/xpcom/threads/nsThread.cpp
+++ b/xpcom/threads/nsThread.cpp
@@ -385,17 +385,17 @@ nsThread::ThreadFunc(void* aArg)
     }
   }
   event->Run();  // unblocks nsThread::Init
   event = nullptr;
 
   {
     // Scope for MessageLoop.
     nsAutoPtr<MessageLoop> loop(
-      new MessageLoop(MessageLoop::TYPE_MOZILLA_NONMAINTHREAD));
+      new MessageLoop(MessageLoop::TYPE_MOZILLA_NONMAINTHREAD, self));
 
     // Now, process incoming events...
     loop->Run();
 
     BackgroundChild::CloseForCurrentThread();
 
     // NB: The main thread does not shut down here!  It shuts down via
     // nsThreadManager::Shutdown.