Bug 1102439 - Clean up child-side PBackground before thread shutdown. r=khuey
authorShih-Chiang Chien <schien@mozilla.com>
Thu, 20 Nov 2014 14:16:36 -0800
changeset 219787 a011ca7ed14c7ed84589e394e9891f5b3832b242
parent 219786 d2d00379a376f2cf4699d58d7bc112733643cfd5
child 219788 8ff55dfa36070e3dbc36a4dff794197e44ec6f84
push id10419
push usercbook@mozilla.com
push dateTue, 16 Dec 2014 12:45:27 +0000
treeherderfx-team@ec87657146eb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs1102439
milestone37.0a1
Bug 1102439 - Clean up child-side PBackground before thread shutdown. r=khuey
ipc/glue/BackgroundImpl.cpp
xpcom/threads/nsThread.cpp
--- a/ipc/glue/BackgroundImpl.cpp
+++ b/ipc/glue/BackgroundImpl.cpp
@@ -333,23 +333,25 @@ class ChildImpl MOZ_FINAL : public Backg
 
   // This is only modified on the main thread. It is the thread-local index that
   // we use to store the BackgroundChild for each thread.
   static unsigned int sThreadLocalIndex;
 
   struct ThreadLocalInfo
   {
     explicit ThreadLocalInfo(nsIIPCBackgroundChildCreateCallback* aCallback)
+      : mClosed(false)
     {
       mCallbacks.AppendElement(aCallback);
     }
 
     nsRefPtr<ChildImpl> mActor;
     nsTArray<nsCOMPtr<nsIIPCBackgroundChildCreateCallback>> mCallbacks;
     nsAutoPtr<BackgroundChildImpl::ThreadLocal> mConsumerThreadLocal;
+    DebugOnly<bool> mClosed;
   };
 
   // This is only modified on the main thread. It is a FIFO queue for actors
   // that are in the process of construction.
   static StaticAutoPtr<nsTArray<nsCOMPtr<nsIEventTarget>>> sPendingTargets;
 
   // This is only modified on the main thread. It prevents us from trying to
   // create the background thread after application shutdown has started.
@@ -426,16 +428,18 @@ private:
   GetThreadLocalForCurrentThread();
 
   static void
   ThreadLocalDestructor(void* aThreadLocal)
   {
     auto threadLocalInfo = static_cast<ThreadLocalInfo*>(aThreadLocal);
 
     if (threadLocalInfo) {
+      MOZ_ASSERT(threadLocalInfo->mClosed);
+
       if (threadLocalInfo->mActor) {
         threadLocalInfo->mActor->Close();
         threadLocalInfo->mActor->AssertActorDestroyed();
 
         // Since the actor is created on the main thread it must only
         // be released on the main thread as well.
         if (!NS_IsMainThread()) {
           ChildImpl* actor;
@@ -1635,16 +1639,24 @@ ChildImpl::Shutdown()
                   !PR_GetThreadPrivate(sThreadLocalIndex));
     return;
   }
 
   sShutdownHasStarted = true;
 
   MOZ_ASSERT(sThreadLocalIndex != kBadThreadLocalIndex);
 
+  auto threadLocalInfo =
+    static_cast<ThreadLocalInfo*>(PR_GetThreadPrivate(sThreadLocalIndex));
+
+  if (threadLocalInfo) {
+    MOZ_ASSERT(!threadLocalInfo->mClosed);
+    threadLocalInfo->mClosed = true;
+  }
+
   DebugOnly<PRStatus> status = PR_SetThreadPrivate(sThreadLocalIndex, nullptr);
   MOZ_ASSERT(status == PR_SUCCESS);
 }
 
 // static
 PBackgroundChild*
 ChildImpl::Alloc(Transport* aTransport, ProcessId aOtherProcess)
 {
@@ -1761,25 +1773,23 @@ ChildImpl::GetOrCreateForCurrentThread(
 void
 ChildImpl::CloseForCurrentThread()
 {
   MOZ_ASSERT(sThreadLocalIndex != kBadThreadLocalIndex,
              "BackgroundChild::Startup() was never called!");
   auto threadLocalInfo =
     static_cast<ThreadLocalInfo*>(PR_GetThreadPrivate(sThreadLocalIndex));
 
-  // If we don't have a thread local we are in one of these conditions:
-  //   1) Startup has not completed and we are racing
-  //   2) We were called again after a previous close or shutdown
-  // For now, these should not happen, so crash.  We can add extra complexity
-  // in the future if it turns out we need to support these cases.
   if (!threadLocalInfo) {
-    MOZ_CRASH("Attempting to close a non-existent PBackground actor!");
+    return;
   }
 
+  MOZ_ASSERT(!threadLocalInfo->mClosed);
+  threadLocalInfo->mClosed = true;
+
   if (threadLocalInfo->mActor) {
     threadLocalInfo->mActor->FlushPendingInterruptQueue();
   }
 
   // Clearing the thread local will synchronously close the actor.
   DebugOnly<PRStatus> status = PR_SetThreadPrivate(sThreadLocalIndex, nullptr);
   MOZ_ASSERT(status == PR_SUCCESS);
 }
--- a/xpcom/threads/nsThread.cpp
+++ b/xpcom/threads/nsThread.cpp
@@ -24,16 +24,17 @@
 #include "prlog.h"
 #include "nsIObserverService.h"
 #include "mozilla/HangMonitor.h"
 #include "mozilla/IOInterposer.h"
 #include "mozilla/ipc/MessageChannel.h"
 #include "mozilla/Services.h"
 #include "nsXPCOMPrivate.h"
 #include "mozilla/ChaosMode.h"
+#include "mozilla/ipc/BackgroundChild.h"
 
 #ifdef MOZ_CRASHREPORTER
 #include "nsServiceManagerUtils.h"
 #include "nsICrashReporter.h"
 #endif
 
 #ifdef XP_LINUX
 #include <sys/time.h>
@@ -318,16 +319,18 @@ SetupCurrentThreadForChaosMode()
     sched_setaffinity(0, sizeof(cpus), &cpus);
   }
 #endif
 }
 
 /*static*/ void
 nsThread::ThreadFunc(void* aArg)
 {
+  using mozilla::ipc::BackgroundChild;
+
   nsThread* self = static_cast<nsThread*>(aArg);  // strong reference
   self->mThread = PR_GetCurrentThread();
   SetupCurrentThreadForChaosMode();
 
   // Inform the ThreadManager
   nsThreadManager::get()->RegisterCurrentThread(self);
 
   mozilla::IOInterposer::RegisterCurrentThread();
@@ -344,16 +347,18 @@ nsThread::ThreadFunc(void* aArg)
   {
     // Scope for MessageLoop.
     nsAutoPtr<MessageLoop> loop(
       new MessageLoop(MessageLoop::TYPE_MOZILLA_NONMAINTHREAD));
 
     // Now, process incoming events...
     loop->Run();
 
+    BackgroundChild::CloseForCurrentThread();
+
     // Do NS_ProcessPendingEvents but with special handling to set
     // mEventsAreDoomed atomically with the removal of the last event. The key
     // invariant here is that we will never permit PutEvent to succeed if the
     // event would be left in the queue after our final call to
     // NS_ProcessPendingEvents.
     while (true) {
       {
         MutexAutoLock lock(self->mLock);