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

Improve signal handling #2488

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from
Draft
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ edit-distance = "2.0.0"
heck = "0.5.0"
lexiclean = "0.0.1"
libc = "0.2.0"
nix = { version = "0.29.0", features = ["user"] }
num_cpus = "1.15.0"
once_cell = "1.19.0"
percent-encoding = "2.3.1"
Expand Down Expand Up @@ -72,6 +73,7 @@ struct_excessive_bools = "allow"
struct_field_names = "allow"
too_many_arguments = "allow"
too_many_lines = "allow"
undocumented_unsafe_blocks = "deny"
unnecessary_wraps = "allow"
wildcard_imports = "allow"

Expand Down
51 changes: 51 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3923,6 +3923,57 @@ the
[`chrono` library docs](https://docs.rs/chrono/latest/chrono/format/strftime/index.html)
for details.

### Signal Handling

[Signals](https://en.wikipedia.org/wiki/Signal_(IPC)) are messsages sent to
running programs to trigger specific behavior. For example, the `SIGINT` signal
is sent to all processes in the forground process group when `ctrl-c` is typed
at the terminal,

`just` tries to exit when requested and avoid leaving behind running child
proccesses, two goals which are somewhat in conflict.

If `just` exits leaving behind child processes, the user will have no recourse
but to `ps aux | grep` for the children and manually `kill` them, a tedious
endevour.

#### Fatal Signals

`SIGHUP`, `SIGINT`, and `SIGQUIT` are generated when the user closes the
terminal, types `ctrl-c`, or types `ctrl-\`, respectively, and are sent to all
processes in the foreground process group.

`SIGTERM` is the default signal sent by the `kill` command, and is delivered
only to its intended victim.

When a child process is not running, `just` will exit immediately on receipt of
any of the above signals.

When a child process *is* running, `just` will wait until it terminates, to
avoid leaving it behind.

Additionally, on receipt of `SIGTERM`, `just` will forward `SIGTERM` to any
running children<sup>master</sup>, since unlike other fatal signals, `SIGTERM`,
was likely sent to `just` alone.

Regardless of whether a child process terminates successfully after `just`
receives a fatal signal, `just` halts execution.

#### `SIGINFO`

`SIGINFO` is sent to all processes in the foreground process group when the
user types `ctrl-t` on
[BSD](https://en.wikipedia.org/wiki/Berkeley_Software_Distribution)-derived
operating systems, including MacOS, but not Linux.

`just` responds by printing a list of all child process IDs and
commands<sup>master</sup>.

#### Windows

On Windows, `just` behaves as if it had received `SIGINT` when the user types
`ctrl-c`. Other signals are unsupported.

Changelog
---------

Expand Down
18 changes: 16 additions & 2 deletions src/command_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ pub(crate) trait CommandExt {
dotenv: &BTreeMap<String, String>,
scope: &Scope,
unexports: &HashSet<String>,
);
) -> &mut Command;

fn export_scope(&mut self, settings: &Settings, scope: &Scope, unexports: &HashSet<String>);

fn status_guard(self) -> (io::Result<ExitStatus>, Option<Signal>);

fn output_guard(self) -> (io::Result<process::Output>, Option<Signal>);
}

impl CommandExt for Command {
Expand All @@ -19,14 +23,16 @@ impl CommandExt for Command {
dotenv: &BTreeMap<String, String>,
scope: &Scope,
unexports: &HashSet<String>,
) {
) -> &mut Command {
for (name, value) in dotenv {
self.env(name, value);
}

if let Some(parent) = scope.parent() {
self.export_scope(settings, parent, unexports);
}

self
}

fn export_scope(&mut self, settings: &Settings, scope: &Scope, unexports: &HashSet<String>) {
Expand All @@ -44,4 +50,12 @@ impl CommandExt for Command {
}
}
}

fn status_guard(self) -> (io::Result<ExitStatus>, Option<Signal>) {
SignalHandler::spawn(self, |mut child| child.wait())
}

fn output_guard(self) -> (io::Result<process::Output>, Option<Signal>) {
SignalHandler::spawn(self, process::Child::wait_with_output)
}
}
8 changes: 8 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ pub(crate) enum Error<'src> {
Internal {
message: String,
},
Interrupted {
signal: Signal,
},
Io {
recipe: &'src str,
io_error: io::Error,
Expand Down Expand Up @@ -291,6 +294,7 @@ impl ColorDisplay for Error<'_> {
OutputError::Code(code) => write!(f, "Backtick failed with exit code {code}")?,
OutputError::Signal(signal) => write!(f, "Backtick was terminated by signal {signal}")?,
OutputError::Unknown => write!(f, "Backtick failed for an unknown reason")?,
OutputError::Interrupted(signal) => write!(f, "Backtick succeeded but `just` was interrupted by signal {signal}")?,
OutputError::Io(io_error) => match io_error.kind() {
io::ErrorKind::NotFound => write!(f, "Backtick could not be run because just could not find the shell:\n{io_error}"),
io::ErrorKind::PermissionDenied => write!(f, "Backtick could not be run because just could not run the shell:\n{io_error}"),
Expand Down Expand Up @@ -340,6 +344,7 @@ impl ColorDisplay for Error<'_> {
OutputError::Code(code) => write!(f, "Cygpath failed with exit code {code} while translating recipe `{recipe}` shebang interpreter path")?,
OutputError::Signal(signal) => write!(f, "Cygpath terminated by signal {signal} while translating recipe `{recipe}` shebang interpreter path")?,
OutputError::Unknown => write!(f, "Cygpath experienced an unknown failure while translating recipe `{recipe}` shebang interpreter path")?,
OutputError::Interrupted(signal) => write!(f, "Cygpath succeeded but `just` was interrupted by {signal}")?,
OutputError::Io(io_error) => {
match io_error.kind() {
io::ErrorKind::NotFound => write!(f, "Could not find `cygpath` executable to translate recipe `{recipe}` shebang interpreter path:\n{io_error}"),
Expand Down Expand Up @@ -402,6 +407,9 @@ impl ColorDisplay for Error<'_> {
write!(f, "Internal runtime error, this may indicate a bug in just: {message} \
consider filing an issue: https://github.com/casey/just/issues/new")?;
}
Interrupted { signal } => {
write!(f, "Interrupted by {signal}")?;
}
Io { recipe, io_error } => {
match io_error.kind() {
io::ErrorKind::NotFound => write!(f, "Recipe `{recipe}` could not be run because just could not find the shell: {io_error}"),
Expand Down
54 changes: 38 additions & 16 deletions src/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,22 +267,44 @@ impl<'src, 'run> Evaluator<'src, 'run> {
.module
.settings
.shell_command(self.context.config);
cmd.arg(command);
cmd.args(args);
cmd.current_dir(self.context.working_directory());
cmd.export(
&self.context.module.settings,
self.context.dotenv,
&self.scope,
&self.context.module.unexports,
);
cmd.stdin(Stdio::inherit());
cmd.stderr(if self.context.config.verbosity.quiet() {
Stdio::null()
} else {
Stdio::inherit()
});
InterruptHandler::guard(|| output(cmd))

cmd
.arg(command)
.args(args)
.current_dir(self.context.working_directory())
.export(
&self.context.module.settings,
self.context.dotenv,
&self.scope,
&self.context.module.unexports,
)
.stdin(Stdio::inherit())
.stderr(if self.context.config.verbosity.quiet() {
Stdio::null()
} else {
Stdio::inherit()
})
.stdout(Stdio::piped());

let (result, caught) = cmd.output_guard();

let output = result.map_err(OutputError::Io)?;

OutputError::result_from_exit_status(output.status)?;

let output = str::from_utf8(&output.stdout).map_err(OutputError::Utf8)?;

if let Some(signal) = caught {
return Err(OutputError::Interrupted(signal));
}

Ok(
output
.strip_suffix("\r\n")
.or_else(|| output.strip_suffix("\n"))
.unwrap_or(output)
.into(),
)
}

pub(crate) fn evaluate_line(
Expand Down
16 changes: 0 additions & 16 deletions src/interrupt_guard.rs

This file was deleted.

86 changes: 0 additions & 86 deletions src/interrupt_handler.rs

This file was deleted.

22 changes: 13 additions & 9 deletions src/justfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,20 +109,20 @@ impl<'src> Justfile<'src> {
Command::new(binary)
};

command.args(arguments);

command.current_dir(&search.working_directory);
command
.args(arguments)
.current_dir(&search.working_directory);

let scope = scope.child();

command.export(&self.settings, &dotenv, &scope, &self.unexports);

let status = InterruptHandler::guard(|| command.status()).map_err(|io_error| {
Error::CommandInvoke {
binary: binary.clone(),
arguments: arguments.clone(),
io_error,
}
let (result, caught) = command.status_guard();

let status = result.map_err(|io_error| Error::CommandInvoke {
binary: binary.clone(),
arguments: arguments.clone(),
io_error,
})?;

if !status.success() {
Expand All @@ -133,6 +133,10 @@ impl<'src> Justfile<'src> {
});
};

if let Some(signal) = caught {
return Err(Error::Interrupted { signal });
}

return Ok(());
}
Subcommand::Evaluate { variable, .. } => {
Expand Down
Loading
Loading