Bug 1386404 - The environment has to live forever. r=jld
☠☠ backed out by 1aa6d3251c70 ☠ ☠
authorGian-Carlo Pascutto <gcp@mozilla.com>
Tue, 31 Oct 2017 15:06:53 +0100
changeset 443370 6224ffae752a05d65fa0c9eb5fe3e89c38820e55
parent 443369 9eba087cf64acab81613424711b160b476340cc0
child 443371 c80acdea24c1c7954c4560c05d4625776ac09134
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjld
bugs1386404
milestone58.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 1386404 - The environment has to live forever. r=jld MozReview-Commit-ID: Lux9D8Ss8fK
ipc/glue/GeckoChildProcessHost.cpp
ipc/glue/GeckoChildProcessHost.h
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -469,40 +469,51 @@ GeckoChildProcessHost::GetChildLogName(c
   // Append child-specific postfix to name
   buffer.AppendLiteral(".child-");
   buffer.AppendInt(mChildCounter);
 }
 
 class AutoSetAndRestoreEnvVarForChildProcess {
 public:
   AutoSetAndRestoreEnvVarForChildProcess(const char* envVar,
-                                         const char* newVal) {
+                                         const char* newVal,
+                                         nsCString& saveString) {
     const char* origVal = PR_GetEnv(envVar);
-    mSetString.Assign(envVar);
-    mSetString.Append('=');
-    mRestoreString.Assign(mSetString);
+    if (origVal != nullptr) {
+      mSetString.Assign(envVar);
+      mSetString.Append('=');
+      saveString.Assign(mSetString);
 
-    mSetString.Append(newVal);
-    mRestoreString.Append(origVal);
+      mSetString.Append(newVal);
+      saveString.Append(origVal);
+      mRestorePtr = saveString.get();
 
-    // Passing to PR_SetEnv is ok here if we keep the the storage alive
-    // for the time we launch the sub-process.  It's copied to the new
-    // environment by PR_DuplicateEnvironment()
-    PR_SetEnv(mSetString.get());
+      // Passing to PR_SetEnv is ok here if we keep the the storage alive
+      // for the time we launch the sub-process.  It's copied to the new
+      // environment by PR_DuplicateEnvironment()
+      PR_SetEnv(mSetString.get());
+    }
   }
   // Delegate helper
   AutoSetAndRestoreEnvVarForChildProcess(const char* envVar,
-                                         nsCString& newVal)
-    : AutoSetAndRestoreEnvVarForChildProcess(envVar, newVal.get()) {}
+                                         nsCString& newVal,
+                                         nsCString& saveString)
+    : AutoSetAndRestoreEnvVarForChildProcess(envVar, newVal.get(),
+                                             saveString) {}
   ~AutoSetAndRestoreEnvVarForChildProcess() {
-    PR_SetEnv(mRestoreString.get());
+    if (mRestorePtr) {
+      PR_SetEnv(mRestorePtr);
+    }
   }
 private:
+  // Needs to live for process launch.
   nsAutoCString mSetString;
-  nsAutoCString mRestoreString;
+  // This storage cannot die for the duration of *any* process in our
+  // inheritance tree, because PR_SetEnv points to it directly.
+  const char* mRestorePtr{nullptr};
 };
 
 bool
 GeckoChildProcessHost::PerformAsyncLaunch(std::vector<std::string> aExtraOpts)
 {
 #ifdef MOZ_GECKO_PROFILER
   AutoSetProfilerEnvVarsForChildProcess profilerEnvironment;
 #endif
@@ -517,28 +528,28 @@ GeckoChildProcessHost::PerformAsyncLaunc
   Maybe<AutoSetAndRestoreEnvVarForChildProcess> rustLogDir;
 
   const char* origNSPRLogName = PR_GetEnv("NSPR_LOG_FILE");
   const char* origMozLogName = PR_GetEnv("MOZ_LOG_FILE");
 
   if (origNSPRLogName) {
     nsAutoCString nsprLogName;
     GetChildLogName(origNSPRLogName, nsprLogName);
-    nsprLogDir.emplace("NSPR_LOG_FILE", nsprLogName);
+    nsprLogDir.emplace("NSPR_LOG_FILE", nsprLogName, mRestoreOrigNSPRLogName);
   }
   if (origMozLogName) {
     nsAutoCString mozLogName;
     GetChildLogName(origMozLogName, mozLogName);
-    mozLogDir.emplace("MOZ_LOG_FILE", mozLogName);
+    mozLogDir.emplace("MOZ_LOG_FILE", mozLogName, mRestoreOrigMozLogName);
   }
 
   // `RUST_LOG_CHILD` is meant for logging child processes only.
   const char* childRustLog = PR_GetEnv("RUST_LOG_CHILD");
   if (childRustLog) {
-    rustLogDir.emplace("RUST_LOG", childRustLog);
+    rustLogDir.emplace("RUST_LOG", childRustLog, mRestoreOrigRustLog);
   }
 
 #if defined(MOZ_CONTENT_SANDBOX)
   Maybe<AutoSetAndRestoreEnvVarForChildProcess> tmpDir;
   Maybe<AutoSetAndRestoreEnvVarForChildProcess> xdgCacheHome;
   Maybe<AutoSetAndRestoreEnvVarForChildProcess> xdgCacheDir;
   Maybe<AutoSetAndRestoreEnvVarForChildProcess> mesaCacheDir;
 
@@ -546,21 +557,21 @@ GeckoChildProcessHost::PerformAsyncLaunc
   nsCOMPtr<nsIFile> mContentTempDir;
   nsresult rv = NS_GetSpecialDirectory(NS_APP_CONTENT_PROCESS_TEMP_DIR,
                                        getter_AddRefs(mContentTempDir));
   if (NS_SUCCEEDED(rv)) {
     rv = mContentTempDir->GetNativePath(tmpDirName);
     if (NS_SUCCEEDED(rv)) {
       // Point a bunch of things that might want to write from content to our
       // shiny new content-process specific tmpdir
-      tmpDir.emplace("TMPDIR", tmpDirName);
-      xdgCacheHome.emplace("XDG_CACHE_HOME", tmpDirName);
-      xdgCacheDir.emplace("XDG_CACHE_DIR", tmpDirName);
+      tmpDir.emplace("TMPDIR", tmpDirName, mRestoreTmpDir);
+      xdgCacheHome.emplace("XDG_CACHE_HOME", tmpDirName, mRestoreXdgCacheHome);
+      xdgCacheDir.emplace("XDG_CACHE_DIR", tmpDirName,  mRestoreXdgCacheDir);
       // Partial fix for bug 1380051 (not persistent - should be)
-      mesaCacheDir.emplace("MESA_GLSL_CACHE_DIR", tmpDirName);
+      mesaCacheDir.emplace("MESA_GLSL_CACHE_DIR", tmpDirName, mRestoreMesaCacheDir);
     }
   }
 #endif
 
   return PerformAsyncLaunchInternal(aExtraOpts);
 }
 
 bool
--- a/ipc/glue/GeckoChildProcessHost.h
+++ b/ipc/glue/GeckoChildProcessHost.h
@@ -192,16 +192,20 @@ private:
   std::queue<IPC::Message> mQueue;
 
   // Remember original env values so we can restore it (there is no other
   // simple way how to change environment of a child process than to modify
   // the current environment).
   nsCString mRestoreOrigNSPRLogName;
   nsCString mRestoreOrigMozLogName;
   nsCString mRestoreOrigRustLog;
+  nsCString mRestoreTmpDir;
+  nsCString mRestoreXdgCacheHome;
+  nsCString mRestoreXdgCacheDir;
+  nsCString mRestoreMesaCacheDir;
 
   static uint32_t sNextUniqueID;
 
   static bool sRunSelfAsContentProc;
 
 #if defined(MOZ_WIDGET_ANDROID)
   void LaunchAndroidService(const char* type,
                             const std::vector<std::string>& argv,