diff --git a/CHANGELOG.md b/CHANGELOG.md index f1043fb..d66904b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,19 @@ has a matching GitHub release whose body comes from its section below. ## [Unreleased] +### Added + +- Cgroup attribution closes the one known stowaway window (chaos scenario 3: a child + whose parent exits within one 2 s tick). On Linux with a systemd user manager, + `berth run` execs the engine in its own transient cgroup scope (`systemd-run --user + --scope`, unit `berth-run--`), which every child inherits on fork: a + child that escapes its process group and reparents stays the lease's, the lease + turns stuck naming it, `berth reclaim` lists it, and a whole-card request is never + granted `contested` over it. A GPU process in a scope no live lease recorded is a + stowaway as before, and a process outside every scope is never attributed to a + scopeless lease. Without a user manager, and on Windows, the run starts as before + and the window stands. + ## [v0.3.3] - 2026-10-21 Fixes from the Windows testers and two gateway additions. On a single-GPU Windows desktop, the diff --git a/README.md b/README.md index c4aab5b..56aadc0 100644 --- a/README.md +++ b/README.md @@ -420,9 +420,12 @@ startup, so a change to the roots applies on the next restart. ones, SIGTERM first and SIGKILL to the survivors after `--grace`, re-checking each PID and start time immediately before every signal. - **Process identity is PID plus start time**, so a reused PID never inherits a lease. -- **Children stay on the lease.** Berth records every descendant of an engine's wrapper each - tick, so a child that escapes its process group and reparents keeps the lease stuck under its - own name instead of turning into a stowaway. +- **Children stay on the lease.** On Linux with a systemd user manager, `berth run` + gives the engine its own cgroup scope, which every child inherits on fork: a child + that escapes its process group and reparents keeps the lease stuck under its own + name instead of turning into a stowaway. Without a user manager, and on Windows, + Berth records every descendant of the wrapper each tick, which covers everything + except a fork whose parent exits inside one tick. - **Whole-card or sized leases.** `--needs-mib N` lets engines that fit share a card. - **Fail-open, never silent.** If the daemon is down, `berth run` still starts the engine but records it as an *unprotected run* that status shows (`--fail-closed` refuses). @@ -1162,8 +1165,6 @@ engine's files during a drain (`prefetch`) and serves measured switch costs at ` In the next v0.3.x releases. Each item has its building blocks merged or a spec written: -- Cgroup attribution, which closes the one known stowaway window (chaos scenario 3: a child - whose parent exits within one 2 s tick). - KV snapshots wired into the drain and start paths, so a sequence of agents skips the prefill after each switch. - Measured switch costs on the live daemon. diff --git a/contrib/chaos/README.md b/contrib/chaos/README.md index da2cd15..b04b5e8 100644 --- a/contrib/chaos/README.md +++ b/contrib/chaos/README.md @@ -45,7 +45,7 @@ the log live; the expectations below quote its event kinds. |---|---|---|---| | 1 | Clean drain | `berth run --as a -- python hog.py 8 polite`, then, once a is in, `berth run --as b --priority foreground -- python hog.py 8 polite` | a is asked to drain, gets SIGTERM and exits; b is granted only after the driver shows a's 8 GiB gone. The event log gets `lease.draining`, `lease.released`, `lease.freed`, `lease.granted` in that order. | | 2 | Ignores SIGTERM | as scenario 1 with `ignore-term` on the first run | SIGTERM goes to a's own process group, nothing happens for the 30 s grace, then SIGKILL goes to that same group. No other process is signalled. After a dies, the handoff is as in scenario 1. | -| 3 | Escaped child | as scenario 1 with `escape-child` on the first run; the parent exits immediately, so start b at any time (`berth run --as b --priority foreground -- python hog.py 8 polite`) | Every tick Berth records each descendant of a's wrapper (PID plus start time), so a child that escapes its group and reparents stays a's: once a exits, the lease turns `stuck` after the 30 s release grace and the log names the child's PID and MiB; b waits and is told why. `berth reclaim` lists it, and `berth reclaim --yes` stops exactly that child. Nothing stops it on its own. One window remains: a child that forks and whose parent exits inside one tick (2 s) before any record shows as a stowaway, and a whole-card b is granted `contested` with a warning. | +| 3 | Escaped child | as scenario 1 with `escape-child` on the first run; the parent exits immediately, so start b at any time (`berth run --as b --priority foreground -- python hog.py 8 polite`) | On Linux with a systemd user manager `berth run` execs the engine in its own transient cgroup scope, which every child inherits on fork: a child that leaves the group and reparents stays a's, once a exits the lease turns `stuck` after the 30 s release grace and the log names the child's PID and MiB, and b waits and is told why, never granted `contested` over a stuck lease. `berth reclaim` lists it, and `berth reclaim --yes` stops exactly that child. Nothing stops it on its own. Without a user manager, and on Windows, the run has no scope of its own and only the per-tick descendant records stand, where the old window remains: a child that forks and whose parent exits inside one tick (2 s) before any record shows as a stowaway, and a whole-card b is granted `contested` with a warning. | | 4 | Stowaway | `python hog.py 8 polite` on its own (no `berth run`), then `berth run --as b --needs-mib 20000 -- python hog.py 4 polite` on a 24 GB card | The hog shows up as a stowaway with 8192 MiB in status, the events log and the harbor. b waits with `squatters hold 8192 MiB`; it is never granted while the hog holds memory, and the hog is never signalled. | | 5 | Unrelated workload survives | run scenario 2 while `glxgears` (or a game) is running | The kill ladder reaches only a's process group: the unrelated window keeps drawing through every step, and the handoff finishes around it. | | 6 | Busy forever | `berth run --as a --drain-slots http://127.0.0.1:18080 --drain-max 10m -- python hog.py 8 busy-slots`, then `berth run --as b --priority foreground -- sleep 30` | The drain waits on a's `/slots`, which is busy forever. Without `drain_max` b waits without limit; the wrapper prints `draining a: 1 slot busy for 6m12s` each minute, so the journal says why. With `drain_max` (the flag, or `drain_max` in a's berth.toml entry) the wait ends after that long: the wrapper says `a still busy after 10m0s (1 slot processing); draining anyway`, and the drain runs the normal ladder, so b gets the card. | diff --git a/internal/broker/broker.go b/internal/broker/broker.go index c7eaa69..e2c946b 100644 --- a/internal/broker/broker.go +++ b/internal/broker/broker.go @@ -154,6 +154,11 @@ type Config struct { // on Linux); "" = a host process. Tests fake it. A GPU process whose // container matches an attached lease's belongs to that lease. Containers func(pid int) (string, bool) + // Scopes reads a process's Berth scope (procs.Scope on Linux): the + // path of the berth-*.scope cgroup it runs in, "" for a process + // outside one. Tests fake it. A GPU process in the scope an attached + // lease recorded belongs to that lease. + Scopes func(pid int) (string, bool) // ContainerName resolves a container id to its runtime name (docker, // then podman, inspect); "" when neither answers. Used to name a // stowaway's container in the report; never affects scheduling. @@ -214,13 +219,18 @@ type lease struct { // ("" for a host process), containerName the name the operator gave. container string containerName string - gpu map[ident]gpu.Proc - desc map[ident]struct{} // descendants recorded while the lease ran - contested uint64 // squatter bytes on the card when granted - peak uint64 // most memory its processes were measured holding - arrived time.Time - granted time.Time - released time.Time + // scope is the berth-*.scope path the attached engine runs in + // ("" when it runs outside one). A cgroup is inherited on fork, so + // every process under that path is the lease's, however its process + // tree is shaped. + scope string + gpu map[ident]gpu.Proc + desc map[ident]struct{} // descendants recorded while the lease ran + contested uint64 // squatter bytes on the card when granted + peak uint64 // most memory its processes were measured holding + arrived time.Time + granted time.Time + released time.Time // releasedRead and parkedRead are the card reads started when the lease // was released or its park call was done: only a tick whose read is // later frees it or marks it parked. Clock-free, so a coarse clock @@ -516,6 +526,9 @@ func New(cfg Config, probe gpu.Probe, tree procs.Tree) *Broker { if cfg.Containers == nil { cfg.Containers = procs.Container } + if cfg.Scopes == nil { + cfg.Scopes = procs.Scope + } if cfg.ContainerName == nil { cfg.ContainerName = containerNameCLI } @@ -623,13 +636,15 @@ func (b *Broker) Tick(ctx context.Context) { b.probeErr = nil b.snap = snap b.recordDescendants() + b.refreshScopes() present := map[ident]bool{} claimed := map[ident]claimedProc{} for _, p := range snap.Procs { id := b.identify(p.PID) present[id] = true - if l := b.owner(id, b.containerOf(p.PID)); l != nil { + scope := b.scopeOf(p.PID) + if l := b.owner(id, b.containerOf(p.PID), scope); l != nil { if l.state != active && l.state != draining && b.runFor(p.PID) != nil { // A released lease keeps blocking while its processes hold // memory, unless a run record says they were left behind on @@ -638,6 +653,13 @@ func (b *Broker) Tick(ctx context.Context) { delete(l.gpu, id) } else { l.gpu[id] = p + if scope != "" && l.scope == scope { + // In the lease's scope but never recorded: a child that + // escaped the tree before any tick. Record it now, so + // reclaim and the stuck report name it exactly like a + // recorded descendant. + l.desc[id] = struct{}{} + } l.peak = max(l.peak, l.used()) if s, ok := b.squatters[id]; ok { b.reportGone(s) @@ -773,10 +795,11 @@ func (b *Broker) reportGone(s *squatter) { // owner finds the lease a GPU process belongs to: one it was already // attributed to, else a recorded descendant of an attached lease, else a -// container the lease names, else the lease whose wrapped engine it -// descends from. The engine's PID only counts while its start time still -// matches, so a reused PID never inherits a lease. -func (b *Broker) owner(id ident, container string) *lease { +// scope the lease recorded, else a container the lease names, else the +// lease whose wrapped engine it descends from. The engine's PID only +// counts while its start time still matches, so a reused PID never +// inherits a lease. An empty scope never matches an empty one. +func (b *Broker) owner(id ident, container, scope string) *lease { for _, l := range b.leases { if _, ok := l.gpu[id]; ok { return l @@ -787,6 +810,9 @@ func (b *Broker) owner(id ident, container string) *lease { return l } } + if l := b.scopeLease(scope); l != nil { + return l + } if container != "" { for _, l := range b.leases { if (l.state == active || l.state == draining || l.state == releasing) && l.container == container { @@ -1250,10 +1276,14 @@ func (l *Lease) Contested() uint64 { } // Attach records the engine holding the berth, by PID plus start time. +// Its Berth scope is read on the next tick, not here: systemd-run moves +// itself into the scope only after a round trip to the user manager, so +// at attach the engine still sits in its wrapper's cgroup. func (l *Lease) Attach(pid int) { start := l.b.tree.StartTime(pid) l.b.mu.Lock() l.l.pid, l.l.pidStart = pid, start + l.l.scope = "" l.b.mu.Unlock() } @@ -1334,6 +1364,7 @@ func (b *Broker) Reattach(r Request, pid int, start uint64) (*Lease, error) { if pid <= 0 || start == 0 || b.tree.StartTime(pid) != start { return nil, fmt.Errorf("%w: pid %d is not the process that started as %s", ErrRefused, pid, r.Engine) } + scope, _ := b.cfg.Scopes(pid) b.mu.Lock() defer b.mu.Unlock() for _, l := range b.leases { @@ -1350,6 +1381,7 @@ func (b *Broker) Reattach(r Request, pid int, start uint64) (*Lease, error) { } l.state, l.reattach = active, true l.pid, l.pidStart = pid, start + l.scope = scope l.granted = now l.grantedN = b.order() close(l.grantCh) diff --git a/internal/broker/chaos_test.go b/internal/broker/chaos_test.go index 53135a3..a60ce59 100644 --- a/internal/broker/chaos_test.go +++ b/internal/broker/chaos_test.go @@ -177,3 +177,122 @@ func TestChaosScenario4Stowaway(t *testing.T) { t.Fatalf("reports = %q, want a single appearance", got) } } + +// Scenario 3, escaped child, on a machine with a systemd user manager: +// `berth run` execs the engine into a berth-run scope, the engine's +// child forks a grandchild that takes the memory and exits itself +// before any tick records a descendant. A cgroup is inherited on fork, +// so the grandchild is still in the scope the lease recorded: it +// belongs to the lease from its first sighting, the lease turns stuck +// after the release grace naming the grandchild, reclaim lists it +// under the lease, and a whole-card requester waits, never granted +// contested. +func TestChaosScenario3EscapedChildInScope(t *testing.T) { + tree := procs.Fake{Parents: map[int]int{100: 1, 102: 1}, Starts: map[int]uint64{100: 1, 102: 5}} + b, c, r := setup(t, tree) + b.cfg.ReleaseGrace = time.Millisecond + table := []procs.Proc{ + {PID: 100, Parent: 1, Start: 1, Name: "berth"}, + {PID: 102, Parent: 1, Start: 5, Name: "python"}, + } + b.cfg.ProcTable = func() []procs.Proc { return table } + b.cfg.Scopes = func(pid int) (string, bool) { + if pid == 100 || pid == 102 { + return runScopeA, true + } + return "", false + } + + a := must(t, acquire(b, Request{Engine: "a"})) + a.Attach(100) + _, live, stop := b.Events(0, true) + defer stop() + + // The intermediate child is gone before the first tick; the + // grandchild takes the card with the process tree already escaped. + c.set(gpu.Proc{PID: 102, Name: "python", Used: 8 * GiB}) + b.Tick(context.Background()) + + st := b.Status() + if len(st.Leases) != 1 || st.Leases[0].Used != 8*GiB { + t.Fatalf("leases = %+v, want the scope's memory under a", st.Leases) + } + if len(st.Squatters) != 0 || strings.Join(r.seen, ",") != "" { + t.Fatalf("squatters = %+v, reports = %v, want no stowaway", st.Squatters, r.seen) + } + + // a lets the lease go; the grandchild keeps holding, so the lease + // turns stuck after the grace and names it in the event. + a.Release() + b.Tick(context.Background()) + time.Sleep(5 * time.Millisecond) + b.Tick(context.Background()) + + st = b.Status() + if len(st.Leases) != 1 || st.Leases[0].State != "stuck" || st.Leases[0].Used != 8*GiB { + t.Fatalf("leases = %+v, want a stuck lease holding the escaped grandchild", st.Leases) + } + deadline := time.Now().Add(time.Second) + for { + select { + case e := <-live: + if e.Kind == proto.EvStuck { + if !strings.Contains(e.Detail, "pid 102 (python)") { + t.Fatalf("stuck event %q does not name the grandchild", e.Detail) + } + goto listed + } + case <-time.After(time.Until(deadline)): + t.Fatal("no stuck event arrived") + } + } +listed: + hs, err := b.Reclaim(a.ID()) + if err != nil { + t.Fatalf("reclaim: %v", err) + } + if len(hs) != 1 || hs[0].PID != 102 || hs[0].Lease != a.ID() || hs[0].Name != "python" { + t.Fatalf("holders = %+v, want the grandchild under a's lease", hs) + } + + // A whole-card requester waits: the stuck lease holds memory, so + // nothing is granted, contested or otherwise. + breq := acquire(b, Request{Engine: "b", Priority: Foreground}) + pending(t, breq) + if st := b.Status(); len(st.Leases) != 1 { + t.Fatalf("leases = %+v, want only the stuck lease", st.Leases) + } +} + +// Scenario 3 without a user manager (no scope to record): the same +// escaped grandchild is a stowaway as today, and once a's wrapper lets +// the lease go a whole-card requester is granted contested. Nothing +// matches an empty scope, so a scopeless process is never silently +// attributed to the lease that also has no scope. +func TestChaosScenario3EscapedChildWithoutScope(t *testing.T) { + tree := procs.Fake{Parents: map[int]int{100: 1, 102: 1}, Starts: map[int]uint64{100: 1, 102: 5}} + b, c, r := setup(t, tree) + b.cfg.ProcTable = func() []procs.Proc { return []procs.Proc{{PID: 100, Parent: 1, Start: 1, Name: "berth"}} } + b.cfg.Scopes = func(int) (string, bool) { return "", false } + + a := must(t, acquire(b, Request{Engine: "a"})) + a.Attach(100) + c.set(gpu.Proc{PID: 102, Name: "python", Used: 8 * GiB}) + b.Tick(context.Background()) + + st := b.Status() + if len(st.Squatters) != 1 || st.Squatters[0].PID != 102 { + t.Fatalf("squatters = %+v, want the escaped grandchild listed", st.Squatters) + } + if got := strings.Join(r.seen, ","); got != "appeared:python" { + t.Fatalf("reports = %q, want a single appearance", got) + } + + a.Release() + b.Tick(context.Background()) + + whole := must(t, acquire(b, Request{Engine: "b", Priority: Foreground})) + if whole.Contested() != 8*GiB { + t.Fatalf("contested = %d, want the stowaway counted", whole.Contested()) + } +} diff --git a/internal/broker/reclaim.go b/internal/broker/reclaim.go index 9f5bffc..4bdac7f 100644 --- a/internal/broker/reclaim.go +++ b/internal/broker/reclaim.go @@ -52,7 +52,7 @@ func (b *Broker) Reclaim(id string) ([]proto.Holder, error) { var hs []proto.Holder for _, p := range b.snap.Procs { pid := b.identify(p.PID) - l := b.owner(pid, b.containerOf(p.PID)) + l := b.owner(pid, b.containerOf(p.PID), b.scopeOf(p.PID)) switch { case l != nil && want[l.id]: hs = append(hs, proto.Holder{PID: p.PID, PIDStart: pid.start, Lease: l.id, Name: p.Name, Used: p.Used}) diff --git a/internal/broker/scope.go b/internal/broker/scope.go new file mode 100644 index 0000000..917bf44 --- /dev/null +++ b/internal/broker/scope.go @@ -0,0 +1,57 @@ +// Copyright 2026 Trevin Tindall (T-Crypt) +// SPDX-License-Identifier: Apache-2.0 + +package broker + +// Cgroup attribution. `berth run` starts its engine in a berth-run +// scope and the gateway starts its runs in berth-engine scopes; a +// cgroup is inherited on fork, so every process under a lease's +// recorded scope is the lease's, even a child that left the process +// tree before any tick could record it (chaos scenario 3). Nothing is +// signalled because of a scope that Berth did not start: the scope +// holds exactly the processes a Berth wrapper execed into it. + +// scopeLease is the one live lease that recorded scope, nil when none +// did or when more than one did: a scope two leases share names neither, +// so attribution falls through to the process tree. +func (b *Broker) scopeLease(scope string) *lease { + if scope == "" { + return nil + } + var found *lease + for _, l := range b.leases { + if (l.state == active || l.state == draining || l.state == releasing) && l.scope == scope { + if found != nil { + return nil + } + found = l + } + } + return found +} + +// refreshScopes reads each attached engine's scope while its PID plus +// start time is still the engine: the engine enters its scope a moment +// after it starts, so the read at attach would name the wrapper's +// cgroup. Once the engine is gone the last scope read stays, which is +// what names a child it left behind. Called with b.mu held. +func (b *Broker) refreshScopes() { + for _, l := range b.leases { + if (l.state != active && l.state != draining && l.state != releasing) || l.pid <= 0 || + b.tree.StartTime(l.pid) != l.pidStart { + continue + } + if s, ok := b.cfg.Scopes(l.pid); ok { + l.scope = s + } + } +} + +// scopeOf is a process's Berth scope path, "" when it runs outside one. +func (b *Broker) scopeOf(pid int) string { + s, ok := b.cfg.Scopes(pid) + if !ok { + return "" + } + return s +} diff --git a/internal/broker/scope_test.go b/internal/broker/scope_test.go new file mode 100644 index 0000000..eeced09 --- /dev/null +++ b/internal/broker/scope_test.go @@ -0,0 +1,124 @@ +// Copyright 2026 Trevin Tindall (T-Crypt) +// SPDX-License-Identifier: Apache-2.0 + +package broker + +// Scope attribution beyond the scenario: a process joins exactly the +// lease that recorded its scope, never a neighbour's, and a process +// outside every recorded scope stays a stranger. + +import ( + "context" + "strings" + "testing" + + "github.com/T-Crypt/berth/internal/gpu" + "github.com/T-Crypt/berth/internal/procs" +) + +const ( + runScopeA = "/user.slice/user-1000.slice/user@1000.service/app.slice/berth-run-a-1a2b3c4d.scope" + runScopeB = "/user.slice/user-1000.slice/user@1000.service/app.slice/berth-run-b-9f8e7d6c.scope" +) + +func TestScopeNeverCrossAttributes(t *testing.T) { + tree := procs.Fake{ + Parents: map[int]int{100: 1, 110: 1, 112: 1, 113: 1}, + Starts: map[int]uint64{100: 1, 110: 2, 112: 5, 113: 6}, + } + b, c, r := setup(t, tree) + b.cfg.Scopes = func(pid int) (string, bool) { + switch pid { + case 100: + return runScopeA, true + case 110, 112: + return runScopeB, true + } + return "", false + } + + a := must(t, acquire(b, Request{Engine: "a", Needs: 4 * GiB})) + a.Attach(100) + mb := must(t, acquire(b, Request{Engine: "mine", Needs: 4 * GiB})) + mb.Attach(110) + + // 112 escaped inside b's scope; 113 runs in no scope at all. + c.set( + gpu.Proc{PID: 112, Name: "python", Used: 8 * GiB}, + gpu.Proc{PID: 113, Name: "hog", Used: 1 * GiB}, + ) + b.Tick(context.Background()) + + st := b.Status() + var byEngine = map[string]uint64{} + for _, l := range st.Leases { + byEngine[l.Engine] = l.Used + } + if len(st.Leases) != 2 || byEngine["a"] != 0 || byEngine["mine"] != 8*GiB { + t.Fatalf("leases = %+v, want 112 only under its own scope's lease", st.Leases) + } + if len(st.Squatters) != 1 || st.Squatters[0].PID != 113 { + t.Fatalf("squatters = %+v, want only the scopeless process", st.Squatters) + } + if got := strings.Join(r.seen, ","); got != "appeared:hog" { + t.Fatalf("reports = %q, want only the scopeless process", got) + } +} + +func TestSharedScopeNamesNeither(t *testing.T) { + tree := procs.Fake{ + Parents: map[int]int{100: 1, 110: 1, 112: 1}, + Starts: map[int]uint64{100: 1, 110: 2, 112: 5}, + } + b, c, _ := setup(t, tree) + b.cfg.Scopes = func(pid int) (string, bool) { return runScopeA, true } + + a := must(t, acquire(b, Request{Engine: "a", Needs: 4 * GiB})) + a.Attach(100) + mb := must(t, acquire(b, Request{Engine: "mine", Needs: 4 * GiB})) + mb.Attach(110) + + // 112 sits in the scope both leases recorded and descends from neither. + c.set(gpu.Proc{PID: 112, Name: "python", Used: 8 * GiB}) + b.Tick(context.Background()) + + st := b.Status() + for _, l := range st.Leases { + if l.Used != 0 { + t.Fatalf("lease %s holds %d, want a shared scope to name neither lease", l.Engine, l.Used) + } + } + if len(st.Squatters) != 1 || st.Squatters[0].PID != 112 { + t.Fatalf("squatters = %+v, want 112 reported as a stowaway", st.Squatters) + } +} + +// systemd-run moves the engine into its scope only after a round trip to +// the user manager, so at attach the engine still reads as outside any +// scope. The lease must still learn the scope once the engine is in it. +func TestScopeReadAfterAttach(t *testing.T) { + tree := procs.Fake{Parents: map[int]int{100: 1, 102: 1}, Starts: map[int]uint64{100: 1, 102: 5}} + b, c, r := setup(t, tree) + moved := false + b.cfg.Scopes = func(pid int) (string, bool) { + if (pid == 100 && moved) || pid == 102 { + return runScopeA, true + } + return "", false + } + + a := must(t, acquire(b, Request{Engine: "a"})) + a.Attach(100) + moved = true + + c.set(gpu.Proc{PID: 102, Name: "python", Used: 8 * GiB}) + b.Tick(context.Background()) + + st := b.Status() + if len(st.Leases) != 1 || st.Leases[0].Used != 8*GiB { + t.Fatalf("leases = %+v, want the scope's memory under a", st.Leases) + } + if len(st.Squatters) != 0 || strings.Join(r.seen, ",") != "" { + t.Fatalf("squatters = %+v, reports = %v, want no stowaway", st.Squatters, r.seen) + } +} diff --git a/internal/cli/run.go b/internal/cli/run.go index fb21e87..ffa1321 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -89,7 +89,7 @@ func runRun(c *Context) int { // diagnostics on stderr, and --quiet never hides the unprotected warning. return runner.Run(runner.OptionsFor(&e, runner.Over{ Socket: *sock, Group: *group, Pinned: *pinned, Wait: *wait, Grace: *grace, - FailClosed: *failClosed, Argv: c.Args, Stderr: c.Err, + FailClosed: *failClosed, Argv: c.Args, Stderr: c.Err, Log: c.Log(), })) } diff --git a/internal/gateway/spawn.go b/internal/gateway/spawn.go index 2bad771..01b1cfd 100644 --- a/internal/gateway/spawn.go +++ b/internal/gateway/spawn.go @@ -20,6 +20,7 @@ import ( "github.com/T-Crypt/berth/internal/procs" "github.com/T-Crypt/berth/internal/runner" + "github.com/T-Crypt/berth/internal/sysd" ) // spawnRun starts the run process for one engine, records it, and waits @@ -46,7 +47,7 @@ func (g *Gateway) spawnRun(name string, s *started, o runner.Options) (int, erro } full = append(full, "--") full = append(full, o.Argv...) - full = scopeArgv(name, full) + full = sysd.ScopeArgv("berth-engine", name, full) cmd := exec.Command(full[0], full[1:]...) detach(cmd) cmd.Stdout, cmd.Stderr = logFile, logFile @@ -68,28 +69,6 @@ func (g *Gateway) spawnRun(name string, s *started, o runner.Options) (int, erro return code, nil } -// unitName maps an engine name onto what a systemd unit name may carry -// (ASCII letters, digits, and . - _ :), replacing every other byte with -// _ and capping the length so a long name still leaves room for the -// suffix. Without this, a name like "qwen3.8/27b" makes systemd-run fail -// and the start fail for the oddest reason. -func unitName(name string) string { - const max = 64 - b := []byte(name) - if len(b) > max { - b = b[:max] - } - for i, c := range b { - switch { - case c >= 'A' && c <= 'Z', c >= 'a' && c <= 'z', c >= '0' && c <= '9', - c == '.', c == '_', c == '-', c == ':': - default: - b[i] = '_' - } - } - return string(b) -} - // runArgv renders the options as `berth run`'s own flags; the engine's // command follows the caller's "--". func runArgv(o runner.Options) []string { diff --git a/internal/gateway/spawn_test.go b/internal/gateway/spawn_test.go deleted file mode 100644 index 29f5c76..0000000 --- a/internal/gateway/spawn_test.go +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2026 Trevin Tindall (T-Crypt) -// SPDX-License-Identifier: Apache-2.0 - -package gateway - -import "testing" - -// unitName maps an engine name onto what a systemd unit name may carry, -// so a name like "qwen3.8/27b" cannot make systemd-run fail. -func TestUnitName(t *testing.T) { - for _, c := range []struct { - name, want string - }{ - {"strata", "strata"}, - {"qwen3.8/27b", "qwen3.8_27b"}, - {"ninfer serve", "ninfer_serve"}, - {"safe.name_:weird", "safe.name_:weird"}, - {"сyrillic", "__yrillic"}, - } { - if got := unitName(c.name); got != c.want { - t.Errorf("unitName(%q) = %q, want %q", c.name, got, c.want) - } - } - long := "a-really-long-engine-name-over-sixty-four-bytes-continues-with-more-words" - if got := unitName(long); got != long[:64] { - t.Errorf("unitName caps the name at 64 bytes: %q", got) - } - if got := unitName("x"); len(got) > 64 { - t.Errorf("unitName length = %d, want at most 64", len(got)) - } -} diff --git a/internal/gateway/spawn_unix.go b/internal/gateway/spawn_unix.go index f546201..75dc179 100644 --- a/internal/gateway/spawn_unix.go +++ b/internal/gateway/spawn_unix.go @@ -6,48 +6,14 @@ package gateway import ( - "crypto/rand" - "fmt" - "os" "os/exec" - "path/filepath" "syscall" - "testing" ) // detach moves the run process to its own session, so a Ctrl-C meant for // the gateway's process group does not reach it. A service stop still -// can: systemd kills by cgroup, which only the scope below leaves. +// can: systemd kills by cgroup, which only the scope the run leaves +// (sysd.ScopeArgv) survives. func detach(cmd *exec.Cmd) { cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} } - -// scopeArgv runs the command through the user manager's scope when one is -// reachable, so the engine leaves the gateway's cgroup and survives a -// gateway restart, stop or upgrade: --collect drops the scope when the -// run ends, --quiet keeps systemd-run's own notice out of the output. -func scopeArgv(name string, argv []string) []string { - // Tests start fake engines: never in the developer's own user manager. - if testing.Testing() || !userScopeReady() { - return argv - } - var rnd [4]byte - _, _ = rand.Read(rnd[:]) - unit := fmt.Sprintf("berth-engine-%s-%x", unitName(name), rnd) - return append([]string{"systemd-run", "--user", "--scope", "--unit", unit, "--collect", "--quiet", "--"}, argv...) -} - -// userScopeReady reports whether a systemd user manager can take a scope: -// systemd-run on PATH, and the manager's private socket in the runtime -// directory. -func userScopeReady() bool { - if _, err := exec.LookPath("systemd-run"); err != nil { - return false - } - dir := os.Getenv("XDG_RUNTIME_DIR") - if dir == "" { - return false - } - _, err := os.Stat(filepath.Join(dir, "systemd", "private")) - return err == nil -} diff --git a/internal/gateway/spawn_windows.go b/internal/gateway/spawn_windows.go index c87c52d..80dcfb3 100644 --- a/internal/gateway/spawn_windows.go +++ b/internal/gateway/spawn_windows.go @@ -11,6 +11,3 @@ import "os/exec" // job object tying it to its own engine already limits what a gateway // restart leaves behind. func detach(*exec.Cmd) {} - -// scopeArgv has no user manager to answer to on this platform. -func scopeArgv(name string, argv []string) []string { return argv } diff --git a/internal/procs/procs.go b/internal/procs/procs.go index 87ef0e3..744fbf1 100644 --- a/internal/procs/procs.go +++ b/internal/procs/procs.go @@ -118,3 +118,41 @@ func hex64(s string) bool { } return true } + +// scopeID parses a cgroups file and returns the path of the nearest +// ancestor cgroup named berth-*.scope, the scope up to and including +// that segment, "" when there is none. Only the cgroup v2 unified line +// ("0::/path") counts: Berth creates scopes on v2 and reads them on v2. +func scopeID(r io.Reader) (string, bool) { + sc := bufio.NewScanner(r) + sc.Buffer(make([]byte, 0, 4096), 64*1024) + for sc.Scan() { + line := sc.Text() + if !strings.HasPrefix(line, "0::") { + continue + } + if p, ok := scopePath(strings.TrimPrefix(line, "0::")); ok { + return p, true + } + } + return "", false +} + +// scopePath reads one v2 cgroup path: the prefix up to and including +// the deepest berth-*.scope segment, so every process nested further +// under the same scope answers with the same path. +func scopePath(path string) (string, bool) { + segs := strings.Split(strings.TrimSpace(path), "/") + for i := len(segs) - 1; i >= 0; i-- { + if berthScope(segs[i]) { + return strings.Join(segs[:i+1], "/"), true + } + } + return "", false +} + +// berthScope is a Berth scope unit name: berth-, something, .scope. +func berthScope(seg string) bool { + return strings.HasPrefix(seg, "berth-") && strings.HasSuffix(seg, ".scope") && + len(seg) > len("berth-.scope") +} diff --git a/internal/procs/procs_linux.go b/internal/procs/procs_linux.go index 06a49e4..b478cb6 100644 --- a/internal/procs/procs_linux.go +++ b/internal/procs/procs_linux.go @@ -69,6 +69,18 @@ func Container(pid int) (string, bool) { return containerID(bytes.NewReader(b)) } +// Scope is the Berth scope that owns pid, from an actual +// /proc//cgroup: the path of the nearest berth-*.scope ancestor, +// "" when the process runs outside one (a plain terminal app, a cgroup +// v1 machine, any process Berth did not start). +func Scope(pid int) (string, bool) { + b, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/cgroup") + if err != nil { + return "", false + } + return scopeID(bytes.NewReader(b)) +} + // statFields reads /proc//stat once: the command name inside the // parenthesis, and the fields after it, so index 0 is field 3 (state). The // name is "(comm)" and may contain spaces or parens, so the split starts diff --git a/internal/procs/procs_other.go b/internal/procs/procs_other.go index 584e9ad..ce23301 100644 --- a/internal/procs/procs_other.go +++ b/internal/procs/procs_other.go @@ -19,3 +19,7 @@ func List() []Proc { return nil } // Container has no cgroups on this platform, so no process is ever in a // container: engines run on the host. func Container(int) (string, bool) { return "", false } + +// Scope has no cgroups on this platform, so no process runs in a Berth +// scope. +func Scope(int) (string, bool) { return "", false } diff --git a/internal/procs/procs_windows.go b/internal/procs/procs_windows.go index acfa71c..8a86bf7 100644 --- a/internal/procs/procs_windows.go +++ b/internal/procs/procs_windows.go @@ -93,3 +93,7 @@ func List() []Proc { return nil } // Container has no cgroups on Windows, so no process is ever in a container: // engines run on the host. func Container(int) (string, bool) { return "", false } + +// Scope has no cgroups on Windows, so no process runs in a Berth scope +// (Job objects come in a later ticket). +func Scope(int) (string, bool) { return "", false } diff --git a/internal/procs/scope_test.go b/internal/procs/scope_test.go new file mode 100644 index 0000000..f1e4ba3 --- /dev/null +++ b/internal/procs/scope_test.go @@ -0,0 +1,153 @@ +// Copyright 2026 Trevin Tindall (T-Crypt) +// SPDX-License-Identifier: Apache-2.0 + +package procs + +// The cgroup shapes a Berth scope reading recognises: the scopes the +// gateway and `berth run` create, nested work under them, and what a +// process outside any Berth scope reads. + +import ( + "strings" + "testing" +) + +func TestScopePath(t *testing.T) { + tests := []struct { + name string + path string + want string + ok bool + }{ + { + name: "gateway engine scope", + path: "/user.slice/user-1000.slice/user@1000.service/app.slice/berth-engine-strata-1a2b3c4d.scope", + want: "/user.slice/user-1000.slice/user@1000.service/app.slice/berth-engine-strata-1a2b3c4d.scope", + ok: true, + }, + { + name: "berth run scope", + path: "/user.slice/user-1000.slice/user@1000.service/app.slice/berth-run-ninfer-aabbccdd.scope", + want: "/user.slice/user-1000.slice/user@1000.service/app.slice/berth-run-ninfer-aabbccdd.scope", + ok: true, + }, + { + name: "nested work under the scope", + path: "/user.slice/user-1000.slice/user@1000.service/app.slice/berth-run-strata-0f0e0d0c.scope/work/1", + want: "/user.slice/user-1000.slice/user@1000.service/app.slice/berth-run-strata-0f0e0d0c.scope", + ok: true, + }, + { + name: "nearest ancestor wins", + path: "/user.slice/user-1000.slice/user@1000.service/app.slice/berth-engine-a-1.scope/berth-run-b-2.scope", + want: "/user.slice/user-1000.slice/user@1000.service/app.slice/berth-engine-a-1.scope/berth-run-b-2.scope", + ok: true, + }, + { + name: "terminal session scope", + path: "/user.slice/user-1000.slice/session-3.scope", + want: "", + ok: false, + }, + { + name: "root", + path: "/", + want: "", + ok: false, + }, + { + name: "berth prefix without the scope suffix", + path: "/user.slice/berth-run-x.service", + want: "", + ok: false, + }, + { + name: "scope suffix without the berth prefix", + path: "/user.slice/app-run-x.scope", + want: "", + ok: false, + }, + { + name: "berth with nothing between", + path: "/user.slice/berth-.scope", + want: "", + ok: false, + }, + { + name: "a longer word is not a scope", + path: "/user.slice/berth-x.scope.evil", + want: "", + ok: false, + }, + { + name: "empty", + path: "", + want: "", + ok: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := scopePath(tt.path) + if got != tt.want || ok != tt.ok { + t.Fatalf("scopePath(%q) = %q, %v, want %q, %v", tt.path, got, ok, tt.want, tt.ok) + } + }) + } +} + +const runScope = "/user.slice/user-1000.slice/user@1000.service/app.slice/berth-run-a-1a2b3c4d.scope" + +func TestScopeID(t *testing.T) { + tests := []struct { + name string + body string + want string + ok bool + }{ + { + name: "unified v2 line", + body: "0::" + runScope + "\n", + want: runScope, + ok: true, + }, + { + name: "nested under the scope", + body: "0::" + runScope + "/work/1\n", + want: runScope, + ok: true, + }, + { + name: "v1 controllers above the v2 line", + body: "12:blkio:/user.slice/user-1000.slice\n11:devices:/user.slice/user-1000.slice\n0::" + runScope + "\n", + want: runScope, + ok: true, + }, + { + name: "cgroup v1 only reads empty", + body: "12:blkio:/user.slice/user-1000.slice/user@1000.service/berth-run-a-1a2b3c4d.scope\n1:name=systemd:/user.slice/user-1000.slice\n", + want: "", + ok: false, + }, + { + name: "v2 present but no berth scope", + body: "0::/user.slice/user-1000.slice/user@1000.service/app.slice/app-org.gnome.Terminal.slice/vte-child.scope\n", + want: "", + ok: false, + }, + { + name: "empty", + body: "", + want: "", + ok: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := scopeID(strings.NewReader(tt.body)) + if got != tt.want || ok != tt.ok { + t.Fatalf("scopeID(%q) = %q, %v, want %q, %v", tt.body, got, ok, tt.want, tt.ok) + } + }) + } +} diff --git a/internal/runner/options.go b/internal/runner/options.go index e3822ec..9e43cc7 100644 --- a/internal/runner/options.go +++ b/internal/runner/options.go @@ -5,6 +5,7 @@ package runner import ( "io" + "log/slog" "time" "github.com/T-Crypt/berth/internal/config" @@ -24,6 +25,7 @@ type Over struct { Argv []string Stderr io.Writer Output io.Writer + Log *slog.Logger } // OptionsFor is the options one run of a configured engine takes: every @@ -55,7 +57,7 @@ func OptionsFor(e *config.Engine, o Over) Options { DrainMax: e.DrainMax.Duration, MinTurn: e.MinTurn.Duration, Container: e.Container, APIKeyEnv: e.APIKeyEnv, APIKeyFile: e.APIKeyFile, ExitComfyUIIdle: e.ExitComfyUIIdle, ExitComfyUIIdleAfter: e.ExitComfyUIIdleAfter.Duration, Grace: grace, FailClosed: o.FailClosed, - Argv: argv, Stderr: o.Stderr, Output: o.Output, + Argv: argv, Stderr: o.Stderr, Output: o.Output, Log: o.Log, } // park = "hook" hands the drain the engine's own offload calls; when // the entry carries no URLs either, the drain probes Berth's park diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 8d40a5b..879f597 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -13,6 +13,7 @@ import ( "errors" "fmt" "io" + "log/slog" "net" "net/http" "net/url" @@ -111,6 +112,10 @@ type Options struct { // (the terminal still gets them too). The gateway keeps its tail, so a // start that dies says why in the engine's words, not berth's. Output io.Writer + // Log is the run's debug log (nil = silent). It records the one + // product decision the run makes without asking: whether the engine + // child starts in its own systemd scope. + Log *slog.Logger } // ExitRefused is EX_TEMPFAIL: no berth was granted. @@ -173,7 +178,7 @@ func Run(o Options) int { } } - cmd := command(o.Argv, o.Output) + cmd := command(o, o.Output) session := comfySession{granted: time.Now()} if err := cmd.Start(); err != nil { say("start %s: %v", o.Argv[0], err) @@ -572,7 +577,7 @@ func slotsBusy(url, key string) (int, error) { } func runUnleased(o Options, reason string) int { - cmd := command(o.Argv, o.Output) + cmd := command(o, o.Output) if err := cmd.Start(); err != nil { fmt.Fprintf(o.Stderr, "berth: start %s: %v\n", o.Argv[0], err) return 127 @@ -603,7 +608,8 @@ func record(o Options, cmd *exec.Cmd, reason string) func() { return remove } -func command(argv []string, out io.Writer) *exec.Cmd { +func command(o Options, out io.Writer) *exec.Cmd { + argv := scopeToChild(o.Engine, o.Argv, o.Log) cmd := exec.Command(argv[0], argv[1:]...) cmd.Stdin, cmd.Stdout, cmd.Stderr = os.Stdin, os.Stdout, os.Stderr if out != nil { diff --git a/internal/runner/runner_unix.go b/internal/runner/runner_unix.go index bd33188..ba3aa20 100644 --- a/internal/runner/runner_unix.go +++ b/internal/runner/runner_unix.go @@ -7,9 +7,13 @@ package runner import ( "errors" + "log/slog" "os" "os/exec" "syscall" + "testing" + + "github.com/T-Crypt/berth/internal/sysd" ) var stopSignals = []os.Signal{syscall.SIGTERM, syscall.SIGINT, syscall.SIGHUP} @@ -23,6 +27,28 @@ func isolate(cmd *exec.Cmd) { // afterStart has nothing to do: the process group was set at start. func afterStart(*exec.Cmd) {} +// scopeToChild starts the engine in its own transient scope, so the +// lease can name every process under it by cgroup even when one escapes +// the process tree (chaos scenario 3: a child that forks and whose +// parent exits before any tick records it). The engine gets a scope of +// its own even when this run already sits in one (a gateway start, or a +// `berth run` inside another lease's engine, as llama-swap runs its +// models): two leases never share a scope. Without a user manager the +// child runs in this cgroup, as always. +func scopeToChild(as string, argv []string, log *slog.Logger) []string { + // Tests start fake engines: never in the developer's own user manager. + if testing.Testing() { + return argv + } + if !sysd.ScopeReady() { + if log != nil { + log.Debug("no systemd user manager; the engine runs in this run's cgroup", "engine", as) + } + return argv + } + return sysd.ScopeArgv("berth-run", as, argv) +} + func signalGroup(cmd *exec.Cmd, s os.Signal) { if sig, ok := s.(syscall.Signal); ok { _ = syscall.Kill(-cmd.Process.Pid, sig) diff --git a/internal/runner/runner_windows.go b/internal/runner/runner_windows.go index 87a1144..0cb413b 100644 --- a/internal/runner/runner_windows.go +++ b/internal/runner/runner_windows.go @@ -7,6 +7,7 @@ package runner import ( "errors" + "log/slog" "os" "os/exec" "sync" @@ -30,6 +31,10 @@ func isolate(cmd *exec.Cmd) { cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: windows.CREATE_NEW_PROCESS_GROUP} } +// scopeToChild has no user manager to answer to on this platform; Job +// objects and the attribution they allow come in a later ticket. +func scopeToChild(as string, argv []string, log *slog.Logger) []string { return argv } + // afterStart puts the engine in a job object. A child it spawns in the first // instant before this lands escapes the job; the drain still reaches the // engine itself, and its memory still shows as a stowaway, never hidden. diff --git a/internal/sysd/sysd.go b/internal/sysd/sysd.go new file mode 100644 index 0000000..bd50fec --- /dev/null +++ b/internal/sysd/sysd.go @@ -0,0 +1,31 @@ +// Copyright 2026 Trevin Tindall (T-Crypt) +// SPDX-License-Identifier: Apache-2.0 + +// Package sysd starts a command inside a transient scope of the systemd +// user manager, so it leaves the cgroup of whoever started it. Berth +// scopes every engine process it starts (the gateway's runs, and a +// `berth run` wrapper's own engine), and a scope's cgroup names those +// processes for attribution even when they escape the process tree. +package sysd + +// UnitName maps an engine name onto what a systemd unit name may carry +// (ASCII letters, digits, and . - _ :), replacing every other byte with +// _ and capping the length so a long name still leaves room for the +// suffix. Without this, a name like "qwen3.8/27b" makes systemd-run fail +// and the start fail for the oddest reason. +func UnitName(name string) string { + const max = 64 + b := []byte(name) + if len(b) > max { + b = b[:max] + } + for i, c := range b { + switch { + case c >= 'A' && c <= 'Z', c >= 'a' && c <= 'z', c >= '0' && c <= '9', + c == '.', c == '_', c == '-', c == ':': + default: + b[i] = '_' + } + } + return string(b) +} diff --git a/internal/sysd/sysd_test.go b/internal/sysd/sysd_test.go new file mode 100644 index 0000000..a045e19 --- /dev/null +++ b/internal/sysd/sysd_test.go @@ -0,0 +1,32 @@ +// Copyright 2026 Trevin Tindall (T-Crypt) +// SPDX-License-Identifier: Apache-2.0 + +package sysd + +// UnitName keeps a unit name one systemd-run will take, so an engine +// name like "qwen3.8/27b" cannot fail the start. + +import "testing" + +func TestUnitName(t *testing.T) { + for _, c := range []struct { + name, want string + }{ + {"strata", "strata"}, + {"qwen3.8/27b", "qwen3.8_27b"}, + {"ninfer serve", "ninfer_serve"}, + {"safe.name_:weird", "safe.name_:weird"}, + {"сyrillic", "__yrillic"}, + } { + if got := UnitName(c.name); got != c.want { + t.Errorf("UnitName(%q) = %q, want %q", c.name, got, c.want) + } + } + long := "a-really-long-engine-name-over-sixty-four-bytes-continues-with-more-words" + if got := UnitName(long); got != long[:64] { + t.Errorf("UnitName caps the name at 64 bytes: %q", got) + } + if got := UnitName("x"); len(got) > 64 { + t.Errorf("UnitName length = %d, want at most 64", len(got)) + } +} diff --git a/internal/sysd/sysd_unix.go b/internal/sysd/sysd_unix.go new file mode 100644 index 0000000..14bb24c --- /dev/null +++ b/internal/sysd/sysd_unix.go @@ -0,0 +1,47 @@ +// Copyright 2026 Trevin Tindall (T-Crypt) +// SPDX-License-Identifier: Apache-2.0 + +//go:build unix + +package sysd + +import ( + "crypto/rand" + "fmt" + "os" + "os/exec" + "path/filepath" + "testing" +) + +// ScopeReady reports whether a systemd user manager can take a scope: +// systemd-run on PATH, and the manager's private socket in the runtime +// directory. +func ScopeReady() bool { + if _, err := exec.LookPath("systemd-run"); err != nil { + return false + } + dir := os.Getenv("XDG_RUNTIME_DIR") + if dir == "" { + return false + } + _, err := os.Stat(filepath.Join(dir, "systemd", "private")) + return err == nil +} + +// ScopeArgv runs the command through the user manager's scope when one is +// reachable, so the command leaves the caller's cgroup and survives a +// restart or upgrade of whoever started it: the unit is named +// --, --collect drops the scope when the run ends, +// --quiet keeps systemd-run's own notice out of the output. With --scope +// systemd-run execs the command in place, so its PID is unchanged. +func ScopeArgv(prefix, name string, argv []string) []string { + // Tests start fake engines: never in the developer's own user manager. + if testing.Testing() || !ScopeReady() { + return argv + } + var rnd [4]byte + _, _ = rand.Read(rnd[:]) + unit := fmt.Sprintf("%s-%s-%x", prefix, UnitName(name), rnd) + return append([]string{"systemd-run", "--user", "--scope", "--unit", unit, "--collect", "--quiet", "--"}, argv...) +} diff --git a/internal/sysd/sysd_windows.go b/internal/sysd/sysd_windows.go new file mode 100644 index 0000000..a785141 --- /dev/null +++ b/internal/sysd/sysd_windows.go @@ -0,0 +1,12 @@ +// Copyright 2026 Trevin Tindall (T-Crypt) +// SPDX-License-Identifier: Apache-2.0 + +//go:build !unix + +package sysd + +// ScopeReady has no user manager to answer to on this platform. +func ScopeReady() bool { return false } + +// ScopeArgv has no user manager to answer to on this platform. +func ScopeArgv(prefix, name string, argv []string) []string { return argv }