flagon-io/g1t

public

Cancelling or timing out a step signals its whole group with kill(2), not the kill program the runner image does not have

The runner image (node bookworm-slim) has no procps, so the kill command failed silently: a cancelled step got no SIGINT or SIGTERM, went straight to being killed, and what it started could outlive it. CI's cancellation test on g1t's runners caught it.

syntaqxcommitted 1d agoParentd6c6d83Browse files

130	130
131	131		/// Sends `signal` to the process's group (it leads its own), so what the
132	132		/// step started hears it too. Windows has no signals: it is left to `kill`.
	133	+	/// Sent with kill(2) itself: a machine without the `kill` program (procps
	134	+	/// is not in every image) would otherwise give the step no SIGINT at all,
	135	+	/// and leave what it started running after the step was killed.
	136	+	#[cfg(unix)]
133	137		fn signal(child: &std::process::Child, signal: &str) {
134		−	if cfg!(unix) {
135		−	let _ = Command::new("kill")
136		−	.args([format!("-{signal}"), "--".into(), format!("-{}", child.id())])
137		−	.stdout(Stdio::null())
138		−	.stderr(Stdio::null())
139		−	.status();
	138	+	let number = match signal {
	139	+	"INT" => libc::SIGINT,
	140	+	"TERM" => libc::SIGTERM,
	141	+	_ => libc::SIGKILL,
	142	+	};
	143	+	let Ok(group) = libc::pid_t::try_from(child.id()) else {
	144	+	return;
	145	+	};
	146	+	// SAFETY: kill(2) with a negative pid signals that process group; it
	147	+	// reads no memory of ours.
	148	+	unsafe {
	149	+	libc::kill(-group, number);
140	150		}
141	151		}
142	152
	153	+	#[cfg(not(unix))]
	154	+	fn signal(_child: &std::process::Child, _signal: &str) {}
	155	+
143	156		/// Runs the command, sending its output (stdout and stderr together, a
144	157		/// line at a time) through `commands` to the log, until it ends or
145	158		/// `timeout` passes.