Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: reading output from orphan process #7402

Merged
merged 3 commits into from Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 58 additions & 1 deletion crates/turborepo-lib/src/process/child.rs
Expand Up @@ -500,7 +500,8 @@ impl Child {

/// Wait for the `Child` to exit, returning the exit code.
pub async fn wait(&mut self) -> Option<ChildExit> {
self.exit_channel.changed().await.ok()?;
// If sending end of exit channel closed, then return last value in the channel
self.exit_channel.changed().await.ok();
*self.exit_channel.borrow()
}

Expand Down Expand Up @@ -671,6 +672,7 @@ impl Child {
let mut stdout_buffer = Vec::new();
let mut stderr_buffer = Vec::new();

let mut is_exited = false;
loop {
tokio::select! {
Some(result) = next_line(&mut stdout_lines, &mut stdout_buffer) => {
Expand All @@ -685,6 +687,14 @@ impl Child {
stdout_pipe.write_all(&stderr_buffer)?;
stderr_buffer.clear();
}
status = self.wait(), if !is_exited => {
is_exited = true;
// We don't abort in the cases of a zero exit code as we could be
// caching this task and should read all the logs it produces.
if status != Some(ChildExit::Finished(Some(0))) {
return Ok(status);
}
}
else => {
// In the case that both futures read a complete line
// the future not chosen in the select will return None if it's at EOF
Expand Down Expand Up @@ -827,6 +837,21 @@ mod test {
assert_matches!(&*state, ChildState::Exited(ChildExit::Killed));
}

#[test_case(false)]
#[test_case(TEST_PTY)]
#[tokio::test]
async fn test_wait(use_pty: bool) {
let script = find_script_dir().join_component("hello_world.js");
let mut cmd = Command::new("node");
cmd.args([script.as_std_path()]);
let mut child = Child::spawn(cmd, ShutdownStyle::Kill, use_pty).unwrap();

let exit1 = child.wait().await;
let exit2 = child.wait().await;
assert_matches!(exit1, Some(ChildExit::Finished(Some(0))));
assert_matches!(exit2, Some(ChildExit::Finished(Some(0))));
}

#[test_case(false)]
#[test_case(TEST_PTY)]
#[tokio::test]
Expand Down Expand Up @@ -1170,6 +1195,38 @@ mod test {
assert_matches!(exit, Some(ChildExit::Killed));
}

#[cfg(unix)]
#[tokio::test]
async fn test_orphan_process() {
let mut cmd = Command::new("sh");
cmd.args(["-c", "echo hello; sleep 120; echo done"]);
let mut child = Child::spawn(cmd, ShutdownStyle::Kill, false).unwrap();

tokio::time::sleep(STARTUP_DELAY).await;

let child_pid = child.pid().unwrap() as i32;
// We don't kill the process group to simulate what an external program might do
unsafe {
libc::kill(child_pid, libc::SIGKILL);
}

let exit = child.wait().await;
assert_matches!(exit, Some(ChildExit::KilledExternal));

let mut output = Vec::new();
match tokio::time::timeout(
Duration::from_millis(500),
child.wait_with_piped_outputs(&mut output),
)
.await
{
Ok(exit_status) => {
assert_matches!(exit_status, Ok(Some(ChildExit::KilledExternal)));
}
Err(_) => panic!("expected wait_with_piped_outputs to exit after it was killed"),
}
}

#[test_case(false)]
#[test_case(TEST_PTY)]
#[tokio::test]
Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-lib/src/process/mod.rs
Expand Up @@ -264,7 +264,7 @@ mod test {
assert_eq!(kill_code, Some(ChildExit::Finished(Some(0))));

let code = child.wait().await;
assert_eq!(code, None);
assert_eq!(code, Some(ChildExit::Finished(Some(0))));
}

#[tokio::test]
Expand Down