Bug 612807 - 'IndexedDB: Transaction thread observer isn't quite safe'. r=jst, a=blocking+.
authorBen Turner <bent.mozilla@gmail.com>
Thu, 18 Nov 2010 14:19:19 -0800
changeset 57871 30484d51bb7b34ec14e98bfb442b438a91944d6c
parent 57870 99a5d8039d3e8134afc04d00ed70dc34fc3e333c
child 57872 364409371144791eab2a44fdb2413219aa5fec3c
push id17053
push userbturner@mozilla.com
push dateThu, 18 Nov 2010 22:19:48 +0000
treeherdermozilla-central@30484d51bb7b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjst, blocking
bugs612807
milestone2.0b8pre
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 612807 - 'IndexedDB: Transaction thread observer isn't quite safe'. r=jst, a=blocking+.
dom/indexedDB/IDBTransaction.cpp
dom/indexedDB/IDBTransaction.h
xpcom/threads/nsIThreadInternal.idl
xpcom/threads/nsThread.cpp
xpcom/threads/nsThread.h
--- a/dom/indexedDB/IDBTransaction.cpp
+++ b/dom/indexedDB/IDBTransaction.cpp
@@ -57,18 +57,16 @@
 #include "TransactionThreadPool.h"
 
 #define SAVEPOINT_NAME "savepoint"
 
 USING_INDEXEDDB_NAMESPACE
 
 namespace {
 
-IDBTransaction::ThreadObserver* gThreadObserver = nsnull;
-
 PLDHashOperator
 DoomCachedStatements(const nsACString& aQuery,
                      nsCOMPtr<mozIStorageStatement>& aStatement,
                      void* aUserArg)
 {
   CommitHelper* helper = static_cast<CommitHelper*>(aUserArg);
   helper->AddDoomedObject(aStatement);
   return PL_DHASH_REMOVE;
@@ -101,30 +99,43 @@ IDBTransaction::Create(IDBDatabase* aDat
   }
 
   if (!transaction->mCachedStatements.Init()) {
     NS_ERROR("Failed to initialize hash!");
     return nsnull;
   }
 
   if (!aDispatchDelayed) {
-    if (!ThreadObserver::BeginObserving(transaction)) {
-      return nsnull;
-    }
+    nsCOMPtr<nsIThreadInternal2> thread =
+      do_QueryInterface(NS_GetCurrentThread());
+    NS_ENSURE_TRUE(thread, nsnull);
+
+    // We need the current recursion depth first.
+    PRUint32 depth;
+    nsresult rv = thread->GetRecursionDepth(&depth);
+    NS_ENSURE_SUCCESS(rv, nsnull);
+
+    NS_ASSERTION(depth, "This should never be 0!");
+    transaction->mCreatedRecursionDepth = depth - 1;
+
+    rv = thread->AddObserver(transaction);
+    NS_ENSURE_SUCCESS(rv, nsnull);
+
     transaction->mCreating = true;
   }
 
   return transaction.forget();
 }
 
 IDBTransaction::IDBTransaction()
 : mReadyState(nsIIDBTransaction::INITIAL),
   mMode(nsIIDBTransaction::READ_ONLY),
   mTimeout(0),
   mPendingRequests(0),
+  mCreatedRecursionDepth(0),
   mSavepointCount(0),
   mAborted(false),
   mCreating(false)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 }
 
 IDBTransaction::~IDBTransaction()
@@ -628,16 +639,17 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_IN
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mOnTimeoutListener)
 
   tmp->mCreatedObjectStores.Clear();
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(IDBTransaction)
   NS_INTERFACE_MAP_ENTRY(nsIIDBTransaction)
+  NS_INTERFACE_MAP_ENTRY(nsIThreadObserver)
   NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(IDBTransaction)
 NS_INTERFACE_MAP_END_INHERITING(nsDOMEventTargetHelper)
 
 NS_IMPL_ADDREF_INHERITED(IDBTransaction, nsDOMEventTargetHelper)
 NS_IMPL_RELEASE_INHERITED(IDBTransaction, nsDOMEventTargetHelper)
 
 DOMCI_DATA(IDBTransaction, IDBTransaction)
 
@@ -814,239 +826,62 @@ IDBTransaction::SetOntimeout(nsIDOMEvent
 nsresult
 IDBTransaction::PreHandleEvent(nsEventChainPreVisitor& aVisitor)
 {
   aVisitor.mCanHandle = PR_TRUE;
   aVisitor.mParentTarget = mDatabase;
   return NS_OK;
 }
 
-IDBTransaction::
-ThreadObserver::ThreadObserver()
-: mBaseRecursionDepth(0),
-  mDone(false)
-{
-  NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
-  NS_ASSERTION(!gThreadObserver, "Multiple observers?!");
-}
-
-IDBTransaction::
-ThreadObserver::~ThreadObserver()
+// XXX Once nsIThreadObserver gets split this method will disappear.
+NS_IMETHODIMP
+IDBTransaction::OnDispatchedEvent(nsIThreadInternal* aThread)
 {
-  NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
-  NS_ASSERTION(gThreadObserver == this, "Multiple observers?!");
-
-#ifdef DEBUG
-  for (PRUint32 i = 0; i < mTransactions.Length(); i++) {
-    NS_ASSERTION(mTransactions[i].transactions.IsEmpty(),
-                 "Unprocessed transactions!");
-  }
-#endif
-
-  // Clear the global.
-  gThreadObserver = nsnull;
+  NS_NOTREACHED("Don't call me!");
+  return NS_ERROR_NOT_IMPLEMENTED;
 }
 
-void
-IDBTransaction::
-ThreadObserver::UpdateNewlyCreatedTransactions(PRUint32 aRecursionDepth)
-{
-  NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
-
-  for (PRUint32 i = 0; i < mTransactions.Length(); i++) {
-    TransactionInfo& info = mTransactions[i];
-
-    if (info.recursionDepth == aRecursionDepth) {
-      for (PRUint32 j = 0; j < info.transactions.Length(); j++) {
-        nsRefPtr<IDBTransaction>& transaction = info.transactions[j];
-
-        // Clear the mCreating flag now.
-        transaction->mCreating = false;
-
-        // And maybe set the readyState to DONE if there were no requests
-        // generated.
-        if (transaction->mReadyState == nsIIDBTransaction::INITIAL) {
-          transaction->mReadyState = nsIIDBTransaction::DONE;
-        }
-      }
-
-      // Don't hang on to transactions any longer than we have to.
-      info.transactions.Clear();
-
-      break;
-    }
-  }
-}
-
-// static
-bool
-IDBTransaction::
-ThreadObserver::BeginObserving(IDBTransaction* aTransaction)
+NS_IMETHODIMP
+IDBTransaction::OnProcessNextEvent(nsIThreadInternal* aThread,
+                                   PRBool aMayWait,
+                                   PRUint32 aRecursionDepth)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
-  NS_ASSERTION(aTransaction, "Null pointer!");
-
-  nsCOMPtr<nsIThreadInternal2> thread(do_QueryInterface(NS_GetCurrentThread()));
-  NS_ENSURE_TRUE(thread, false);
-
-  // We need the current recursion depth first.
-  PRUint32 depth;
-  nsresult rv = thread->GetRecursionDepth(&depth);
-  NS_ENSURE_SUCCESS(rv, false);
-
-  NS_ASSERTION(depth, "This should never be 0!");
-  depth--;
-
-  // If we've already got an observer created then simply append this
-  // transaction to its list.
-  if (gThreadObserver) {
-    for (PRUint32 i = 0; i < gThreadObserver->mTransactions.Length(); i++) {
-      TransactionInfo& info = gThreadObserver->mTransactions[i];
-      if (info.recursionDepth == depth) {
-        if (!info.transactions.AppendElement(aTransaction)) {
-          NS_WARNING("Out of memory!");
-          return false;
-        }
-        return true;
-      }
-    }
-
-    // No transactions at this depth yet, make a new entry
-    TransactionInfo* newInfo = gThreadObserver->mTransactions.AppendElement();
-    if (!newInfo || !newInfo->transactions.AppendElement(aTransaction)) {
-      NS_WARNING("Out of memory!");
-      return false;
-    }
-    newInfo->recursionDepth = depth;
-
-    return true;
-  }
-
-  // Make a new thread observer and install it.
-  nsRefPtr<ThreadObserver> observer(new ThreadObserver());
-
-  TransactionInfo* info = observer->mTransactions.AppendElement();
-  NS_ASSERTION(info, "This should never fail!");
-
-  info->recursionDepth = observer->mBaseRecursionDepth = depth;
-
-  if (!info->transactions.AppendElement(aTransaction)) {
-    NS_WARNING("Out of memory!");
-    return false;
-  }
-
-  // We need to keep the thread observer chain intact so grab the previous
-  // observer.
-  rv = thread->GetObserver(getter_AddRefs(observer->mPreviousObserver));
-  NS_ENSURE_SUCCESS(rv, false);
-
-  // Now set our new observer.
-  rv = thread->SetObserver(observer);
-  NS_ENSURE_SUCCESS(rv, false);
-
-  // And set the global so that we don't recreate it later.
-  gThreadObserver = observer;
-  return true;
-}
-
-NS_IMPL_THREADSAFE_ISUPPORTS1(IDBTransaction::ThreadObserver, nsIThreadObserver)
-
-NS_IMETHODIMP
-IDBTransaction::
-ThreadObserver::OnDispatchedEvent(nsIThreadInternal* aThread)
-{
-  // This may be called on any thread!
-
-  // Nothing special is needed here, just call the previous observer.
-  if (mPreviousObserver) {
-    return mPreviousObserver->OnDispatchedEvent(aThread);
-  }
-
+  NS_ASSERTION(aRecursionDepth > mCreatedRecursionDepth,
+               "Should be impossible!");
+  NS_ASSERTION(mCreating, "Should be true!");
   return NS_OK;
 }
 
 NS_IMETHODIMP
-IDBTransaction::
-ThreadObserver::OnProcessNextEvent(nsIThreadInternal* aThread,
-                                   PRBool aMayWait,
-                                   PRUint32 aRecursionDepth)
-{
-  NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
-  NS_ASSERTION(aThread, "This should never be null!");
-  NS_ASSERTION(!mKungFuDeathGrip, "Shouldn't have a self-ref here!");
-
-  // If we're at the base recursion depth here then we're ready to unset
-  // ourselves as the thread observer.
-  if (aRecursionDepth == mBaseRecursionDepth || mDone) {
-    // From here on we'll continue to try to unset ourselves.
-    mDone = true;
-
-    nsCOMPtr<nsIThreadObserver> currentObserver;
-    if (NS_FAILED(aThread->GetObserver(getter_AddRefs(currentObserver)))) {
-      NS_WARNING("Can't get current observer?!");
-    }
-
-    // We can only set the previous observer if this is the current observer.
-    // Otherwise someone else has installed themselves into the chain and we
-    // have to hang around until they unset themselves.
-    if (currentObserver == this) {
-      // Setting a different thread observer could delete us. Maintain a
-      // reference until AfterProcessNextEvent is called.
-      mKungFuDeathGrip = this;
-
-      // Set our previous observer back on the thread.
-      if (NS_FAILED(aThread->SetObserver(mPreviousObserver))) {
-        NS_ERROR("This should never fail!");
-      }
-    }
-  }
-
-  // Take care of any transactions that were created at this recursion depth.
-  UpdateNewlyCreatedTransactions(aRecursionDepth);
-
-  // And call the previous observer.
-  if (mPreviousObserver) {
-    return mPreviousObserver->OnProcessNextEvent(aThread, aMayWait,
-                                                 aRecursionDepth);
-  }
-
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-IDBTransaction::
-ThreadObserver::AfterProcessNextEvent(nsIThreadInternal* aThread,
+IDBTransaction::AfterProcessNextEvent(nsIThreadInternal* aThread,
                                       PRUint32 aRecursionDepth)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
   NS_ASSERTION(aThread, "This should never be null!");
-
-  nsRefPtr<ThreadObserver> kungFuDeathGrip;
-  nsCOMPtr<nsIThreadObserver> observer;
+  NS_ASSERTION(aRecursionDepth >= mCreatedRecursionDepth,
+               "Should be impossible!");
+  NS_ASSERTION(mCreating, "Should be true!");
 
-  if (mKungFuDeathGrip) {
-    NS_ASSERTION(mDone, "Huh?!");
-
-    // We can drop the reference to this observer after this call.
-    kungFuDeathGrip.swap(mKungFuDeathGrip);
+  if (aRecursionDepth == mCreatedRecursionDepth) {
+    // We're back at the event loop, no longer newborn.
+    mCreating = false;
 
-    // And we don't need the previous observer after this call either.
-    observer.swap(mPreviousObserver);
-  }
-  else {
-    // Still call the previous observer.
-    observer = mPreviousObserver;
-  }
+    // Maybe set the readyState to DONE if there were no requests generated.
+    if (mReadyState == nsIIDBTransaction::INITIAL) {
+      mReadyState = nsIIDBTransaction::DONE;
+    }
 
-  // We may have collected more transactions while the event was processed.
-  // Update them now.
-  UpdateNewlyCreatedTransactions(aRecursionDepth);
+    // No longer need to observe thread events.
+    nsCOMPtr<nsIThreadInternal2> thread = do_QueryInterface(aThread);
+    NS_ASSERTION(thread, "This must never fail!");
 
-  if (observer) {
-    return observer->AfterProcessNextEvent(aThread, aRecursionDepth);
+    if(NS_FAILED(thread->RemoveObserver(this))) {
+      NS_ERROR("Failed to remove observer!");
+    }
   }
 
   return NS_OK;
 }
 
 CommitHelper::CommitHelper(IDBTransaction* aTransaction)
 : mTransaction(aTransaction),
   mAborted(!!aTransaction->mAborted),
--- a/dom/indexedDB/IDBTransaction.h
+++ b/dom/indexedDB/IDBTransaction.h
@@ -61,26 +61,28 @@ class nsIThread;
 BEGIN_INDEXEDDB_NAMESPACE
 
 class AsyncConnectionHelper;
 class CommitHelper;
 struct ObjectStoreInfo;
 class TransactionThreadPool;
 
 class IDBTransaction : public nsDOMEventTargetHelper,
-                       public nsIIDBTransaction
+                       public nsIIDBTransaction,
+                       public nsIThreadObserver
 {
   friend class AsyncConnectionHelper;
   friend class CommitHelper;
   friend class ThreadObserver;
   friend class TransactionThreadPool;
 
 public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_NSIIDBTRANSACTION
+  NS_DECL_NSITHREADOBSERVER
 
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(IDBTransaction,
                                            nsDOMEventTargetHelper)
 
   static already_AddRefed<IDBTransaction>
   Create(IDBDatabase* aDatabase,
          nsTArray<nsString>& aObjectStoreNames,
          PRUint16 aMode,
@@ -154,57 +156,29 @@ public:
     NS_ASSERTION(mDatabase, "This should never be null!");
     return mDatabase;
   }
 
   already_AddRefed<IDBObjectStore>
   GetOrCreateObjectStore(const nsAString& aName,
                          ObjectStoreInfo* aObjectStoreInfo);
 
-  class ThreadObserver : public nsIThreadObserver
-  {
-  public:
-    NS_DECL_ISUPPORTS
-    NS_DECL_NSITHREADOBSERVER
-
-    static bool BeginObserving(IDBTransaction* aTransaction);
-
-  private:
-    ThreadObserver();
-    ~ThreadObserver();
-
-    void UpdateNewlyCreatedTransactions(PRUint32 aRecursionDepth);
-
-    struct TransactionInfo
-    {
-      PRUint32 recursionDepth;
-      nsTArray<nsRefPtr<IDBTransaction> > transactions;
-    };
-
-    nsAutoTArray<TransactionInfo, 1> mTransactions;
-
-    nsCOMPtr<nsIThreadObserver> mPreviousObserver;
-    nsRefPtr<ThreadObserver> mKungFuDeathGrip;
-
-    PRUint32 mBaseRecursionDepth;
-    bool mDone;
-  };
-
 private:
   IDBTransaction();
   ~IDBTransaction();
 
   nsresult CommitOrRollback();
 
   nsRefPtr<IDBDatabase> mDatabase;
   nsTArray<nsString> mObjectStoreNames;
   PRUint16 mReadyState;
   PRUint16 mMode;
   PRUint32 mTimeout;
   PRUint32 mPendingRequests;
+  PRUint32 mCreatedRecursionDepth;
 
   // Only touched on the main thread.
   nsRefPtr<nsDOMEventListenerWrapper> mOnErrorListener;
   nsRefPtr<nsDOMEventListenerWrapper> mOnCompleteListener;
   nsRefPtr<nsDOMEventListenerWrapper> mOnAbortListener;
   nsRefPtr<nsDOMEventListenerWrapper> mOnTimeoutListener;
 
   nsInterfaceHashtable<nsCStringHashKey, mozIStorageStatement>
--- a/xpcom/threads/nsIThreadInternal.idl
+++ b/xpcom/threads/nsIThreadInternal.idl
@@ -97,16 +97,20 @@ interface nsIThreadInternal : nsIThread
  *       }
  *     }
  *   };
  *
  * NOTE: The implementation of this interface must be threadsafe.
  * 
  * NOTE: It is valid to change the thread's observer during a call to an
  *       observer method.
+ *
+ * NOTE: Will be split into two interfaces soon: one for onProcessNextEvent and
+ *       afterProcessNextEvent, then another that inherits the first and adds
+ *       onDispatchedEvent.
  */
 [scriptable, uuid(81D0B509-F198-4417-8020-08EB4271491F)]
 interface nsIThreadObserver : nsISupports
 {
   /**
    * This method is called after an event has been dispatched to the thread.
    * This method may be called from any thread. 
    *
@@ -161,17 +165,33 @@ interface nsIThreadEventFilter : nsISupp
    * WARNING: This method must not make any calls on the thread object.
    */
   [notxpcom] boolean acceptEvent(in nsIRunnable event);
 };
 
 /**
  * Temporary interface, will be merged into nsIThreadInternal.
  */
-[scriptable, uuid(718e9346-74cb-4859-8bcc-c9ec37bfb668)]
+[scriptable, uuid(4531f101-fddc-4d36-80e7-35260a2f3afe)]
 interface nsIThreadInternal2 : nsIThreadInternal
 {
   /**
    * The current recursion depth, 0 when no events are running, 1 when a single
-   * event is running, and higher when nested events are running.
+   * event is running, and higher when nested events are running. Must only be
+   * called on the target thread.
    */
   readonly attribute unsigned long recursionDepth;
+
+  /**
+   * Add an observer that will *only* receive onProcessNextEvent and
+   * afterProcessNextEvent callbacks. Always called on the target thread, and
+   * the implementation does not have to be threadsafe. Order of callbacks is
+   * not guaranteed (i.e. afterProcessNextEvent may be called first depending on
+   * whether or not the observer is added in a nested loop). Holds a strong ref.
+   */
+  void addObserver(in nsIThreadObserver observer);
+
+  /**
+   * Remove an observer added via the addObserver call. Once removed the
+   * observer will never be called again by the thread.
+   */
+  void removeObserver(in nsIThreadObserver observer);
 };
--- a/xpcom/threads/nsThread.cpp
+++ b/xpcom/threads/nsThread.cpp
@@ -555,32 +555,48 @@ void canary_alarm_handler (int signum)
   const char msg[29] = "event took too long to run:\n";
   // use write to be safe in the signal handler
   write(Canary::sOutputFD, msg, sizeof(msg)); 
   backtrace_symbols_fd(array, backtrace(array, 30), Canary::sOutputFD);
 }
 
 #endif
 
+#define NOTIFY_EVENT_OBSERVERS(func_, params_)                                 \
+  PR_BEGIN_MACRO                                                               \
+    if (!mEventObservers.IsEmpty()) {                                          \
+      nsAutoTObserverArray<nsCOMPtr<nsIThreadObserver>, 2>::ForwardIterator    \
+        iter_(mEventObservers);                                                \
+      nsCOMPtr<nsIThreadObserver> obs_;                                        \
+      while (iter_.HasMore()) {                                                \
+        obs_ = iter_.GetNext();                                                \
+        obs_ -> func_ params_ ;                                                \
+      }                                                                        \
+    }                                                                          \
+  PR_END_MACRO
+
 NS_IMETHODIMP
 nsThread::ProcessNextEvent(PRBool mayWait, PRBool *result)
 {
   LOG(("THRD(%p) ProcessNextEvent [%u %u]\n", this, mayWait, mRunningEvent));
 
   NS_ENSURE_STATE(PR_GetCurrentThread() == mThread);
 
   PRBool notifyGlobalObserver = (sGlobalObserver != nsnull);
   if (notifyGlobalObserver) 
     sGlobalObserver->OnProcessNextEvent(this, mayWait && !ShuttingDown(),
                                         mRunningEvent);
 
   nsCOMPtr<nsIThreadObserver> obs = mObserver;
   if (obs)
     obs->OnProcessNextEvent(this, mayWait && !ShuttingDown(), mRunningEvent);
 
+  NOTIFY_EVENT_OBSERVERS(OnProcessNextEvent,
+                         (this, mayWait && !ShuttingDown(), mRunningEvent));
+
   ++mRunningEvent;
 
 #ifdef MOZ_CANARY
   Canary canary;
 #endif
   nsresult rv = NS_OK;
 
   {
@@ -611,16 +627,19 @@ nsThread::ProcessNextEvent(PRBool mayWai
     } else if (mayWait) {
       NS_ASSERTION(ShuttingDown(),
                    "This should only happen when shutting down");
       rv = NS_ERROR_UNEXPECTED;
     }
   }
 
   --mRunningEvent;
+
+  NOTIFY_EVENT_OBSERVERS(AfterProcessNextEvent, (this, mRunningEvent));
+
   if (obs)
     obs->AfterProcessNextEvent(this, mRunningEvent);
 
   if (notifyGlobalObserver && sGlobalObserver)
     sGlobalObserver->AfterProcessNextEvent(this, mRunningEvent);
 
   return rv;
 }
@@ -740,20 +759,51 @@ nsThread::nsChainedEventQueue::PutEvent(
 
 //-----------------------------------------------------------------------------
 // nsIThreadInternal2
 
 NS_IMETHODIMP
 nsThread::GetRecursionDepth(PRUint32 *depth)
 {
   NS_ENSURE_ARG_POINTER(depth);
+  NS_ENSURE_STATE(PR_GetCurrentThread() == mThread);
+
   *depth = mRunningEvent;
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsThread::AddObserver(nsIThreadObserver *observer)
+{
+  NS_ENSURE_ARG_POINTER(observer);
+  NS_ENSURE_STATE(PR_GetCurrentThread() == mThread);
+
+  NS_WARN_IF_FALSE(!mEventObservers.Contains(observer),
+                   "Adding an observer twice!");
+
+  if (!mEventObservers.AppendElement(observer)) {
+    NS_WARNING("Out of memory!");
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+nsThread::RemoveObserver(nsIThreadObserver *observer)
+{
+  NS_ENSURE_STATE(PR_GetCurrentThread() == mThread);
+
+  if (observer && !mEventObservers.RemoveElement(observer)) {
+    NS_WARNING("Removing an observer that was never added!");
+  }
+
+  return NS_OK;
+}
+
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 nsThreadSyncDispatch::Run()
 {
   if (mSyncTask) {
     mResult = mSyncTask->Run();
     mSyncTask = nsnull;
--- a/xpcom/threads/nsThread.h
+++ b/xpcom/threads/nsThread.h
@@ -41,16 +41,17 @@
 
 #include "nsIThreadInternal.h"
 #include "nsISupportsPriority.h"
 #include "nsEventQueue.h"
 #include "nsThreadUtils.h"
 #include "nsString.h"
 #include "nsAutoLock.h"
 #include "nsAutoPtr.h"
+#include "nsTObserverArray.h"
 
 // A native thread
 class nsThread : public nsIThreadInternal2, public nsISupportsPriority
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIEVENTTARGET
   NS_DECL_NSITHREAD
@@ -129,16 +130,19 @@ private:
   // All of those fields are only modified on the thread itself (never from
   // another thread).  This means that we can avoid holding the lock while
   // using mObserver and mEvents on the thread itself.  When calling PutEvent
   // on mEvents, we have to hold the lock to synchronize with PopEventQueue.
   PRLock *mLock;
 
   nsCOMPtr<nsIThreadObserver> mObserver;
 
+  // Only accessed on the target thread.
+  nsAutoTObserverArray<nsCOMPtr<nsIThreadObserver>, 2> mEventObservers;
+
   nsChainedEventQueue *mEvents;   // never null
   nsChainedEventQueue  mEventsRoot;
 
   PRInt32   mPriority;
   PRThread *mThread;
   PRUint32  mRunningEvent;  // counter
 
   struct nsThreadShutdownContext *mShutdownContext;