Bug 999888: Make IOInterposer leakcheck-friendly; f=ehsan, r=froydnj
authorAaron Klotz <aklotz@mozilla.com>
Tue, 29 Apr 2014 11:29:43 -0600
changeset 181192 1d166a3a207d0f328a51304403802cdacdc4fe9e
parent 181191 79924a38cc00ad5b1ecdc3ed3805a6dac0f567fc
child 181193 4033c3fdd78aff42a66739c88b7151715f05150f
push id272
push userpvanderbeken@mozilla.com
push dateMon, 05 May 2014 16:31:18 +0000
reviewersfroydnj
bugs999888
milestone32.0a1
Bug 999888: Make IOInterposer leakcheck-friendly; f=ehsan, r=froydnj
toolkit/xre/nsAppRunner.cpp
xpcom/build/IOInterposer.cpp
xpcom/build/IOInterposer.h
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -4031,33 +4031,33 @@ XREMain::XRE_mainRun()
  */
 int
 XREMain::XRE_main(int argc, char* argv[], const nsXREAppData* aAppData)
 {
   char aLocal;
   GeckoProfilerInitRAII profilerGuard(&aLocal);
   PROFILER_LABEL("Startup", "XRE_Main");
 
-  mozilla::IOInterposerInit ioInterposerGuard;
-
   nsresult rv = NS_OK;
 
   gArgc = argc;
   gArgv = argv;
 
   NS_ENSURE_TRUE(aAppData, 2);
 
   mAppData = new ScopedAppData(aAppData);
   if (!mAppData)
     return 1;
   // used throughout this file
   gAppData = mAppData;
 
   ScopedLogging log;
 
+  mozilla::IOInterposerInit ioInterposerGuard;
+
 #if defined(MOZ_WIDGET_GTK)
 #if defined(MOZ_MEMORY) || defined(__FreeBSD__) || defined(__NetBSD__)
   // Disable the slice allocator, since jemalloc already uses similar layout
   // algorithms, and using a sub-allocator tends to increase fragmentation.
   // This must be done before g_thread_init() is called.
   g_slice_set_config(G_SLICE_CONFIG_ALWAYS_MALLOC, 1);
 #endif
   g_thread_init(nullptr);
--- a/xpcom/build/IOInterposer.cpp
+++ b/xpcom/build/IOInterposer.cpp
@@ -6,25 +6,17 @@
 #include <vector>
 
 #include "IOInterposer.h"
 
 #include "IOInterposerPrivate.h"
 #include "MainThreadIOLogger.h"
 #include "mozilla/Atomics.h"
 #include "mozilla/Mutex.h"
-#if defined(MOZILLA_INTERNAL_API)
-// We need to undefine MOZILLA_INTERNAL_API for RefPtr.h because IOInterposer
-// does not clean up its data before shutdown.
-#undef MOZILLA_INTERNAL_API
 #include "mozilla/RefPtr.h"
-#define MOZILLA_INTERNAL_API
-#else
-#include "mozilla/RefPtr.h"
-#endif // defined(MOZILLA_INTERNAL_API)
 #include "mozilla/StaticPtr.h"
 #include "mozilla/ThreadLocal.h"
 #if !defined(XP_WIN)
 #include "NSPRInterposer.h"
 #endif // !defined(XP_WIN)
 #include "nsXULAppAPI.h"
 #include "PoisonIOInterposer.h"
 
@@ -81,16 +73,22 @@ struct ObserverLists : public mozilla::A
 class PerThreadData
 {
 public:
   PerThreadData(bool aIsMainThread = false)
     : mIsMainThread(aIsMainThread)
     , mIsHandlingObservation(false)
     , mCurrentGeneration(0)
   {
+    MOZ_COUNT_CTOR(PerThreadData);
+  }
+
+  ~PerThreadData()
+  {
+    MOZ_COUNT_DTOR(PerThreadData);
   }
 
   void
   CallObservers(IOInterposeObserver::Observation& aObservation)
   {
     // Prevent recursive reporting.
     if (mIsHandlingObservation) {
       return;
@@ -169,34 +167,45 @@ public:
 
   inline void
   SetObserverLists(uint32_t aNewGeneration, RefPtr<ObserverLists>& aNewLists)
   {
     mCurrentGeneration = aNewGeneration;
     mObserverLists = aNewLists;
   }
 
+  inline void
+  ClearObserverLists()
+  {
+    if (mObserverLists) {
+      mCurrentGeneration = 0;
+      mObserverLists = nullptr;
+    }
+  }
+
 private:
   bool                  mIsMainThread;
   bool                  mIsHandlingObservation;
   uint32_t              mCurrentGeneration;
   RefPtr<ObserverLists> mObserverLists;
 };
 
 class MasterList
 {
 public:
   MasterList()
     : mObservedOperations(IOInterposeObserver::OpNone)
     , mIsEnabled(true)
   {
+    MOZ_COUNT_CTOR(MasterList);
   }
 
   ~MasterList()
   {
+    MOZ_COUNT_DTOR(MasterList);
   }
 
   inline void
   Disable()
   {
     mIsEnabled = false;
   }
 
@@ -308,17 +317,17 @@ public:
       if (newLists->mStageObservers.empty()) {
         mObservedOperations = (IOInterposeObserver::Operation)
                          (mObservedOperations & ~IOInterposeObserver::OpNextStage);
       }
     }
     mObserverLists = newLists;
     mCurrentGeneration++;
   }
- 
+
   void
   Update(PerThreadData &aPtd)
   {
     if (mCurrentGeneration == aPtd.GetCurrentGeneration()) {
       return;
     }
     // If the generation counts don't match then we need to update the current
     // thread's observer list with the new master list.
@@ -469,43 +478,51 @@ IOInterposeObserver::IsMainThread()
     return false;
   }
   return ptd->IsMainThread();
 }
 
 void
 IOInterposer::Clear()
 {
+  /* Clear() is a no-op on opt builds so that we may continue to trap I/O until
+     process termination. In debug builds we need to shut down IOInterposer so
+     that all references are properly released and refcnt log remains clean. */
+#if defined(DEBUG) || defined(FORCE_BUILD_REFCNT_LOGGING)
+  UnregisterCurrentThread();
   sMasterList = nullptr;
+#endif
 }
 
 void
 IOInterposer::Disable()
 {
   if (!sMasterList) {
     return;
   }
   sMasterList->Disable();
 }
 
 void
 IOInterposer::Report(IOInterposeObserver::Observation& aObservation)
 {
-  MOZ_ASSERT(sMasterList);
-  if (!sMasterList) {
-    return;
-  }
-
   PerThreadData* ptd = sThreadLocalData.get();
   if (!ptd) {
     // In this case the current thread is not registered with IOInterposer.
     // Alternatively we could take the slow path and just lock everything if
     // we're not registered. That could potentially perform poorly, though.
     return;
   }
+
+  if (!sMasterList) {
+    // If there is no longer a master list then we should clear the local one.
+    ptd->ClearObserverLists();
+    return;
+  }
+
   sMasterList->Update(*ptd);
 
   // Don't try to report if there's nobody listening.
   if (!IOInterposer::IsObservedOperation(aObservation.ObservedOperation())) {
     return;
   }
 
   ptd->CallObservers(aObservation);
--- a/xpcom/build/IOInterposer.h
+++ b/xpcom/build/IOInterposer.h
@@ -276,15 +276,19 @@ class IOInterposerInit
 public:
   IOInterposerInit()
   {
 #if defined(MOZ_ENABLE_PROFILER_SPS)
     IOInterposer::Init();
 #endif
   }
 
-  // No destructor needed at the moment -- this stuff stays active for the
-  // life of the process. This may change in the future.
+  ~IOInterposerInit()
+  {
+#if defined(MOZ_ENABLE_PROFILER_SPS)
+    IOInterposer::Clear();
+#endif
+  }
 };
 
 } // namespace mozilla
 
 #endif // mozilla_IOInterposer_h