feat(process): make the signal diagnostic discriminating (Bets 2-4)
Framing acceptances 2, 3, 4, 5, 7 and 8. Evidence collection only: no tolerance rule, no change to which process is signalled, no disposition change. Three distinct failures previously rendered as one string. **The PTY fallback is now named.** When a PTY's foreground-group lookup yields no group, the target falls back to the leader — and until now that rendered "leader-pid", identical to a pipe child that never had a terminal. `portable-pty::MasterPty::process_group_leader` collapses every failure into `None` before pmacs can see it, so the errno was gone too. pmacs now performs the query itself and reports four distinct outcomes: no master fd, a failed duplicate with its errno, a failed `tcgetpgrp` with its errno, and a non-positive answer. Doing that without `unsafe` is the interesting part. `nix::unistd:: tcgetpgrp` needs `AsFd`; `MasterPty` exposes only `Option<RawFd>`; and every std route between them is `unsafe`, which this crate forbids. `filedescriptor::OwnedHandle::dup` takes any `AsRawFd` through a safe blanket impl and returns an owned handle that IS `AsFd`, so a lifetime-tied view implementing one safe trait is the whole bridge. The borrow is what makes it sound: the view cannot outlive the master, so the descriptor cannot close underneath it. **The report names the signal.** A failed SIGUSR1 and a failed SIGTERM were the same text. Note this is a reporting gap only — every failed `kill` returns before the fatal-signal branch, so failed signals are disposition-identical whatever they are. A separate control pins that the fatal/non-fatal difference is real for calls that SUCCEED, which is what gives the first test its meaning. **`measured_group` is a real observation.** `expected_group` is `-leader_pid`, and on the spawn-group path the target is `-leader_pid` too, so the report printed the same number three times and their agreement was arithmetic rather than evidence. `getpgid` supplies the one field that can disagree. It establishes no identity — it is read inside the same read-then-act window, and no portable mechanism closes that for a group. Bites, each by an actual revert, all observed to fail: - collapsing the PTY fallback back into a bare "leader-pid"; - dropping `signal=` from the report; - making the measured group restate the pid it was handed; - replacing the job-control fixture with a plain `sleep`, as a positive control on the divergence fixture itself. All four exact-string sites were updated individually, never by a blanket rewrite: a wholesale rewrite of expected strings is how a format regression hides. `:2501`'s first-call disposition pin is retained and updated for the new format rather than replaced. `nix`'s `process` feature is now declared explicitly. It already arrived transitively — nix's own `signal` feature depends on it — which is stable but invisible, and a real requirement resting on another feature's internals is one refactor away from vanishing. `filedescriptor` is declared directly for the same reason: pmacs now calls its API. The reap ledger's comment claiming "EPERM cannot happen for our own children" is corrected. Its bounded-growth policy is unchanged, but the justification was wrong: the probe targets a group, and owning the spawned child says nothing about a group unless the child is still a member — which nothing measures. The handoff records this together with the limit of the evidence: the occurrence does NOT establish that the child itself received EPERM.
This commit is contained in:
parent
0d2ad49969
commit
b27df705bb
|
|
@ -2574,6 +2574,7 @@ dependencies = [
|
|||
"codebook-tree-sitter-latex",
|
||||
"crossbeam",
|
||||
"crossterm",
|
||||
"filedescriptor",
|
||||
"loro",
|
||||
"mlua",
|
||||
"nix 0.29.0",
|
||||
|
|
|
|||
18
Cargo.toml
18
Cargo.toml
|
|
@ -262,12 +262,28 @@ arborium-lean = "2.18"
|
|||
# T M4.4 process supervisor: signal sending without `unsafe`. Keep
|
||||
# the feature surface tight to keep build time low. `poll` feeds the
|
||||
# compile-mode group readers (cancellable poll-based reads, Q#CM3).
|
||||
nix = { version = "0.29", default-features = false, features = ["signal", "user", "fs", "term", "socket", "poll"] }
|
||||
#
|
||||
# `process` is listed explicitly even though it already arrives
|
||||
# transitively: nix's own `signal` feature depends on it, so `getpgid`
|
||||
# and `tcgetpgrp` compile today without being asked for. That is
|
||||
# stable but invisible, and a real requirement that depends on another
|
||||
# feature's internals is one refactor away from vanishing. The signal
|
||||
# diagnostic calls both directly.
|
||||
nix = { version = "0.29", default-features = false, features = ["signal", "user", "fs", "term", "socket", "poll", "process"] }
|
||||
# T M4.4 PTY mode: portable abstraction over openpty / fork+exec
|
||||
# with controlling-tty wiring. The crate uses internal `unsafe`
|
||||
# but exposes a fully safe API; pmacs's own `unsafe_code = "forbid"`
|
||||
# rule still holds.
|
||||
portable-pty = "0.9"
|
||||
# Safe file-descriptor duplication, already in the tree through
|
||||
# `portable-pty`. Declared directly because the signal diagnostic calls
|
||||
# `OwnedHandle::dup` itself: `MasterPty` exposes only a `RawFd`, and
|
||||
# every std route from a raw fd to something implementing `AsFd` is
|
||||
# `unsafe`. `dup` takes any `AsRawFd` through a safe blanket impl and
|
||||
# returns an owned handle that IS `AsFd`, which is what lets pmacs call
|
||||
# `nix::unistd::tcgetpgrp` — and keep the errno portable-pty discards —
|
||||
# without a single `unsafe` block of its own.
|
||||
filedescriptor = "0.8"
|
||||
# T M4.5 LSP wire format: JSON-RPC 2.0 bodies inside Content-Length
|
||||
# framing. Used only on LSP and similar protocols that require JSON
|
||||
# specifically; in-process workers continue to use MessagePack
|
||||
|
|
|
|||
|
|
@ -223,6 +223,31 @@ commands, read `docs/active-work.md` immediately after this file.
|
|||
remain parked pending the evidence this diagnostic produces, as does
|
||||
`terminate` idempotence for an already-reaped process (a different
|
||||
failure, so a different PR).
|
||||
- **The diagnostic fired, and what it showed.** macOS CI, PR #191,
|
||||
[run 30553376486](https://github.com/levineuwirth/pmacs/actions/runs/30553376486/job/90907461258):
|
||||
`target=-8619 via group, leader_pid=8619, expected_group=-8619,
|
||||
leader=live` — the `spec.group` pipe path, not the PTY path, with the
|
||||
leader observed alive by a real `try_wait`. A rerun of the identical
|
||||
head passed 12/12, so it is intermittent.
|
||||
- **Owning the child does not license dismissing a group error, and
|
||||
the occurrence does not prove the child received EPERM.** The failed
|
||||
target was the *group* `-8619`; `try_wait` observed the *process*
|
||||
`8619`. Nothing measured `getpgid(8619)`, so the two are not known to
|
||||
refer to the same thing. What is settled is narrower and still
|
||||
enough: a group target computed from the spawn-time `pgid == pid`
|
||||
assumption returned EPERM while the leader was alive, which retires
|
||||
"EPERM cannot happen for our own children" as a reason to discard an
|
||||
arbitrary group-directed error. Attributing the errno to the child
|
||||
would repeat the exact error that killed three tolerance rules —
|
||||
concluding something about one entity from something about another.
|
||||
- **A field named like an observation can be a restatement of its
|
||||
input.** `expected_group` is `-leader_pid`, and on the spawn-group
|
||||
path the target is `-leader_pid` too, so the report printed the same
|
||||
number three times and their agreement was arithmetic. Stage B adds a
|
||||
`measured_group` from `getpgid`, which is the only field able to
|
||||
disagree — and it still establishes no identity, because it is read
|
||||
inside the same read-then-act window and no portable mechanism closes
|
||||
that for a *group* (`pidfd` covers a process; macOS has neither).
|
||||
- **Lean 4 arc (Arc 8) — stages 1, 2, 3a, 3b, 4a, 4b ALL LANDED**
|
||||
(`docs/lean4-mode-framing.md`; #160, #161, #167, #170, #179, #181). pmacs edits Lean 4: `arborium-lean` highlighting, a
|
||||
`lean4` major mode, `⟨⟩ ⦃⦄ ⟮⟯` pairs, and a `lake serve` language
|
||||
|
|
|
|||
523
src/process.rs
523
src/process.rs
|
|
@ -477,6 +477,9 @@ pub struct ProcessSupervisor {
|
|||
/// observation still runs against the real child handle; a stubbed
|
||||
/// observation would bypass the code path under test.
|
||||
forced_kill_errno: Option<nix::errno::Errno>,
|
||||
/// Test seam for the PTY foreground-group lookup (see
|
||||
/// `force_next_pty_lookup_failure`). Always `None` outside tests.
|
||||
forced_pty_lookup: Option<PtyLookupFailure>,
|
||||
}
|
||||
|
||||
/// One armed group in the reap ledger.
|
||||
|
|
@ -726,16 +729,58 @@ enum TargetSource {
|
|||
ForegroundGroup,
|
||||
/// A `group = true` pipe child leading its own process group.
|
||||
SpawnGroup,
|
||||
/// The child's own pid.
|
||||
/// The child's own pid, for a pipe child that leads no group.
|
||||
LeaderPid,
|
||||
/// A **PTY** child whose foreground-group lookup did not yield a
|
||||
/// group, so the target fell back to the leader pid.
|
||||
///
|
||||
/// Distinct from [`Self::LeaderPid`] on purpose. Before this
|
||||
/// variant existed both rendered "leader-pid", so a PTY whose
|
||||
/// terminal query failed was indistinguishable in the report from
|
||||
/// an ordinary pipe child that never had a terminal — two very
|
||||
/// different situations reading as one.
|
||||
PtyForegroundFallback(PtyLookupFailure),
|
||||
}
|
||||
|
||||
/// Why a PTY's foreground-group lookup produced no group.
|
||||
///
|
||||
/// Each arm is a different fact and none is forged into another: a
|
||||
/// missing fd is not an errno, and a failure to *duplicate* the master
|
||||
/// is not a failure to *query* the terminal.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum PtyLookupFailure {
|
||||
/// The master reported no file descriptor to query.
|
||||
NoMasterFd,
|
||||
/// Duplicating the master fd failed, so the terminal was never
|
||||
/// queried at all.
|
||||
Duplicate(nix::errno::Errno),
|
||||
/// `tcgetpgrp` itself failed on a successfully duplicated fd.
|
||||
Query(nix::errno::Errno),
|
||||
/// The terminal answered, but with a non-positive group id, which
|
||||
/// names no group.
|
||||
NonPositive(i32),
|
||||
}
|
||||
|
||||
impl PtyLookupFailure {
|
||||
fn render(self) -> String {
|
||||
match self {
|
||||
Self::NoMasterFd => "no-master-fd".to_owned(),
|
||||
Self::Duplicate(e) => format!("duplicate-master-fd: {e}"),
|
||||
Self::Query(e) => format!("tcgetpgrp: {e}"),
|
||||
Self::NonPositive(v) => format!("tcgetpgrp-non-positive: {v}"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl TargetSource {
|
||||
fn as_str(self) -> &'static str {
|
||||
fn render(self) -> String {
|
||||
match self {
|
||||
Self::ForegroundGroup => "tcgetpgrp",
|
||||
Self::SpawnGroup => "group",
|
||||
Self::LeaderPid => "leader-pid",
|
||||
Self::ForegroundGroup => "tcgetpgrp".to_owned(),
|
||||
Self::SpawnGroup => "group".to_owned(),
|
||||
Self::LeaderPid => "leader-pid".to_owned(),
|
||||
Self::PtyForegroundFallback(why) => {
|
||||
format!("pty-leader-fallback({})", why.render())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -745,6 +790,85 @@ impl TargetSource {
|
|||
}
|
||||
}
|
||||
|
||||
/// A lifetime-tied view of a `MasterPty`'s file descriptor.
|
||||
///
|
||||
/// `MasterPty` exposes only `Option<RawFd>`, and every std route from a
|
||||
/// raw fd to something implementing `AsFd` — `BorrowedFd::borrow_raw`,
|
||||
/// `OwnedFd::from_raw_fd`, `File::from_raw_fd` — is `unsafe`, which this
|
||||
/// crate forbids. `filedescriptor::OwnedHandle::dup` accepts any
|
||||
/// `AsRawFd` through a safe blanket impl and hands back an owned handle
|
||||
/// that *is* `AsFd`, so implementing this one safe trait is the whole
|
||||
/// bridge.
|
||||
///
|
||||
/// The borrow is what makes it sound: the view cannot outlive the master
|
||||
/// it read the descriptor from, so the fd cannot have been closed
|
||||
/// underneath it.
|
||||
struct MasterFdView<'a> {
|
||||
fd: std::os::fd::RawFd,
|
||||
_master: &'a (dyn portable_pty::MasterPty + Send),
|
||||
}
|
||||
|
||||
impl std::os::fd::AsRawFd for MasterFdView<'_> {
|
||||
fn as_raw_fd(&self) -> std::os::fd::RawFd {
|
||||
self.fd
|
||||
}
|
||||
}
|
||||
|
||||
/// Recover the OS errno from a `filedescriptor` error.
|
||||
///
|
||||
/// Its error type is an enum of thiserror variants, each carrying a
|
||||
/// `std::io::Error` as a `#[source]` rather than exposing
|
||||
/// `raw_os_error` itself. Walking the source chain and downcasting keeps
|
||||
/// every variant working, including ones added later, instead of
|
||||
/// matching the one arm that exists today.
|
||||
///
|
||||
/// Returns `UnknownErrno` when the chain carries no OS error, rather
|
||||
/// than inventing a plausible one — a forged errno in a diagnostic is
|
||||
/// worse than an honest absence.
|
||||
fn os_errno_of(err: &filedescriptor::Error) -> nix::errno::Errno {
|
||||
let mut current: Option<&(dyn std::error::Error + 'static)> = Some(err);
|
||||
while let Some(e) = current {
|
||||
if let Some(io) = e.downcast_ref::<std::io::Error>()
|
||||
&& let Some(code) = io.raw_os_error()
|
||||
{
|
||||
return nix::errno::Errno::from_raw(code);
|
||||
}
|
||||
current = e.source();
|
||||
}
|
||||
nix::errno::Errno::UnknownErrno
|
||||
}
|
||||
|
||||
/// Read the terminal's foreground process group, keeping the errno.
|
||||
///
|
||||
/// `portable_pty::MasterPty::process_group_leader` collapses every
|
||||
/// failure into `None`, so pmacs could not tell "this tty has no
|
||||
/// foreground group" from "the query failed and here is why". This does
|
||||
/// the query itself and returns the reason on every non-success path.
|
||||
fn pty_foreground_group(
|
||||
master: &(dyn portable_pty::MasterPty + Send),
|
||||
) -> Result<i32, PtyLookupFailure> {
|
||||
let Some(fd) = master.as_raw_fd() else {
|
||||
return Err(PtyLookupFailure::NoMasterFd);
|
||||
};
|
||||
let view = MasterFdView {
|
||||
fd,
|
||||
_master: master,
|
||||
};
|
||||
let owned = filedescriptor::OwnedHandle::dup(&view)
|
||||
.map_err(|e| PtyLookupFailure::Duplicate(os_errno_of(&e)))?;
|
||||
match nix::unistd::tcgetpgrp(&owned) {
|
||||
Ok(pgrp) => {
|
||||
let raw = pgrp.as_raw();
|
||||
if raw > 0 {
|
||||
Ok(raw)
|
||||
} else {
|
||||
Err(PtyLookupFailure::NonPositive(raw))
|
||||
}
|
||||
}
|
||||
Err(e) => Err(PtyLookupFailure::Query(e)),
|
||||
}
|
||||
}
|
||||
|
||||
/// The entity a signal was actually aimed at, plus the branch that
|
||||
/// chose it. Carried so a failure can report the target as a fact
|
||||
/// separate from the leader's state (Q#PD1).
|
||||
|
|
@ -754,18 +878,36 @@ struct SignalTarget {
|
|||
source: TargetSource,
|
||||
}
|
||||
|
||||
fn signal_target(proc: &ManagedProcess, pid: u32) -> Result<SignalTarget, String> {
|
||||
fn signal_target(
|
||||
proc: &ManagedProcess,
|
||||
pid: u32,
|
||||
forced_lookup: Option<PtyLookupFailure>,
|
||||
) -> Result<SignalTarget, String> {
|
||||
if let Some(runtime) = proc.runtime.as_ref()
|
||||
&& let ChildHandle::Pty {
|
||||
_master: master, ..
|
||||
} = &runtime.child
|
||||
&& let Some(pgrp) = master.process_group_leader()
|
||||
&& pgrp > 0
|
||||
{
|
||||
return Ok(SignalTarget {
|
||||
pid: Pid::from_raw(-pgrp),
|
||||
source: TargetSource::ForegroundGroup,
|
||||
});
|
||||
// A PTY child is always group-directed when the terminal names a
|
||||
// foreground group. When it does not, the target falls back to
|
||||
// the leader — and *why* it fell back is carried into the source
|
||||
// so the report can say it. Previously every one of these paths
|
||||
// produced a bare `LeaderPid`, identical to a pipe child that
|
||||
// never had a terminal at all.
|
||||
let lookup = match forced_lookup {
|
||||
Some(failure) => Err(failure),
|
||||
None => pty_foreground_group(master.as_ref()),
|
||||
};
|
||||
return match lookup {
|
||||
Ok(pgrp) => Ok(SignalTarget {
|
||||
pid: Pid::from_raw(-pgrp),
|
||||
source: TargetSource::ForegroundGroup,
|
||||
}),
|
||||
Err(why) => Ok(SignalTarget {
|
||||
pid: Pid::from_raw(i32::try_from(pid).map_err(|e| e.to_string())?),
|
||||
source: TargetSource::PtyForegroundFallback(why),
|
||||
}),
|
||||
};
|
||||
}
|
||||
// `group = true` pipe children lead a fresh process group
|
||||
// (`process_group(0)` at spawn ⇒ pgid == pid), so fatal signals
|
||||
|
|
@ -824,12 +966,45 @@ fn observe_leader(proc: &mut ManagedProcess) -> LeaderObservation {
|
|||
}
|
||||
}
|
||||
|
||||
/// Render a failing `kill` as the five facts of Q#PD1. The disposition
|
||||
/// is unchanged (Q#PD2) — this only replaces a message that said
|
||||
/// nothing but the errno.
|
||||
/// The leader's process group as the kernel reports it, for a target
|
||||
/// that was *computed* from the spawn-time assumption `pgid == pid`.
|
||||
///
|
||||
/// `expected_group` is that assumption restated — it is `-leader_pid`,
|
||||
/// and on the `SpawnGroup` path the target is `-leader_pid` too, so the
|
||||
/// two agreeing is arithmetic rather than evidence. This is the only
|
||||
/// field in the report that can disagree with the input, which is what
|
||||
/// makes it worth printing.
|
||||
///
|
||||
/// **It does not establish identity** (framing §1.5). It is read before
|
||||
/// the `kill` in the same read-then-act window, and a number cannot
|
||||
/// distinguish the original group from a recycled one. No portable
|
||||
/// mechanism can: `pidfd` closes pid reuse for a process, not a group,
|
||||
/// and macOS has none at all. This records an observation; it settles
|
||||
/// nothing.
|
||||
fn measured_group_of(leader_pid: u32) -> String {
|
||||
let Ok(raw) = i32::try_from(leader_pid) else {
|
||||
return ", measured_group=unobservable(pid out of range)".to_owned();
|
||||
};
|
||||
match nix::unistd::getpgid(Some(Pid::from_raw(raw))) {
|
||||
Ok(pgid) => format!(", measured_group=-{}", pgid.as_raw()),
|
||||
Err(e) => format!(", measured_group=unobservable({e})"),
|
||||
}
|
||||
}
|
||||
|
||||
/// Render a failing `kill` as the facts of Q#PD1. The disposition is
|
||||
/// unchanged (Q#PD2) — this only replaces a message that said nothing
|
||||
/// but the errno.
|
||||
///
|
||||
/// The signal is named because it could not be recovered otherwise: a
|
||||
/// failed `SIGUSR1` and a failed `SIGTERM` were previously identical
|
||||
/// text. Note this is a *reporting* gap only — every failed `kill`
|
||||
/// returns before the fatal-signal branch, so failed signals are
|
||||
/// disposition-identical whatever they are. The disposition difference
|
||||
/// is real only for calls that succeed.
|
||||
fn signal_failure_report(
|
||||
target: SignalTarget,
|
||||
leader_pid: u32,
|
||||
signal: Signal,
|
||||
errno: nix::errno::Errno,
|
||||
leader: &LeaderObservation,
|
||||
) -> String {
|
||||
|
|
@ -841,10 +1016,19 @@ fn signal_failure_report(
|
|||
} else {
|
||||
String::new()
|
||||
};
|
||||
// Only the spawn-group path computes its target from the assumption,
|
||||
// so it is the only one where a measurement can contradict anything.
|
||||
// A PTY target came from the terminal and a leader-directed target is
|
||||
// not a group at all.
|
||||
let measured = if matches!(target.source, TargetSource::SpawnGroup) {
|
||||
measured_group_of(leader_pid)
|
||||
} else {
|
||||
String::new()
|
||||
};
|
||||
format!(
|
||||
"kill: {errno} (target={} via {}, leader_pid={leader_pid}{expected}, leader={})",
|
||||
"kill: {errno} (signal={signal:?}, target={} via {}, leader_pid={leader_pid}{expected}{measured}, leader={})",
|
||||
target.pid.as_raw(),
|
||||
target.source.as_str(),
|
||||
target.source.render(),
|
||||
leader.render(),
|
||||
)
|
||||
}
|
||||
|
|
@ -950,6 +1134,7 @@ impl ProcessSupervisor {
|
|||
reap_ledger: HashMap::new(),
|
||||
group_term_grace: GROUP_TERM_GRACE,
|
||||
forced_kill_errno: None,
|
||||
forced_pty_lookup: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -963,6 +1148,21 @@ impl ProcessSupervisor {
|
|||
self.forced_kill_errno = Some(errno);
|
||||
}
|
||||
|
||||
/// Test seam for the PTY foreground-group lookup, on the same terms
|
||||
/// as [`Self::force_next_kill_errno`] and for the same reason.
|
||||
///
|
||||
/// The three non-success arms — no master fd, a failed duplicate, a
|
||||
/// failed `tcgetpgrp` — cannot be produced on demand from a healthy
|
||||
/// PTY: they need an exhausted descriptor table or a master that has
|
||||
/// stopped being a terminal. Injecting only the *lookup result*
|
||||
/// leaves the branch itself, the fallback target choice, the leader
|
||||
/// observation against the real child, and the report construction
|
||||
/// all running as production code. Consumed by one call.
|
||||
#[cfg(test)]
|
||||
fn force_next_pty_lookup_failure(&mut self, failure: PtyLookupFailure) {
|
||||
self.forced_pty_lookup = Some(failure);
|
||||
}
|
||||
|
||||
/// Override the SIGTERM-to-SIGKILL grace window. Test helper.
|
||||
pub fn set_grace_period(&mut self, d: Duration) {
|
||||
self.grace_period = d;
|
||||
|
|
@ -1080,7 +1280,8 @@ impl ProcessSupervisor {
|
|||
else {
|
||||
return Err(format!("process {id} is not running"));
|
||||
};
|
||||
let target = signal_target(proc, pid)?;
|
||||
let forced_lookup = self.forced_pty_lookup.take();
|
||||
let target = signal_target(proc, pid, forced_lookup)?;
|
||||
// Q#PD4: the seam injects the KILL attempt's result only —
|
||||
// never the observation below — so target selection, the real
|
||||
// `ChildHandle::try_wait` against the real child, and the error
|
||||
|
|
@ -1094,7 +1295,7 @@ impl ProcessSupervisor {
|
|||
// disposition is unchanged — this still returns `Err`,
|
||||
// with no state transition and no ledger arming.
|
||||
let leader = observe_leader(proc);
|
||||
return Err(signal_failure_report(target, pid, errno, &leader));
|
||||
return Err(signal_failure_report(target, pid, signal, errno, &leader));
|
||||
}
|
||||
if matches!(signal, Signal::SIGTERM | Signal::SIGKILL | Signal::SIGHUP) {
|
||||
proc.state = ProcessState::Exiting {
|
||||
|
|
@ -1243,9 +1444,24 @@ impl ProcessSupervisor {
|
|||
let now = Instant::now();
|
||||
self.reap_ledger.retain(|pgid, entry| {
|
||||
// ESRCH: no such group — done. Any other probe error is
|
||||
// also treated as "nothing left we can reach" (EPERM
|
||||
// cannot happen for our own children) so the ledger
|
||||
// cannot grow without bound.
|
||||
// also treated as "nothing left we can reach", so the
|
||||
// ledger cannot grow without bound.
|
||||
//
|
||||
// **That is a bounded-growth policy, not a claim that the
|
||||
// group is gone.** This comment previously justified it with
|
||||
// "EPERM cannot happen for our own children". That reasoning
|
||||
// does not hold: the probe targets a *group*, and owning the
|
||||
// spawned child says nothing about a group unless the child
|
||||
// is still a member of it — which nothing here measures. A
|
||||
// group-directed EPERM against a live leader has since been
|
||||
// observed in CI (macOS, PR #191, run 30553376486), via an
|
||||
// explicit signal rather than this probe.
|
||||
//
|
||||
// So this arm can silently cancel an escalation, and the
|
||||
// `SIGKILL` below can fail while the entry is marked killed.
|
||||
// Both are known and deliberately unchanged here: the
|
||||
// diagnostic lane that found them does not alter
|
||||
// disposition. Fixing it is its own lane.
|
||||
if nix::sys::signal::kill(Pid::from_raw(-*pgid), None).is_err() {
|
||||
return false;
|
||||
}
|
||||
|
|
@ -2388,6 +2604,20 @@ mod tests {
|
|||
/// the availability guard and the spawn cannot drift apart.
|
||||
const BASH: &str = "/bin/bash";
|
||||
|
||||
/// A plain PTY child, for tests that care about the PTY *branch*
|
||||
/// rather than about job control.
|
||||
fn spawn_live_pty(sup: &mut ProcessSupervisor, name: &str) -> (ProcessId, u32) {
|
||||
let mut spec = ProcessSpec::new(name, "/bin/sleep");
|
||||
spec.args = vec!["30".into()];
|
||||
spec.mode = ProcessMode::Pty {
|
||||
rows: 24,
|
||||
cols: 80,
|
||||
mode: TerminalMode::Canonical,
|
||||
};
|
||||
let id = sup.spawn(spec).expect("spawn");
|
||||
(id, spawn_started_pid(sup, id))
|
||||
}
|
||||
|
||||
/// The tty's current foreground process group, read through the same
|
||||
/// `MasterPty` accessor production uses. `None` for a pipe
|
||||
/// generation, or when the terminal reports no foreground group.
|
||||
|
|
@ -2524,7 +2754,7 @@ mod tests {
|
|||
let err = sup.terminate(id).expect_err("injected EPERM must fail");
|
||||
|
||||
let expected = format!(
|
||||
"kill: {} (target=-{fg} via tcgetpgrp, leader_pid={pid}, expected_group=-{pid}, leader=live)",
|
||||
"kill: {} (signal=SIGTERM, target=-{fg} via tcgetpgrp, leader_pid={pid}, expected_group=-{pid}, leader=live)",
|
||||
nix::errno::Errno::EPERM
|
||||
);
|
||||
assert_eq!(
|
||||
|
|
@ -2549,6 +2779,247 @@ mod tests {
|
|||
let _ = sup.signal(id, Signal::SIGKILL);
|
||||
}
|
||||
|
||||
/// Q#DC2 / acceptance 2 — a PTY whose foreground-group lookup fails
|
||||
/// is distinguishable from a pipe child that never had a terminal.
|
||||
///
|
||||
/// Before this, both rendered "leader-pid". The PTY fallback was
|
||||
/// therefore invisible: a terminal query that failed, and a process
|
||||
/// with no terminal at all, produced the same word. Each arm now
|
||||
/// names its own stage, and `portable-pty`'s
|
||||
/// `process_group_leader` — which collapses every failure into
|
||||
/// `None` before pmacs can see it — is bypassed so the errno
|
||||
/// survives.
|
||||
#[test]
|
||||
fn a_pty_foreground_lookup_failure_names_its_stage() {
|
||||
let arms = [
|
||||
(PtyLookupFailure::NoMasterFd, "no-master-fd".to_owned()),
|
||||
(
|
||||
PtyLookupFailure::Duplicate(nix::errno::Errno::EMFILE),
|
||||
format!("duplicate-master-fd: {}", nix::errno::Errno::EMFILE),
|
||||
),
|
||||
(
|
||||
PtyLookupFailure::Query(nix::errno::Errno::ENOTTY),
|
||||
format!("tcgetpgrp: {}", nix::errno::Errno::ENOTTY),
|
||||
),
|
||||
(
|
||||
PtyLookupFailure::NonPositive(0),
|
||||
"tcgetpgrp-non-positive: 0".to_owned(),
|
||||
),
|
||||
];
|
||||
|
||||
for (failure, rendered) in arms {
|
||||
let mut sup = ProcessSupervisor::new();
|
||||
let (id, pid) = spawn_live_pty(&mut sup, "diag-pty-fallback");
|
||||
|
||||
sup.force_next_pty_lookup_failure(failure);
|
||||
sup.force_next_kill_errno(nix::errno::Errno::EPERM);
|
||||
let err = sup.terminate(id).expect_err("injected EPERM must fail");
|
||||
|
||||
// The target falls back to the leader — positive, not a
|
||||
// negated group — and the source says why.
|
||||
let expected = format!(
|
||||
"kill: {} (signal=SIGTERM, target={pid} via pty-leader-fallback({rendered}), leader_pid={pid}, leader=live)",
|
||||
nix::errno::Errno::EPERM
|
||||
);
|
||||
assert_eq!(err, expected, "arm {failure:?} must name its own stage");
|
||||
|
||||
// And it must NOT read like a pipe child.
|
||||
assert!(
|
||||
!err.contains("via leader-pid,"),
|
||||
"a PTY fallback must not render as a bare pipe leader target: {err}"
|
||||
);
|
||||
|
||||
let _ = sup.signal(id, Signal::SIGKILL);
|
||||
}
|
||||
}
|
||||
|
||||
/// The companion half of acceptance 2: a genuine pipe child still
|
||||
/// renders "leader-pid", so the two really are distinct strings
|
||||
/// rather than both having moved.
|
||||
///
|
||||
/// Asserted here as well as in the leader-directed test because a
|
||||
/// rename of one side would otherwise pass every test — the pair is
|
||||
/// the point, not either string alone.
|
||||
#[test]
|
||||
fn a_pipe_child_still_renders_a_bare_leader_target() {
|
||||
let mut sup = ProcessSupervisor::new();
|
||||
let mut spec = ProcessSpec::new("diag-pipe-leader", "/bin/sleep");
|
||||
spec.args = vec!["30".into()];
|
||||
let id = sup.spawn(spec).expect("spawn");
|
||||
let pid = spawn_started_pid(&mut sup, id);
|
||||
|
||||
sup.force_next_kill_errno(nix::errno::Errno::EPERM);
|
||||
let err = sup.terminate(id).expect_err("injected EPERM must fail");
|
||||
|
||||
assert!(
|
||||
err.contains(&format!("target={pid} via leader-pid,")),
|
||||
"a pipe child with no group renders the bare leader source: {err}"
|
||||
);
|
||||
assert!(
|
||||
!err.contains("pty-leader-fallback"),
|
||||
"a pipe child never took the PTY branch: {err}"
|
||||
);
|
||||
|
||||
let _ = sup.signal(id, Signal::SIGKILL);
|
||||
}
|
||||
|
||||
/// Q#DC3 / acceptance 3(a) — the report names the signal, so two
|
||||
/// failures that differ only in which signal was sent are no longer
|
||||
/// the same text.
|
||||
///
|
||||
/// **They differ in text only.** Every failed `kill` returns before
|
||||
/// the fatal-signal branch, so both leave the state and the ledger
|
||||
/// exactly as they were. That is asserted here rather than assumed,
|
||||
/// because revision 2 of the framing claimed the opposite.
|
||||
#[test]
|
||||
fn a_failed_signal_names_which_signal_and_changes_nothing() {
|
||||
let mut reports = Vec::new();
|
||||
for signal in [Signal::SIGTERM, Signal::SIGUSR1] {
|
||||
let mut sup = ProcessSupervisor::new();
|
||||
let mut spec = ProcessSpec::new("diag-signal-name", "/bin/sh");
|
||||
spec.args = vec!["-c".into(), "sleep 30".into()];
|
||||
spec.group = true;
|
||||
let id = sup.spawn(spec).expect("spawn");
|
||||
let pid = spawn_started_pid(&mut sup, id);
|
||||
|
||||
sup.force_next_kill_errno(nix::errno::Errno::EPERM);
|
||||
let err = sup
|
||||
.signal(id, signal)
|
||||
.expect_err("injected EPERM must fail");
|
||||
|
||||
assert!(
|
||||
err.contains(&format!("signal={signal:?},")),
|
||||
"the report must name {signal:?}: {err}"
|
||||
);
|
||||
assert!(
|
||||
matches!(
|
||||
sup.processes.get(&id).expect("record").state,
|
||||
ProcessState::Running { .. }
|
||||
),
|
||||
"a failed {signal:?} must not transition the record"
|
||||
);
|
||||
assert!(
|
||||
sup.reap_ledger.is_empty(),
|
||||
"a failed {signal:?} must not arm the ledger"
|
||||
);
|
||||
|
||||
reports.push(err.replace(&format!("{pid}"), "<pid>"));
|
||||
let _ = nix::sys::signal::kill(
|
||||
Pid::from_raw(-i32::try_from(pid).unwrap()),
|
||||
Signal::SIGKILL,
|
||||
);
|
||||
}
|
||||
|
||||
assert_ne!(
|
||||
reports[0], reports[1],
|
||||
"SIGTERM and SIGUSR1 failures must no longer be identical text"
|
||||
);
|
||||
}
|
||||
|
||||
/// Q#DC3 / acceptance 3(b) — the disposition control. A *successful*
|
||||
/// non-fatal signal changes nothing, while a *successful* fatal one
|
||||
/// transitions the record and arms the ledger.
|
||||
///
|
||||
/// This is the check that gives the previous test its meaning: it
|
||||
/// shows the fatal/non-fatal distinction is real, and therefore that
|
||||
/// "failed signals are disposition-identical" is a statement about
|
||||
/// the failure path rather than about signals generally.
|
||||
#[test]
|
||||
fn a_successful_signal_disposition_depends_on_whether_it_is_fatal() {
|
||||
let mut sup = ProcessSupervisor::new();
|
||||
let mut spec = ProcessSpec::new("diag-disposition-live", "/bin/sh");
|
||||
// Ignore USR1 so the successful non-fatal signal cannot end the
|
||||
// child and confuse the state assertion with a real exit.
|
||||
spec.args = vec!["-c".into(), "trap '' USR1; sleep 30".into()];
|
||||
spec.group = true;
|
||||
let id = sup.spawn(spec).expect("spawn");
|
||||
let pid = spawn_started_pid(&mut sup, id);
|
||||
|
||||
sup.signal(id, Signal::SIGUSR1).expect("USR1 delivers");
|
||||
assert!(
|
||||
matches!(
|
||||
sup.processes.get(&id).expect("record").state,
|
||||
ProcessState::Running { .. }
|
||||
),
|
||||
"a successful non-fatal signal leaves the record Running"
|
||||
);
|
||||
assert!(
|
||||
sup.reap_ledger.is_empty(),
|
||||
"a successful non-fatal signal arms no ledger entry"
|
||||
);
|
||||
|
||||
sup.terminate(id).expect("TERM delivers");
|
||||
assert!(
|
||||
matches!(
|
||||
sup.processes.get(&id).expect("record").state,
|
||||
ProcessState::Exiting { .. }
|
||||
),
|
||||
"a successful fatal signal transitions the record to Exiting"
|
||||
);
|
||||
assert!(
|
||||
!sup.reap_ledger.is_empty(),
|
||||
"a successful fatal signal arms the group reap ledger"
|
||||
);
|
||||
|
||||
let _ =
|
||||
nix::sys::signal::kill(Pid::from_raw(-i32::try_from(pid).unwrap()), Signal::SIGKILL);
|
||||
}
|
||||
|
||||
/// Q#DC4 / acceptance 4 — the measured group is a real observation,
|
||||
/// not a restatement of the input.
|
||||
///
|
||||
/// `expected_group` is `-leader_pid` by construction, so on the
|
||||
/// spawn-group path it can never disagree with the target. The
|
||||
/// measured field is the only one that can, and this proves it does:
|
||||
/// a child placed into an *anchor* group reports that group, not its
|
||||
/// own pid.
|
||||
///
|
||||
/// Without this the field would be exactly the vacuous readout the
|
||||
/// framing was written to eliminate — an implementation returning
|
||||
/// `-pid` unconditionally would satisfy every other test.
|
||||
#[test]
|
||||
fn the_measured_group_reports_the_real_group_not_the_pid() {
|
||||
use std::os::unix::process::CommandExt as _;
|
||||
|
||||
// An anchor process leading its own group.
|
||||
let mut anchor = std::process::Command::new("/bin/sleep");
|
||||
anchor.arg("30");
|
||||
anchor.process_group(0);
|
||||
let mut anchor = anchor.spawn().expect("spawn anchor");
|
||||
let anchor_pgid = i32::try_from(anchor.id()).expect("pid fits i32");
|
||||
|
||||
// A second process placed INTO the anchor's group, so its pgid
|
||||
// is genuinely not its own pid.
|
||||
let mut joiner = std::process::Command::new("/bin/sleep");
|
||||
joiner.arg("30");
|
||||
joiner.process_group(anchor_pgid);
|
||||
let mut joiner = joiner.spawn().expect("spawn joiner");
|
||||
let joiner_pid = joiner.id();
|
||||
|
||||
assert_ne!(
|
||||
i32::try_from(joiner_pid).unwrap(),
|
||||
anchor_pgid,
|
||||
"precondition: the joiner must not be the anchor itself"
|
||||
);
|
||||
|
||||
let rendered = measured_group_of(joiner_pid);
|
||||
assert_eq!(
|
||||
rendered,
|
||||
format!(", measured_group=-{anchor_pgid}"),
|
||||
"the measurement must report the group the kernel actually has"
|
||||
);
|
||||
assert_ne!(
|
||||
rendered,
|
||||
format!(", measured_group=-{joiner_pid}"),
|
||||
"and must NOT restate the pid it was given"
|
||||
);
|
||||
|
||||
let _ = joiner.kill();
|
||||
let _ = joiner.wait();
|
||||
let _ = anchor.kill();
|
||||
let _ = anchor.wait();
|
||||
}
|
||||
|
||||
/// Q#PD1 acceptance 2 — a leader-directed failure records the
|
||||
/// fallback branch and a positive target, and omits the group field
|
||||
/// that would be meaningless for it. Exact message again.
|
||||
|
|
@ -2564,7 +3035,7 @@ mod tests {
|
|||
let err = sup.terminate(id).expect_err("injected ESRCH must fail");
|
||||
|
||||
let expected = format!(
|
||||
"kill: {} (target={pid} via leader-pid, leader_pid={pid}, leader=live)",
|
||||
"kill: {} (signal=SIGTERM, target={pid} via leader-pid, leader_pid={pid}, leader=live)",
|
||||
nix::errno::Errno::ESRCH
|
||||
);
|
||||
assert_eq!(
|
||||
|
|
@ -2614,7 +3085,7 @@ mod tests {
|
|||
let err = terminate_until_leader_exited(&mut sup, id, Duration::from_secs(10));
|
||||
|
||||
let expected = format!(
|
||||
"kill: {} (target={pid} via leader-pid, leader_pid={pid}, leader=exited(code 3))",
|
||||
"kill: {} (signal=SIGTERM, target={pid} via leader-pid, leader_pid={pid}, leader=exited(code 3))",
|
||||
nix::errno::Errno::EPERM
|
||||
);
|
||||
assert_eq!(
|
||||
|
|
@ -2646,7 +3117,7 @@ mod tests {
|
|||
let err = sup.terminate(id).expect_err("injected EPERM must fail");
|
||||
|
||||
let expected = format!(
|
||||
"kill: {} (target=-{pid} via group, leader_pid={pid}, expected_group=-{pid}, leader=live)",
|
||||
"kill: {} (signal=SIGTERM, target=-{pid} via group, leader_pid={pid}, expected_group=-{pid}, measured_group=-{pid}, leader=live)",
|
||||
nix::errno::Errno::EPERM
|
||||
);
|
||||
assert_eq!(err, expected, "a group=true pipe child reports via group");
|
||||
|
|
|
|||
Loading…
Reference in New Issue