Fix Windows process implementation to only inherit the three

runtime/bin/process_win.cc

Index: runtime/bin/process_win.cc
diff --git a/runtime/bin/process_win.cc b/runtime/bin/process_win.cc
index 0a9eb520415c2fc738ffa33e0bdd388b762c2bce..7f01d6aee4345a67e2a579dbe933e7b81bb222b3 100644
--- a/runtime/bin/process_win.cc
+++ b/runtime/bin/process_win.cc
@@ -389,13 +389,55 @@ int Process::Start(const char* path,
   }
 
   // Setup info structures.
-  STARTUPINFO startup_info;
+  STARTUPINFOEX startup_info;
   ZeroMemory(&startup_info, sizeof(startup_info));
-  startup_info.cb = sizeof(startup_info);
-  startup_info.hStdInput = stdin_handles[kReadHandle];
-  startup_info.hStdOutput = stdout_handles[kWriteHandle];
-  startup_info.hStdError = stderr_handles[kWriteHandle];
-  startup_info.dwFlags = STARTF_USESTDHANDLES;
+  startup_info.StartupInfo.cb = sizeof(startup_info);
+  startup_info.StartupInfo.hStdInput = stdin_handles[kReadHandle];
+  startup_info.StartupInfo.hStdOutput = stdout_handles[kWriteHandle];
+  startup_info.StartupInfo.hStdError = stderr_handles[kWriteHandle];
+  startup_info.StartupInfo.dwFlags = STARTF_USESTDHANDLES;
+
+  // Setup the handles to inherit. We only want to inherit the three handles
+  // for stdin, stdout and stderr.
+  SIZE_T size = 0;
+  if (!InitializeProcThreadAttributeList(NULL, 1, 0, &size) &&
+      GetLastError() != ERROR_INSUFFICIENT_BUFFER) {

[ahe, 2012/12/10 10:30:51]

Why are you ignoring ERROR_INSUFFICIENT_BUFFER?

[Mads Ager (google), 2012/12/10 10:45:29]

I will add a comment about that. When you pass in NULL to
InitializeProcThreadAttributeList it retuns the size needed and reports
INSUFFICIENT_BUFFER. This is slightly weird but is apparently the way to use
this API to catch other potential errors.

+    int error_code = SetOsErrorMessage(os_error_message, os_error_message_len);
+    CloseProcessPipes(
+        stdin_handles, stdout_handles, stderr_handles, exit_handles);
+    return error_code;
+  }
+  LPPROC_THREAD_ATTRIBUTE_LIST attribute_list =
+      reinterpret_cast<LPPROC_THREAD_ATTRIBUTE_LIST>(malloc(size));
+  ZeroMemory(attribute_list, size);
+  if (!InitializeProcThreadAttributeList(attribute_list, 1, 0, &size)) {
+    free(attribute_list);

[ahe, 2012/12/10 10:30:51]

Shouldn't you call free after getting the error message?

[Mads Ager (google), 2012/12/10 10:45:29]

Yes, thanks!

+    int error_code = SetOsErrorMessage(os_error_message, os_error_message_len);
+    CloseProcessPipes(
+        stdin_handles, stdout_handles, stderr_handles, exit_handles);
+    return error_code;
+  }
+  int num_inherited_handles = 3;
+  HANDLE* inherited_handles = new HANDLE[num_inherited_handles];

[kustermann, 2012/12/08 11:53:42]

Is there a reason why you allocate this array on the heap and not on the stack?

[Mads Ager (google), 2012/12/10 10:45:29]

Thanks for catching that Martin. No, there is no reason for heap allocation
here.

+  inherited_handles[0] = stdin_handles[kReadHandle];
+  inherited_handles[1] = stdout_handles[kWriteHandle];
+  inherited_handles[2] = stderr_handles[kWriteHandle];
+  if (!UpdateProcThreadAttribute(attribute_list,
+                                 0,
+                                 PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
+                                 inherited_handles,
+                                 num_inherited_handles * sizeof(HANDLE),
+                                 NULL,
+                                 NULL)) {
+    delete[] inherited_handles;
+    DeleteProcThreadAttributeList(attribute_list);
+    free(attribute_list);
+    int error_code = SetOsErrorMessage(os_error_message, os_error_message_len);
+    CloseProcessPipes(
+        stdin_handles, stdout_handles, stderr_handles, exit_handles);
+    return error_code;
+  }
+  startup_info.lpAttributeList = attribute_list;
 
   PROCESS_INFORMATION process_info;
   ZeroMemory(&process_info, sizeof(process_info));
@@ -415,6 +457,9 @@ int Process::Start(const char* path,
   command_line_length += arguments_length + 1;
   static const int kMaxCommandLineLength = 32768;
   if (command_line_length > kMaxCommandLineLength) {
+    delete[] inherited_handles;
+    DeleteProcThreadAttributeList(attribute_list);
+    free(attribute_list);
     int error_code = SetOsErrorMessage(os_error_message, os_error_message_len);

[ahe, 2012/12/10 10:30:51]

Shouldn't you call SetOsErrorMessage first?

[Mads Ager (google), 2012/12/10 10:45:29]

Yes, thanks!

     CloseProcessPipes(
         stdin_handles, stdout_handles, stderr_handles, exit_handles);
@@ -482,10 +527,10 @@ int Process::Start(const char* path,
                               NULL,   // ProcessAttributes
                               NULL,   // ThreadAttributes
                               TRUE,   // InheritHandles
-                              0,      // CreationFlags
+                              EXTENDED_STARTUPINFO_PRESENT,
                               environment_block,
                               working_directory,
-                              &startup_info,
+                              reinterpret_cast<STARTUPINFO*>(&startup_info),
                               &process_info);
 
   // Deallocate command-line and environment block strings.
@@ -495,6 +540,10 @@ int Process::Start(const char* path,
     free(const_cast<char*>(working_directory));
   }
 
+  delete[] inherited_handles;
+  DeleteProcThreadAttributeList(attribute_list);
+  free(attribute_list);
+
   if (result == 0) {
     int error_code = SetOsErrorMessage(os_error_message, os_error_message_len);
     CloseProcessPipes(