Bug 1390488 - Pass the childProcessTmpDir from the parent process to the GPU process. r=froydnj
☠☠ backed out by 0fd97303a92d ☠ ☠
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 13 Sep 2017 10:18:15 -0400
changeset 430063 b80e267bdf307f08a49810729a31a567e39fa0c5
parent 430062 fdffa51b68d174710c7dc502d23f827b8bed468a
child 430064 1873e0e3b4c84f052309a61bb9c9e6bcc36a1d8d
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1390488
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1390488 - Pass the childProcessTmpDir from the parent process to the GPU process. r=froydnj The GPU process doesn't have the directory service enabled, so it can't find a tmp dir to put its .extra files for crash reports. Even if we do enable the directory service, we still don't get the correct "content process tmp dir" in the GPU process, because the UUID baked into that folder is passed via the preferences service, and that isn't initialized in the GPU process either. Rather than unneccessarily initialize all this stuff in the GPU process just to get one folder name, we can pass that folder name directly in the argv list. See comments 12-19 on the bug for further discussion of the various solutions attempted/explored. MozReview-Commit-ID: 1sFg27hIe7S
ipc/glue/GeckoChildProcessHost.cpp
toolkit/crashreporter/nsExceptionHandler.cpp
toolkit/crashreporter/nsExceptionHandler.h
toolkit/xre/nsEmbedFunctions.cpp
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -856,16 +856,28 @@ GeckoChildProcessHost::PerformAsyncLaunc
       childArgv.push_back("-appomni");
       childArgv.push_back(path.get());
     }
   }
 
   // Add the application directory path (-appdir path)
   AddAppDirToCommandLine(childArgv);
 
+  // Tmp dir that the GPU process should use for crash reports. This arg is
+  // always populated (but possibly with an empty value) for a GPU child process.
+  if (mProcessType == GeckoProcessType_GPU) {
+    nsCOMPtr<nsIFile> file;
+    CrashReporter::GetChildProcessTmpDir(getter_AddRefs(file));
+    nsAutoCString path;
+    if (file) {
+      file->GetNativePath(path);
+    }
+    childArgv.push_back(path.get());
+  }
+
   childArgv.push_back(pidstring);
 
 # if defined(MOZ_CRASHREPORTER)
 #  if defined(OS_LINUX) || defined(OS_BSD) || defined(OS_SOLARIS)
   int childCrashFd, childCrashRemapFd;
   if (!CrashReporter::CreateNotificationPipeForChild(
         &childCrashFd, &childCrashRemapFd))
     return false;
@@ -1099,16 +1111,29 @@ GeckoChildProcessHost::PerformAsyncLaunc
 
   // XXX Command line params past this point are expected to be at
   // the end of the command line string, and in a specific order.
   // See XRE_InitChildProcess in nsEmbedFunction.
 
   // Win app model id
   cmdLine.AppendLooseValue(mGroupId.get());
 
+  // Tmp dir that the GPU process should use for crash reports. This arg is
+  // always populated (but possibly with an empty value) for a GPU child process.
+  if (mProcessType == GeckoProcessType_GPU) {
+    nsCOMPtr<nsIFile> file;
+    CrashReporter::GetChildProcessTmpDir(getter_AddRefs(file));
+    nsString path;
+    if (file) {
+      MOZ_ALWAYS_SUCCEEDS(file->GetPath(path));
+    }
+    std::wstring wpath(path.get());
+    cmdLine.AppendLooseValue(wpath);
+  }
+
   // Process id
   cmdLine.AppendLooseValue(UTF8ToWide(pidstring));
 
 # if defined(MOZ_CRASHREPORTER)
   cmdLine.AppendLooseValue(
     UTF8ToWide(CrashReporter::GetChildNotificationPipe()));
 # endif // defined(MOZ_CRASHREPORTER)
 
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -3585,16 +3585,27 @@ OOPDeinit()
   pidToMinidump = nullptr;
 
 #if defined(XP_WIN) || defined(XP_MACOSX)
   free(childCrashNotifyPipe);
   childCrashNotifyPipe = nullptr;
 #endif
 }
 
+void
+GetChildProcessTmpDir(nsIFile** aOutTmpDir)
+{
+  MOZ_ASSERT(XRE_IsParentProcess());
+#if (defined(XP_MACOSX) || defined(XP_WIN))
+  if (childProcessTmpDir) {
+    CreateFileFromPath(*childProcessTmpDir, aOutTmpDir);
+  }
+#endif
+}
+
 #if defined(XP_WIN) || defined(XP_MACOSX)
 // Parent-side API for children
 const char*
 GetChildNotificationPipe()
 {
   if (!GetEnabled())
     return kNullNotifyPipe;
 
@@ -3739,19 +3750,24 @@ GetLastRunCrashID(nsAString& id)
   }
 
   id = *lastRunCrashID;
   return true;
 }
 
 #if defined(XP_WIN) || defined(XP_MACOSX)
 void
-InitChildProcessTmpDir()
+InitChildProcessTmpDir(nsIFile* aDirOverride)
 {
   MOZ_ASSERT(!XRE_IsParentProcess());
+  if (aDirOverride) {
+    childProcessTmpDir = CreatePathFromFile(aDirOverride);
+    return;
+  }
+
   // When retrieved by the child process, this will always resolve to the
   // correct directory regardless of sandbox level.
   nsCOMPtr<nsIFile> tmpDir;
   nsresult rv = NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(tmpDir));
   if (NS_SUCCEEDED(rv)) {
     childProcessTmpDir = CreatePathFromFile(tmpDir);
   }
 }
--- a/toolkit/crashreporter/nsExceptionHandler.h
+++ b/toolkit/crashreporter/nsExceptionHandler.h
@@ -212,16 +212,20 @@ CreateMinidumpsAndPair(ProcessHandle aTa
 // a minidump (|parentMinidump|).
 // The resulting dump will get the id of the parent and use the |name| as
 // an extension.
 bool CreateAdditionalChildMinidump(ProcessHandle childPid,
                                    ThreadId childBlamedThread,
                                    nsIFile* parentMinidump,
                                    const nsACString& name);
 
+// Parent-side API, returns the tmp dir for child processes to use, accounting
+// for sandbox considerations.
+void GetChildProcessTmpDir(nsIFile** aOutTmpDir);
+
 #  if defined(XP_WIN32) || defined(XP_MACOSX)
 // Parent-side API for children
 const char* GetChildNotificationPipe();
 
 #ifdef MOZ_CRASHREPORTER_INJECTOR
 // Inject a crash report client into an arbitrary process, and inform the
 // callback object when it crashes. Parent process only.
 
@@ -242,17 +246,17 @@ public:
 
 // This method implies OOPInit
 void InjectCrashReporterIntoProcess(DWORD processID, InjectorCrashCallback* cb);
 void UnregisterInjectorCallback(DWORD processID);
 #endif
 
 // Child-side API
 bool SetRemoteExceptionHandler(const nsACString& crashPipe);
-void InitChildProcessTmpDir();
+void InitChildProcessTmpDir(nsIFile* aDirOverride = nullptr);
 
 #  elif defined(XP_LINUX)
 // Parent-side API for children
 
 // Set the outparams for crash reporter server's fd (|childCrashFd|)
 // and the magic fd number it should be remapped to
 // (|childCrashRemapFd|) before exec() in the child process.
 // |SetRemoteExceptionHandler()| in the child process expects to find
--- a/toolkit/xre/nsEmbedFunctions.cpp
+++ b/toolkit/xre/nsEmbedFunctions.cpp
@@ -558,16 +558,30 @@ XRE_InitChildProcess(int aArgc,
   const char* const parentPIDString = aArgv[aArgc-1];
   MOZ_ASSERT(parentPIDString, "NULL parent PID");
   --aArgc;
 
   char* end = 0;
   base::ProcessId parentPID = strtol(parentPIDString, &end, 10);
   MOZ_ASSERT(!*end, "invalid parent PID");
 
+  nsCOMPtr<nsIFile> crashReportTmpDir;
+  if (XRE_GetProcessType() == GeckoProcessType_GPU) {
+    aArgc--;
+    if (strlen(aArgv[aArgc])) { // if it's empty, ignore it
+      nsresult rv = XRE_GetFileFromPath(aArgv[aArgc], getter_AddRefs(crashReportTmpDir));
+      if (NS_FAILED(rv)) {
+        // If we don't have a valid tmp dir we can probably still run ok, but
+        // crash report .extra files might not get picked up by the parent
+        // process. Debug-assert because this shouldn't happen in practice.
+        MOZ_ASSERT(false, "GPU process started without valid tmp dir!");
+      }
+    }
+  }
+
 #ifdef XP_MACOSX
   mozilla::ipc::SharedMemoryBasic::SetupMachMemory(parentPID, ports_in_receiver, ports_in_sender,
                                                    ports_out_sender, ports_out_receiver, true);
 #endif
 
 #if defined(XP_WIN)
   // On Win7+, register the application user model id passed in by
   // parent. This insures windows created by the container properly
@@ -657,17 +671,17 @@ XRE_InitChildProcess(int aArgc,
       }
 
       if (!process->Init(aArgc, aArgv)) {
         return NS_ERROR_FAILURE;
       }
 
 #ifdef MOZ_CRASHREPORTER
 #if defined(XP_WIN) || defined(XP_MACOSX)
-      CrashReporter::InitChildProcessTmpDir();
+      CrashReporter::InitChildProcessTmpDir(crashReportTmpDir);
 #endif
 #endif
 
 #if defined(XP_WIN)
       // Set child processes up such that they will get killed after the
       // chrome process is killed in cases where the user shuts the system
       // down or logs off.
       ::SetProcessShutdownParameters(0x280 - 1, SHUTDOWN_NORETRY);