Skip to content

Commit 4813f2a

Browse files
authored
Fix run_output to prevent infinite blocking (#1627)
If the child is blocked on writing to stderr and run_output blocked on reading stdout, then it'd be hung forever * Add new fn spawn_and_wait_for_output And use it in mod tool
1 parent 0a1bc19 commit 4813f2a

File tree

2 files changed

+36
-16
lines changed

2 files changed

+36
-16
lines changed

src/command_helpers.rs

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::{
99
hash::Hasher,
1010
io::{self, Read, Write},
1111
path::Path,
12-
process::{Child, ChildStderr, Command, Stdio},
12+
process::{Child, ChildStderr, Command, Output, Stdio},
1313
sync::{
1414
atomic::{AtomicBool, Ordering},
1515
Arc,
@@ -348,24 +348,45 @@ pub(crate) fn run(cmd: &mut Command, cargo_output: &CargoOutput) -> Result<(), E
348348
wait_on_child(cmd, &mut child, cargo_output)
349349
}
350350

351-
pub(crate) fn run_output(cmd: &mut Command, cargo_output: &CargoOutput) -> Result<Vec<u8>, Error> {
351+
pub(crate) fn spawn_and_wait_for_output(
352+
cmd: &mut Command,
353+
cargo_output: &CargoOutput,
354+
) -> Result<Output, Error> {
352355
// We specifically need the output to be captured, so override default
353356
let mut captured_cargo_output = cargo_output.clone();
354357
captured_cargo_output.output = OutputKind::Capture;
355-
let mut child = spawn(cmd, &captured_cargo_output)?;
358+
spawn(cmd, &captured_cargo_output)?
359+
.wait_with_output()
360+
.map_err(|e| {
361+
Error::new(
362+
ErrorKind::ToolExecError,
363+
format!("failed to wait on spawned child process `{cmd:?}`: {e}"),
364+
)
365+
})
366+
}
367+
368+
pub(crate) fn run_output(cmd: &mut Command, cargo_output: &CargoOutput) -> Result<Vec<u8>, Error> {
369+
let Output {
370+
status,
371+
stdout,
372+
stderr,
373+
} = spawn_and_wait_for_output(cmd, cargo_output)?;
356374

357-
let mut stdout = vec![];
358-
child
359-
.stdout
360-
.take()
361-
.unwrap()
362-
.read_to_end(&mut stdout)
363-
.unwrap();
375+
stderr
376+
.split(|&b| b == b'\n')
377+
.filter(|part| !part.is_empty())
378+
.for_each(write_warning);
364379

365-
// Don't care about this output, use the normal settings
366-
wait_on_child(cmd, &mut child, cargo_output)?;
380+
cargo_output.print_debug(&status);
367381

368-
Ok(stdout)
382+
if status.success() {
383+
Ok(stdout)
384+
} else {
385+
Err(Error::new(
386+
ErrorKind::ToolExecError,
387+
format!("command did not execute successfully (status code {status}): {cmd:?}"),
388+
))
389+
}
369390
}
370391

371392
pub(crate) fn spawn(cmd: &mut Command, cargo_output: &CargoOutput) -> Result<Child, Error> {

src/tool.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::{
2-
command_helpers::{run_output, spawn, CargoOutput},
2+
command_helpers::{run_output, spawn_and_wait_for_output, CargoOutput},
33
run,
44
tempfile::NamedTempfile,
55
Error, ErrorKind, OutputKind,
@@ -221,13 +221,12 @@ impl Tool {
221221
// But with clang-cl it can be part of stderr instead and exit with a
222222
// non-zero exit code.
223223
let mut captured_cargo_output = compiler_detect_output.clone();
224-
captured_cargo_output.output = OutputKind::Capture;
225224
captured_cargo_output.warnings = true;
226225
let Output {
227226
status,
228227
stdout,
229228
stderr,
230-
} = spawn(&mut cmd, &captured_cargo_output)?.wait_with_output()?;
229+
} = spawn_and_wait_for_output(&mut cmd, &captured_cargo_output)?;
231230

232231
let stdout = if [&stdout, &stderr]
233232
.iter()

0 commit comments

Comments
 (0)