Bug 953435 - Browser hang on Mac if an AfterProcessNextEvent callback tries to spin the event loop, r=nfroyd+smichaud
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Thu, 09 Jan 2014 00:10:06 +0200
changeset 162639 e176126d2b4270428f539e422dd38d964471d848
parent 162638 010f5faed9cf9da0703cc69258aceec07a865b89
child 162640 49c6393d37f3dc43e24fd147843fbbb1075e2f04
push id38254
push useropettay@mozilla.com
push dateWed, 08 Jan 2014 22:53:17 +0000
treeherdermozilla-inbound@e176126d2b42 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd
bugs953435
milestone29.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 953435 - Browser hang on Mac if an AfterProcessNextEvent callback tries to spin the event loop, r=nfroyd+smichaud
widget/cocoa/nsAppShell.h
widget/cocoa/nsAppShell.mm
xpcom/threads/LazyIdleThread.cpp
xpcom/threads/nsIThread.idl
xpcom/threads/nsThread.cpp
xpcom/threads/nsThread.h
--- a/widget/cocoa/nsAppShell.h
+++ b/widget/cocoa/nsAppShell.h
@@ -104,15 +104,14 @@ protected:
   uint32_t               mHadMoreEventsCount;
   // Setting kHadMoreEventsCountMax to '10' contributed to a fairly large
   // (about 10%) increase in the number of calls to malloc (though without
   // effecting the total amount of memory used).  Cutting it to '3'
   // reduced the number of calls by 6%-7% (reducing the original regression
   // to 3%-4%).  See bmo bug 395397.
   static const uint32_t  kHadMoreEventsCountMax = 3;
 
-  int32_t            mRecursionDepth;
   int32_t            mNativeEventCallbackDepth;
   // Can be set from different threads, so must be modified atomically
   int32_t            mNativeEventScheduledDepth;
 };
 
 #endif // nsAppShell_h_
--- a/widget/cocoa/nsAppShell.mm
+++ b/widget/cocoa/nsAppShell.mm
@@ -205,17 +205,16 @@ nsAppShell::nsAppShell()
 , mDelegate(nullptr)
 , mCFRunLoop(NULL)
 , mCFRunLoopSource(NULL)
 , mRunningEventLoop(false)
 , mStarted(false)
 , mTerminated(false)
 , mSkippedNativeCallback(false)
 , mHadMoreEventsCount(0)
-, mRecursionDepth(0)
 , mNativeEventCallbackDepth(0)
 , mNativeEventScheduledDepth(0)
 {
   // A Cocoa event loop is running here if (and only if) we've been embedded
   // by a Cocoa app (like Camino).
   mRunningCocoaEmbedded = [NSApp isRunning] ? true : false;
 }
 
@@ -707,21 +706,24 @@ nsAppShell::ProcessNextNativeEvent(bool 
 // won't be any other Gecko event-processing call on the stack (e.g.
 // NS_ProcessNextEvent() or NS_ProcessPendingEvents()).  (In the current
 // nsAppShell implementation, what counts as the "main" event loop is what
 // nsBaseAppShell::NativeEventCallback() does to process Gecko events.  We
 // don't currently use nsBaseAppShell::Run().)
 bool
 nsAppShell::InGeckoMainEventLoop()
 {
-  if ((gXULModalLevel > 0) || (mRecursionDepth > 0))
+  if (gXULModalLevel > 0)
     return false;
   if (mNativeEventCallbackDepth <= 0)
     return false;
-  return true;
+
+  bool isProcessingEvents = false;
+  NS_GetCurrentThread()->GetIsProcessingEvents(&isProcessingEvents);
+  return !isProcessingEvents;
 }
 
 // Run
 //
 // Overrides the base class's Run() method to call [NSApp run] (which spins
 // the native run loop until the application quits).  Since (unlike the base
 // class's Run() method) we don't process any Gecko events here, they need
 // to be processed elsewhere (in NativeEventCallback(), called from
@@ -813,18 +815,16 @@ nsAppShell::Exit(void)
 //
 // public
 NS_IMETHODIMP
 nsAppShell::OnProcessNextEvent(nsIThreadInternal *aThread, bool aMayWait,
                                uint32_t aRecursionDepth)
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;
 
-  mRecursionDepth = aRecursionDepth;
-
   NS_ASSERTION(mAutoreleasePools,
                "No stack on which to store autorelease pool");
 
   NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
   ::CFArrayAppendValue(mAutoreleasePools, pool);
 
   return nsBaseAppShell::OnProcessNextEvent(aThread, aMayWait, aRecursionDepth);
 
@@ -840,18 +840,16 @@ nsAppShell::OnProcessNextEvent(nsIThread
 // public
 NS_IMETHODIMP
 nsAppShell::AfterProcessNextEvent(nsIThreadInternal *aThread,
                                   uint32_t aRecursionDepth,
                                   bool aEventWasProcessed)
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;
 
-  mRecursionDepth = aRecursionDepth;
-
   CFIndex count = ::CFArrayGetCount(mAutoreleasePools);
 
   NS_ASSERTION(mAutoreleasePools && count,
                "Processed an event, but there's no autorelease pool?");
 
   const NSAutoreleasePool* pool = static_cast<const NSAutoreleasePool*>
     (::CFArrayGetValueAtIndex(mAutoreleasePools, count - 1));
   ::CFArrayRemoveValueAtIndex(mAutoreleasePools, count - 1);
--- a/xpcom/threads/LazyIdleThread.cpp
+++ b/xpcom/threads/LazyIdleThread.cpp
@@ -458,16 +458,27 @@ LazyIdleThread::ProcessNextEvent(bool aM
 {
   // This is only supposed to be called from the thread itself so it's not
   // implemented here.
   NS_NOTREACHED("Shouldn't ever call this!");
   return NS_ERROR_UNEXPECTED;
 }
 
 NS_IMETHODIMP
+LazyIdleThread::GetIsProcessingEvents(bool* aIsProcessing)
+{
+  if (mThread) {
+    return mThread->GetIsProcessingEvents(aIsProcessing);
+  }
+
+  *aIsProcessing = false;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 LazyIdleThread::Notify(nsITimer* aTimer)
 {
   ASSERT_OWNING_THREAD();
 
   {
     MutexAutoLock lock(mMutex);
 
     if (mPendingEventCount || mIdleNotificationCount) {
--- a/xpcom/threads/nsIThread.idl
+++ b/xpcom/threads/nsIThread.idl
@@ -12,17 +12,17 @@
  * This interface provides a high-level abstraction for an operating system
  * thread.
  *
  * Threads have a built-in event queue, and a thread is an event target that
  * can receive nsIRunnable objects (events) to be processed on the thread.
  *
  * See nsIThreadManager for the API used to create and locate threads.
  */
-[scriptable, uuid(9c889946-a73a-4af3-ae9a-ea64f7d4e3ca)]
+[scriptable, uuid(4df07d3a-e759-4256-ba4e-7e2265354ec3)]
 interface nsIThread : nsIEventTarget
 {
   /**
    * @returns
    *   The NSPR thread object corresponding to this nsIThread.
    */
   [noscript] readonly attribute PRThread PRThread;
 
@@ -77,9 +77,15 @@ interface nsIThread : nsIEventTarget
    * @returns
    *   A boolean value that if "true" indicates that an event was processed.
    *
    * @throws NS_ERROR_UNEXPECTED
    *   Indicates that this method was erroneously called when this thread was
    *   not the current thread.
    */
   boolean processNextEvent(in boolean mayWait);
+
+  /**
+   * true if we're processing runnables or thread observers and if this is the
+   * current thread.
+   */
+  readonly attribute boolean isProcessingEvents;
 };
--- a/xpcom/threads/nsThread.cpp
+++ b/xpcom/threads/nsThread.cpp
@@ -299,16 +299,17 @@ int sCanaryOutputFD = -1;
 
 nsThread::nsThread(MainThreadFlag aMainThread, uint32_t aStackSize)
   : mLock("nsThread.mLock")
   , mEvents(&mEventsRoot)
   , mPriority(PRIORITY_NORMAL)
   , mThread(nullptr)
   , mRunningEvent(0)
   , mStackSize(aStackSize)
+  , mProcessingEvent(0)
   , mShutdownContext(nullptr)
   , mShutdownRequired(false)
   , mEventsAreDoomed(false)
   , mIsMainThread(aMainThread)
 {
 }
 
 nsThread::~nsThread()
@@ -591,16 +592,18 @@ nsThread::ProcessNextEvent(bool mayWait,
                             mpPending == MemPressure_New ? lowMem.get() :
                                                            lowMemOngoing.get());
       } else {
         NS_WARNING("Can't get observer service!");
       }
     }
   }
 
+  ++mProcessingEvent;
+
   bool notifyMainThreadObserver =
     (MAIN_THREAD == mIsMainThread) && sMainThreadObserver;
   if (notifyMainThreadObserver) 
    sMainThreadObserver->OnProcessNextEvent(this, mayWait && !ShuttingDown(),
                                            mRunningEvent);
 
   nsCOMPtr<nsIThreadObserver> obs = mObserver;
   if (obs)
@@ -645,19 +648,32 @@ nsThread::ProcessNextEvent(bool mayWait,
                          (this, mRunningEvent, *result));
 
   if (obs)
     obs->AfterProcessNextEvent(this, mRunningEvent, *result);
 
   if (notifyMainThreadObserver && sMainThreadObserver)
     sMainThreadObserver->AfterProcessNextEvent(this, mRunningEvent, *result);
 
+  --mProcessingEvent;
+
   return rv;
 }
 
+NS_IMETHODIMP
+nsThread::GetIsProcessingEvents(bool* aIsProcessing)
+{
+  if (NS_WARN_IF(PR_GetCurrentThread() != mThread)) {
+    return NS_ERROR_NOT_SAME_THREAD;
+  }
+
+  *aIsProcessing = mProcessingEvent != 0;
+  return NS_OK;
+}
+
 //-----------------------------------------------------------------------------
 // nsISupportsPriority
 
 NS_IMETHODIMP
 nsThread::GetPriority(int32_t *priority)
 {
   *priority = mPriority;
   return NS_OK;
--- a/xpcom/threads/nsThread.h
+++ b/xpcom/threads/nsThread.h
@@ -145,16 +145,18 @@ protected:
   nsChainedEventQueue *mEvents;   // never null
   nsChainedEventQueue  mEventsRoot;
 
   int32_t   mPriority;
   PRThread *mThread;
   uint32_t  mRunningEvent;  // counter
   uint32_t  mStackSize;
 
+  uint32_t  mProcessingEvent;
+
   struct nsThreadShutdownContext *mShutdownContext;
 
   bool mShutdownRequired;
   // Set to true when events posted to this thread will never run.
   bool mEventsAreDoomed;
   MainThreadFlag mIsMainThread;
 };