Bug 1386404 - The environment has to live forever. r?jld draft
authorGian-Carlo Pascutto <gcp@mozilla.com>
Tue, 31 Oct 2017 15:06:53 +0100
changeset 693642 331b83546cc477dffe83c39b51f24363a59fa8e5
parent 693641 5b9d7d1827c1039373263c6dda3ee15c39cc6b94
child 693643 09242b2902ff7af3d6f01d7121e851f089c50f4c
push id87881
push usergpascutto@mozilla.com
push dateMon, 06 Nov 2017 18:00:57 +0000
reviewersjld
bugs1386404
milestone58.0a1
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,