Skip to content

Commit f40aade

Browse files
committed
fixed tetris and pipes example regression
Last commit replace wait_with_output with wait to get process running result, which could cause partial read.
1 parent 0423e12 commit f40aade

File tree

2 files changed

+26
-37
lines changed

2 files changed

+26
-37
lines changed

src/child.rs

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -75,18 +75,15 @@ impl CmdChildren {
7575
}
7676
}
7777

78-
pub fn wait_with_pipe(&mut self, f: &mut dyn FnMut(PipeReader) -> CmdResult) -> CmdResult {
78+
pub fn wait_with_pipe(&mut self, f: &mut dyn FnMut(Box<dyn Read>) -> CmdResult) -> CmdResult {
7979
let handle = self.0.pop().unwrap();
8080
let mut ret = Ok(());
8181
match handle {
8282
ProcChild {
83-
mut child,
84-
stderr,
85-
stdout,
86-
..
83+
mut child, stderr, ..
8784
} => {
88-
if let Some(stdout) = stdout {
89-
ret = f(stdout);
85+
if let Some(stdout) = child.stdout.take() {
86+
ret = f(Box::new(stdout));
9087
let _ = child.kill();
9188
}
9289
CmdChild::log_stderr_output(stderr);
@@ -97,7 +94,7 @@ impl CmdChildren {
9794
SyncChild { stderr, stdout, .. } => {
9895
CmdChild::log_stderr_output(stderr);
9996
if let Some(stdout) = stdout {
100-
ret = f(stdout);
97+
ret = f(Box::new(stdout));
10198
}
10299
}
103100
};
@@ -111,7 +108,6 @@ pub enum CmdChild {
111108
ProcChild {
112109
child: Child,
113110
cmd: String,
114-
stdout: Option<PipeReader>,
115111
stderr: Option<PipeReader>,
116112
},
117113
ThreadChild {
@@ -146,12 +142,7 @@ impl CmdChild {
146142
..
147143
} => {
148144
Self::log_stderr_output(stderr);
149-
if let Some(stdout) = child.stdout.take() {
150-
BufReader::new(stdout)
151-
.lines()
152-
.filter_map(|line| line.ok())
153-
.for_each(|line| println!("{}", line));
154-
}
145+
Self::print_stdout_output(child.stdout.take());
155146
let status = child.wait()?;
156147
if !status.success() && (is_last || pipefail) {
157148
return Err(Self::status_to_io_error(
@@ -181,37 +172,25 @@ impl CmdChild {
181172
}
182173
SyncChild { stdout, stderr, .. } => {
183174
Self::log_stderr_output(stderr);
184-
if let Some(mut out) = stdout {
185-
let mut buf = vec![];
186-
check_result(out.read_to_end(&mut buf).map(|_| ()))?;
187-
print!("{}", String::from_utf8_lossy(&buf));
188-
}
175+
Self::print_stdout_output(stdout);
189176
}
190177
}
191178
Ok(())
192179
}
193180

194181
pub fn wait_with_output(self) -> Result<Vec<u8>> {
195182
match self {
196-
ProcChild {
197-
mut child,
198-
cmd,
199-
stdout,
200-
stderr,
201-
} => {
202-
let mut buf = vec![];
203-
if let Some(mut stdout) = stdout {
204-
stdout.read_to_end(&mut buf)?;
205-
}
183+
ProcChild { child, cmd, stderr } => {
206184
Self::log_stderr_output(stderr);
207-
let status = child.wait()?;
208-
if !status.success() {
185+
let output = child.wait_with_output()?;
186+
if !output.status.success() {
209187
return Err(Self::status_to_io_error(
210-
status,
188+
output.status,
211189
&format!("{} exited with error", cmd),
212190
));
191+
} else {
192+
Ok(output.stdout)
213193
}
214-
Ok(buf)
215194
}
216195
ThreadChild { cmd, .. } => {
217196
panic!("{} thread should not be waited for output", cmd);
@@ -228,6 +207,15 @@ impl CmdChild {
228207
}
229208
}
230209

210+
fn print_stdout_output(stdout: Option<impl Read>) {
211+
if let Some(stdout) = stdout {
212+
BufReader::new(stdout)
213+
.lines()
214+
.filter_map(|line| line.ok())
215+
.for_each(|line| println!("{}", line));
216+
}
217+
}
218+
231219
fn log_stderr_output(stderr: Option<impl Read>) {
232220
if let Some(stderr) = stderr {
233221
BufReader::new(stderr)

src/process.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use std::fmt;
1010
use std::fs::{File, OpenOptions};
1111
use std::io::{Error, ErrorKind, Read, Result, Write};
1212
use std::path::Path;
13-
use std::process::Command;
13+
use std::process::{Command, Stdio};
1414
use std::sync::Mutex;
1515

1616
/// Environment for builtin or custom commands
@@ -404,6 +404,8 @@ impl Cmd {
404404
// update stdout
405405
if let Some(redirect_out) = self.stdout_redirect.take() {
406406
cmd.stdout(redirect_out);
407+
} else {
408+
cmd.stdout(Stdio::piped());
407409
}
408410

409411
// update stderr
@@ -415,7 +417,6 @@ impl Cmd {
415417
let child = cmd.spawn()?;
416418
Ok(CmdChild::ProcChild {
417419
cmd: full_cmd,
418-
stdout: self.stdout_logging,
419420
stderr: self.stderr_logging,
420421
child,
421422
})
@@ -471,7 +472,7 @@ impl Cmd {
471472

472473
if let Some(pipe) = pipe_out {
473474
self.stdout_redirect = Some(CmdOut::CmdPipe(pipe));
474-
} else {
475+
} else if self.in_cmd_map {
475476
// set up stdout pipe
476477
let (pipe_reader, pipe_writer) = os_pipe::pipe()?;
477478
self.stdout_redirect = Some(CmdOut::CmdPipe(pipe_writer));

0 commit comments

Comments
 (0)