Fix Windows process implementation to only inherit the three

runtime/bin/process_win.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include <process.h>
 
 #include "bin/builtin.h"
 #include "bin/process.h"
 #include "bin/eventhandler.h"
 #include "bin/log.h"
@@ skipping 371 matching lines @@
     return error_code;
   }
   if (!CreateProcessPipe(exit_handles, pipe_names[3], kInheritNone)) {
     int error_code = SetOsErrorMessage(os_error_message, os_error_message_len);
     CloseProcessPipes(
         stdin_handles, stdout_handles, stderr_handles, exit_handles);
     return error_code;
   }
 
   // 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));
 
   // Transform input strings to system format.
   path = StringUtils::Utf8ToSystemString(path);
   for (int i = 0; i < arguments_length; i++) {
      arguments[i] = StringUtils::Utf8ToSystemString(arguments[i]);
   }
 
   // Compute command-line length.
   int command_line_length = strlen(path);
   for (int i = 0; i < arguments_length; i++) {
     command_line_length += strlen(arguments[i]);
   }
   // Account for null termination and one space per argument.
   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);
     free(const_cast<char*>(path));
     for (int i = 0; i < arguments_length; i++) free(arguments[i]);
     return error_code;
   }
 
   // Put together command-line string.
   char* command_line = new char[command_line_length];
   int len = 0;
@@ skipping 46 matching lines @@
   if (working_directory != NULL) {
     working_directory = StringUtils::Utf8ToSystemString(working_directory);
   }
 
   // Create process.
   BOOL result = CreateProcess(NULL,   // ApplicationName
                               command_line,
                               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.
   delete[] command_line;
   delete[] environment_block;
   if (working_directory != NULL) {
     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(
         stdin_handles, stdout_handles, stderr_handles, exit_handles);
     return error_code;
   }
 
   ProcessInfoList::AddProcess(process_info.dwProcessId,
                               process_info.hProcess,
                               exit_handles[kWriteHandle]);
@@ skipping 36 matching lines @@
 
 
 void Process::TerminateExitCodeHandler() {
   // Nothing needs to be done on Windows.
 }
 
 
 intptr_t Process::CurrentProcessId() {
   return static_cast<intptr_t>(GetCurrentProcessId());
 }