Skip to content

Commit

Permalink
utils: Add a lifecycle_bind helper for Command
Browse files Browse the repository at this point in the history
In almost all children we fork, we want the child to reliably
exit if we do (e.g. especially if we panic). The Linux
PR_SET_PDEATHSIG is just great for this.

Signed-off-by: Colin Walters <[email protected]>
  • Loading branch information
cgwalters committed Dec 6, 2024
1 parent 3ead145 commit af207a0
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 1 deletion.
2 changes: 2 additions & 0 deletions Cargo.lock

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

3 changes: 2 additions & 1 deletion lib/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,8 @@ async fn install_container(
/// Run a command in the host mount namespace
pub(crate) fn run_in_host_mountns(cmd: &str) -> Command {
let mut c = Command::new("/proc/self/exe");
c.args(["exec-in-host-mount-namespace", cmd]);
c.lifecycle_bind()
.args(["exec-in-host-mount-namespace", cmd]);
c
}

Expand Down
2 changes: 2 additions & 0 deletions lib/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::process::Command;
use std::time::Duration;

use anyhow::{Context, Result};
use bootc_utils::CommandRunExt;
#[cfg(feature = "install")]
use camino::Utf8Path;
use cap_std_ext::cap_std::fs::Dir;
Expand Down Expand Up @@ -119,6 +120,7 @@ pub(crate) fn spawn_editor(tmpf: &tempfile::NamedTempFile) -> Result<()> {
let status = Command::new(argv0)
.args(editor_args)
.arg(tmpf.path())
.lifecycle_bind()
.status()
.context("Spawning editor")?;
if !status.success() {
Expand Down
1 change: 1 addition & 0 deletions ostree-ext/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ ostree = { features = ["v2022_6"], version = "0.19.0" }

# Private dependencies
anyhow = { workspace = true }
bootc-utils = { path = "../utils" }
camino = { workspace = true, features = ["serde1"] }
chrono = { workspace = true }
olpc-cjson = "0.1.1"
Expand Down
2 changes: 2 additions & 0 deletions ostree-ext/src/container/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::os::fd::BorrowedFd;
use std::process::Command;

use anyhow::Result;
use bootc_utils::CommandRunExt;
use cap_std_ext::cmdext::CapStdExtCommandExt;
use fn_error_context::context;
use ocidir::cap_std::fs::Dir;
Expand Down Expand Up @@ -148,6 +149,7 @@ pub async fn deploy(
let st = Command::new("/proc/self/exe")
.args(["internals", "bootc-install-completion", ".", stateroot])
.cwd_dir(sysroot_dir.try_clone()?)
.lifecycle_bind()
.status()?;
if !st.success() {
anyhow::bail!("Failed to complete bootc install");
Expand Down
1 change: 1 addition & 0 deletions utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ repository = "https://github.com/containers/bootc"

[dependencies]
anyhow = { workspace = true }
rustix = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
tempfile = { workspace = true }
Expand Down
17 changes: 17 additions & 0 deletions utils/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
use std::{
io::{Read, Seek},
os::unix::process::CommandExt,
process::Command,
};

Expand All @@ -15,6 +16,9 @@ pub trait CommandRunExt {
/// Execute the child process.
fn run(&mut self) -> Result<()>;

/// Ensure the child does not outlive the parent.
fn lifecycle_bind(&mut self) -> &mut Self;

/// Execute the child process and capture its output. This uses `run` internally
/// and will return an error if the child process exits abnormally.
fn run_get_output(&mut self) -> Result<Box<dyn std::io::BufRead>>;
Expand Down Expand Up @@ -84,6 +88,19 @@ impl CommandRunExt for Command {
self.status()?.check_status(stderr)
}

#[allow(unsafe_code)]
fn lifecycle_bind(&mut self) -> &mut Self {
// SAFETY: This API is safe to call in a forked child.
unsafe {
self.pre_exec(|| {
rustix::process::set_parent_process_death_signal(Some(
rustix::process::Signal::Term,
))
.map_err(Into::into)
})
}
}

/// Output a debug-level log message with this command.
fn log_debug(&mut self) -> &mut Self {
// We unconditionally log at trace level, so avoid double logging
Expand Down

0 comments on commit af207a0

Please sign in to comment.