Bug 498010 - Threads should release their observers when they are shut down; r=benjamin
authorben turner <bent.mozilla@gmail.com>
Mon, 22 Jun 2009 15:08:04 +0200
changeset 29437 21964b89824c4dc9c64ce7b296d44d1872d909d1
parent 29436 40690e93dfc028453d89e9c5d62ea194d3eb7807
child 29438 ce59685483a696c97aa3cc62d9eceefb7f4a395b
push id7623
push usersgautherie.bz@free.fr
push dateMon, 22 Jun 2009 13:08:27 +0000
treeherdermozilla-central@21964b89824c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbenjamin
bugs498010
milestone1.9.2a1pre
Bug 498010 - Threads should release their observers when they are shut down; r=benjamin
xpcom/threads/nsIThreadInternal.idl
xpcom/threads/nsThread.cpp
xpcom/threads/nsThreadManager.cpp
--- a/xpcom/threads/nsIThreadInternal.idl
+++ b/xpcom/threads/nsIThreadInternal.idl
@@ -46,17 +46,19 @@ interface nsIThreadEventFilter;
  * to observe dispatch activity on the thread.
  */
 [scriptable, uuid(f89b5063-b06d-42f8-bf23-4dfcf2d80d6a)]
 interface nsIThreadInternal : nsIThread
 {
   /**
    * Get/set the current thread observer (may be null).  This attribute may be
    * read from any thread, but must only be set on the thread corresponding to
-   * this thread object.
+   * this thread object.  The observer will be released on the thread
+   * corresponding to this thread object after all other events have been
+   * processed during a call to Shutdown.
    */
   attribute nsIThreadObserver observer;
 
   /**
    * This method causes any events currently enqueued on the thread to be
    * suppressed until PopEventQueue is called.  Additionally, any new events
    * dispatched to the thread will only be processed if they are accepted by
    * the given filter.  If the filter is null, then new events are accepted.
--- a/xpcom/threads/nsThread.cpp
+++ b/xpcom/threads/nsThread.cpp
@@ -275,16 +275,19 @@ nsThread::ThreadFunc(void *arg)
 
   // Inform the threadmanager that this thread is going away
   nsThreadManager::get()->UnregisterCurrentThread(self);
 
   // Dispatch shutdown ACK
   event = new nsThreadShutdownAckEvent(self->mShutdownContext);
   self->mShutdownContext->joiningThread->Dispatch(event, NS_DISPATCH_NORMAL);
 
+  // Release any observer of the thread here.
+  self->SetObserver(nsnull);
+
   NS_RELEASE(self);
 }
 
 //-----------------------------------------------------------------------------
 
 nsThread::nsThread()
   : mLock(PR_NewLock())
   , mEvents(&mEventsRoot)
@@ -463,16 +466,24 @@ nsThread::Shutdown()
   // Process events on the current thread until we receive a shutdown ACK.
   while (!context.shutdownAck)
     NS_ProcessNextEvent(context.joiningThread);
 
   // Now, it should be safe to join without fear of dead-locking.
 
   PR_JoinThread(mThread);
   mThread = nsnull;
+
+#ifdef DEBUG
+  {
+    nsAutoLock lock(mLock);
+    NS_ASSERTION(!mObserver, "Should have been cleared at shutdown!");
+  }
+#endif
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsThread::HasPendingEvents(PRBool *result)
 {
   NS_ENSURE_STATE(PR_GetCurrentThread() == mThread);
 
--- a/xpcom/threads/nsThreadManager.cpp
+++ b/xpcom/threads/nsThreadManager.cpp
@@ -150,16 +150,21 @@ nsThreadManager::Shutdown()
   // There are no more background threads at this point.
 
   // Clear the table of threads.
   {
     nsAutoLock lock(mLock);
     mThreadsByPRThread.Clear();
   }
 
+  // Normally thread shutdown clears the observer for the thread, but since the
+  // main thread is special we do it manually here after we're sure all events
+  // have been processed.
+  mMainThread->SetObserver(nsnull);
+
   // Release main thread object.
   mMainThread = nsnull;
 
   // Remove the TLS entry for the main thread.
   PR_SetThreadPrivate(mCurThreadIndex, nsnull);
 
   // We don't need this lock anymore.
   PR_DestroyLock(mLock);