Bug 1321875: Fix relative lifetimes of lpAttributeList and handlesToInherit in LaunchApp. r=aklotz
authorDavid Major <dmajor@mozilla.com>
Fri, 02 Dec 2016 14:49:41 -0600
changeset 325165 72cd631fdd7957fccec0beea95f6b89df27fe65a
parent 325164 eb3ac9d2775b8c4e7b089cab4d33d953d6605f0e
child 325166 18109e54e6ccf6f250f6ff62fe2408f087d7c18b
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewersaklotz
bugs1321875
milestone53.0a1
Bug 1321875: Fix relative lifetimes of lpAttributeList and handlesToInherit in LaunchApp. r=aklotz CreateThreadAttributeList warns: // Note that the pointer to the HANDLE array ends up embedded in the result of // this function and must stay alive until FreeThreadAttributeList is called, // hence it is passed in so the owner is the caller of this function. but the caller was passing a |handlesToInherit| that was declared inside a block scope that ends before we're finished using lpAttributeList. This happened to work on MSVC but leads to badness under clang-cl. + Bonus fix for a sometimes-uninitialized warning in CreateThreadAttributeList. MozReview-Commit-ID: 6uu3ICjfj5k
ipc/chromium/src/base/process_util_win.cc
--- a/ipc/chromium/src/base/process_util_win.cc
+++ b/ipc/chromium/src/base/process_util_win.cc
@@ -234,17 +234,17 @@ LPPROC_THREAD_ATTRIBUTE_LIST CreateThrea
     LoadThreadAttributeFunctions();
   // shouldn't happen as we are only called for Vista+, but better safe than sorry...
   if (!InitializeProcThreadAttributeListPtr ||
       !DeleteProcThreadAttributeListPtr ||
       !UpdateProcThreadAttributePtr)
     return NULL;
 
   SIZE_T threadAttrSize;
-  LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList;
+  LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList = NULL;
 
   if (!(*InitializeProcThreadAttributeListPtr)(NULL, 1, 0, &threadAttrSize) &&
       GetLastError() != ERROR_INSUFFICIENT_BUFFER)
     goto fail;
   lpAttributeList = reinterpret_cast<LPPROC_THREAD_ATTRIBUTE_LIST>
                               (malloc(threadAttrSize));
   if (!lpAttributeList ||
       !(*InitializeProcThreadAttributeListPtr)(lpAttributeList, 1, 0, &threadAttrSize))
@@ -291,23 +291,26 @@ bool LaunchApp(const std::wstring& cmdli
   // 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);
   startup_info.dwFlags = STARTF_USESHOWWINDOW;
   startup_info.wShowWindow = start_hidden ? SW_HIDE : SW_SHOW;
 
+  // Per the comment in CreateThreadAttributeList, lpAttributeList will contain
+  // a pointer to handlesToInherit, so make sure they have the same lifetime.
   LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList = NULL;
+  HANDLE handlesToInherit[2];
+  int handleCount = 0;
+
   // Don't even bother trying pre-Vista...
   if (mozilla::IsVistaOrLater()) {
     // setup our handle array first - if we end up with no handles that can
     // be inherited we can avoid trying to do the ThreadAttributeList dance...
-    HANDLE handlesToInherit[2];
-    int handleCount = 0;
     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;