Bug 1883903 - Try to get a profile when we encounter a hang r=florian,profiler-reviewers,ahal

Differential Revision: https://phabricator.services.mozilla.com/D214842

testing/mochitest/runtests.py

@@ -3883,6 +3883,32 @@ toolbar#nav-bar {
         self.log.warning("Force-terminating active process(es).")
 
         browser_pid = browser_pid or proc.pid
+
+        # Send a signal to start the profiler - if we're running on Linux or MacOS
+        if mozinfo.isLinux or mozinfo.isMac:
+            self.log.info(
+                "Attempting to start the profiler to help with diagnosing the hang."
+            )
+            self.log.info("Sending SIGUSR1 to pid %d start the profiler." % browser_pid)
+            os.kill(browser_pid, signal.SIGUSR1)
+            self.log.info("Waiting 10s to capture a profile.")
+            time.sleep(10)
+            self.log.info("Sending SIGUSR2 to pid %d stop the profiler." % browser_pid)
+            os.kill(browser_pid, signal.SIGUSR2)
+            # We trigger `killPid` further down in this function, which will
+            # stop the profiler writing to disk. As we might still be writing a
+            # profile when we run `killPid`, we might end up with a truncated
+            # profile. Instead, we give the profiler ten seconds to write a
+            # profile (which should be plenty of time!) See Bug 1906151 for more
+            # details, and Bug 1905929 for an intermediate solution that would
+            # allow this test to watch for the profile file being completed.
+            self.log.info("Wait 10s for Firefox to write the profile to disk.")
+            time.sleep(10)
+        else:
+            self.log.info(
+                "Not sending a signal to start the profiler - not on MacOS or Linux. See Bug 1823370."
+            )
+
         child_pids = self.extract_child_pids(processLog, browser_pid)
         self.log.info("Found child pids: %s" % child_pids)
 

tools/profiler/core/platform.cpp

@@ -961,7 +961,7 @@ class CorePS {
   PS_GET_AND_SET(const nsACString&, ProcessName)
   PS_GET_AND_SET(const nsACString&, ETLDplus1)
 #if !defined(XP_WIN)
-  PS_GET_AND_SET(const Maybe<nsCOMPtr<nsIFile>>&, DownloadDirectory)
+  PS_GET_AND_SET(const Maybe<nsCOMPtr<nsIFile>>&, AsyncSignalDumpDirectory)
 #endif
 
   static void SetBandwidthCounter(ProfilerBandwidthCounter* aBandwidthCounter) {
@@ -1027,7 +1027,7 @@ class CorePS {
 
   // Cached download directory for when we need to dump profiles to disk.
 #if !defined(XP_WIN)
-  Maybe<nsCOMPtr<nsIFile>> mDownloadDirectory;
+  Maybe<nsCOMPtr<nsIFile>> mAsyncSignalDumpDirectory;
 #endif
 };
 
@@ -5673,10 +5673,10 @@ Maybe<nsAutoCString> profiler_find_dump_path() {
     // Acquire the lock so that we can get things from CorePS
     PSAutoLock lock;
     Maybe<nsCOMPtr<nsIFile>> downloadDir = Nothing();
-    downloadDir = CorePS::DownloadDirectory(lock);
+    downloadDir = CorePS::AsyncSignalDumpDirectory(lock);
 
     // This needs to be done within the context of the lock, as otherwise
-    // another thread might modify CorePS::mDownloadDirectory while we're
+    // another thread might modify CorePS::mAsyncSignalDumpDirectory while we're
     // cloning the pointer.
     if (downloadDir) {
       nsCOMPtr<nsIFile> d;
@@ -6943,12 +6943,12 @@ bool profiler_is_paused() {
 }
 
 // See `ProfilerControl.h` for more details.
-void profiler_lookup_download_directory() {
+void profiler_lookup_async_signal_dump_directory() {
 // This implementation is causing issues on Windows (see Bug 1890154) but as it
 // only exists to support the posix signal handling (on non-windows platforms)
 // we can remove it for now.
 #if !defined(XP_WIN)
-  LOG("profiler_lookup_download_directory");
+  LOG("profiler_lookup_async_signal_dump_directory");
 
   MOZ_ASSERT(
       NS_IsMainThread(),
@@ -6959,16 +6959,47 @@ void profiler_lookup_download_directory() {
 
   // take the lock so that we can write to CorePS
   PSAutoLock lock;
+  nsresult rv;
 
-  nsCOMPtr<nsIFile> tDownloadDir;
-  nsresult rv = NS_GetSpecialDirectory(NS_OS_DEFAULT_DOWNLOAD_DIR,
-                                       getter_AddRefs(tDownloadDir));
-  if (NS_FAILED(rv)) {
-    LOG("Failed to find download directory. Profiler signal handling will not "
-        "be able to save to disk. Error: %s",
-        GetStaticErrorName(rv));
+  // Check to see if we have a `MOZ_UPLOAD_DIR` first - i.e., check to see if
+  // we're running in CI.
+  LOG("Checking if MOZ_UPLOAD_DIR exists");
+  const char* mozUploadDir = getenv("MOZ_UPLOAD_DIR");
+  if (mozUploadDir && mozUploadDir[0] != '\0') {
+    LOG("Found MOZ_UPLOAD_DIR at: %s", mozUploadDir);
+    // We want to do the right thing, and turn this into an nsIFile. Go through
+    // the motions here:
+    nsCOMPtr<nsIFile> mozUploadDirFile =
+        do_CreateInstance("@mozilla.org/file/local;1", &rv);
+
+    if (NS_FAILED(rv)) {
+      LOG("Failed to create nsIFile for MOZ_UPLOAD_DIR: %s, Error: %s",
+          mozUploadDir, GetStaticErrorName(rv));
+      return;
+    }
+
+    rv = mozUploadDirFile->InitWithNativePath(nsDependentCString(mozUploadDir));
+
+    if (NS_FAILED(rv)) {
+      LOG("Failed to assign a filepath while creating MOZ_UPLOAD_DIR file "
+          "%s, Error %s ",
+          mozUploadDir, GetStaticErrorName(rv));
+      return;
+    }
+
+    CorePS::SetAsyncSignalDumpDirectory(lock, Some(mozUploadDirFile));
   } else {
-    CorePS::SetDownloadDirectory(lock, Some(tDownloadDir));
+    LOG("Defaulting to the user's Download directory for profile dumps");
+    nsCOMPtr<nsIFile> tDownloadDir;
+    rv = NS_GetSpecialDirectory(NS_OS_DEFAULT_DOWNLOAD_DIR,
+                                getter_AddRefs(tDownloadDir));
+    if (NS_FAILED(rv)) {
+      LOG("Failed to find download directory. Profiler signal handling will "
+          "not be able to save to disk. Error: %s",
+          GetStaticErrorName(rv));
+    } else {
+      CorePS::SetAsyncSignalDumpDirectory(lock, Some(tDownloadDir));
+    }
   }
 #endif
 }

tools/profiler/public/ProfilerControl.h

@@ -40,7 +40,7 @@ static inline void profiler_init(void* stackTop) {}
 static inline void profiler_shutdown(
     IsFastShutdown aIsFastShutdown = IsFastShutdown::No) {}
 
-static inline void profiler_lookup_download_directory() {}
+static inline void profiler_lookup_async_signal_dump_directory() {}
 
 #else  // !MOZ_GECKO_PROFILER
 
@@ -138,7 +138,7 @@ void profiler_ensure_started(
 // third option, by giving us a hook to look for the download directory when
 // the time is right. This might be triggered internally (e.g. when we start
 // profiling), or externally, e.g. after the directory service is initialised.
-void profiler_lookup_download_directory();
+void profiler_lookup_async_signal_dump_directory();
 
 //---------------------------------------------------------------------------
 // Control the profiler

tools/profiler/tests/browser/browser_test_feature_multiprocess_capture_with_signal.js

@@ -2,16 +2,6 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-async function get_profile_path_on_disk(pid) {
-  // Get the system downloads directory, and use it to build a profile file
-  let profile = FileUtils.File(await Downloads.getSystemDownloadsDirectory());
-
-  // use the pid to construct the name of the profile, and resulting file
-  profile.append(`profile_0_${pid}.json`);
-
-  return profile;
-}
-
 function check_profile_contains_parent_and_content_pids(
   parent_pid,
   content_pid,
@@ -153,14 +143,14 @@ add_task(
 
       // Check that we have a profile written to disk:
       info(`Retrieving profile file.`);
-      let profile_file = await get_profile_path_on_disk(parent_pid);
+      let profile_file = await getFullProfilePath(parent_pid);
       Assert.ok(
-        await IOUtils.exists(profile_file.path),
+        await IOUtils.exists(profile_file),
         "A profile file should be written to disk."
       );
 
       // Read the profile from the json file
-      let profile = await IOUtils.readJSON(profile_file.path);
+      let profile = await IOUtils.readJSON(profile_file);
       info("Found this many proceses: " + profile.processes.length);
 
       // check for processes and the synthetic marker

tools/profiler/tests/shared-head.js

@@ -391,6 +391,21 @@ function escapeStringRegexp(string) {
 
 /** ------ Utility functions and definitions for raising POSIX signals ------ */
 
+ChromeUtils.defineESModuleGetters(this, {
+  Downloads: "resource://gre/modules/Downloads.sys.mjs",
+});
+
+// Find the path for a profile written to disk because of signals
+async function getFullProfilePath(pid) {
+  // Initially look for "MOZ_UPLOAD_DIR". If this exists, firefox will write the
+  // profile file here, so this is where we need to look.
+  let path = Services.env.get("MOZ_UPLOAD_DIR");
+  if (!path) {
+    path = await Downloads.getSystemDownloadsDirectory();
+  }
+  return PathUtils.join(path, `profile_0_${pid}.json`);
+}
+
 // Hardcode the constants SIGUSR1 and SIGUSR2.
 // This is an absolutely terrible idea, as they are implementation defined!
 // However, it turns out that for 99% of the platforms we care about, and for

tools/profiler/tests/xpcshell/test_feature_posix_signals.js

@@ -3,24 +3,15 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 ChromeUtils.defineESModuleGetters(this, {
-  Downloads: "resource://gre/modules/Downloads.sys.mjs",
-  FileUtils: "resource://gre/modules/FileUtils.sys.mjs",
   TestUtils: "resource://testing-common/TestUtils.sys.mjs",
 });
 
 async function cleanupAfterTest() {
-  // We need to cleanup written profiles after a test
-  // Get the system downloads directory, and use it to build a profile file
-  let profile = FileUtils.File(await Downloads.getSystemDownloadsDirectory());
-
-  // Get the process ID
-  let pid = Services.appinfo.processID;
-
-  // write it to the profile file name
-  profile.append(`profile_0_${pid}.json`);
+  // Get the full path to the profile
+  let profile = await getFullProfilePath(Services.appinfo.processID);
 
   // remove the file!
-  await IOUtils.remove(profile.path, { ignoreAbsent: true });
+  await IOUtils.remove(profile, { ignoreAbsent: true });
 
   // Make sure the profiler is fully stopped, even if the test failed
   await Services.profiler.StopProfiler();
@@ -101,7 +92,7 @@ add_task(async () => {
 
 add_task(async () => {
   info(
-    "Test that stopping the profiler with a posix signal writes a profile file to the system download directory."
+    "Test that stopping the profiler with a posix signal writes a profile file disk."
   );
   registerCleanupFunction(cleanupAfterTest);
 
@@ -115,14 +106,9 @@ add_task(async () => {
   const threads = [];
   const features = [];
 
-  // Get the system downloads directory, and use it to build a profile file
-  let profile = FileUtils.File(await Downloads.getSystemDownloadsDirectory());
-
-  // Get the process ID
+  // get the full path to the expected profile file:
   let pid = Services.appinfo.processID;
-
-  // use the pid to construct the name of the profile, and resulting file
-  profile.append(`profile_0_${pid}.json`);
+  let profile = await getFullProfilePath(pid);
 
   // Start the profiler, and ensure that it's active
   await Services.profiler.StartProfiler(entries, interval, threads, features);
@@ -140,7 +126,7 @@ add_task(async () => {
 
   // Now that it's stopped, make sure that we have a profile file
   Assert.ok(
-    await IOUtils.exists(profile.path),
+    await IOUtils.exists(profile),
     "A profile file should be written to disk."
   );
 

xpcom/build/XPCOMInit.cpp

@@ -478,7 +478,7 @@ NS_InitXPCOM(nsIServiceManager** aResult, nsIFile* aBinDirectory,
   // Now that both the profiler and directory services have been started
   // we can find the download directory, where the profiler can write
   // profiles if necessary
-  profiler_lookup_download_directory();
+  profiler_lookup_async_signal_dump_directory();
 
   // Init mozilla::SharedThreadPool (which needs the service manager).
   mozilla::SharedThreadPool::InitStatics();