command_windows: fix deadlock when waiting on stdout and stderr of process

Using WaitForMultipleObjects on the handles to stdout and stderr streams
is apparently not the right way to do this. Instead, we create 2
threads, one that reads stdout, and one that reads stderr, and we wait
on those threads, along with the process handle.

Bug: tint:812
Change-Id: I5975e4d649b58d0f8bffb76dec5a3f4079513e04
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53060
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Auto-Submit: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This commit is contained in:
Antonio Maiorano 2021-06-02 22:57:24 +00:00 committed by Tint LUCI CQ
parent a70c14dbce
commit a7f47b49e7
1 changed files with 45 additions and 38 deletions

View File

@ -69,7 +69,7 @@ class Handle {
class Pipe { class Pipe {
public: public:
/// Constructs the pipe /// Constructs the pipe
Pipe() { explicit Pipe(bool for_read) {
SECURITY_ATTRIBUTES sa; SECURITY_ATTRIBUTES sa;
sa.nLength = sizeof(SECURITY_ATTRIBUTES); sa.nLength = sizeof(SECURITY_ATTRIBUTES);
sa.bInheritHandle = TRUE; sa.bInheritHandle = TRUE;
@ -81,7 +81,8 @@ class Pipe {
read = Handle(hread); read = Handle(hread);
write = Handle(hwrite); write = Handle(hwrite);
// Ensure the read handle to the pipe is not inherited // Ensure the read handle to the pipe is not inherited
if (!SetHandleInformation(read, HANDLE_FLAG_INHERIT, 0)) { if (!SetHandleInformation(for_read ? read : write, HANDLE_FLAG_INHERIT,
0)) {
read.Close(); read.Close();
write.Close(); write.Close();
} }
@ -147,9 +148,9 @@ bool Command::Found() const {
Command::Output Command::Exec( Command::Output Command::Exec(
std::initializer_list<std::string> arguments) const { std::initializer_list<std::string> arguments) const {
Pipe stdout_pipe; Pipe stdout_pipe(true);
Pipe stderr_pipe; Pipe stderr_pipe(true);
Pipe stdin_pipe; Pipe stdin_pipe(false);
if (!stdin_pipe || !stdout_pipe || !stderr_pipe) { if (!stdin_pipe || !stdout_pipe || !stderr_pipe) {
Output output; Output output;
output.err = "Command::Exec(): Failed to create pipes"; output.err = "Command::Exec(): Failed to create pipes";
@ -195,46 +196,52 @@ Command::Output Command::Exec(
return out; return out;
} }
stdin_pipe.read.Close();
stdout_pipe.write.Close(); stdout_pipe.write.Close();
stderr_pipe.write.Close(); stderr_pipe.write.Close();
struct StreamReadThreadArgs {
HANDLE stream;
std::string output;
};
auto stream_read_thread = [](LPVOID user) -> DWORD {
auto* thread_args = reinterpret_cast<StreamReadThreadArgs*>(user);
DWORD n = 0;
char buf[256];
while (ReadFile(thread_args->stream, buf, sizeof(buf), &n, NULL)) {
auto s = std::string(buf, buf + n);
thread_args->output += std::string(buf, buf + n);
}
return 0;
};
StreamReadThreadArgs stdout_read_args{stdout_pipe.read, {}};
auto* stdout_read_thread = ::CreateThread(nullptr, 0, stream_read_thread,
&stdout_read_args, 0, nullptr);
StreamReadThreadArgs stderr_read_args{stderr_pipe.read, {}};
auto* stderr_read_thread = ::CreateThread(nullptr, 0, stream_read_thread,
&stderr_read_args, 0, nullptr);
HANDLE handles[] = {pi.hProcess, stdout_read_thread, stderr_read_thread};
constexpr DWORD num_handles = sizeof(handles) / sizeof(handles[0]);
Output output; Output output;
char buf[256]; auto res = WaitForMultipleObjects(num_handles, handles, /* wait_all = */ TRUE,
HANDLE handles[] = {stdout_pipe.read, stderr_pipe.read}; INFINITE);
if (res >= WAIT_OBJECT_0 && res < WAIT_OBJECT_0 + num_handles) {
bool stdout_open = true; output.out = stdout_read_args.output;
bool stderr_open = true; output.err = stderr_read_args.output;
while (stdout_open || stderr_open) { DWORD exit_code = 0;
auto res = WaitForMultipleObjects(2, handles, FALSE, INFINITE); GetExitCodeProcess(pi.hProcess, &exit_code);
switch (res) { output.error_code = static_cast<int>(exit_code);
case WAIT_FAILED: } else {
output.err = "Command::Exec() WaitForMultipleObjects() returned " + output.err = "Command::Exec() WaitForMultipleObjects() returned " +
std::to_string(res); std::to_string(res);
return output;
case WAIT_OBJECT_0: { // stdout
DWORD n = 0;
if (ReadFile(stdout_pipe.read, buf, sizeof(buf), &n, NULL)) {
output.out += std::string(buf, buf + n);
} else {
stdout_open = false;
}
break;
}
case WAIT_OBJECT_0 + 1: { // stderr
DWORD n = 0;
if (ReadFile(stderr_pipe.read, buf, sizeof(buf), &n, NULL)) {
output.err += std::string(buf, buf + n);
} else {
stderr_open = false;
}
break;
}
}
} }
WaitForSingleObject(pi.hProcess, INFINITE);
return output; return output;
} }