Skip to content

Commit b933188

Browse files
committed
Terminate child containers on program termination.
Upon receiving a termination signal, ensuring any spawned containers are also terminated. This is required since we intentional do not attach stdint to the container. This means, however, that interrupts such as Ctrl+C did not stop the build process. Likewise, termination signals such as SIGTERM merely exited cross, and did not terminate the container.
1 parent 2a313f0 commit b933188

File tree

3 files changed

+33
-3
lines changed

3 files changed

+33
-3
lines changed

src/docker/local.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::io;
22
use std::path::Path;
33
use std::process::{Command, ExitStatus};
4+
use std::sync::atomic::Ordering;
45

56
use super::shared::*;
67
use crate::errors::Result;
@@ -50,6 +51,8 @@ pub(crate) fn run(
5051
msg_info,
5152
)?;
5253

54+
let container = toolchain_dirs.unique_container_identifier(options.target.target())?;
55+
docker.args(["--name", &container]);
5356
docker.arg("--rm");
5457

5558
docker_seccomp(&mut docker, engine.kind, &options.target, &paths.metadata)
@@ -121,9 +124,22 @@ pub(crate) fn run(
121124
.wrap_err("when building custom image")?;
122125
}
123126

124-
docker
127+
Container::create(engine.clone(), container)?;
128+
let status = docker
125129
.arg(&image_name)
126130
.args(["sh", "-c", &build_command(toolchain_dirs, &cmd)])
127131
.run_and_get_status(msg_info, false)
128-
.map_err(Into::into)
132+
.map_err(Into::into);
133+
134+
// `cargo` generally returns 0 or 101 on completion, but isn't guaranteed
135+
// to. `ExitStatus::code()` may be None if a signal caused the process to
136+
// terminate or it may be a known interrupt return status (130, 137, 143).
137+
// simpler: just test if the program termination handler was called.
138+
// SAFETY: an atomic load.
139+
let is_terminated = unsafe { crate::errors::TERMINATED.load(Ordering::SeqCst) };
140+
if !is_terminated {
141+
Container::exit_static();
142+
}
143+
144+
status
129145
}

src/docker/shared.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,19 @@ impl Container {
549549
unsafe { CONTAINER.exists() }
550550
}
551551

552+
// when the `docker run` command finished.
553+
// the container has already exited, so no cleanup required.
554+
pub fn exit(&mut self) {
555+
self.exists.store(false, Ordering::SeqCst);
556+
}
557+
558+
pub fn exit_static() {
559+
// SAFETY: an atomic store.
560+
unsafe {
561+
CONTAINER.exit();
562+
}
563+
}
564+
552565
// when the `docker exec` command finished.
553566
pub fn finish(&mut self, is_tty: bool, msg_info: &mut MessageInfo) {
554567
// relax the no-timeout and lack of output

src/errors.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ unsafe fn termination_handler() {
104104
// makes this safe regardless.
105105
docker::CONTAINER.terminate();
106106

107-
// EOWNERDEAD, seems to be the same on linux, macos, and bash on windows.
107+
// all termination exit codes are 128 + signal code. the exit code is
108+
// 130 for Ctrl+C or SIGINT (signal code 2) for linux, macos, and windows.
108109
std::process::exit(130);
109110
}
110111

0 commit comments

Comments
 (0)