[stable] Fix Process::Exec stdio handle inheritance
Issue description: dart run crashes on Windows in GitBash or similar
terminals.
What is the fix: Fix Process::Exec to account for situations when
stdout and stderr are the same handles.
Why cherry-pick: Currently dart run is completely broken on certain
terminal emulators.
Risk: Low, landed on main and tested.
Issue link(s): https://github.com/dart-lang/sdk/issues/61981
TEST=vm/dart/regress_61981
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/464382
Change-Id: I357725f8982814c02f28267a8629ce5099fe4b39
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/465560
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
diff --git a/BUILD.gn b/BUILD.gn
index 25f4b6a..6f33af7 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -51,8 +51,8 @@
"samples/embedder:kernel",
"samples/ffi/http:fake_http",
"samples/ffi/httpIG:fake_httpIG",
- "utils/dartdev:dartdev",
- "utils/kernel-service:kernel-service",
+ "utils/dartdev",
+ "utils/kernel-service",
]
# The following dependencies allow dartdev to start the resident frontend
@@ -68,8 +68,8 @@
]
} else {
deps += [
- "utils/dds:dds",
- "utils/dtd:dtd",
+ "utils/dds",
+ "utils/dtd",
"utils/kernel-service:frontend_server",
]
}
@@ -104,6 +104,8 @@
deps += [ "runtime/bin:abstract_socket_test" ]
} else if (is_fuchsia) {
deps += [ ":fuchsia_test_package" ]
+ } else if (is_win) {
+ deps += [ "runtime/bin:create_process_test_helper" ]
}
}
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4f13164..24c539e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,9 @@
- Fixes an issue with the analyzer not emitting an error when using a dot
shorthand with type arguments on a factory constructor in an abstract class.
(issue [dart-lang/sdk#61978])
+- Fixes an issue with `dart run` not working and simply exiting with
+ `Process::Exec - (null)` under GitBash on Windows.
+ (issue [dart-lang/sdk#61981])
[Dart-Code/Dart-Code#61978]: https://github.com/Dart-Code/Dart-Code/issues/5810
[dart-lang/sdk#61996]: https://github.com/dart-lang/sdk/issues/61996
@@ -23,6 +26,7 @@
[flutter/flutter#178740]: https://github.com/flutter/flutter/issues/178740
[dart-lang/sdk#62136]: https://github.com/dart-lang/sdk/issues/62136
[dart-lang/sdk#61978]: https://github.com/dart-lang/sdk/issues/61978
+[dart-lang/sdk#61981]: https://github.com/dart-lang/sdk/issues/61981
## 3.10.2
diff --git a/runtime/bin/BUILD.gn b/runtime/bin/BUILD.gn
index b3b71b3..cfb5ede 100644
--- a/runtime/bin/BUILD.gn
+++ b/runtime/bin/BUILD.gn
@@ -1142,6 +1142,11 @@
include_dirs = [ ".." ]
}
+executable("create_process_test_helper") {
+ sources = [ "create_process_test_helper.cc" ]
+ include_dirs = [ ".." ]
+}
+
source_set("run_vm_tests_set") {
if (target_os == "fuchsia") {
testonly = true
diff --git a/runtime/bin/create_process_test_helper.cc b/runtime/bin/create_process_test_helper.cc
new file mode 100644
index 0000000..5e37e03
--- /dev/null
+++ b/runtime/bin/create_process_test_helper.cc
@@ -0,0 +1,138 @@
+// Copyright (c) 2025, 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.
+
+// This is a utility program for testing that Dart binary correctly handles
+// situations when stdout and stderr handles are the same.
+//
+// This can happen in certain terminal emulators, e.g. one used by GitBash
+// see https://github.com/dart-lang/sdk/issues/61981 for an example.
+
+#include "platform/globals.h"
+#if defined(DART_HOST_OS_WINDOWS)
+
+#include <cstdio>
+#include <source_location>
+#include <sstream>
+#include <vector>
+
+struct StdioHandles {
+ HANDLE in;
+ HANDLE out;
+ HANDLE err;
+};
+
+static void ReportErrorAndAbort(
+ std::source_location location = std::source_location::current()) {
+ const auto code = GetLastError();
+
+ wchar_t buffer[512];
+ auto message_size =
+ FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
+ nullptr, code, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+ buffer, ARRAY_SIZE(buffer), nullptr);
+ if (message_size == 0) {
+ _snwprintf(buffer, ARRAY_SIZE(buffer), L"OS Error %d", code);
+ }
+ fprintf(stderr, "error at %s:%d: %ls", location.file_name(), location.line(),
+ buffer);
+ abort();
+}
+
+static void LaunchProcessWith(wchar_t* command_line,
+ const StdioHandles& stdio_handles) {
+ fprintf(stderr, "LAUNCHING %ls\n", command_line);
+
+ // Setup info
+ STARTUPINFOEXW startup_info;
+ ZeroMemory(&startup_info, sizeof(startup_info));
+ startup_info.StartupInfo.cb = sizeof(startup_info);
+
+ // Setup the handles to inherit. We only want to inherit the three
+ // handles for stdin, stdout and stderr.
+ startup_info.StartupInfo.hStdInput = stdio_handles.in;
+ startup_info.StartupInfo.hStdOutput = stdio_handles.out;
+ startup_info.StartupInfo.hStdError = stdio_handles.err;
+ startup_info.StartupInfo.dwFlags = STARTF_USESTDHANDLES;
+ SIZE_T size = 0;
+ // The call to determine the size of an attribute list always fails with
+ // ERROR_INSUFFICIENT_BUFFER and that error should be ignored.
+ if (!InitializeProcThreadAttributeList(nullptr, 1, 0, &size) &&
+ (GetLastError() != ERROR_INSUFFICIENT_BUFFER)) {
+ return ReportErrorAndAbort();
+ }
+ auto attribute_list =
+ reinterpret_cast<LPPROC_THREAD_ATTRIBUTE_LIST>(malloc(size));
+ ZeroMemory(attribute_list, size);
+ if (!InitializeProcThreadAttributeList(attribute_list, 1, 0, &size)) {
+ return ReportErrorAndAbort();
+ }
+ std::vector<HANDLE> inherited_handles = {stdio_handles.in};
+ if (stdio_handles.out != stdio_handles.in) {
+ inherited_handles.push_back(stdio_handles.out);
+ }
+ if (stdio_handles.err != stdio_handles.out &&
+ stdio_handles.err != stdio_handles.in) {
+ inherited_handles.push_back(stdio_handles.err);
+ }
+ if (!UpdateProcThreadAttribute(
+ attribute_list, 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
+ inherited_handles.data(), inherited_handles.size() * sizeof(HANDLE),
+ nullptr, nullptr)) {
+ return ReportErrorAndAbort();
+ }
+ startup_info.lpAttributeList = attribute_list;
+
+ PROCESS_INFORMATION process_info;
+ ZeroMemory(&process_info, sizeof(process_info));
+
+ // Create process.
+ BOOL result = CreateProcessW(
+ /*lpApplicationName=*/nullptr, command_line,
+ /*lpProcessAttributes=*/nullptr,
+ /*lpThreadAttributes=*/nullptr,
+ /*bInheritHandles=*/TRUE,
+ /*dwCreationFlags=*/EXTENDED_STARTUPINFO_PRESENT,
+ /*lpEnvironment=*/nullptr,
+ /*lpCurrentDirectory=*/nullptr,
+ reinterpret_cast<STARTUPINFOW*>(&startup_info), &process_info);
+
+ if (result == 0) {
+ return ReportErrorAndAbort();
+ }
+
+ WaitForSingleObject(process_info.hProcess, INFINITE);
+ CloseHandle(process_info.hProcess);
+ CloseHandle(process_info.hThread);
+}
+
+int main(int argc, char* argv[]) {
+ if (argc <= 1) {
+ fprintf(stderr, "Usage: %s <executable> <arg0> ... <argN>\n", argv[0]);
+ return -1;
+ }
+
+ // Generate command line. Assume that it does not contain any white-space
+ // in the arguments.
+ std::wstringstream wstr;
+ for (int i = 1; i < argc; i++) {
+ wstr << (i > 1 ? " " : "") << argv[i];
+ }
+
+ HANDLE stdin_handle = GetStdHandle(STD_INPUT_HANDLE);
+ HANDLE stdout_handle = GetStdHandle(STD_OUTPUT_HANDLE);
+ LaunchProcessWith(
+ wstr.str().data(),
+ {.in = stdin_handle, .out = stdout_handle, .err = stdout_handle});
+ CloseHandle(stdin_handle);
+ CloseHandle(stdout_handle);
+ return 0;
+}
+
+#else
+
+int main() {
+ return -1;
+}
+
+#endif // defined(DART_HOST_OS_WINDOWS)
diff --git a/runtime/bin/process.cc b/runtime/bin/process.cc
index 027734d..4691823 100644
--- a/runtime/bin/process.cc
+++ b/runtime/bin/process.cc
@@ -155,7 +155,7 @@
Dart_Handle stderr_handle = Dart_GetNativeArgument(args, 9);
Dart_Handle exit_handle = Dart_GetNativeArgument(args, 10);
intptr_t pid = -1;
- char* os_error_message = nullptr; // Scope allocated by Process::Start.
+ char* os_error_message = nullptr;
int error_code = Process::Start(
namespc, path, string_args, args_length, working_directory,
diff --git a/runtime/bin/process_win.cc b/runtime/bin/process_win.cc
index fc61eba..635e8d9 100644
--- a/runtime/bin/process_win.cc
+++ b/runtime/bin/process_win.cc
@@ -9,6 +9,7 @@
#include <process.h> // NOLINT
#include <psapi.h> // NOLINT
+#include <source_location>
#include <vector>
#include "bin/builtin.h"
@@ -294,12 +295,15 @@
CloseProcessPipe(handles4);
}
-static int SetOsErrorMessage(char** os_error_message) {
+static int SetOsErrorMessage(
+ CStringUniquePtr* os_error_message,
+ std::source_location location = std::source_location::current()) {
int error_code = GetLastError();
const int kMaxMessageLength = 256;
wchar_t message[kMaxMessageLength];
FormatMessageIntoBuffer(error_code, message, kMaxMessageLength);
- *os_error_message = StringUtilsWin::WideToUtf8(message);
+ os_error_message->reset(Utils::SCreate(
+ "%ls (at %s:%d)", message, location.file_name(), location.line()));
return error_code;
}
@@ -356,7 +360,7 @@
intptr_t* err,
intptr_t* id,
intptr_t* exit_handler,
- char** os_error_message)
+ CStringUniquePtr* os_error_message)
: path_(path),
working_directory_(working_directory),
mode_(mode),
@@ -617,7 +621,13 @@
if (!InitializeProcThreadAttributeList(attribute_list_, 1, 0, &size)) {
return CleanupAndReturnError();
}
- inherited_handles_ = {stdin_handle, stdout_handle, stderr_handle};
+ inherited_handles_ = {stdin_handle};
+ if (stdout_handle != stdin_handle) {
+ inherited_handles_.push_back(stdout_handle);
+ }
+ if (stderr_handle != stdout_handle && stderr_handle != stdin_handle) {
+ inherited_handles_.push_back(stderr_handle);
+ }
if (!UpdateProcThreadAttribute(
attribute_list_, 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
inherited_handles_.data(),
@@ -707,8 +717,9 @@
return 0;
}
- int CleanupAndReturnError() {
- int error_code = SetOsErrorMessage(os_error_message_);
+ int CleanupAndReturnError(
+ std::source_location location = std::source_location::current()) {
+ int error_code = SetOsErrorMessage(os_error_message_, location);
CloseProcessPipes(stdin_handles_, stdout_handles_, stderr_handles_,
exit_handles_);
return error_code;
@@ -734,7 +745,7 @@
intptr_t* err_;
intptr_t* id_;
intptr_t* exit_handler_;
- char** os_error_message_;
+ CStringUniquePtr* os_error_message_;
private:
DISALLOW_ALLOCATION();
@@ -755,10 +766,15 @@
intptr_t* id,
intptr_t* exit_handler,
char** os_error_message) {
+ CStringUniquePtr error;
ProcessStarter starter(path, arguments, arguments_length, working_directory,
environment, environment_length, mode, in, out, err,
- id, exit_handler, os_error_message);
- return starter.Start();
+ id, exit_handler, &error);
+ const auto result = starter.Start();
+ if (result != 0 && error != nullptr) {
+ *os_error_message = DartUtils::ScopedCopyCString(error.get());
+ }
+ return result;
}
class BufferList : public BufferListBase {
@@ -981,7 +997,7 @@
HANDLE hjob = CreateJobObject(nullptr, nullptr);
if (hjob == nullptr) {
BufferFormatter f(errmsg, errmsg_len);
- f.Printf("Process::Exec - CreateJobObject failed %d\n", GetLastError());
+ f.Printf("Process::Exec - CreateJobObject failed %d", GetLastError());
return -1;
}
JOBOBJECT_EXTENDED_LIMIT_INFORMATION info;
@@ -991,7 +1007,7 @@
sizeof(JOBOBJECT_EXTENDED_LIMIT_INFORMATION),
&qresult)) {
BufferFormatter f(errmsg, errmsg_len);
- f.Printf("Process::Exec - QueryInformationJobObject failed %d\n",
+ f.Printf("Process::Exec - QueryInformationJobObject failed %d",
GetLastError());
return -1;
}
@@ -1005,7 +1021,7 @@
if (!SetInformationJobObject(hjob, JobObjectExtendedLimitInformation, &info,
sizeof(JOBOBJECT_EXTENDED_LIMIT_INFORMATION))) {
BufferFormatter f(errmsg, errmsg_len);
- f.Printf("Process::Exec - SetInformationJobObject failed %d\n",
+ f.Printf("Process::Exec - SetInformationJobObject failed %d",
GetLastError());
return -1;
}
@@ -1015,7 +1031,7 @@
// we haven't spawned any children yet this race is harmless)
if (!AssignProcessToJobObject(hjob, GetCurrentProcess())) {
BufferFormatter f(errmsg, errmsg_len);
- f.Printf("Process::Exec - AssignProcessToJobObject failed %d\n",
+ f.Printf("Process::Exec - AssignProcessToJobObject failed %d",
GetLastError());
return -1;
}
@@ -1028,14 +1044,14 @@
// as the value passed in 'path', we strip that off when starting the
// process.
intptr_t pid = -1;
- char* os_error_message = nullptr; // Scope allocated by Process::Start.
+ CStringUniquePtr os_error_message;
ProcessStarter starter(path, &(arguments[1]), (arguments_length - 1),
working_directory, nullptr, 0, kInheritStdio, nullptr,
nullptr, nullptr, &pid, nullptr, &os_error_message);
int result = starter.StartForExec(hjob);
if (result != 0) {
BufferFormatter f(errmsg, errmsg_len);
- f.Printf("Process::Exec - %s\n", os_error_message);
+ f.Printf("ProcessStarter::StartForExec failed: %s", os_error_message.get());
return -1;
}
@@ -1047,14 +1063,14 @@
DWORD wait_result = WaitForSingleObject(child_process, INFINITE);
if (wait_result != WAIT_OBJECT_0) {
BufferFormatter f(errmsg, errmsg_len);
- f.Printf("Process::Exec - WaitForSingleObject failed %d\n", GetLastError());
+ f.Printf("Process::Exec - WaitForSingleObject failed %d", GetLastError());
CloseHandle(child_process);
return -1;
}
int retval;
if (!GetExitCodeProcess(child_process, reinterpret_cast<DWORD*>(&retval))) {
BufferFormatter f(errmsg, errmsg_len);
- f.Printf("Process::Exec - GetExitCodeProcess failed %d\n", GetLastError());
+ f.Printf("Process::Exec - GetExitCodeProcess failed %d", GetLastError());
CloseHandle(child_process);
return -1;
}
diff --git a/runtime/bin/utils_win.cc b/runtime/bin/utils_win.cc
index c73d972..7a0217e 100644
--- a/runtime/bin/utils_win.cc
+++ b/runtime/bin/utils_win.cc
@@ -46,9 +46,17 @@
code, GetLastError());
}
_snwprintf(buffer, buffer_length, L"OS Error %d", code);
+ return;
}
- // Ensure string termination.
- buffer[buffer_length - 1] = 0;
+
+ // Strip trailing whitespace and a dot.
+ while (message_size > 0 && iswspace(buffer[message_size - 1])) {
+ message_size--;
+ }
+ if (message_size > 0 && buffer[message_size - 1] == L'.') {
+ message_size--;
+ }
+ buffer[Utils::Minimum<int>(message_size, buffer_length - 1)] = 0;
}
FILETIME GetFiletimeFromMillis(int64_t millis) {
diff --git a/runtime/tests/vm/dart/regress_61981_test.dart b/runtime/tests/vm/dart/regress_61981_test.dart
new file mode 100644
index 0000000..2fa8b76
--- /dev/null
+++ b/runtime/tests/vm/dart/regress_61981_test.dart
@@ -0,0 +1,44 @@
+// Copyright (c) 2025, 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.
+
+// Test that Dart binary correctly handles situations when stdout and stderr
+// handles are the same.
+//
+// This can happen in certain terminal emulators, e.g. one used by GitBash
+// see https://github.com/dart-lang/sdk/issues/61981 for an example.
+
+import 'dart:io';
+
+import 'package:expect/expect.dart';
+import 'package:path/path.dart' as p;
+
+void main(List<String> args) {
+ if (args case ['child-process']) {
+ print('OK');
+ return;
+ }
+
+ if (!Platform.isWindows ||
+ p.basenameWithoutExtension(Platform.executable) != 'dart') {
+ return;
+ }
+
+ final createProcessHelper = p.join(
+ p.dirname(Platform.executable),
+ 'create_process_test_helper.exe',
+ );
+
+ final result = Process.runSync(createProcessHelper, [
+ Platform.executable,
+ 'run',
+ Platform.script.toFilePath(),
+ 'child-process',
+ ]);
+ if (result.exitCode != 0) {
+ print(result.stdout);
+ print(result.stderr);
+ Expect.fail('process exited with ${result.exitCode}');
+ }
+ Expect.equals('OK', (result.stdout as String).trim());
+}
