Skip to content

Commit fe573c9

Browse files
committed
sh: properly synchronise pipeline with parent process
1 parent 8b3643e commit fe573c9

File tree

1 file changed

+47
-22
lines changed

1 file changed

+47
-22
lines changed

sh/src/shell/mod.rs

Lines changed: 47 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@ use crate::utils::{
3434
use crate::wordexp::{expand_word, expand_word_to_string, word_to_pattern};
3535
use nix::errno::Errno;
3636
use nix::libc;
37+
use nix::sys::signal::kill;
38+
use nix::sys::signal::Signal as NixSignal;
3739
use nix::sys::wait::{WaitPidFlag, WaitStatus};
38-
use nix::unistd::{
39-
getcwd, getgid, getpgid, getpgrp, getpid, getppid, setpgid, tcsetpgrp, ForkResult, Pid,
40-
};
40+
use nix::unistd::{getcwd, getpgid, getpgrp, getpid, getppid, setpgid, tcsetpgrp, ForkResult, Pid};
4141
use std::collections::HashMap;
4242
use std::ffi::{CString, OsString};
4343
use std::fmt::{Display, Formatter};
@@ -734,6 +734,11 @@ impl Shell {
734734
// this should never fail as both arguments are valid
735735
setpgid(Pid::from_raw(0), Pid::from_raw(0)).unwrap();
736736
let pipeline_pgid = getpgrp();
737+
// wait for the parent process to put the subshell in the foreground
738+
if let Err(err) = kill(Pid::from_raw(0), NixSignal::SIGTSTP) {
739+
self.eprint(&format!("sh: internal call to kill failed ({err})"));
740+
self.exit(1);
741+
}
737742

738743
let mut current_stdin = libc::STDIN_FILENO;
739744
for command in pipeline.commands.head() {
@@ -765,26 +770,46 @@ impl Shell {
765770
self.exit(return_status);
766771
}
767772
ForkResult::Parent { child } => {
768-
if is_process_in_foreground() {
769-
// unwrap should never fail as child is a valid process id and in the
770-
// same session as the shell process
771-
let mut child_pgid = getpgid(Some(child)).unwrap();
772-
// loop until child is in another group
773-
while child_pgid.as_raw() as u32 == getgid().as_raw() {
774-
self.handle_async_events();
775-
std::thread::sleep(Duration::from_millis(16));
776-
// cannot fail, same reason as above
777-
child_pgid = getpgid(Some(child)).unwrap();
773+
loop {
774+
match waitpid(child, Some(WaitPidFlag::WNOHANG | WaitPidFlag::WUNTRACED))? {
775+
WaitStatus::Exited(_, _) => {
776+
// the only way this happened is if there was an error before going
777+
// the child went to sleep
778+
return Ok(1);
779+
}
780+
WaitStatus::Signaled(_, _, _) => {
781+
self.eprint("sh: unsynchronised pipeline was terminated by another process\n");
782+
return Ok(1);
783+
}
784+
WaitStatus::Continued(_) => {
785+
self.eprint("sh: unsynchronised pipeline was restarted by another process\n");
786+
return Ok(1);
787+
}
788+
WaitStatus::Stopped(_, _) => {
789+
if is_process_in_foreground() {
790+
// should never fail as child is a valid process id and
791+
// in the same session as the current shell
792+
let child_gpid = getpgid(Some(child)).unwrap();
793+
// should never fail as stdin is a valid file descriptor and
794+
// child gpid is valid and in the same session
795+
tcsetpgrp(io::stdin().as_fd(), child_gpid).unwrap();
796+
kill(child, NixSignal::SIGCONT).unwrap();
797+
pipeline_exit_status = self.wait_child_process(child)?;
798+
// should never fail
799+
tcsetpgrp(io::stdin().as_fd(), getpgrp()).unwrap();
800+
break;
801+
} else {
802+
kill(child, NixSignal::SIGCONT).unwrap();
803+
pipeline_exit_status = self.wait_child_process(child)?;
804+
break;
805+
}
806+
}
807+
WaitStatus::StillAlive => {
808+
self.handle_async_events();
809+
std::thread::sleep(Duration::from_millis(16));
810+
}
811+
_ => unreachable!(),
778812
}
779-
// should never fail as stdin is a valid file descriptor and
780-
// child is a valid group id and is in the same session
781-
// as the shell process
782-
tcsetpgrp(io::stdin().as_fd(), child_pgid).unwrap();
783-
pipeline_exit_status = self.wait_child_process(child)?;
784-
// should never fail
785-
tcsetpgrp(io::stdin().as_fd(), getpgrp()).unwrap();
786-
} else {
787-
pipeline_exit_status = self.wait_child_process(child)?;
788813
}
789814
}
790815
}

0 commit comments

Comments
 (0)