Bug 1037445: When pre-Vista, for testing purposes allow std handles to be inherited by child process when an env var is set. r=jmaher,dvander
authorBob Owen <bobowencode@gmail.com>
Wed, 15 Oct 2014 08:26:39 +0100
changeset 210505 522b1d79b69e24d8dff4b3204ed13360f075e521
parent 210504 8d0aca89e1b264e607f151fd1f23f6b15ccf3e0d
child 210506 ec4e1f0988cd4ceedebd57c5f9bce536b823ed97
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersjmaher, dvander
bugs1037445
milestone36.0a1
Bug 1037445: When pre-Vista, for testing purposes allow std handles to be inherited by child process when an env var is set. r=jmaher,dvander
ipc/chromium/src/base/process_util_win.cc
testing/mochitest/runtests.py
--- a/ipc/chromium/src/base/process_util_win.cc
+++ b/ipc/chromium/src/base/process_util_win.cc
@@ -12,16 +12,17 @@
 #include <winternl.h>
 #include <psapi.h>
 
 #include "base/histogram.h"
 #include "base/logging.h"
 #include "base/win_util.h"
 
 #include <algorithm>
+#include "prenv.h"
 
 namespace {
 
 // System pagesize. This value remains constant on x86/64 architectures.
 const int PAGESIZE_KB = 4;
 
 // HeapSetInformation function pointer.
 typedef BOOL (WINAPI* HeapSetFn)(HANDLE, HEAP_INFORMATION_CLASS, PVOID, SIZE_T);
@@ -270,20 +271,19 @@ void FreeThreadAttributeList(LPPROC_THRE
 bool LaunchApp(const std::wstring& cmdline,
                bool wait, bool start_hidden, ProcessHandle* process_handle) {
 
   // We want to inherit the std handles so dump() statements and assertion
   // messages in the child process can be seen - but we *do not* want to
   // blindly have all handles inherited.  Vista and later has a technique
   // where only specified handles are inherited - so we use this technique if
   // we can.  If that technique isn't available (or it fails), we just don't
-  // inherit anything.  This means that dump() etc isn't going to be seen on
-  // XP release builds, but that's OK (developers who really care can run a
-  // debug build on XP, where the processes are marked as "console" apps, so
-  // things work without these hoops)
+  // inherit anything.  This can cause us a problem for Windows XP testing,
+  // because we sometimes need the handles to get inherited for test logging to
+  // work. So we also inherit when a specific environment variable is set.
   DWORD dwCreationFlags = 0;
   BOOL bInheritHandles = FALSE;
   // We use a STARTUPINFOEX, but if we can't do the thread attribute thing, we
   // just pass the size of a STARTUPINFO.
   STARTUPINFOEX startup_info_ex;
   ZeroMemory(&startup_info_ex, sizeof(startup_info_ex));
   STARTUPINFO &startup_info = startup_info_ex.StartupInfo;
   startup_info.cb = sizeof(startup_info);
@@ -300,31 +300,40 @@ bool LaunchApp(const std::wstring& cmdli
     HANDLE stdOut = ::GetStdHandle(STD_OUTPUT_HANDLE);
     HANDLE stdErr = ::GetStdHandle(STD_ERROR_HANDLE);
 
     if (IsInheritableHandle(stdOut))
       handlesToInherit[handleCount++] = stdOut;
     if (stdErr != stdOut && IsInheritableHandle(stdErr))
       handlesToInherit[handleCount++] = stdErr;
 
-    if (handleCount)
+    if (handleCount) {
       lpAttributeList = CreateThreadAttributeList(handlesToInherit, handleCount);
-  }
-
-  if (lpAttributeList) {
-    // it's safe to inherit handles, so arrange for that...
-    startup_info.cb = sizeof(startup_info_ex);
+      if (lpAttributeList) {
+        // it's safe to inherit handles, so arrange for that...
+        startup_info.cb = sizeof(startup_info_ex);
+        startup_info.dwFlags |= STARTF_USESTDHANDLES;
+        startup_info.hStdOutput = stdOut;
+        startup_info.hStdError = stdErr;
+        startup_info.hStdInput = INVALID_HANDLE_VALUE;
+        startup_info_ex.lpAttributeList = lpAttributeList;
+        dwCreationFlags |= EXTENDED_STARTUPINFO_PRESENT;
+        bInheritHandles = TRUE;
+      }
+    }
+  } else if (PR_GetEnv("MOZ_WIN_INHERIT_STD_HANDLES_PRE_VISTA")) {
+    // Even if we can't limit what gets inherited, we sometimes want to inherit
+    // stdout/err for testing purposes.
     startup_info.dwFlags |= STARTF_USESTDHANDLES;
     startup_info.hStdOutput = ::GetStdHandle(STD_OUTPUT_HANDLE);
     startup_info.hStdError = ::GetStdHandle(STD_ERROR_HANDLE);
     startup_info.hStdInput = INVALID_HANDLE_VALUE;
-    startup_info_ex.lpAttributeList = lpAttributeList;
-    dwCreationFlags |= EXTENDED_STARTUPINFO_PRESENT;
     bInheritHandles = TRUE;
   }
+
   PROCESS_INFORMATION process_info;
   BOOL createdOK = CreateProcess(NULL,
                      const_cast<wchar_t*>(cmdline.c_str()), NULL, NULL,
                      bInheritHandles, dwCreationFlags, NULL, NULL,
                      &startup_info, &process_info);
   if (lpAttributeList)
     FreeThreadAttributeList(lpAttributeList);
   if (!createdOK)
--- a/testing/mochitest/runtests.py
+++ b/testing/mochitest/runtests.py
@@ -1233,16 +1233,24 @@ class Mochitest(MochitestUtilsMixin):
     browserEnv = self.environment(xrePath=options.xrePath, env=env,
                                   debugger=debugger, dmdPath=options.dmdPath,
                                   lsanPath=lsanPath)
 
     # These variables are necessary for correct application startup; change
     # via the commandline at your own risk.
     browserEnv["XPCOM_DEBUG_BREAK"] = "stack"
 
+    # When creating child processes on Windows pre-Vista (e.g. Windows XP) we
+    # don't normally inherit stdout/err handles, because you can only do it by
+    # inheriting all other inheritable handles as well.
+    # We need to inherit them for plain mochitests for test logging purposes, so
+    # we do so on the basis of a specific environment variable.
+    if self.getTestFlavor(options) == "mochitest":
+      browserEnv["MOZ_WIN_INHERIT_STD_HANDLES_PRE_VISTA"] = "1"
+
     # interpolate environment passed with options
     try:
       browserEnv.update(dict(parseKeyValue(options.environment, context='--setenv')))
     except KeyValueParseError, e:
       self.log.error(str(e))
       return None
 
     browserEnv["XPCOM_MEM_BLOAT_LOG"] = self.leak_report_file