Backout Bug 1365309 for test failures on a CLOSED TREE

MozReview-Commit-ID: 143ac9l7Goq

dom/media/gmp/GMPProcessChild.cpp

@@ -44,6 +44,8 @@ GMPProcessChild::Init(int aArgc, char* aArgv[])
 #error Not implemented
 #endif
 
+  BackgroundHangMonitor::Startup();
+
   return mPlugin.Init(pluginFilename,
                       ParentPid(),
                       IOThreadChild::message_loop(),
@@ -53,6 +55,7 @@ GMPProcessChild::Init(int aArgc, char* aArgv[])
 void
 GMPProcessChild::CleanUp()
 {
+  BackgroundHangMonitor::Shutdown();
 }
 
 } // namespace gmp

toolkit/components/telemetry/tests/unit/xpcshell.ini

@@ -52,7 +52,7 @@ tags = addons
 tags = addons
 [test_TelemetrySession_activeTicks.js]
 [test_ThreadHangStats.js]
-skip-if = os == "android" || (os == "linux" && bits == 32) # BHR is disabled on 32-bit linux
+skip-if = os == "android" || os == "linux" # BHR is disabled on linux (bug 1365309)
 run-sequentially = Bug 1046307, test can fail intermittently when CPU load is high
 [test_TelemetrySend.js]
 [test_ChildHistograms.js]

tools/profiler/core/platform-linux-android.cpp

@@ -59,6 +59,7 @@
 #include <errno.h>
 #include <stdarg.h>
 
+#include "prenv.h"
 #include "mozilla/LinuxSignal.h"
 #include "mozilla/PodOperations.h"
 #include "mozilla/DebugOnly.h"
@@ -254,7 +255,19 @@ Sampler::Sampler(PSLockRef aLock)
   // SuspendAndSampleAndResumeThread().
   , mSamplerTid(-1)
 {
-  profiler_initialize_stackwalk();
+#if defined(USE_EHABI_STACKWALK)
+  mozilla::EHABIStackWalkInit();
+#elif defined(USE_LUL_STACKWALK)
+  bool createdLUL = false;
+  lul::LUL* lul = CorePS::Lul(aLock);
+  if (!lul) {
+    CorePS::SetLul(aLock, MakeUnique<lul::LUL>(logging_sink_for_LUL));
+    // Read all the unwind info currently available.
+    lul = CorePS::Lul(aLock);
+    read_procmaps(lul);
+    createdLUL = true;
+  }
+#endif
 
   // Request profiling signals.
   struct sigaction sa;
@@ -264,6 +277,21 @@ Sampler::Sampler(PSLockRef aLock)
   if (sigaction(SIGPROF, &sa, &mOldSigprofHandler) != 0) {
     MOZ_CRASH("Error installing SIGPROF handler in the profiler");
   }
+
+#if defined(USE_LUL_STACKWALK)
+  if (createdLUL) {
+    // Switch into unwind mode. After this point, we can't add or remove any
+    // unwind info to/from this LUL instance. The only thing we can do with
+    // it is Unwind() calls.
+    lul->EnableUnwinding();
+
+    // Has a test been requested?
+    if (PR_GetEnv("MOZ_PROFILER_LUL_TEST")) {
+      int nTests = 0, nTestsPassed = 0;
+      RunLulUnitTests(&nTests, &nTestsPassed, lul);
+    }
+  }
+#endif
 }
 
 void

tools/profiler/core/platform.cpp

@@ -64,7 +64,6 @@
 #include "shared-libraries.h"
 #include "prdtoa.h"
 #include "prtime.h"
-#include "prenv.h"
 
 #ifdef MOZ_TASK_TRACER
 #include "GeckoTaskTracer.h"
@@ -170,6 +169,9 @@ class CorePS
 private:
   CorePS()
     : mProcessStartTime(TimeStamp::ProcessCreation())
+#ifdef USE_LUL_STACKWALK
+    , mLul(nullptr)
+#endif
   {}
 
   ~CorePS()
@@ -201,17 +203,18 @@ public:
   // thread that we don't have to worry about it being racy.
   static bool Exists() { return !!sInstance; }
 
-  static size_t SizeOf(PSLockRef, MallocSizeOf aMallocSizeOf)
+  static void AddSizeOf(PSLockRef, MallocSizeOf aMallocSizeOf,
+                        size_t& aProfSize, size_t& aLulSize)
   {
-    size_t profSize = aMallocSizeOf(sInstance);
+    aProfSize += aMallocSizeOf(sInstance);
 
     for (uint32_t i = 0; i < sInstance->mLiveThreads.size(); i++) {
-      profSize +=
+      aProfSize +=
         sInstance->mLiveThreads.at(i)->SizeOfIncludingThis(aMallocSizeOf);
     }
 
     for (uint32_t i = 0; i < sInstance->mDeadThreads.size(); i++) {
-      profSize +=
+      aProfSize +=
         sInstance->mDeadThreads.at(i)->SizeOfIncludingThis(aMallocSizeOf);
     }
 
@@ -222,7 +225,11 @@ public:
     // - CorePS::mDeadThreads itself (ditto)
     // - CorePS::mInterposeObserver
 
-    return profSize;
+#if defined(USE_LUL_STACKWALK)
+    if (sInstance->mLul) {
+      aLulSize += sInstance->mLul->SizeOfIncludingThis(aMallocSizeOf);
+    }
+#endif
   }
 
   // No PSLockRef is needed for this field because it's immutable.
@@ -231,6 +238,14 @@ public:
   PS_GET(ThreadVector&, LiveThreads)
   PS_GET(ThreadVector&, DeadThreads)
 
+#ifdef USE_LUL_STACKWALK
+  static lul::LUL* Lul(PSLockRef) { return sInstance->mLul.get(); }
+  static void SetLul(PSLockRef, UniquePtr<lul::LUL> aLul)
+  {
+    sInstance->mLul = Move(aLul);
+  }
+#endif
+
 private:
   // The singleton instance
   static CorePS* sInstance;
@@ -245,6 +260,11 @@ private:
   // threads.
   ThreadVector mLiveThreads;
   ThreadVector mDeadThreads;
+
+#ifdef USE_LUL_STACKWALK
+  // LUL's state. Null prior to the first activation, non-null thereafter.
+  UniquePtr<lul::LUL> mLul;
+#endif
 };
 
 CorePS* CorePS::sInstance = nullptr;
@@ -606,82 +626,6 @@ MOZ_THREAD_LOCAL(PseudoStack*) AutoProfilerLabel::sPseudoStack;
 // The name of the main thread.
 static const char* const kMainThreadName = "GeckoMain";
 
-#if defined(USE_LUL_STACKWALK)
-// When we are using LUL, we are going to need to perform expensive
-// initialization before we can start getting backtraces. We don't want to be
-// holding the profiler mutex while doing that, as the main thread sometimes
-// wants to grant that and we never want to get in the way of that. Because of
-// that sLul is behind a separate mutex. We just directly use
-// mozilla::StaticMutex here as we never have to pass this lock around.
-class LulPS {
-public:
-  // Determine if LulPS::sLul has been initialized yet. If this returns true it
-  // is safe to access the unsynchronized LulPS::Get().
-  static bool Exists() {
-    // NOTE: Hold the lock here, in case we are mid initialization while this
-    // method is being called.
-    mozilla::StaticMutexAutoLock lock(sLulMutex);
-    return !!sLul;
-  }
-
-  // Get the LUL object without synchronization. This method must be called
-  // guarded by either a call to `Exists`, or after calling `GetOrCreate`
-  // earlier in the same thread.
-  //
-  // This is safe because LulPS::sLul has approximately the same lifetime as
-  // CorePS. It is initialized after CorePS, but will live until CorePS is
-  // killed. Once it has been initialized, it is safe to read without
-  // synchronization.
-  static lul::LUL* Get()
-  {
-    MOZ_RELEASE_ASSERT(sLul, "Lul must have been initialized when Get() is called");
-    return sLul;
-  }
-
-  // This method gets the LUL object, and creates it if it does not exist.
-  static lul::LUL* GetOrCreate()
-  {
-    mozilla::StaticMutexAutoLock lock(sLulMutex);
-    if (sLul) {
-      return sLul;
-    }
-
-    lul::LUL* lul = new lul::LUL(logging_sink_for_LUL);
-    read_procmaps(lul);
-
-    // Switch into unwind mode. After this point, we can't add or remove any
-    // unwind info to/from this LUL instance. The only thing we can do with
-    // it is Unwind() calls.
-    lul->EnableUnwinding();
-
-    // If unit tests were requested, run them now.
-    if (PR_GetEnv("MOZ_PROFILER_LUL_TEST")) {
-      int nTests = 0, nTestsPassed = 0;
-      RunLulUnitTests(&nTests, &nTestsPassed, lul);
-    }
-
-    sLul = lul;
-    return sLul;
-  }
-
-  static void Destroy()
-  {
-    mozilla::StaticMutexAutoLock lock(sLulMutex);
-    if (sLul) {
-      delete sLul;
-      sLul = nullptr;
-    }
-  }
-
-private:
-  static mozilla::StaticMutex sLulMutex;
-  static lul::LUL* sLul;
-};
-
-mozilla::StaticMutex LulPS::sLulMutex;
-lul::LUL* LulPS::sLul = nullptr;
-#endif
-
 ////////////////////////////////////////////////////////////////////////
 // BEGIN sampling/unwinding code
 
@@ -1279,7 +1223,7 @@ DoNativeBacktrace(PSLockRef aLock, const ThreadInfo& aThreadInfo,
   }
 
   size_t framePointerFramesAcquired = 0;
-  lul::LUL* lul = LulPS::Get();
+  lul::LUL* lul = CorePS::Lul(aLock);
   lul->Unwind(reinterpret_cast<uintptr_t*>(aNativeStack.mPCs),
               reinterpret_cast<uintptr_t*>(aNativeStack.mSPs),
               &aNativeStack.mCount, &framePointerFramesAcquired,
@@ -1736,7 +1680,7 @@ PrintUsageThenExit(int aExitCode)
     "  If set, the profiler saves a profile to the named file on shutdown.\n"
     "\n"
     "  MOZ_PROFILER_LUL_TEST\n"
-    "  If set to any value, runs LUL unit tests when it is initialized.\n"
+    "  If set to any value, runs LUL unit tests at startup.\n"
     "\n"
     "  This platform %s native unwinding.\n"
     "\n",
@@ -1947,8 +1891,7 @@ SamplerThread::Run()
         // involves doing I/O (fprintf, __android_log_print, etc.) and so
         // can't safely be done from the critical section inside
         // SuspendAndSampleAndResumeThread, which is why it is done here.
-        lul::LUL* lul = LulPS::Get();
+        CorePS::Lul(lock)->MaybeShowStats();
-        lul->MaybeShowStats();
 #endif
       }
     }
@@ -2012,12 +1955,13 @@ GeckoProfilerReporter::CollectReports(nsIHandleReportCallback* aHandleReport,
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   size_t profSize = 0;
+  size_t lulSize = 0;
 
   {
     PSAutoLock lock(gPSMutex);
 
     if (CorePS::Exists()) {
-      profSize += CorePS::SizeOf(lock, GeckoProfilerMallocSizeOf);
+      CorePS::AddSizeOf(lock, GeckoProfilerMallocSizeOf, profSize, lulSize);
     }
 
     if (ActivePS::Exists(lock)) {
@@ -2031,12 +1975,6 @@ GeckoProfilerReporter::CollectReports(nsIHandleReportCallback* aHandleReport,
     "by LUL).");
 
 #if defined(USE_LUL_STACKWALK)
-  size_t lulSize = 0;
-
-  if (LulPS::Exists()) {
-    lulSize += LulPS::Get()->SizeOfIncludingThis(GeckoProfilerMallocSizeOf);
-  }
-
   MOZ_COLLECT_REPORT(
     "explicit/profiler/lul", KIND_HEAP, UNITS_BYTES, lulSize,
     "Memory used by LUL, a stack unwinder used by the Gecko Profiler.");
@@ -2217,7 +2155,7 @@ profiler_init(void* aStackTop)
     // Setup support for pushing/popping labels in mozglue.
     RegisterProfilerLabelEnterExit(MozGlueLabelEnter, MozGlueLabelExit);
 
-    // (Linux-only) We could create LulPS::sLul and read unwind info into it
+    // (Linux-only) We could create CorePS::mLul and read unwind info into it
     // at this point. That would match the lifetime implied by destruction of
     // it in profiler_shutdown() just below. However, that gives a big delay on
     // startup, even if no profiling is actually to be done. So, instead, it is
@@ -2293,10 +2231,6 @@ profiler_shutdown()
 
     CorePS::Destroy(lock);
 
-#ifdef USE_LUL_STACKWALK
-    LulPS::Destroy();
-#endif
-
     // We just destroyed CorePS and the ThreadInfos it contains, so we can
     // clear this thread's TLSInfo.
     TLSInfo::SetInfo(lock, nullptr);
@@ -3145,16 +3079,5 @@ profiler_suspend_and_sample_thread(
   }
 }
 
-void
-profiler_initialize_stackwalk()
-{
-#if defined(USE_EHABI_STACKWALK)
-  mozilla::EHABIStackWalkInit();
-#elif defined(USE_LUL_STACKWALK)
-  lul::LUL* lul = LulPS::GetOrCreate();
-  MOZ_RELEASE_ASSERT(lul);
-#endif
-}
-
 // END externally visible functions
 ////////////////////////////////////////////////////////////////////////

tools/profiler/public/GeckoProfiler.h

@@ -272,14 +272,6 @@ PROFILER_FUNC_VOID(
                                      const std::function<ProfilerStackCallback>& aCallback,
                                      bool aSampleNative = true))
 
-// This method tries to initialize any internal state used in order to sample
-// threads. This initialization may be expensive on some platforms, so this
-// method may block for a while. Do not run this method on the main thread.
-//
-// This method may be safely called multiple times, and will be a no-op when
-// called after the first time.
-PROFILER_FUNC_VOID(profiler_initialize_stackwalk());
-
 struct ProfilerBacktraceDestructor
 {
 #ifdef MOZ_GECKO_PROFILER

xpcom/threads/BackgroundHangMonitor.cpp

@@ -240,7 +240,7 @@ public:
 
 
 StaticRefPtr<BackgroundHangManager> BackgroundHangManager::sInstance;
-bool BackgroundHangManager::sDisabled = true;
+bool BackgroundHangManager::sDisabled = false;
 
 MOZ_THREAD_LOCAL(BackgroundHangThread*) BackgroundHangThread::sTlsKey;
 bool BackgroundHangThread::sTlsKeyInitialized;
@@ -277,12 +277,6 @@ BackgroundHangManager::~BackgroundHangManager()
 void
 BackgroundHangManager::RunMonitorThread()
 {
-  // Make sure to initialize any state required to perform stack walking as soon
-  // as possible, so it does not interfere with us collecting hang stacks. We
-  // don't want to be on the main thread, or holding the BHR lock, because this
-  // can take a long time, so we do it before grabbing the lock off-main-thread.
-  profiler_initialize_stackwalk();
-
   // Keep us locked except when waiting
   MonitorAutoLock autoLock(mLock);
 
@@ -720,10 +714,6 @@ BackgroundHangMonitor::Startup()
 #ifdef MOZ_ENABLE_BACKGROUND_HANG_MONITOR
   MOZ_ASSERT(!BackgroundHangManager::sInstance, "Already initialized");
 
-  // Enable the BackgroundHangManager by setting sDisabled to false in Startup.
-  MOZ_ASSERT(BackgroundHangManager::sDisabled);
-  BackgroundHangManager::sDisabled = false;
-
   if (!strcmp(NS_STRINGIFY(MOZ_UPDATE_CHANNEL), "beta")) {
     if (XRE_IsParentProcess()) { // cached ClientID hasn't been read yet
       BackgroundHangThread::Startup();

xpcom/threads/ThreadStackHelper.h

@@ -22,24 +22,22 @@
 #include <mach/mach.h>
 #endif
 
-// Support pseudostack and native stack on these platforms. We don't support
+// Support pseudostack and native stack on these platforms.
-// collecting this information on x86 linux builds, as our LUL stackwalking
+#if defined(XP_LINUX) || defined(XP_WIN) || defined(XP_MACOSX)
-// initialization code can occasionally crash. (see bug 1365309 comment 8).
-#if (defined(XP_LINUX) && defined(HAVE_64BIT_BUILD)) || defined(XP_WIN) || defined(XP_MACOSX)
 #  ifdef MOZ_GECKO_PROFILER
 #    define MOZ_THREADSTACKHELPER_PSEUDO
 #    define MOZ_THREADSTACKHELPER_NATIVE
 #  endif
 #endif
 
-// Android x86 builds consistently crash in the Background Hang Reporter. bug
+// NOTE: Currently, due to a problem with LUL stackwalking initialization taking
-// 1368520.
+// a long time (bug 1365309), we don't perform pseudostack or native stack
-#if defined(__ANDROID__)
+// walking on Linux.
-#  undef MOZ_THREADSTACKHELPER_PSEUDO
+#if defined(XP_LINUX)
 #  undef MOZ_THREADSTACKHELPER_NATIVE
+#  undef MOZ_THREADSTACKHELPER_PSEUDO
 #endif
 
-
 namespace mozilla {
 
 /**