Bug 716167 - Only push null contexts in XPConnect for main thread events, and remove infrastructure from bug 326777. r=bz

js/xpconnect/src/nsXPConnect.cpp

@@ -49,11 +49,11 @@
 #include "jsatom.h"
 #include "jsfriendapi.h"
 #include "jsgc.h"
-#include "nsThreadUtilsInternal.h"
 #include "dom_quickstubs.h"
 #include "nsNullPrincipal.h"
 #include "nsIURI.h"
 #include "nsJSEnvironment.h"
+#include "nsThreadUtils.h"
 
 #include "XrayWrapper.h"
 #include "WrapperFactory.h"
@@ -66,6 +66,7 @@
 
 #include "mozilla/Assertions.h"
 #include "mozilla/Base64.h"
+#include "mozilla/Util.h"
 
 #include "nsWrapperCacheInlines.h"
 
@@ -108,6 +109,7 @@ nsXPConnect::nsXPConnect()
         mDefaultSecurityManagerFlags(0),
         mShuttingDown(false),
         mNeedGCBeforeCC(true),
+        mEventDepth(0),
         mCycleCollectionContext(nsnull)
 {
     mRuntime = XPCJSRuntime::newXPCJSRuntime(this);
@@ -184,7 +186,14 @@ nsXPConnect::GetXPConnect()
         // Initial extra ref to keep the singleton alive
         // balanced by explicit call to ReleaseXPConnectSingleton()
         NS_ADDREF(gSelf);
-        if (NS_FAILED(NS_SetGlobalThreadObserver(gSelf))) {
+
+        // Add XPConnect as an thread observer.
+        //
+        // The cycle collector sometimes calls GetXPConnect, but it should never
+        // be the one that initializes gSelf.
+        MOZ_ASSERT(NS_IsMainThread());
+        nsCOMPtr<nsIThreadInternal> thread = do_QueryInterface(NS_GetCurrentThread());
+        if (NS_FAILED(thread->AddObserver(gSelf))) {
             NS_RELEASE(gSelf);
             // Fall through to returning null
         }
@@ -207,7 +216,14 @@ nsXPConnect::ReleaseXPConnectSingleton()
 {
     nsXPConnect* xpc = gSelf;
     if (xpc) {
-        NS_SetGlobalThreadObserver(nsnull);
+
+        // The thread subsystem may have been shut down already, so make sure
+        // to check for null here.
+        nsCOMPtr<nsIThreadInternal> thread = do_QueryInterface(NS_GetCurrentThread());
+        if (thread) {
+            MOZ_ASSERT(NS_IsMainThread());
+            thread->RemoveObserver(xpc);
+        }
 
 #ifdef DEBUG
         // force a dump of the JavaScript gc heap if JS is still alive
@@ -2354,8 +2370,12 @@ NS_IMETHODIMP
 nsXPConnect::OnProcessNextEvent(nsIThreadInternal *aThread, bool aMayWait,
                                 PRUint32 aRecursionDepth)
 {
+    // Record this event.
+    mEventDepth++;
+
     // Push a null JSContext so that we don't see any script during
     // event processing.
+    MOZ_ASSERT(NS_IsMainThread());
     return Push(nsnull);
 }
 
@@ -2363,10 +2383,14 @@ NS_IMETHODIMP
 nsXPConnect::AfterProcessNextEvent(nsIThreadInternal *aThread,
                                    PRUint32 aRecursionDepth)
 {
+    // Watch out for unpaired events during observer registration.
+    if (NS_UNLIKELY(mEventDepth == 0))
+        return NS_OK;
+    mEventDepth--;
+
     // Call cycle collector occasionally.
-    if (NS_IsMainThread()) {
+    MOZ_ASSERT(NS_IsMainThread());
-        nsJSContext::MaybePokeCC();
+    nsJSContext::MaybePokeCC();
-    }
 
     return Pop(nsnull);
 }

js/xpconnect/src/xpcprivate.h

@@ -597,6 +597,13 @@ private:
     PRUint16                 mDefaultSecurityManagerFlags;
     JSBool                   mShuttingDown;
     JSBool                   mNeedGCBeforeCC;
+
+    // nsIThreadInternal doesn't remember which observers it called
+    // OnProcessNextEvent on when it gets around to calling AfterProcessNextEvent.
+    // So if XPConnect gets initialized mid-event (which can happen), we'll get
+    // an 'after' notification without getting an 'on' notification. If we don't
+    // watch out for this, we'll do an unmatched |pop| on the context stack.
+    PRUint16                   mEventDepth;
 #ifdef DEBUG_CC
     PLDHashTable             mJSRoots;
 #endif

xpcom/threads/Makefile.in

@@ -68,7 +68,6 @@ EXPORTS		= \
 		nsThread.h \
 		nsProcess.h \
 		nsEventQueue.h \
-		nsThreadUtilsInternal.h \
 		$(NULL)
 
 EXPORTS_mozilla = \

xpcom/threads/nsThread.cpp

@@ -44,7 +44,6 @@
 #include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
 #include "prlog.h"
-#include "nsThreadUtilsInternal.h"
 #include "nsIObserverService.h"
 #include "mozilla/HangMonitor.h"
 #include "mozilla/Services.h"
@@ -81,8 +80,6 @@ static PRLogModuleInfo *sLog = PR_NewLogModule("nsThread");
 
 NS_DECL_CI_INTERFACE_GETTER(nsThread)
 
-nsIThreadObserver* nsThread::sGlobalObserver;
-
 namespace mozilla {
 
 // Fun fact: Android's GCC won't convert bool* to PRInt32*, so we can't
@@ -617,11 +614,6 @@ nsThread::ProcessNextEvent(bool mayWait, bool *result)
     }
   }
 
-  bool notifyGlobalObserver = (sGlobalObserver != nsnull);
-  if (notifyGlobalObserver) 
-    sGlobalObserver->OnProcessNextEvent(this, mayWait && !ShuttingDown(),
-                                        mRunningEvent);
-
   nsCOMPtr<nsIThreadObserver> obs = mObserver;
   if (obs)
     obs->OnProcessNextEvent(this, mayWait && !ShuttingDown(), mRunningEvent);
@@ -677,9 +669,6 @@ nsThread::ProcessNextEvent(bool mayWait, bool *result)
   if (obs)
     obs->AfterProcessNextEvent(this, mRunningEvent);
 
-  if (notifyGlobalObserver && sGlobalObserver)
-    sGlobalObserver->AfterProcessNextEvent(this, mRunningEvent);
-
   return rv;
 }
 
@@ -844,18 +833,3 @@ nsThreadSyncDispatch::Run()
   }
   return NS_OK;
 }
-
-nsresult
-NS_SetGlobalThreadObserver(nsIThreadObserver* aObserver)
-{
-  if (aObserver && nsThread::sGlobalObserver) {
-    return NS_ERROR_NOT_AVAILABLE;
-  }
-
-  if (!NS_IsMainThread()) {
-    return NS_ERROR_UNEXPECTED;
-  }
-
-  nsThread::sGlobalObserver = aObserver;
-  return NS_OK;
-}

xpcom/threads/nsThread.h

@@ -80,9 +80,6 @@ public:
   // Clear the observer list.
   void ClearObservers() { mEventObservers.Clear(); }
 
-  // The global thread observer
-  static nsIThreadObserver* sGlobalObserver;
-
 private:
   friend class nsThreadShutdownEvent;
 

xpcom/threads/nsThreadUtilsInternal.h

@@ -1,61 +0,0 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim:set ts=2 sw=2 sts=2 et cindent: */
-/* ***** BEGIN LICENSE BLOCK *****
- * Version: MPL 1.1/GPL 2.0/LGPL 2.1
- *
- * The contents of this file are subject to the Mozilla Public License Version
- * 1.1 (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- * http://www.mozilla.org/MPL/
- *
- * Software distributed under the License is distributed on an "AS IS" basis,
- * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
- * for the specific language governing rights and limitations under the
- * License.
- *
- * The Original Code is Mozilla code.
- *
- * The Initial Developer of the Original Code is the Mozilla Corporation.
- * Portions created by the Initial Developer are Copyright (C) 2007
- * the Initial Developer. All Rights Reserved.
- *
- * Contributor(s):
- *  Boris Zbarsky <bzbarsky@mit.edu>
- *
- * Alternatively, the contents of this file may be used under the terms of
- * either the GNU General Public License Version 2 or later (the "GPL"), or
- * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
- * in which case the provisions of the GPL or the LGPL are applicable instead
- * of those above. If you wish to allow use of your version of this file only
- * under the terms of either the GPL or the LGPL, and not to allow others to
- * use your version of this file under the terms of the MPL, indicate your
- * decision by deleting the provisions above and replace them with the notice
- * and other provisions required by the GPL or the LGPL. If you do not delete
- * the provisions above, a recipient may use your version of this file under
- * the terms of any one of the MPL, the GPL or the LGPL.
- *
- * ***** END LICENSE BLOCK ***** */
-
-#ifndef nsThreadUtilsInternal_h_
-#define nsThreadUtilsInternal_h_
-
-#ifdef MOZILLA_INTERNAL_API
-
-class nsIThreadObserver;
-
-/**
- * Function to set a "global" thread observer that all threads will notify when
- * they process an event.  This observer will not be notified when events are
- * posted to threads.  Only one global observer can be set at a time; an
- * attempt to change the value without setting it to null first will throw.
- * This function does NOT take a reference to the observer; the caller of this
- * function is responsible for setting the observer to null when it goes away.
- * This method may only be called on the main thread; attempts to do it on
- * other threads will return an error.
- */
-extern nsresult
-NS_SetGlobalThreadObserver(nsIThreadObserver* aObserver);
-
-#endif // MOZILLA_INTERNAL_API
-
-#endif  // nsThreadUtilsInternal_h_