961aed714 · carlos/platform

diff --git a/internal/activator/activator_test.go b/internal/activator/activator_test.go

index ca59699ee..154e307e6 100644

--- a/internal/activator/activator_test.go

+++ b/internal/activator/activator_test.go

@@ -5892,6 +5892,11 @@ func TestSeedCreatedTheDB(t *testing.T) {

// probe — serving HTTP on a unix socket needs a real program, and the test

// binary is the one program every `go test` run is guaranteed to have.

func TestMain(m *testing.M) {

+ // This binary places processes on a real cgroupfs in its kernel tests

+ // (TestGenerationStartRefusedBesideADirectParentMember,

+ // TestClaimContainmentCleanupRespawn), so it serves the placement shim

+ // like the carlos binary does (cgroup.RunShimIfRequested).

+ cgroup.RunShimIfRequested()

if os.Getenv("ACTIVATOR_FAKE_APP") == "1" {

fakeAppMain()

return

diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go

index 4a6df2a32..0578016a5 100644

--- a/internal/activator/backend_exec.go

+++ b/internal/activator/backend_exec.go

@@ -2129,15 +2129,16 @@ func (b *ExecBackend) spawnChildFrom(ctx context.Context, rt registry.Route, soc

// Spawned through the cgroup manager (nil-tolerant: no delegation means

// a plain start, exactly as before) so the child lands in

// carlos-exec@<host> and the sampler can account it. build may run

- // twice — the manager's clone3-refusal fallback — so everything about

- // the Cmd is configured inside it.

+ // twice — a flat start's PID-move fallback after a failed placement —

+ // so everything about the Cmd is configured inside it.

//

// The caller's exec gate (final review F1) is the manager's before-start

// hook (launchGate): asked after every step above (the token, the config

// read, the control socket's bind) AND the cgroup preparation (the

- // parent, the child, its limits), immediately before each start attempt,

- // the PID-move retry included. The lease, a fence or the attempt can

- // change during any of it. A refusal starts nothing; the manager drops

+ // parent, the child, its limits, and the shim's placement), immediately

+ // before each exec, the flat PID-move fallback's included. The lease, a

+ // fence or the attempt can change during any of it. A refusal runs

+ // nothing of the command; the manager drops

// the child cgroup and the error path below unbinds the listener, as for

// a failed spawn. With no gate on ctx (every live-link spawn) there is no

// hook and the spawn is exactly as it was, unless LiveExecGate is set:

@@ -2917,8 +2918,9 @@ func (b *ExecBackend) StartWorkerStaged(ctx context.Context, rt registry.Route,

// comment names.

//

// Gated (spawnInCgroupGated): ownership can be lost while the cgroup is

- // prepared or between a failed clone3 start and its PID-move retry, so

- // the caller's gate is asked at each start, not once before all of it.

+ // prepared and the process placed, or between a failed placement and

+ // its PID-move fallback, so the caller's gate is asked at each exec, not

+ // once before all of it.

// With no gate on ctx (an unplanned wake's spawn passes none) launchGate

// is nil and this is spawnInCgroup exactly. The zero tier is

// spawnInCgroup's too: the route's live tier (b.tier), as a worker has

diff --git a/internal/activator/claimspawn_kernel_linux_test.go b/internal/activator/claimspawn_kernel_linux_test.go

index 849b5aa8a..65bf09bee 100644

--- a/internal/activator/claimspawn_kernel_linux_test.go

+++ b/internal/activator/claimspawn_kernel_linux_test.go

@@ -32,7 +32,11 @@ func TestClaimSpawnKernelChild(t *testing.T) {

// same path again, on a real kernel. On the dev box (amadan #272: arm64,

// kernel 6.12.105, systemd 257) a clone3(CLONE_INTO_CGROUP) into a cgroup

// that took cgroup.kill is SIGKILLed at once, so each respawn below must

-// land in a fresh cgroup and stay alive. Three sequences: the strict

+// land in a fresh cgroup and stay alive. The strict primitive still places

+// with clone3; the Manager's starts (the worker's and the sidecar's) now

+// place through the exec-shim, whose cgroup.procs write is not clone3's,

+// and must still land in a fresh cgroup, limits written again, and stay

+// up. Three sequences: the strict

// primitive (PrepareStrict, then a cleanup handle's kill), a worker's

// KillHost then its respawn into carlos-exec@<host>, and the legacy

// sidecar's KillSidecar then its respawn into role-sidecar.

diff --git a/internal/activator/exec_gate_test.go b/internal/activator/exec_gate_test.go

index cd25a8df4..acae4e4dc 100644

--- a/internal/activator/exec_gate_test.go

+++ b/internal/activator/exec_gate_test.go

@@ -472,19 +472,23 @@ func TestAWorkerStartIsGatedAfterItsCgroupPreparation(t *testing.T) {

}

-// The same gate is asked again before the PID-move retry that follows a

-// failed clone3 start (cgroup.StartGated's contract), so ownership lost

-// between the two starts the worker on neither. The temp-dir subtree is no

-// cgroupfs: the clone3-into-cgroup start fails and the manager retries.

-// Before the fix the worker's gate was never asked at a start attempt, and

-// the retry started it.

-func TestAWorkerStartRetryReasksTheGate(t *testing.T) {

- a, eb, rt, out, _ := workerGateService(t, nil)

+// The same gate is the manager's before hook at the PID-move fallback a

+// flat start takes after a failed placement (cgroup.StartGated's

+// contract), so ownership lost by then starts nothing. The temp-dir subtree

+// is no cgroupfs: the shim's checks refuse the placement before any fork,

+// so the gate (asked after a placement) is first asked at the fallback, and

+// refuses there. That a placement which got past the gate and then failed

+// asks it again at the fallback is internal/cgroup's

+// TestFlatPlacementFailureFallsBackToPIDMove: the hook passed through is

+// this same gate. Before the clone3 start left (amadan #272) this asked

+// twice, at the clone3 start and its retry.

+func TestAWorkerStartFallbackAsksTheGate(t *testing.T) {

+ a, eb, rt, out, dir := workerGateService(t, nil)

var asked atomic.Int32

eb.launchHook = func(string) { asked.Add(1) }

superseded := errors.New("the attempt was superseded")

gate := func() error {

- if asked.Load() >= 2 {

+ if asked.Load() >= 1 {

return superseded

}

return nil

@@ -493,8 +497,11 @@ func TestAWorkerStartRetryReasksTheGate(t *testing.T) {

if p != nil {

_ = p.Stop(context.Background())

}

- if n := asked.Load(); n != 2 {

- t.Fatalf("the gate was asked %d times at the starts, want twice: the clone3 start and its retry", n)

+ if n := asked.Load(); n != 1 {

+ t.Fatalf("the gate was asked %d times at the starts, want once: the PID-move fallback's", n)

+ }

+ if b, err := os.ReadFile(filepath.Join(dir, "cgroup.procs")); err == nil && len(b) != 0 {

+ t.Fatalf("the worker's cgroup was given pid %s after a refusal at its start", b)

}

if !errors.Is(err, superseded) || p != nil {

t.Fatalf("startWorker = %v, %v; want no process and the gate's refusal", p, err)

@@ -508,9 +515,9 @@ func TestAWorkerStartRetryReasksTheGate(t *testing.T) {

// Final review wave 2 (F1 completion, Astra round 2 finding 1): a flat

// staged spawn's exec gate is the cgroup manager's before-start hook, so

// it is asked AFTER the cgroup preparation (the parent's tier, the child

-// cgroup and its limits, their log lines) and before EACH start attempt,

-// the PID-move retry after a failed clone3 start included. Ownership lost

-// during that preparation, or between a failed start and its retry,

+// cgroup and its limits, their log lines) and the placement, before EACH

+// exec, a flat start's PID-move fallback included. Ownership lost during

+// that preparation, or between a failed placement and its fallback,

// starts nothing: no process, no marker, the control socket unbound, the

// generation's child cgroup removed, abandonment with no execution

// evidence. Before the fix the gate ran once, before the cgroup

@@ -521,6 +528,7 @@ func TestTheExecGateIsAskedAtEachCgroupStart(t *testing.T) {

b := newLaunchBackend(t, "flat", f.b)

b.eb.GenContainment = true

parent := gatedCgroups(t, b.eb, b.rt.Host, func() { f.rev.Add(1) })

+ cgroup.FakePlacementForTest(t) // a temp dir is no cgroupfs, and a generation is never moved by PID

f.a.backend = b

mark := filepath.Join(t.TempDir(), "v2.ran")

id := markerScript(t, t.TempDir(), "v2", mark)

@@ -566,14 +574,16 @@ func TestTheExecGateIsAskedAtEachCgroupStart(t *testing.T) {

}

b.prepGone(t, b.gens()[0])

})

- t.Run("overlap successor, a failed clone3 start is not retried by PID move", func(t *testing.T) {

+ t.Run("overlap successor, a failed placement is not retried by PID move", func(t *testing.T) {

f := newSwapFixture(t)

b := newLaunchBackend(t, "flat", f.b)

b.eb.GenContainment = true

- // The temp-dir subtree is no cgroupfs: the clone3-into-cgroup start

- // fails (EBADF). A generation is never started and then moved by

- // PID (readiness round 2 Task 7), so there is no retry and no second

- // gate: the start is refused as the box's condition.

+ // The temp-dir subtree is no cgroupfs: the shim's checks refuse the

+ // placement before any fork, so the gate, asked only after a

+ // placement (amadan #272, Ruling 13), is never asked. A generation is

+ // never started and then moved by PID (readiness round 2 Task 7), so

+ // there is no retry and no gate at all: the start is refused as the

+ // box's condition.

parent := gatedCgroups(t, b.eb, b.rt.Host, nil)

var asked atomic.Int32

b.eb.launchHook = func(string) { asked.Add(1) }

@@ -583,15 +593,15 @@ func TestTheExecGateIsAskedAtEachCgroupStart(t *testing.T) {

at := f.attempt(ModeOverlap)

at.StagedPath, at.Digest = id.StagedPath, id.Digest

res := f.a.Swap(context.Background(), at, f.newest)

- if n := asked.Load(); n != 1 {

- t.Fatalf("the gate was asked %d times, want once: the clone3 start, and no retry", n)

+ if n := asked.Load(); n != 0 {

+ t.Fatalf("the gate was asked %d times, want never: no placement, and no retry", n)

}

if res.Outcome != SwapAbandoned || !strings.Contains(res.Reason, cgroup.ErrLimitNotApplied.Error()) || res.Executed {

t.Fatalf("Swap = %+v, want abandoned (limits not applied), nothing executed", res)

}

time.Sleep(100 * time.Millisecond)

if b.launched(t, id, mark) {

- t.Fatal("the successor was launched after its clone3 placement failed")

+ t.Fatal("the successor was launched after its placement failed")

}

gens := b.gens()

b.prepGone(t, gens[0])

diff --git a/internal/activator/genbackend.go b/internal/activator/genbackend.go

index a74d5ed3b..240327d29 100644

--- a/internal/activator/genbackend.go

+++ b/internal/activator/genbackend.go

@@ -397,9 +397,10 @@ func (b *SplitBackend) StartSidecarGen(ctx context.Context, rt registry.Route, g

// host cgroup with the instance, and the terminal stop's evidence on that

// cgroup is the proof (errNoGenContainment, as KillGen answers).

//

-// The role child is kept after the kill, and a clone3 into a cgroup that

-// took cgroup.kill is SIGKILLed at once (amadan #272), so the next sidecar

-// start (StartChildGated) removes it and makes it fresh first, or refuses.

+// The role child is kept after the kill, and the next sidecar start

+// (StartChildGated) removes it and makes it fresh first, or refuses: a

+// cgroup that took cgroup.kill is never spawned into again (amadan #272,

+// internal/cgroup/recreate_linux.go).

func (b *ExecBackend) KillSidecar(ctx context.Context, rt registry.Route) error {

switch p := b.pathsFor(rt); p.layout {

case LayoutNested:

@@ -492,9 +493,10 @@ func (b *SplitBackend) GenPaths(rt registry.Route, n uint64) (Gen, bool) {

// so a nested-looking worker is proven there too. An absent cgroup is

// empty. No cgroup delegation at all is an unproven kill.

//

-// The host cgroup is kept after the kill, and a clone3 into a cgroup that

-// took cgroup.kill is SIGKILLed at once (amadan #272), so the worker's next

-// start (StartGated) removes it and makes it fresh first, or refuses. That

+// The host cgroup is kept after the kill, and the worker's next start

+// (StartGated) removes it and makes it fresh first, or refuses: a cgroup

+// that took cgroup.kill is never spawned into again (amadan #272,

+// internal/cgroup/recreate_linux.go). That

// holds with the flag off too, where only Task 15's F6 path reaches here.

func (b *ExecBackend) KillHost(ctx context.Context, rt registry.Route) error {

if b.Cgroups == nil {

diff --git a/internal/activator/logsink_test.go b/internal/activator/logsink_test.go

index 15e0530f6..767035f86 100644

--- a/internal/activator/logsink_test.go

+++ b/internal/activator/logsink_test.go

@@ -229,7 +229,7 @@ func TestMemberSafeWakeReasonHidesAKilledCgroupRefusal(t *testing.T) {

func TestMemberSafeWakeReasonHidesALimitNotAppliedRefusal(t *testing.T) {

for _, err := range []error{

fmt.Errorf("h.example.com: %w: g2: memory.max: open /sys/fs/cgroup/x/carlos-exec@h.example.com/g2/memory.max: permission denied", cgroup.ErrLimitNotApplied),

- fmt.Errorf("%w: h.example.com: clone3-into-cgroup unavailable", cgroup.ErrLimitNotApplied),

+ fmt.Errorf("%w: h.example.com: the placement shim is not available", cgroup.ErrLimitNotApplied),

} {

got := memberSafeWakeReason(err)

if got != memberSafeWakeReason(ErrFenced) || strings.Contains(got, "cgroup") || strings.Contains(got, "memory") {

diff --git a/internal/activator/swap.go b/internal/activator/swap.go

index 8365047c2..6834fd490 100644

--- a/internal/activator/swap.go

+++ b/internal/activator/swap.go

@@ -204,7 +204,7 @@ const (

// would run under could not be applied, or it could only be started

// outside its cgroup and moved after (cgroup.ErrLimitNotApplied, zdd

// readiness round 2 Task 7): it refuses before it builds the command

-// (or, for a failed clone3 placement, the kernel refused the clone),

+// (or, for a failed placement, the shim proved it never reached exec),

// so nothing ran, and the condition is the box's;

// - root refusing the nested staged executable (instancectl.ErrStagedExe,

// Task 9): it validates before it mounts or starts anything;

diff --git a/internal/activator/swap_executed_test.go b/internal/activator/swap_executed_test.go

index 4dbd5dc22..3d02cdb81 100644

--- a/internal/activator/swap_executed_test.go

+++ b/internal/activator/swap_executed_test.go

@@ -61,7 +61,7 @@ func TestSwapResultReportsWhetherTheAttemptsBuildExecuted(t *testing.T) {

f.b.genErr = fmt.Errorf("%w: g2: memory.max: is a directory", cgroup.ErrLimitNotApplied)

}, SwapAbandoned, false},

{"hold, the generation's limits were not applied", ModeHold, func(f *swapFixture) {

- f.b.genErr = fmt.Errorf("%w: h.example.com: clone3-into-cgroup unavailable", cgroup.ErrLimitNotApplied)

+ f.b.genErr = fmt.Errorf("%w: h.example.com: the placement shim is not available", cgroup.ErrLimitNotApplied)

}, SwapAbandoned, false},

{"overlap, superseded at the switch", ModeOverlap, func(f *swapFixture) {

f.a.genHealth = func(context.Context, Gen) error { f.rev.Add(1); return nil }

diff --git a/internal/activator/swap_overlap_test.go b/internal/activator/swap_overlap_test.go

index da60172d1..d2d3fc582 100644

--- a/internal/activator/swap_overlap_test.go

+++ b/internal/activator/swap_overlap_test.go

@@ -361,7 +361,7 @@ func TestVerdictOf(t *testing.T) {

// all be applied (or it could only be started then moved) is refused

// before the exec; no build is at fault.

{cgroup.ErrLimitNotApplied, gateNoOwnership},

- {fmt.Errorf("h: %w: clone3-into-cgroup unavailable", cgroup.ErrLimitNotApplied), gateNoOwnership},

+ {fmt.Errorf("h: %w: the placement shim is not available", cgroup.ErrLimitNotApplied), gateNoOwnership},

// A row that cannot run a staged generation: nothing staged spawns.

{errUnitNoGens, gateNoOwnership},

{instancectl.ErrStagedExe, gateNoOwnership},

diff --git a/internal/activator/swap_sidecar_test.go b/internal/activator/swap_sidecar_test.go

index 1b0832e67..388b19ddf 100644

--- a/internal/activator/swap_sidecar_test.go

+++ b/internal/activator/swap_sidecar_test.go

@@ -589,7 +589,7 @@ func TestStartSidecarGenKeepsTheRoleSidecarChild(t *testing.T) {

t.Fatal(err)

}

// Checked before the stop: the exit's cleanup removes the empty child

- // (with clone3 placement nothing is written into it to keep it).

+ // (with placement before exec nothing is written into it to keep it).

if fi, err := os.Stat(filepath.Join(parent, "role-sidecar")); err != nil || !fi.IsDir() {

_ = p.Stop(context.Background())

t.Fatalf("no role-sidecar child: %v", err)

diff --git a/internal/activator/workerbuild.go b/internal/activator/workerbuild.go

index 835f6b37a..b02884fbd 100644

--- a/internal/activator/workerbuild.go

+++ b/internal/activator/workerbuild.go

@@ -98,9 +98,10 @@ func runBeforeExec(ctx context.Context, id release.StagedIdentity) error {

// gate, when set, is the caller's ownership check at the exec (final review

// F1). Through the staged capability it rides the context as the exec gate

// (withExecGate), and the backend asks it as an instance spawn's: after the

-// materialization, the publication and the cgroup's preparation, and before

-// EACH start attempt, the PID-move retry after a failed clone3 start

-// included (Astra R3 §5 item 4, ruling 206). In the before-exec hook, as it

+// materialization, the publication, the cgroup's preparation and the

+// process's placement, and before EACH exec, the PID-move fallback after a

+// failed flat placement included (Astra R3 §5 item 4, ruling 206). In the

+// before-exec hook, as it

// was, it was asked once and before the cgroup was prepared, so ownership

// lost during that preparation, or between a failed start and its retry,

// still started the worker. On a backend without the staged capability it

diff --git a/internal/cgroup/cgroup.go b/internal/cgroup/cgroup.go

index 839a29483..bab3730a5 100644

--- a/internal/cgroup/cgroup.go

+++ b/internal/cgroup/cgroup.go

@@ -18,6 +18,7 @@

package cgroup

import (

+ "context"

"errors"

"fmt"

"os"

@@ -173,11 +174,6 @@ func ParseSelfCgroup(data string) (string, bool) {

type Manager struct {

root string

logf func(string, ...any)

- // fallback, once latched, means clone3-into-cgroup is refused here (a

- // seccomp filter older than the syscall, an old kernel) and children are

- // started normally then moved by PID. A filter is static for the life of

- // the process, so retrying the syscall on every spawn buys nothing.

- fallback atomic.Bool

// locks holds each host's start lock (hostLock), made on first use.

locksMu sync.Mutex

locks map[string]*sync.Mutex

@@ -193,7 +189,7 @@ type Manager struct {

// removes, a cgroup of the host that took cgroup.kill (freshen, amadan

// #272), so two starts of one host never both remove one. It is the

// innermost lock: nothing is taken while it is held, and it is never held

-// across a before-start hook or a clone3.

+// across a before-start hook or a start.

func (m *Manager) hostLock(host string) *sync.Mutex {

m.locksMu.Lock()

defer m.locksMu.Unlock()

@@ -333,10 +329,18 @@ func (m *Manager) sweepStale() {

// child at once, so nothing is started.

var ErrKilledCgroupKept = errors.New("cgroup: a cgroup that took cgroup.kill could not be made fresh before a spawn")

-// startCmd is Start's spawn seam — a package var so the clone3 refusal

-// path is testable without a seccomp filter in the loop.

+// startCmd is the package's one spawn seam: every exec.Cmd.Start, the

+// shim's own included, goes through it, so a test can fake a start without

+// a process.

var startCmd = func(cmd *exec.Cmd) error { return cmd.Start() }

+// placeStart is startIn's placement: the exec-shim (startViaShim) puts the

+// process in dir before any of its code runs. FakePlacementForTest swaps

+// it for a test whose cgroupfs is a temp directory.

+var placeStart = func(m *Manager, dir string, cmd *exec.Cmd, before func() error) (bool, error) {

+ return m.startViaShim(dir, cmd, before)

+}

+

// hostDir is the per-host cgroup: instance, sidecar and seed all share it —

// a seed's first-boot cost and a sidecar's load ARE that instance's work.

func (m *Manager) hostDir(host string) string { return filepath.Join(m.root, ExecName(host)) }

@@ -349,31 +353,33 @@ func (m *Manager) hostDir(host string) string { return filepath.Join(m.root, Exe

// a Cmd cannot be reused after a failed Start. On a non-nil error the

// returned Cmd may be nil.

//

-// Primary mechanism: clone3 with CLONE_INTO_CGROUP (SysProcAttr.UseCgroupFD)

-// — the child never spends a cycle in the wrong cgroup. If that start

-// fails, the cgroup fd may or may not be why (a seccomp filter older than

-// clone3 answers EPERM; so does a missing binary): retry WITHOUT the fd,

-// and only if the retry succeeds — proving the cgroup was the problem — and

-// only if the failure actually means the mechanism is gone (shouldLatch),

-// latch the fallback and move the child by PID. A transient, spawn-specific

-// error (e.g. the cgroup dir vanishing between os.Open and start) falls

-// back for this spawn only; the next Start still tries clone3. The window

-// between start and move charges the child's first moments to the agent:

-// mis-accounting in the safe direction. Note the PID write migrates only

-// the child process itself — any grandchildren it has already forked by

-// then stay charged to the agent's cgroup, not the child's.

+// Primary mechanism: the placement exec-shim (startViaShim, amadan #272).

+// The child starts as this binary, writes its own PID into the cgroup,

+// and only then execs the command, so none of the command's code runs in

+// the wrong cgroup. clone3 CLONE_INTO_CGROUP did this until the shipped

+// unit's RestrictNamespaces=yes made clone3 answer ENOSYS. If the shim

+// proves it could not place the child before exec (a placement error),

+// this one spawn falls back to starting the command normally and moving it

+// by PID; the next Start still uses the shim. The window between start and

+// move charges the child's first moments to the agent: mis-accounting in

+// the safe direction. Note the PID write migrates only the child process

+// itself — any grandchildren it has already forked by then stay charged to

+// the agent's cgroup, not the child's. A failure that is the command's own

+// (a missing binary, a bad Dir) is returned as is and never retried.

func (m *Manager) Start(host string, limit Limit, build func() *exec.Cmd) (*exec.Cmd, error) {

return m.StartGated(host, limit, build, nil)

}

// StartGated is Start with a before-start hook. before, when set, is asked

-// immediately before EACH start attempt, the clone3 one and the PID-move

-// retry alike, so after the cgroup is made and its limits are written

-// (zero-downtime deploys final review F1: the caller's ownership can change

-// during that preparation). A refusal starts nothing, undoes that attempt's

+// immediately before EACH exec of the command, after the cgroup is made and

+// its limits are written (zero-downtime deploys final review F1: the

+// caller's ownership can change during that preparation). Through the shim

+// that is after the child is placed and before it execs (Ruling 13); on

+// the PID-move fallback it is before the start. A refusal runs nothing of

+// the command (a placed shim is killed and reaped), undoes that attempt's

// preparation as a failed start does (the cgroup dir this spawn would have

// used is removed if empty), and comes back as before's own error, never

-// joined with an earlier start's. A nil before is Start exactly.

+// joined with an earlier attempt's. A nil before is Start exactly.

//

// A host cgroup that took cgroup.kill (KillHost keeps it) is removed and

// made again before anything is spawned into it, and a start whose killed

@@ -390,12 +396,16 @@ func (m *Manager) StartGated(host string, limit Limit, build func() *exec.Cmd, b

}

// FakePlacementForTest is for another package's tests over a fake cgroupfs:

-// a temp directory is no cgroup, so the clone3 CLONE_INTO_CGROUP start

-// answers EBADF there, and a generation start (StartChildGated) is never

-// retried by start-and-move. Until t's cleanup, every start runs without

-// the cgroup fd, as if the kernel had placed it. t is a *testing.T. It

-// panics outside a test binary. It swaps a package global, so a test that

-// calls it, and every other test in that binary, must not use t.Parallel.

+// a temp directory is no cgroup, so the placement shim cannot place a

+// process there, and a generation start (StartChildGated) is never retried

+// by start-and-move. Until t's cleanup, every start that would go through

+// the shim instead starts the command directly behind its before hook, as

+// if it had been placed, and the shim counts as available even in a test

+// binary whose TestMain has no RunShimIfRequested. That is safe because the

+// fake never runs a shim, so it cannot make a test binary re-run itself.

+// Both are restored at t's cleanup. t is a *testing.T. It panics outside a

+// test binary. It swaps package globals, so a test that calls it, and every

+// other test in that binary, must not use t.Parallel.

func FakePlacementForTest(t interface {

Helper()

Cleanup(func())

@@ -404,30 +414,47 @@ func FakePlacementForTest(t interface {

panic("cgroup: FakePlacementForTest called outside a test")

}

t.Helper()

- orig := startCmd

- startCmd = func(cmd *exec.Cmd) error {

- clearInto(cmd)

- return orig(cmd)

- }

- t.Cleanup(func() { startCmd = orig })

+ orig := placeStart

+ placeStart = func(_ *Manager, _ string, cmd *exec.Cmd, before func() error) (bool, error) {

+ refused, _, err := startGated(cmd, before, nil)

+ return refused, err

+ }

+ forced := shimForcedForTest.Swap(true)

+ t.Cleanup(func() {

+ placeStart = orig

+ shimForcedForTest.Store(forced)

+ })

}

// startGated starts cmd once before, when set, has agreed. refused reports

// that before said no: nothing was started, and err is before's own error.

-func startGated(cmd *exec.Cmd, before func() error) (refused bool, err error) {

+//

+// register, when set, is called after before agrees and immediately before

+// the start, and the done it returns comes back for the caller to call once

+// the process is where it belongs. A start that fails ends it here, and

+// with no register done is a no-op; either way done may be called.

+func startGated(cmd *exec.Cmd, before func() error, register func() func()) (refused bool, done func(), err error) {

+ done = func() {}

if before != nil {

if err := before(); err != nil {

- return true, err

+ return true, done, err

}

- return false, startCmd(cmd)

+ if register != nil {

+ done = register()

+ }

+ if err := startCmd(cmd); err != nil {

+ done()

+ return false, done, err

+ }

+ return false, done, nil

}

// startPlain is the unaccounted start (no manager, or no cgroup dir): one

// start behind before. With before nil it is today's `cmd, startCmd(cmd)`.

func startPlain(build func() *exec.Cmd, before func() error) (*exec.Cmd, error) {

cmd := build()

- refused, err := startGated(cmd, before)

+ refused, _, err := startGated(cmd, before, nil)

if refused {

return nil, err

}

@@ -435,33 +462,30 @@ func startPlain(build func() *exec.Cmd, before func() error) (*exec.Cmd, error)

}

// startIn is Start against an explicit cgroup directory: the host's own

-// (Start) or one of its children (StartChild). The clone3 attempt, the

-// fallback and its latch are Start's, unchanged; before is StartGated's.

+// (Start) or one of its children (StartChild). Every start places its

+// process through the shim (placeStart); before is StartGated's.

//

// required is the generation layout's (StartChildGated, Task 7): every

-// limit must be in place before the process exists. With it, a child dir

-// that cannot be made or a limit write that fails refuses the start with

-// ErrLimitNotApplied, and the process is only ever placed by clone3

-// CLONE_INTO_CGROUP: a latched fallback, a platform without clone3

-// placement, or a dir that cannot be opened for the clone refuses before

-// build() runs, and a clone3 start that fails is not retried by

-// start-and-move. Without it (StartGated, the flat start) every one of

-// those stays best-effort, exactly as before.

+// limit must be in place before the process exists. With it, a process

+// that no shim can place here (a binary without RunShimIfRequested) is

+// refused before the child dir is made or build() runs, a child dir that

+// cannot be made or a limit write that fails refuses the start, and a

+// placement that fails refuses it too: all with ErrLimitNotApplied, the

+// box's condition. A required start is never started outside its cgroup

+// and moved by PID: it never reaches startAndMoveIn. Without required

+// (StartGated, the flat start) every one of those stays best-effort, and a

+// placement failure falls back to startAndMoveIn for that spawn (Ruling 2).

+//

+// Either way a failure that is the command's own (a missing binary, a bad

+// Dir) is returned as is, unwrapped, and never retried, and an empty dir

+// this spawn made is removed. A start whose exec.CommandContext context

+// ends during the placement is the caller's own condition too, never the

+// box's: the shim reports it as a placement failure (it could not start,

+// or was killed before placement), so startIn tells it apart (startEnded).

func (m *Manager) startIn(host, dir string, limit Limit, required bool, build func() *exec.Cmd, before func() error) (*exec.Cmd, error) {

- // clone3 is whether this start places its process with clone3

- // CLONE_INTO_CGROUP. A generation start reads the latch ONCE, here, and

- // that one value decides both the refusal below and the start further

- // down (Task 7 fix round 1): a concurrent flat start may latch the

- // fallback while this one makes its child and writes its limits, and a

- // second read would then send the generation to start-and-move. The flat

- // start keeps its one read where it always was, after its limits.

- clone3 := false

- if required {

- clone3 = intoCgroupSupported && !m.fallback.Load()

- if !clone3 {

- m.logf("cgroup: %s: clone3-into-cgroup is unavailable here, and a generation is never started then moved — generation start refused", host)

- return nil, fmt.Errorf("%w: %s: clone3-into-cgroup unavailable", ErrLimitNotApplied, host)

- }

+ if required && !shimAvailable() {

+ m.logf("cgroup: %s: the placement shim is not available in this process, and a generation is never started then moved — generation start refused", host)

+ return nil, fmt.Errorf("%w: %s: the placement shim is not available", ErrLimitNotApplied, host)

}

if err := os.MkdirAll(dir, 0o755); err != nil {

if required {

@@ -476,84 +500,68 @@ func (m *Manager) startIn(host, dir string, limit Limit, required bool, build fu

m.logf("cgroup: %s: %s: %v — generation start refused", host, filepath.Base(dir), err)

return nil, fmt.Errorf("%w: %s: %w", ErrLimitNotApplied, filepath.Base(dir), err)

}

- if !required {

- clone3 = intoCgroupSupported && !m.fallback.Load()

+ if !required && !shimAvailable() {

+ cmd, _, err := m.startAndMoveIn(host, dir, build, before)

+ return cmd, err

}

- if clone3 {

- f, err := os.Open(dir)

- if err != nil {

- if required {

- _ = os.Remove(dir)

- m.logf("cgroup: %s: open cgroup dir: %v — generation start refused", host, err)

- return nil, fmt.Errorf("%w: %s: open %s for clone3: %w", ErrLimitNotApplied, host, filepath.Base(dir), err)

- }

- m.logf("cgroup: %s: open cgroup dir: %v — using PID-move fallback for this spawn", host, err)

- cmd, _, err := m.startAndMoveIn(host, dir, build, before)

- return cmd, err

- }

- cmd := build()

- setInto(cmd, int(f.Fd()))

- refused, err := startGated(cmd, before)

- f.Close()

- if refused {

- // Nothing started, and no retry: the dir goes as it does after a

- // spawn that never happened (startAndMoveIn).

- _ = os.Remove(dir)

- return nil, err

- }

- if err == nil {

- m.biasOOM(host, cmd)

- return cmd, nil

- }

- firstErr := err

- if required {

- // Never start-and-move a generation: no retry, so nothing proves

- // the cgroup was the cause, and nothing latches. A failure that

- // is the placement's (placementFailed) is the box's condition;

- // any other is the start's own, as for a flat start whose retry

- // failed too. placementFailed may probe dir, so it runs before

- // the dir goes.

- placement := placementFailed(firstErr, dir)

- _ = os.Remove(dir)

- if placement {

- m.logf("cgroup: %s: clone3-into-cgroup failed (%v) — generation start refused, not retried by PID move", host, firstErr)

- return nil, fmt.Errorf("%w: %s: clone3-into-cgroup: %w", ErrLimitNotApplied, host, firstErr)

- }

- return nil, firstErr

- }

- retried, refused, retryErr := m.startAndMoveIn(host, dir, build, before)

- if refused {

- // The hook said no between the failed clone3 start and its

- // retry: its own answer, not a spawn failure to join.

- return nil, retryErr

- }

- if retryErr != nil {

- // startAndMoveIn already dropped the ghost dir on its own

- // failure; join both errors so the operator can see a clone3

- // attempt happened at all, not just that the retry failed.

- return nil, errors.Join(firstErr, retryErr)

- }

- if shouldLatch(firstErr) {

- m.fallback.Store(true)

- m.logf("cgroup: clone3-into-cgroup refused (%v) — latching PID-move fallback for the life of this process", firstErr)

- } else {

- m.logf("cgroup: clone3-into-cgroup failed for this spawn (%v) — not latching", firstErr)

- }

- return retried, nil

- }

- if required {

- // Unreachable while clone3 is read once above (a required start

- // without it was refused before its child was made). Kept so no

- // later edit can send a generation to start-and-move: a generation

- // is never started outside its cgroup and moved by PID.

+ cmd := build()

+ ended := watchCancel(cmd)

+ refused, err := placeStart(m, dir, cmd, before)

+ switch {

+ case refused:

+ // Nothing of the command ran, and no retry: the dir goes as it does

+ // after a spawn that never happened (startAndMoveIn).

_ = os.Remove(dir)

- m.logf("cgroup: %s: a generation start reached the PID-move fallback — generation start refused", host)

- return nil, fmt.Errorf("%w: %s: a generation is never started then moved", ErrLimitNotApplied, host)

- }

- cmd, _, err := m.startAndMoveIn(host, dir, build, before)

+ return nil, err

+ case err == nil:

+ m.biasOOM(host, cmd)

+ return cmd, nil

+ case !isPlacementError(err):

+ // The command's own failure: nothing ran, and a retry would fail the

+ // same way.

+ _ = os.Remove(dir)

+ return nil, err

+ case required && startEnded(err, ended):

+ // The caller's context ended: nothing ran, and it says nothing

+ // about the box.

+ _ = os.Remove(dir)

+ return nil, fmt.Errorf("cgroup: %s: the start's context ended before exec: %w", host, err)

+ case required:

+ _ = os.Remove(dir)

+ m.logf("cgroup: %s: placement before exec failed (%v) — generation start refused", host, err)

+ return nil, fmt.Errorf("%w: %s: %w", ErrLimitNotApplied, host, err)

+ }

+ // The flat start alone, after a placement failure the shim proved

+ // happened before exec. A cancelled context lands here too (the shim

+ // could not start), and the retry's Start then answers ctx's error at once.

+ m.logf("cgroup: %s: placement before exec failed (%v) — using the PID-move fallback for this spawn", host, err)

+ cmd, _, err = m.startAndMoveIn(host, dir, build, before)

return cmd, err

}

+// watchCancel records whether cmd's exec.CommandContext context ended

+// while it was being started: exec.Cmd calls Cancel (by default, a kill)

+// when the context ends after Start, which during a placement kills the

+// shim. A cmd without a context has no Cancel, and nothing is recorded.

+func watchCancel(cmd *exec.Cmd) *atomic.Bool {

+ var ended atomic.Bool

+ if cmd.Cancel != nil {

+ cancel := cmd.Cancel

+ cmd.Cancel = func() error {

+ ended.Store(true)

+ return cancel()

+ }

+ return &ended

+}

+

+// startEnded reports whether a failed placement was the start's context

+// ending: before the shim could start (Start answers the context's error,

+// which the placement error wraps), or during it (Cancel ran).

+func startEnded(err error, ended *atomic.Bool) bool {

+ return errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || ended.Load()

+}

+

// applyLimit writes limit into dir. Best-effort in the package's usual

// sense: a refused write (a kernel without the memory controller here, a

// control file that isn't ours) is one logged line and an uncapped child.

@@ -627,10 +635,17 @@ func (m *Manager) startAndMove(host string, build func() *exec.Cmd) (*exec.Cmd,

// startAndMoveIn is startAndMove into an explicit cgroup directory, behind

// StartGated's before. refused reports that before said no (err is its

-// own error).

+// own error). It is the flat start's fallback only: a generation start

+// never comes here.

+//

+// From the moment before agrees until the PID write has returned, the

+// child runs in the agent's cgroup, not dir, so dir's placement is in

+// flight for that window (the registry, inflight.go): a cleanup's proof

+// that dir is empty must not miss it, as it must not miss a shim's.

func (m *Manager) startAndMoveIn(host, dir string, build func() *exec.Cmd, before func() error) (*exec.Cmd, bool, error) {

cmd := build()

- if refused, err := startGated(cmd, before); err != nil {

+ refused, done, err := startGated(cmd, before, func() func() { return m.beginPlacement(dir) })

+ if err != nil {

// The spawn never happened: drop the now-ownerless dir so a failed

// wake doesn't leave the sampler reporting permanent zeros for a

// host with nothing running. rmdir-safe: os.Remove refuses a

@@ -642,11 +657,14 @@ func (m *Manager) startAndMoveIn(host, dir string, build func() *exec.Cmd, befor

// A broken startCmd seam reporting success with no process: this

// package must never panic the agent over it. Log and skip the

// move; the child, if any, simply stays unaccounted.

+ done()

m.logf("cgroup: %s: start reported success with no process — skipping pid move", host)

return cmd, false, nil

}

procs := filepath.Join(dir, "cgroup.procs")

- if err := os.WriteFile(procs, []byte(strconv.Itoa(cmd.Process.Pid)), 0o644); err != nil {

+ err = os.WriteFile(procs, []byte(strconv.Itoa(cmd.Process.Pid)), 0o644)

+ done()

+ if err != nil {

m.logf("cgroup: %s: move pid %d: %v — child stays unaccounted", host, cmd.Process.Pid, err)

}

m.biasOOM(host, cmd)

diff --git a/internal/cgroup/cgroup_linux.go b/internal/cgroup/cgroup_linux.go

index 864f6bba1..961f3b615 100644

--- a/internal/cgroup/cgroup_linux.go

+++ b/internal/cgroup/cgroup_linux.go

@@ -4,20 +4,15 @@ package cgroup

import (

"errors"

- "os"

"os/exec"

- "path/filepath"

"syscall"

)

-// intoCgroupSupported gates the clone3 CLONE_INTO_CGROUP attempt to

-// platforms whose SysProcAttr has the fields — everywhere else Start goes

-// straight to the PID-move fallback.

-const intoCgroupSupported = true

-

// setInto arms cmd for clone3-into-cgroup without clobbering any

// SysProcAttr build() already configured (Setpgid, Pdeathsig, ...) — build()

// owns all Cmd configuration; Start only adds to it, never replaces it.

+// Only the strict claims path ((*Prepared).Start) places with clone3 now;

+// the Manager's starts place through the shim and never set these fields.

func setInto(cmd *exec.Cmd, fd int) {

if cmd.SysProcAttr == nil {

cmd.SysProcAttr = &syscall.SysProcAttr{}

@@ -28,68 +23,12 @@ func setInto(cmd *exec.Cmd, fd int) {

// shouldLatch reports whether err proves clone3-into-cgroup is unavailable

// for the life of this process — EPERM (a seccomp filter predating clone3,

-// or one that forbids it outright) and ENOSYS (a kernel predating clone3)

-// mean the mechanism itself is gone; EINVAL means the kernel understood

-// clone3 but rejected CLONE_INTO_CGROUP. Anything else (e.g. ENOENT or

-// ENODEV from the cgroup dir disappearing between os.Open and start, raced

-// by a concurrent CleanupHost) is spawn-specific and says nothing about the

-// mechanism — latching on it would permanently downgrade the process over a

-// fluke and log a lie ("refused").

+// or one that forbids it outright) and ENOSYS (a kernel predating clone3,

+// or RestrictNamespaces=yes's filter) mean the mechanism itself is gone;

+// EINVAL means the kernel understood clone3 but rejected CLONE_INTO_CGROUP.

+// The strict claims path answers ErrStrictUnsupported for these. Anything

+// else (e.g. ENOENT or ENODEV from the cgroup dir disappearing) is

+// spawn-specific and says nothing about the mechanism.

func shouldLatch(err error) bool {

return errors.Is(err, syscall.EPERM) || errors.Is(err, syscall.ENOSYS) || errors.Is(err, syscall.EINVAL)

}

-

-// placementFailed reports whether a failed clone3 CLONE_INTO_CGROUP start

-// into dir failed on the placement or the box rather than on the command. A

-// generation start is never retried by start-and-move, so this is the only

-// evidence of which it was (Task 7). A placement or box failure:

-// - the mechanism refused (shouldLatch: EPERM, ENOSYS, EINVAL)

-// - the kernel would not place into the cgroup the fd names: EBADF (not

-// a cgroup), ENODEV (a cgroup the kernel has already marked dead),

-// EBUSY (a domain controller enabled in it), EOPNOTSUPP (a

-// domain-invalid cgroup)

-// - the box is out of room to fork: ENOMEM, or EAGAIN (a process or

-// pids.max limit)

-//

-// ENOENT and EACCES are ambiguous. Exec answers them for a missing or

-// unexecutable binary. The placement answers ENOENT for a cgroup removed

-// under the open fd, and EACCES when cgroup.procs is not writable. For

-// those two, dir is probed (placementLive): a child that is still a live,

-// writable cgroup puts the failure on the command; otherwise it is the

-// placement's. Any other error (ENOEXEC, ...) is the command's own.

-func placementFailed(err error, dir string) bool {

- switch {

- case shouldLatch(err), errors.Is(err, syscall.EBADF), errors.Is(err, syscall.ENODEV),

- errors.Is(err, syscall.EBUSY), errors.Is(err, syscall.EOPNOTSUPP),

- errors.Is(err, syscall.ENOMEM), errors.Is(err, syscall.EAGAIN):

- return true

- case errors.Is(err, syscall.ENOENT), errors.Is(err, syscall.EACCES):

- return placementLive(dir) != nil

- }

- return false

-}

-

-// placementLive probes the cgroup a failed clone3 start was placing into:

-// dir must still be a live cgroup (its cgroup.events can be read) and its

-// cgroup.procs must open for writing. It writes nothing: the open without

-// O_TRUNC changes no file, and a cgroup.procs that is opened and closed

-// moves no process. A nil answer means the placement was sound, so the

-// start's failure was the command's.

-func placementLive(dir string) error {

- if _, err := os.ReadFile(filepath.Join(dir, "cgroup.events")); err != nil {

- return err

- }

- f, err := os.OpenFile(filepath.Join(dir, "cgroup.procs"), os.O_WRONLY, 0)

- if err != nil {

- return err

- }

- return f.Close()

-}

-

-// clearInto undoes setInto: the start runs without the cgroup fd.

-func clearInto(cmd *exec.Cmd) {

- if cmd.SysProcAttr != nil {

- cmd.SysProcAttr.UseCgroupFD = false

- cmd.SysProcAttr.CgroupFD = 0

- }

-}

diff --git a/internal/cgroup/cgroup_linux_test.go b/internal/cgroup/cgroup_linux_test.go

index f397d7991..de01fb20b 100644

--- a/internal/cgroup/cgroup_linux_test.go

+++ b/internal/cgroup/cgroup_linux_test.go

@@ -3,228 +3,182 @@

package cgroup

import (

+ "context"

"errors"

+ "fmt"

"os"

"os/exec"

"path/filepath"

+ "slices"

+ "strconv"

"strings"

"syscall"

"testing"

)

-// TestStartUsesCgroupFDFirst: with delegation live and no latch, the child

-// starts via clone3-into-cgroup — SysProcAttr set, host dir created, and NO

-// cgroup.procs write (the kernel placed it, no PID move needed).

-func TestStartUsesCgroupFDFirst(t *testing.T) {

- root := newDelegatedRoot(t)

- m, err := SetupAt(root, t.Logf)

- if err != nil {

- t.Fatal(err)

+// Every Manager start places through the shim, and the shim keeps the

+// SysProcAttr the caller built (Setpgid, Pdeathsig, ...). No start may set

+// the clone3 cgroup fields: the shim would carry them to its own fork, and

+// the shipped unit answers clone3 with ENOSYS (amadan #272). Only the

+// strict claims path sets them (strict_linux.go).

+func TestManagerStartsNeverSetTheCgroupFD(t *testing.T) {

+ const host = "h.example.com"

+ m, _ := genManager(t, host)

+ var seen []*syscall.SysProcAttr

+ stubPlacement(t, func(_ *Manager, _ string, cmd *exec.Cmd, _ func() error) (bool, error) {

+ seen = append(seen, cmd.SysProcAttr)

+ cmd.Process, _ = os.FindProcess(os.Getpid())

+ return false, nil

+ })

+ build := func() *exec.Cmd {

+ cmd := exec.Command("/bin/true")

+ cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}

+ return cmd

}

- calls := fakeStart(t, func(*exec.Cmd) error { return nil })

- if _, err := m.Start("h.example.com", Limit{}, buildTrue); err != nil {

+ if _, err := m.Start("flat.example.com", Limit{}, build); err != nil {

t.Fatal(err)

}

- if len(*calls) != 1 {

- t.Fatalf("started %d times, want 1", len(*calls))

- }

- attr := (*calls)[0].SysProcAttr

- if attr == nil || !attr.UseCgroupFD {

- t.Fatalf("SysProcAttr = %+v, want UseCgroupFD", attr)

+ if _, err := m.StartChild(host, GenChild(1), TierDefault, TierDefault, build); err != nil {

+ t.Fatal(err)

}

- dir := filepath.Join(root, ExecName("h.example.com"))

- if _, err := os.Stat(dir); err != nil {

- t.Fatalf("host cgroup dir not created: %v", err)

+ if len(seen) != 2 {

+ t.Fatalf("placed %d starts, want the flat and the generation start", len(seen))

}

- if _, err := os.Stat(filepath.Join(dir, "cgroup.procs")); !os.IsNotExist(err) {

- t.Errorf("clone3 path must not write cgroup.procs (the kernel places the child): %v", err)

+ for i, attr := range seen {

+ if attr == nil || !attr.Setpgid {

+ t.Errorf("start %d: SysProcAttr = %+v, want build()'s Setpgid kept", i, attr)

+ } else if attr.UseCgroupFD || attr.CgroupFD != 0 {

+ t.Errorf("start %d: SysProcAttr = %+v, want no clone3 cgroup fd", i, attr)

+ }

}

-// TestStartFallsBackAndLatches pins the refusal path: clone3 refused

-// (seccomp EPERM), the retry WITHOUT the cgroup fd succeeds — proving the

-// cgroup was the problem — the child's PID is moved by file write, and the

-// latch makes every later spawn skip clone3 entirely.

-func TestStartFallsBackAndLatches(t *testing.T) {

+// Ruling 2: a flat start whose placement the shim proved failed before exec

+// falls back, for that spawn, to starting the command and moving its PID.

+// The fallback builds a fresh command (a Cmd cannot be reused) and asks

+// before again, since ownership can change between the two attempts.

+func TestFlatPlacementFailureFallsBackToPIDMove(t *testing.T) {

root := newDelegatedRoot(t)

m, err := SetupAt(root, t.Logf)

if err != nil {

t.Fatal(err)

}

- calls := fakeStart(t, func(cmd *exec.Cmd) error {

- if cmd.SysProcAttr != nil {

- return syscall.EPERM // the filter refusing clone3

+ quietOOM(t)

+ stubPlacement(t, func(_ *Manager, _ string, _ *exec.Cmd, before func() error) (bool, error) {

+ // The shim was placed and before agreed; then it died before exec.

+ if err := before(); err != nil {

+ return true, err

}

- return nil

+ return false, &placementError{errors.New("the shim exited before exec")}

})

- if _, err := m.Start("h.example.com", Limit{}, buildTrue); err != nil {

- t.Fatal(err)

- }

- if len(*calls) != 2 {

- t.Fatalf("first spawn: %d start calls, want 2 (clone3 attempt + plain retry)", len(*calls))

- }

- procs := filepath.Join(root, ExecName("h.example.com"), "cgroup.procs")

- if _, err := os.Stat(procs); err != nil {

- t.Fatalf("fallback did not move the PID: %v", err)

- }

- if !m.fallback.Load() {

- t.Fatal("refusal did not latch the fallback")

- }

-

- if _, err := m.Start("h2.example.com", Limit{}, buildTrue); err != nil {

- t.Fatal(err)

+ calls := fakeStart(t, func(*exec.Cmd) error { return nil })

+ builds, asked := 0, 0

+ cmd, err := m.StartGated("h.example.com", Limit{}, func() *exec.Cmd { builds++; return buildTrue() }, func() error { asked++; return nil })

+ if err != nil || cmd == nil {

+ t.Fatalf("StartGated = %v, %v: want the PID-move start", cmd, err)

}

- if len(*calls) != 3 {

- t.Fatalf("latched spawn: %d total start calls, want 3 (no clone3 re-attempt)", len(*calls))

+ if builds != 2 || asked != 2 || len(*calls) != 1 {

+ t.Fatalf("built %d, asked %d, started %d: want two builds, two asks, one start-and-move", builds, asked, len(*calls))

}

- if (*calls)[2].SysProcAttr != nil {

- t.Fatal("latched spawn still set SysProcAttr")

+ if got := readFixture(t, filepath.Join(m.hostDir("h.example.com"), "cgroup.procs")); got != strconv.Itoa(os.Getpid()) {

+ t.Fatalf("cgroup.procs = %q, want the child's PID moved in", got)

}

-// TestStartMergesIntoBuilderSysProcAttr: build() owns all Cmd configuration

-// — Start must ADD UseCgroupFD/CgroupFD to whatever SysProcAttr build()

-// already set, never replace it. A builder setting Setpgid or Pdeathsig

-// must keep that field on the clone3 attempt.

-func TestStartMergesIntoBuilderSysProcAttr(t *testing.T) {

+// A failure that is the command's own (a missing binary) is returned as

+// is, with no PID-move retry: a retry would fail the same way, and an

+// uncertain start must never run a tenant twice.

+func TestFlatCommandFailureIsNotRetried(t *testing.T) {

root := newDelegatedRoot(t)

m, err := SetupAt(root, t.Logf)

if err != nil {

t.Fatal(err)

}

+ missing := &os.PathError{Op: "fork/exec", Path: "/x", Err: syscall.ENOENT}

+ stubPlacement(t, func(*Manager, string, *exec.Cmd, func() error) (bool, error) { return false, missing })

calls := fakeStart(t, func(*exec.Cmd) error { return nil })

- build := func() *exec.Cmd {

- cmd := exec.Command("/bin/true")

- cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}

- return cmd

- }

- if _, err := m.Start("h.example.com", Limit{}, build); err != nil {

- t.Fatal(err)

+ builds := 0

+ cmd, err := m.Start("h.example.com", Limit{}, func() *exec.Cmd { builds++; return buildTrue() })

+ if err != missing || cmd != nil {

+ t.Fatalf("Start = %v, %v: want the command's own error as is", cmd, err)

}

- attr := (*calls)[0].SysProcAttr

- if attr == nil || !attr.Setpgid {

- t.Fatalf("SysProcAttr = %+v, want Setpgid preserved from build()", attr)

+ if builds != 1 || len(*calls) != 0 {

+ t.Fatalf("built %d, started %d: want one build and no retry", builds, len(*calls))

}

- if !attr.UseCgroupFD {

- t.Fatalf("SysProcAttr = %+v, want UseCgroupFD added by Start", attr)

+ if _, err := os.Stat(m.hostDir("h.example.com")); !os.IsNotExist(err) {

+ t.Fatalf("host dir after a failed start: %v, want it removed", err)

}

-// TestStartTransientClone3FailureDoesNotLatch: ENODEV (e.g. the cgroup dir

-// disappearing between os.Open and start, raced by a concurrent

-// CleanupHost) says nothing about whether clone3-into-cgroup works at all —

-// the spawn falls back for THIS call only, and the NEXT Start still

-// attempts clone3.

-func TestStartTransientClone3FailureDoesNotLatch(t *testing.T) {

+// A flat start whose context is already cancelled (Task 1's review carry):

+// the shim cannot start, which is a placement error by the protocol, so the

+// fallback runs once, and its own Start answers the context's error at

+// once. That error comes back as is, with no box condition beyond the one

+// fallback line, and nothing ran. This uses the real placement: a temp dir

+// is refused by the shim's checks before any fork, as a cancelled start is

+// refused at its fork.

+func TestFlatCancelledContextIsTheCommandsError(t *testing.T) {

root := newDelegatedRoot(t)

m, err := SetupAt(root, t.Logf)

if err != nil {

t.Fatal(err)

}

- calls := fakeStart(t, func(cmd *exec.Cmd) error {

- if cmd.SysProcAttr != nil {

- return syscall.ENODEV

- }

- return nil

- })

- if _, err := m.Start("h.example.com", Limit{}, buildTrue); err != nil {

- t.Fatal(err)

- }

- if len(*calls) != 2 {

- t.Fatalf("first spawn: %d start calls, want 2 (clone3 attempt + plain retry)", len(*calls))

- }

- if m.fallback.Load() {

- t.Fatal("a transient clone3 failure latched the fallback")

+ var lines []string

+ m.logf = func(format string, args ...any) { lines = append(lines, fmt.Sprintf(format, args...)) }

+ ctx, cancel := context.WithCancel(t.Context())

+ cancel()

+ cmd, err := m.Start("h.example.com", Limit{}, func() *exec.Cmd { return exec.CommandContext(ctx, "/bin/true") })

+ if !errors.Is(err, context.Canceled) || cmd != nil {

+ t.Fatalf("Start = %v, %v: want context.Canceled and no command", cmd, err)

}

-

- if _, err := m.Start("h2.example.com", Limit{}, buildTrue); err != nil {

- t.Fatal(err)

+ if isPlacementError(err) || errors.Is(err, ErrLimitNotApplied) {

+ t.Fatalf("a cancelled start read as a placement failure: %v", err)

}

- // The fake keeps refusing clone3 with ENODEV for h2 too, so the second

- // spawn again pays for a clone3 attempt (fails) plus a plain retry

- // (succeeds): two more calls. What matters is call index 2 — the second

- // spawn's FIRST attempt — still carries SysProcAttr: proof clone3 was

- // retried rather than skipped by a latched fallback.

- if len(*calls) != 4 {

- t.Fatalf("second spawn: %d total start calls, want 4 (clone3 attempted again + its own retry)", len(*calls))

+ if len(lines) != 1 || !strings.Contains(lines[0], "using the PID-move fallback for this spawn") {

+ t.Fatalf("log lines = %q, want only the one fallback line", lines)

}

- if (*calls)[2].SysProcAttr == nil || !(*calls)[2].SysProcAttr.UseCgroupFD {

- t.Fatal("second spawn did not attempt clone3 again — fallback must not have latched")

+ if _, err := os.Stat(m.hostDir("h.example.com")); !os.IsNotExist(err) {

+ t.Fatalf("host dir after a cancelled start: %v, want it removed", err)

}

-// TestStartJoinsBothErrorsOnTotalFailure: when the clone3 attempt AND the

-// PID-move retry both fail, the operator needs to see that a clone3 attempt

-// happened at all — the returned error must reference both, and the ghost

-// host dir must not survive.

-func TestStartJoinsBothErrorsOnTotalFailure(t *testing.T) {

+// startAndMoveIn's child runs in the agent's cgroup from its start until its

+// PID is written, so its target is registered in flight for exactly that

+// window: not while before is asked, through the start, and no longer once

+// the PID write has returned.

+func TestStartAndMoveRegistersItsTarget(t *testing.T) {

root := newDelegatedRoot(t)

m, err := SetupAt(root, t.Logf)

if err != nil {

t.Fatal(err)

}

- errClone3 := errors.New("clone3 boom")

- errRetry := errors.New("retry boom")

- first := true

- fakeStart(t, func(*exec.Cmd) error {

- if first {

- first = false

- return errClone3

- }

- return errRetry

- })

- _, err = m.Start("h.example.com", Limit{}, buildTrue)

- if err == nil {

- t.Fatal("Start with two failing spawns succeeded, want error")

- }

- if !errors.Is(err, errClone3) || !errors.Is(err, errRetry) {

- t.Fatalf("Start error = %v, want it to wrap both %v and %v", err, errClone3, errRetry)

- }

- if _, statErr := os.Stat(filepath.Join(root, ExecName("h.example.com"))); !os.IsNotExist(statErr) {

- t.Errorf("ghost host dir survived total spawn failure: %v", statErr)

- }

-}

-

-// Zero-downtime final review wave 2 (F1 completion): the before-start hook

-// is asked again before the PID-move retry that follows a failed clone3

-// start. A refusal there starts nothing more, comes back as the hook's own

-// error (not joined with the clone3 failure), latches nothing, and drops

-// the host dir as a failed spawn does.

-func TestStartGatedAsksAgainBeforeTheRetry(t *testing.T) {

- root := newDelegatedRoot(t)

- m, err := SetupAt(root, t.Logf)

- if err != nil {

+ quietOOM(t)

+ dir := m.hostDir("h.example.com")

+ if err := os.MkdirAll(dir, 0o755); err != nil {

t.Fatal(err)

}

- calls := fakeStart(t, func(cmd *exec.Cmd) error {

- if cmd.SysProcAttr != nil && cmd.SysProcAttr.UseCgroupFD {

- return syscall.EPERM

- }

- return nil

- })

- refused := errors.New("the attempt was superseded")

- var asked int

- cmd, err := m.StartGated("h.example.com", Limit{}, buildTrue, func() error {

- if asked++; asked == 2 {

- return refused

- }

- return nil

- })

- if err != refused || cmd != nil {

- t.Fatalf("StartGated = %v, %v: want the hook's own error, unjoined, and no command", cmd, err)

+ var atBefore, atStart int

+ fakeStart(t, func(*exec.Cmd) error { atStart = m.placementsAt(dir); return nil })

+ cmd, refused, err := m.startAndMoveIn("h.example.com", dir, buildTrue, func() error { atBefore = m.placementsAt(dir); return nil })

+ if err != nil || refused || cmd == nil {

+ t.Fatalf("startAndMoveIn = %v, %v, %v", cmd, refused, err)

}

- if asked != 2 || len(*calls) != 1 {

- t.Fatalf("hook asked %d times, %d starts: want 2 asks and only the clone3 start", asked, len(*calls))

+ if atBefore != 0 || atStart != 1 || m.placementsAt(dir) != 0 {

+ t.Fatalf("in flight: %d at before, %d at the start, %d after the move; want 0, 1, 0", atBefore, atStart, m.placementsAt(dir))

}

- if m.fallback.Load() {

- t.Fatal("a refused retry latched the PID-move fallback")

+ // A failed start ends its registration too.

+ fakeStart(t, func(*exec.Cmd) error { return errors.New("no such binary") })

+ if _, _, err := m.startAndMoveIn("h.example.com", dir, buildTrue, nil); err == nil {

+ t.Fatal("a failing start succeeded")

}

- if _, err := os.Stat(filepath.Join(root, ExecName("h.example.com"))); !os.IsNotExist(err) {

- t.Fatalf("host dir after a refused start: %v, want it removed", err)

+ if n := m.placementsUnder(root); n != 0 {

+ t.Fatalf("%d placements left in flight after a failed start", n)

}

-// A refusal before the clone3 start itself: nothing starts, no retry.

+// A refusal before any start: nothing starts, no retry. The real placement

+// cannot place in a temp dir (its checks refuse before any fork, so before

+// is not asked there); the fallback then asks before, which refuses.

func TestStartGatedRefusedBeforeTheFirstStart(t *testing.T) {

root := newDelegatedRoot(t)

m, err := SetupAt(root, t.Logf)

@@ -244,211 +198,85 @@ func TestStartGatedRefusedBeforeTheFirstStart(t *testing.T) {

}

-// Task 7, G: a generation start whose clone3 placement fails is never

-// retried by start-and-move. A failure that is the placement's or the

-// box's is ErrLimitNotApplied: the mechanism gone (EPERM, ENOSYS, EINVAL),

-// a cgroup fd the kernel would not place into (EBADF, ENODEV, EBUSY,

-// EOPNOTSUPP), or no room to fork (ENOMEM, EAGAIN). ENOENT and EACCES are

-// the start's own error only while the child is still a live, writable

-// cgroup (fix round 1: placementLive); a removed or unwritable child makes

-// them the placement's. Nothing latches (no retry proved the cgroup was

-// the cause), and nothing is moved by PID.

-func TestAGenerationStartIsNeverRetriedByPIDMove(t *testing.T) {

- const host = "h.example.com"

- for name, tc := range map[string]struct {

- fail error

- live bool // the child is still a live, writable cgroup at the failure

- want error

- }{

- "clone3 refused": {syscall.EPERM, true, ErrLimitNotApplied},

- "clone3 not understood": {syscall.EINVAL, true, ErrLimitNotApplied},

- "not a cgroup": {syscall.EBADF, true, ErrLimitNotApplied},

- "cgroup dead": {syscall.ENODEV, true, ErrLimitNotApplied},

- "out of memory": {syscall.ENOMEM, true, ErrLimitNotApplied},

- "out of processes": {syscall.EAGAIN, true, ErrLimitNotApplied},

- "a missing binary": {syscall.ENOENT, true, syscall.ENOENT},

- "a cgroup removed under the fd": {syscall.ENOENT, false, ErrLimitNotApplied},

- "an unexecutable binary": {syscall.EACCES, true, syscall.EACCES},

- "a cgroup that cannot be entered": {syscall.EACCES, false, ErrLimitNotApplied},

- } {

- t.Run(name, func(t *testing.T) {

- m, parent := genManager(t, host)

- g1 := filepath.Join(parent, "g1")

- calls := fakeStart(t, func(cmd *exec.Cmd) error {

- if tc.live {

- // What cgroupfs shows in a live child: the kernel's own

- // files, cgroup.procs empty and writable.

- writeFixture(t, filepath.Join(g1, "cgroup.events"), "populated 0\nfrozen 0\n")

- writeFixture(t, filepath.Join(g1, "cgroup.procs"), "")

- }

- if cmd.SysProcAttr != nil && cmd.SysProcAttr.UseCgroupFD {

- return &os.PathError{Op: "fork/exec", Path: cmd.Path, Err: tc.fail}

- }

- return nil

- })

- cmd, err := m.StartChild(host, GenChild(1), TierDefault, TierDefault, buildTrue)

- if !errors.Is(err, tc.want) || cmd != nil {

- t.Fatalf("StartChild = %v, %v: want %v and no command", cmd, err, tc.want)

- }

- if tc.want != ErrLimitNotApplied && errors.Is(err, ErrLimitNotApplied) {

- t.Errorf("a spawn failure read as a limit failure: %v", err)

- }

- if len(*calls) != 1 {

- t.Fatalf("started %d times, want the one clone3 attempt", len(*calls))

- }

- if m.fallback.Load() {

- t.Error("a generation start latched the PID-move fallback")

- }

- if got, err := os.ReadFile(filepath.Join(g1, "cgroup.procs")); err == nil && len(got) != 0 {

- t.Errorf("a PID was moved into g1: cgroup.procs %q", got)

- } else if err != nil && !tc.live && !os.IsNotExist(err) {

- t.Errorf("g1's cgroup.procs: %v", err)

- }

- })

- }

-}

-

-// Fix round 1, finding 1: a generation start reads the PID-move latch once.

-// A flat start elsewhere in the agent may latch the fallback while this

-// generation's child is made and its limits written; the generation still

-// starts the one way it decided on, placed by clone3, never started in the

-// agent's cgroup and moved by PID. The latch is set from applyLimit's

-// summary log line, which comes after the old first read and before the

-// old second one.

-func TestAGenerationStartReadsTheLatchOnce(t *testing.T) {

+// A refusal through the shim (before said no after placement): nothing of

+// the command ran, the refusal comes back as the hook's own error, and

+// there is no PID-move retry, for a flat start and a generation alike.

+func TestARefusalAtPlacementIsNotRetried(t *testing.T) {

const host = "h.example.com"

m, parent := genManager(t, host)

- latched := false

- m.logf = func(format string, args ...any) {

- t.Logf(format, args...)

- if !latched && strings.HasPrefix(format, "cgroup: %s: memory.max=") {

- latched = true

- m.fallback.Store(true) // a concurrent flat start's shouldLatch

- }

- builds := 0

+ refused := errors.New("the attempt was superseded")

+ stubPlacement(t, func(_ *Manager, _ string, _ *exec.Cmd, before func() error) (bool, error) {

+ return true, before()

+ })

calls := fakeStart(t, func(*exec.Cmd) error { return nil })

- cmd, err := m.StartChild(host, GenChild(1), TierDefault, TierDefault, func() *exec.Cmd { builds++; return buildTrue() })

- if !latched {

- t.Fatal("the latch was never set during the start: applyLimit's summary line did not fire")

- }

- if err != nil || cmd == nil {

- t.Fatalf("StartChild = %v, %v: want the clone3 start it decided on", cmd, err)

- }

- if builds != 1 || len(*calls) != 1 {

- t.Fatalf("built %d, started %d times: want one of each", builds, len(*calls))

+ if cmd, err := m.StartGated("flat.example.com", Limit{}, buildTrue, func() error { return refused }); err != refused || cmd != nil {

+ t.Fatalf("StartGated = %v, %v: want the hook's own error", cmd, err)

}

- if attr := (*calls)[0].SysProcAttr; attr == nil || !attr.UseCgroupFD {

- t.Fatal("the generation was started without its cgroup fd, to be moved by PID")

+ if cmd, err := m.StartChildGated(host, GenChild(1), TierDefault, Limit{}, buildTrue, func() error { return refused }); err != refused || cmd != nil {

+ t.Fatalf("StartChildGated = %v, %v: want the hook's own error", cmd, err)

}

- if _, err := os.Stat(filepath.Join(parent, "g1", "cgroup.procs")); !os.IsNotExist(err) {

- t.Errorf("a PID was moved into g1: %v", err)

+ if len(*calls) != 0 {

+ t.Fatalf("started %d times after a refusal at placement, want none", len(*calls))

}

-}

-

-// placementLive is the ENOENT/EACCES probe: a child that is still a live

-// cgroup with a writable cgroup.procs puts a failed start on the command;

-// one removed, one that is no cgroup, or one whose cgroup.procs cannot be

-// written puts it on the placement. The probe writes nothing.

-func TestPlacementLiveProbesTheChild(t *testing.T) {

- live := func(t *testing.T) string {

- dir := t.TempDir()

- writeFixture(t, filepath.Join(dir, "cgroup.events"), "populated 0\nfrozen 0\n")

- writeFixture(t, filepath.Join(dir, "cgroup.procs"), "")

- return dir

- }

- t.Run("live and writable", func(t *testing.T) {

- dir := live(t)

- if err := placementLive(dir); err != nil {

- t.Fatalf("placementLive = %v, want nil", err)

+ for _, dir := range []string{m.hostDir("flat.example.com"), filepath.Join(parent, "g1")} {

+ if _, err := os.Stat(dir); !os.IsNotExist(err) {

+ t.Errorf("%s after a refused start: %v, want it removed", dir, err)

}

- if got := readFixture(t, filepath.Join(dir, "cgroup.procs")); got != "" {

- t.Errorf("the probe wrote cgroup.procs: %q", got)

- }

- for _, e := range []error{syscall.ENOENT, syscall.EACCES} {

- if placementFailed(&os.PathError{Op: "fork/exec", Path: "/bin/x", Err: e}, dir) {

- t.Errorf("%v into a live child read as the placement's", e)

- }

- })

- gone := map[string]func(t *testing.T) string{

- "removed": func(t *testing.T) string {

- dir := live(t)

- if err := os.RemoveAll(dir); err != nil {

- t.Fatal(err)

- }

- return dir

- },

- "no cgroup.events": func(t *testing.T) string {

- dir := live(t)

- if err := os.Remove(filepath.Join(dir, "cgroup.events")); err != nil {

- t.Fatal(err)

- }

- return dir

- },

- "no cgroup.procs": func(t *testing.T) string {

- dir := live(t)

- if err := os.Remove(filepath.Join(dir, "cgroup.procs")); err != nil {

- t.Fatal(err)

- }

- return dir

- },

- }

- if os.Geteuid() != 0 { // root opens a read-only file for writing

- gone["cgroup.procs not writable"] = func(t *testing.T) string {

- dir := live(t)

- if err := os.Chmod(filepath.Join(dir, "cgroup.procs"), 0o444); err != nil {

- t.Fatal(err)

- }

- return dir

- }

- for name, mk := range gone {

- t.Run(name, func(t *testing.T) {

- dir := mk(t)

- if err := placementLive(dir); err == nil {

- t.Fatal("placementLive = nil, want the probe to fail")

- }

- for _, e := range []error{syscall.ENOENT, syscall.EACCES} {

- if !placementFailed(&os.PathError{Op: "fork/exec", Path: "/bin/x", Err: e}, dir) {

- t.Errorf("%v into a gone child read as the command's", e)

- }

- if placementFailed(&os.PathError{Op: "fork/exec", Path: "/bin/x", Err: syscall.ENOEXEC}, dir) {

- t.Error("ENOEXEC read as the placement's")

- }

- })

}

// FakePlacementForTest lets another package's fixture (a temp directory,

-// where clone3 CLONE_INTO_CGROUP answers EBADF) start a generation: the

-// clone3 start runs without the cgroup fd, as if placed, and is restored at

-// the test's cleanup.

-func TestFakePlacementForTestStartsWithoutTheFdAndRestores(t *testing.T) {

+// where the shim cannot place) start a generation: the command starts

+// directly behind its before hook, as if placed, and the shim counts as

+// available even where the binary has no hook. Both are restored at the

+// test's cleanup.

+func TestFakePlacementForTestStartsWithoutTheShimAndRestores(t *testing.T) {

const host = "h.example.com"

- m, _ := genManager(t, host)

+ m, _ := genFixture(t, host)

t.Run("faked", func(t *testing.T) {

- var seen []bool

- orig := startCmd

- startCmd = func(cmd *exec.Cmd) error {

- seen = append(seen, cmd.SysProcAttr != nil && cmd.SysProcAttr.UseCgroupFD)

- return syscall.EBADF

- }

- t.Cleanup(func() { startCmd = orig })

+ noShim(t) // as in a test binary whose TestMain has no hook

FakePlacementForTest(t)

- if _, err := m.StartChild(host, GenChild(1), TierDefault, TierDefault, buildTrue); !errors.Is(err, syscall.EBADF) {

- t.Fatalf("StartChild = %v, want the seam's own answer", err)

+ if !shimAvailable() {

+ t.Fatal("the fake did not make the shim available")

+ }

+ var asked int

+ calls := fakeStart(t, func(*exec.Cmd) error { return nil })

+ if _, err := m.StartChildGated(host, GenChild(1), TierDefault, TierDefault, buildTrue, func() error { asked++; return nil }); err != nil {

+ t.Fatalf("StartChildGated under the fake = %v", err)

}

- if len(seen) != 1 || seen[0] {

- t.Fatalf("starts with the cgroup fd = %v, want one start without it", seen)

+ if len(*calls) != 1 || asked != 1 {

+ t.Fatalf("started %d times, asked %d: want one direct start behind one ask", len(*calls), asked)

+ }

+ if slices.ContainsFunc(*calls, func(c *exec.Cmd) bool { return slices.Contains(c.Args, ShimVerb) }) {

+ t.Fatal("the fake started a shim")

}

})

+ if shimForcedForTest.Load() {

+ t.Fatal("the fake's availability override outlived its test")

+ }

calls := fakeStart(t, func(*exec.Cmd) error { return nil })

- if _, err := m.StartChild(host, GenChild(2), TierDefault, TierDefault, buildTrue); err != nil {

- t.Fatal(err)

+ if _, err := m.StartChild(host, GenChild(2), TierDefault, TierDefault, buildTrue); !errors.Is(err, ErrLimitNotApplied) || !isPlacementError(err) {

+ t.Fatalf("StartChild after the fake = %v: want the real shim's placement refusal on a temp dir", err)

}

- if len(*calls) != 1 || !(*calls)[0].SysProcAttr.UseCgroupFD {

+ if len(*calls) != 0 {

t.Fatal("the fake placement outlived its test")

}

+

+// The real shim cannot place a process in a temp directory (no cgroup2

+// filesystem). A generation start there is refused as the box's condition,

+// having built the command once and started nothing.

+func TestAGenerationTheShimCannotPlaceIsRefused(t *testing.T) {

+ const host = "h.example.com"

+ m, parent := genFixture(t, host)

+ calls := fakeStart(t, func(*exec.Cmd) error { return nil })

+ builds := 0

+ cmd, err := m.StartChild(host, RoleChild("seed"), TierDefault, Limit{}, func() *exec.Cmd { builds++; return buildTrue() })

+ if !errors.Is(err, ErrLimitNotApplied) || !isPlacementError(err) || cmd != nil {

+ t.Fatalf("StartChild = %v, %v: want ErrLimitNotApplied from the shim's placement error", cmd, err)

+ }

+ if builds != 1 || len(*calls) != 0 {

+ t.Fatalf("built %d, started %d: want one build and nothing started", builds, len(*calls))

+ }

+ noGenerationChild(t, parent)

+}

diff --git a/internal/cgroup/cgroup_other.go b/internal/cgroup/cgroup_other.go

deleted file mode 100644

index 3a754cb04..000000000

--- a/internal/cgroup/cgroup_other.go

+++ /dev/null

@@ -1,18 +0,0 @@

-//go:build !linux

-

-package cgroup

-

-import "os/exec"

-

-// intoCgroupSupported gates the clone3 CLONE_INTO_CGROUP attempt to

-// platforms whose SysProcAttr has the fields — everywhere else Start goes

-// straight to the PID-move fallback.

-const intoCgroupSupported = false

-

-func setInto(cmd *exec.Cmd, fd int) {}

-

-func shouldLatch(error) bool { return false }

-

-func placementFailed(error, string) bool { return false }

-

-func clearInto(*exec.Cmd) {}

diff --git a/internal/cgroup/cgroup_test.go b/internal/cgroup/cgroup_test.go

index bd3706473..5511c202a 100644

--- a/internal/cgroup/cgroup_test.go

+++ b/internal/cgroup/cgroup_test.go

@@ -185,10 +185,10 @@ func TestStartNilManagerSpawnsPlainly(t *testing.T) {

}

-// TestStartGenuineSpawnFailureDoesNotLatch: when the retry-without-cgroup

-// ALSO fails, the cgroup was not the problem — the error propagates and the

-// fallback must not latch (the next spawn should still try clone3).

-func TestStartGenuineSpawnFailureDoesNotLatch(t *testing.T) {

+// TestStartGenuineSpawnFailureIsReturned: when the start itself fails (here

+// the PID-move start a temp dir's placement failure falls back to), the

+// error propagates and the host dir this spawn made does not survive it.

+func TestStartGenuineSpawnFailureIsReturned(t *testing.T) {

root := newDelegatedRoot(t)

m, err := SetupAt(root, t.Logf)

if err != nil {

@@ -198,8 +198,8 @@ func TestStartGenuineSpawnFailureDoesNotLatch(t *testing.T) {

if _, err := m.Start("h.example.com", Limit{}, buildTrue); err == nil {

t.Fatal("Start with a failing spawn succeeded, want error")

}

- if m.fallback.Load() {

- t.Fatal("a genuine spawn failure latched the clone3 fallback")

+ if _, err := os.Stat(m.hostDir("h.example.com")); !os.IsNotExist(err) {

+ t.Errorf("ghost host dir survived a failed spawn: %v", err)

}

diff --git a/internal/cgroup/cleanup_linux.go b/internal/cgroup/cleanup_linux.go

index 3d775e74d..de8918ac8 100644

--- a/internal/cgroup/cleanup_linux.go

+++ b/internal/cgroup/cleanup_linux.go

@@ -9,6 +9,7 @@ import (

"fmt"

"io"

"os"

+ "path/filepath"

"slices"

"strconv"

"strings"

@@ -22,6 +23,12 @@ type Cleanup struct{ state *cleanupState }

type cleanupState struct {

pin *Prepared

procs *os.File

+ // m and path are the manager that opened the cleanup and the directory

+ // it pinned, as the manager's starts name it: the in-flight registry's

+ // key (inflight.go). A process being placed there or below counts as a

+ // member (Ruling 6).

+ m *Manager

+ path string

}

func cleanupFilesystem(root *os.File) error {

@@ -63,7 +70,11 @@ func (m *Manager) openCleanupAt(host, child string, checkFS func(*os.File) error

return nil, err

}

p := &Prepared{strictPreparedState: &strictPreparedState{root: root, host: host}}

- c := &Cleanup{state: &cleanupState{pin: p}}

+ path := filepath.Join(m.root, ExecName(host))

+ if child != "" {

+ path = filepath.Join(path, child)

+ }

+ c := &Cleanup{state: &cleanupState{pin: p, m: m, path: path}}

defer func() {

if err != nil {

if c.state.procs != nil {

@@ -131,11 +142,19 @@ func (c *Cleanup) Populated() (bool, error) {

return c.populationLocked()

}

+// populationLocked is the pinned cgroup's population. A process being

+// placed there or below it (the in-flight registry) makes it populated

+// before cgroup.events is read: a shim spends its first milliseconds in the

+// agent's cgroup, and a flat fallback's child runs there until its PID is

+// moved, so the kernel cannot see either yet.

func (c *Cleanup) populationLocked() (bool, error) {

p := c.state.pin

if err := p.identityOK(); err != nil {

return false, err

}

+ if c.state.m.placementsUnder(c.state.path) > 0 {

+ return true, nil

+ }

v, err := strictPopulation(p.events)

if err != nil {

return false, err

@@ -168,11 +187,16 @@ const (

// downtime spec §7.3), so stop evidence taken on it must read its

// generations'. A flat tenant runs as the agent's uid and can make cgroups

// of its own under its generation, so the walk goes to any depth, bounded.

+// While a process is being placed there or below it, its PID is not in any

+// cgroup.procs yet, so the list would be short: ErrPlacementInFlight.

func (c *Cleanup) membersLocked() ([]int, error) {

p := c.state.pin

if err := p.identityOK(); err != nil {

return nil, err

}

+ if c.state.m.placementsUnder(c.state.path) > 0 {

+ return nil, ErrPlacementInFlight

+ }

members, err := readProcs(c.state.procs)

if err != nil {

return nil, err

@@ -284,6 +308,25 @@ func childGroups(dir *os.File, depth int, groups *int) ([]string, error) {

}

+// cleanupKillWrite writes a pinned cgroup's cgroup.kill: a seam so a test

+// can act right after the kill lands.

+var cleanupKillWrite = func(f *os.File) error { return strictWrite(f, "1") }

+

+// KillAndEmpty kills everything in the pinned cgroup and below it and

+// returns nil only once it is proven empty: unpopulated and no members.

+// Any error is unproven, and ctx must carry a deadline.

+//

+// A process being placed into the cgroup or below it (the in-flight

+// registry, Ruling 6) is a member the kernel cannot see yet. KillAndEmpty

+// first waits for every such placement to resolve, bounded by ctx; an

+// error there is unproven (ErrPlacementInFlight with ctx's error), and

+// nothing is killed. Every emptiness check after the kill goes through the

+// same registry, so a placement that registers after the wait keeps the

+// proof from finishing until the placed process has been killed with the

+// rest, or ctx ends (unproven). Callers still exclude new starts during a

+// proof as they always have (the lease, and the before hook a start asks

+// immediately before exec); the registry only closes the window a shim, or

+// a flat start awaiting its PID move, spends outside its cgroup.

func (c *Cleanup) KillAndEmpty(ctx context.Context) error {

if c == nil || c.state == nil {

return ErrStrictState

@@ -301,6 +344,9 @@ func (c *Cleanup) KillAndEmpty(ctx context.Context) error {

if !time.Now().Before(deadline) {

return context.DeadlineExceeded

}

+ if err := c.state.m.waitPlacementsUnder(ctx, c.state.path); err != nil {

+ return fmt.Errorf("%w: %w", ErrPlacementInFlight, err)

+ }

p := c.state.pin

lockTick := time.NewTicker(time.Millisecond)

defer lockTick.Stop()

@@ -322,7 +368,7 @@ func (c *Cleanup) KillAndEmpty(ctx context.Context) error {

// the kernel kills anything cloned into one that took cgroup.kill

// (amadan #272, recreate_linux.go).

markKilled(p.identity)

- if err := strictWrite(p.kill, "1"); err != nil {

+ if err := cleanupKillWrite(p.kill); err != nil {

return err

}

tick := time.NewTicker(5 * time.Millisecond)

@@ -336,6 +382,10 @@ func (c *Cleanup) KillAndEmpty(ctx context.Context) error {

return err

}

members, err := c.membersLocked()

+ if errors.Is(err, ErrPlacementInFlight) {

+ // Registered since the population read: not empty yet.

+ populated, members, err = true, nil, nil

+ }

if err != nil {

return err

}

diff --git a/internal/cgroup/cleanup_linux_test.go b/internal/cgroup/cleanup_linux_test.go

index d9dcdb4a9..331059b5d 100644

--- a/internal/cgroup/cleanup_linux_test.go

+++ b/internal/cgroup/cleanup_linux_test.go

@@ -25,7 +25,7 @@ func cleanupFixture(t *testing.T) (*Manager, string, string) {

t.Fatal(err)

}

- return &Manager{root: root}, host, dir

+ return &Manager{root: root, logf: t.Logf}, host, dir

}

func fixtureCleanup(t *testing.T, m *Manager, host string) *Cleanup {

@@ -333,3 +333,78 @@ func TestOpenCleanupChildRefusesForeignNamesAndMissingChildren(t *testing.T) {

t.Errorf("missing g9: %v, want ErrNoCgroup", err)

}

+

+// Review focus 4, Ruling 6: a process being placed under the host (here a

+// generation into g1) is a member the kernel cannot see yet. While it is in

+// flight the host's cleanup reports populated, refuses to list members,

+// and kills nothing; once it resolves, Populated reads cgroup.events again.

+func TestCleanupCountsPlacementInFlight(t *testing.T) {

+ m, host, dir := cleanupFixture(t)

+ writeFixture(t, filepath.Join(dir, "cgroup.events"), "populated 0\n")

+ writeFixture(t, filepath.Join(dir, "cgroup.procs"), "")

+ c := fixtureCleanup(t, m, host)

+ done := m.beginPlacement(filepath.Join(dir, "g1"))

+ defer done()

+ if populated, err := c.Populated(); err != nil || !populated {

+ t.Fatalf("Populated = %v, %v with a placement in flight: want true", populated, err)

+ }

+ if members, err := c.Members(); !errors.Is(err, ErrPlacementInFlight) {

+ t.Fatalf("Members = %v, %v with a placement in flight: want ErrPlacementInFlight", members, err)

+ }

+ ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond)

+ defer cancel()

+ if err := c.KillAndEmpty(ctx); err == nil {

+ t.Fatal("KillAndEmpty proved the host empty with a placement in flight")

+ }

+ if got := readFixture(t, filepath.Join(dir, "cgroup.kill")); got != "" {

+ t.Fatalf("cgroup.kill = %q: the kill was written before the placement resolved", got)

+ }

+ done()

+ if populated, err := c.Populated(); err != nil || populated {

+ t.Fatalf("Populated = %v, %v once the placement resolved: want cgroup.events' answer, false", populated, err)

+ }

+ if members, err := c.Members(); err != nil || len(members) != 0 {

+ t.Fatalf("Members = %v, %v once the placement resolved: want none", members, err)

+ }

+}

+

+// Ruling 6: a placement that registers after KillAndEmpty's wait, here

+// right after the kill is written, keeps the proof from finishing: every

+// emptiness check after the kill counts it. The fake cgroupfs says empty

+// throughout, so only the registry stands between the proof and a false

+// empty.

+func TestKillAndEmptySeesAPlacementRegisteredMidProof(t *testing.T) {

+ m, host, dir := cleanupFixture(t)

+ writeFixture(t, filepath.Join(dir, "cgroup.events"), "populated 0\n")

+ writeFixture(t, filepath.Join(dir, "cgroup.procs"), "")

+ c := fixtureCleanup(t, m, host)

+ var done func()

+ orig := cleanupKillWrite

+ cleanupKillWrite = func(f *os.File) error {

+ err := orig(f)

+ done = m.beginPlacement(filepath.Join(dir, "g1"))

+ return err

+ }

+ t.Cleanup(func() {

+ cleanupKillWrite = orig

+ if done != nil {

+ done()

+ }

+ })

+ ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)

+ defer cancel()

+ if err := c.KillAndEmpty(ctx); err == nil {

+ t.Fatal("KillAndEmpty proved the host empty while a placement registered mid-proof was in flight")

+ }

+ if done == nil {

+ t.Fatal("the kill was never written: the test proves nothing")

+ }

+ // The control: with nothing in flight the same proof finishes.

+ done()

+ ctx2, cancel2 := context.WithTimeout(t.Context(), time.Second)

+ defer cancel2()

+ cleanupKillWrite = orig

+ if err := c.KillAndEmpty(ctx2); err != nil {

+ t.Fatalf("KillAndEmpty with nothing in flight = %v, want the proof", err)

+ }

+}

diff --git a/internal/cgroup/gen.go b/internal/cgroup/gen.go

index 91b9dd172..0b7b604be 100644

--- a/internal/cgroup/gen.go

+++ b/internal/cgroup/gen.go

@@ -138,12 +138,13 @@ var ErrLegacyParentPopulated = errors.New("cgroup: the host's parent cgroup hold

// controllers could not be delegated (or a live parent does not delegate

// memory), the parent's backstop could not be written, the generation's own

// child could not be made or one of its four limit writes failed, or the

-// clone3 CLONE_INTO_CGROUP placement is unavailable, so the process could

-// only be started outside its cgroup and moved after, or it failed on the

-// placement or the box rather than the command (placementFailed). The flat

-// start (StartGated) stays best-effort and never returns it. Nothing is

-// exec'd, and it is no fault of the build: like ErrLegacyParentPopulated,

-// the condition is the box's (Task 7, zdd flag readiness round 2).

+// placement shim is not available in this process, so the process could

+// only be started outside its cgroup and moved after, or the shim could

+// not place it before exec (a placement error, never the command's own

+// failure). The flat start (StartGated) stays best-effort and never

+// returns it. Nothing is exec'd, and it is no fault of the build: like

+// ErrLegacyParentPopulated, the condition is the box's (Task 7, zdd flag

+// readiness round 2).

var ErrLimitNotApplied = errors.New("cgroup: a generation's limits could not be applied")

// delegatesMemory reports whether dir hands the memory controller to its

@@ -182,10 +183,13 @@ func parentProcs(dir string) (int, error) {

// ensureParent makes carlos-exec@<host> a generation parent. It first

// refuses a parent that holds a process directly, whatever children it has

// (plan review #12): a stale empty g<n> beside a legacy process does not make

-// the parent one. Then, when nothing is alive under it (a cold wake), it

-// delegates the controllers and writes the tier with memory.oom.group=0. With

-// a live generation under it, the parent is left as it is, because a swap

-// raised it on purpose, once it is shown to delegate memory to its children.

+// the parent one. A process being placed (the in-flight registry) counts as

+// one already there: into the parent itself, as a direct process; anywhere

+// below it, as something alive under it. Then, when nothing is alive under

+// it (a cold wake), it delegates the controllers and writes the tier with

+// memory.oom.group=0. With a live generation under it, the parent is left

+// as it is, because a swap raised it on purpose, once it is shown to

+// delegate memory to its children.

//

// Nothing here is best-effort (Task 7): a generation started past any of

// these failures would run without a limit, with nothing to say so. A

@@ -206,11 +210,16 @@ func (m *Manager) ensureParent(host string, tier Limit) error {

m.logf("cgroup: %s: generation start refused: cannot read the parent's cgroup.procs: %v", host, err)

return fmt.Errorf("%w (cgroup.procs unreadable, treated as populated): %v", ErrLegacyParentPopulated, err)

}

+ // A flat start being placed into the parent itself is a direct process

+ // the moment it lands (Ruling 6): count it now.

+ n += m.placementsAt(dir)

if n > 0 {

m.logf("cgroup: %s: generation start refused: the parent cgroup holds %d process(es) directly, so no memory limit could apply to a generation under it; stop them or let them exit", host, n)

return fmt.Errorf("%w: %s holds %d process(es)", ErrLegacyParentPopulated, host, n)

}

- if parentPopulated(dir) {

+ // A sibling generation being placed is a live one: the parent is not

+ // cold, and its limits (a swap may have raised them) are left alone.

+ if parentPopulated(dir) || m.placementsUnder(dir) > 0 {

if err := delegatesMemory(dir); err != nil {

m.logf("cgroup: %s: generation start refused: %v", host, err)

return fmt.Errorf("%s: %w", host, err)

@@ -247,12 +256,13 @@ func (m *Manager) StartChild(host, child string, parent, own Limit, build func()

}

// StartChildGated is StartChild with StartGated's before-start hook: asked

-// immediately before each start attempt, after the parent and the child

-// are prepared. A refusal starts nothing, leaves the child removed (if

-// empty) and the parent as a failed start leaves it, and comes back as

-// before's own error. A nil before is StartChild exactly. A parent that

-// holds a process directly refuses the start with ErrLegacyParentPopulated

-// before anything is created or exec'd (ensureParent).

+// after the parent and the child are prepared and the process is placed in

+// the child, immediately before it execs. A refusal runs nothing of the

+// command, leaves the child removed (if empty) and the parent as a failed

+// start leaves it, and comes back as before's own error. A nil before is

+// StartChild exactly. A parent that holds a process directly refuses the

+// start with ErrLegacyParentPopulated before anything is created or exec'd

+// (ensureParent).

func (m *Manager) StartChildGated(host, child string, parent, own Limit, build func() *exec.Cmd, before func() error) (*exec.Cmd, error) {

if !isChildName(child) {

return nil, fmt.Errorf("cgroup: %s: %q is not a child cgroup name", host, child)

diff --git a/internal/cgroup/gen_kernel_linux_test.go b/internal/cgroup/gen_kernel_linux_test.go

index 0881c68f2..063f3836b 100644

--- a/internal/cgroup/gen_kernel_linux_test.go

+++ b/internal/cgroup/gen_kernel_linux_test.go

@@ -104,7 +104,9 @@ func TestGenKernelChild(t *testing.T) {

fmt.Fprintln(os.Stderr, "hog: survived 1 GiB: not confined")

os.Exit(3)

default:

- t.Skip("synthetic child only")

+ // Synthetic child only. It returns rather than skips, so a kernel

+ // run's -test.run 'GenKernel' shows no SKIP line (as TestShimKernelChild).

+ return

}

@@ -478,3 +480,281 @@ func TestGenStrictKernel(t *testing.T) {

}

m.CleanupHost(host)

}

+

+// sleepKid starts nothing itself: it is the build for a tenant that sleeps

+// until killed.

+func sleepKid() *exec.Cmd { return exec.Command("/bin/sleep", "60") }

+

+// staysUp fails the test if k's process exits within a moment of its start

+// (amadan #272's symptom: a child SIGKILLed as it lands), and checks that

+// the process is the tenant, exec'd, in want (a path below /sys/fs/cgroup).

+func staysUp(t *testing.T, label string, k *kernelKid, want string) {

+ t.Helper()

+ select {

+ case <-k.reaped():

+ t.Fatalf("%s: the child died as it landed: %v", label, k.err)

+ case <-time.After(500 * time.Millisecond):

+ }

+ pid := k.cmd.Process.Pid

+ rel, err := cgroupOf(pid)

+ if err != nil || "/sys/fs/cgroup/"+rel != want {

+ t.Fatalf("%s: pid %d runs in %q (%v), want %s", label, pid, rel, err, want)

+ }

+ if comm, err := os.ReadFile(fmt.Sprintf("/proc/%d/comm", pid)); err != nil || strings.TrimSpace(string(comm)) != "sleep" {

+ t.Fatalf("%s: pid %d is %q (%v), want the exec'd tenant, sleep", label, pid, comm, err)

+ }

+}

+

+func inodeOf(t *testing.T, path string) uint64 {

+ t.Helper()

+ var st syscall.Stat_t

+ if err := syscall.Lstat(path, &st); err != nil {

+ t.Fatalf("stat %s: %v", path, err)

+ }

+ return st.Ino

+}

+

+// The respawn scenario amadan #272 found with clone3 (a clone3 into a

+// cgroup that took cgroup.kill is SIGKILLed at once), now through the

+// shim: a flat start (a worker's) lands in carlos-exec@<host> placed before

+// exec, a cleanup kills and proves it empty, and each respawn gets a fresh

+// cgroup (new inode, the tier written again) and stays up as the tenant.

+// The activator's TestClaimContainmentCleanupRespawn runs the same sequence

+// through ExecBackend.KillHost, but needs a delegation with io to arm

+// cgroup.Setup; this one runs under a user manager's (cpu memory pids).

+func TestGenKernelShimRespawnAfterKill(t *testing.T) {

+ m := genKernelManager(t)

+ const host = "respawn.example"

+ dir := m.hostDir(host)

+ t.Cleanup(func() { m.CleanupHost(host) })

+ tier := strconv.FormatInt(TierDefault.MemoryMax, 10)

+ var last uint64

+ for attempt := range 3 {

+ label := fmt.Sprintf("spawn %d", attempt)

+ cmd, err := m.Start(host, TierDefault, sleepKid)

+ if err != nil {

+ t.Fatalf("%s: %v", label, err)

+ }

+ k := &kernelKid{cmd: cmd, done: make(chan struct{})}

+ t.Cleanup(func() { _ = cmd.Process.Kill(); <-k.reaped() })

+ staysUp(t, label, k, dir)

+ ino := inodeOf(t, dir)

+ if attempt > 0 && ino == last {

+ t.Fatalf("%s reused the killed host cgroup", label)

+ }

+ last = ino

+ if got := strings.TrimSpace(readFixture(t, filepath.Join(dir, "memory.max"))); got != tier {

+ t.Fatalf("%s: memory.max = %q, want the tier", label, got)

+ }

+ c, err := m.OpenCleanup(host)

+ if err != nil {

+ t.Fatal(err)

+ }

+ ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second)

+ err = c.KillAndEmpty(ctx)

+ cancel()

+ _ = c.Close()

+ if err != nil {

+ t.Fatalf("%s: KillAndEmpty: %v", label, err)

+ }

+ select {

+ case <-k.reaped():

+ case <-time.After(5 * time.Second):

+ t.Fatalf("%s: the killed child was not reaped", label)

+ }

+}

+

+// amadan #272, real-unit check (d), through the shim: a flat start puts its

+// process directly in carlos-exec@<host>, and a generation start beside it

+// is refused (ErrLegacyParentPopulated) before it builds anything or makes

+// a g* child. The recovery control: once the direct member has exited, the

+// same host starts a capped g1 under a parent that holds no process.

+func TestGenKernelGenerationRefusedBesideAFlatMember(t *testing.T) {

+ m := genKernelManager(t)

+ const host = "legacy.example"

+ parent := m.hostDir(host)

+ t.Cleanup(func() {

+ m.RemoveChild(host, GenChild(1))

+ m.CleanupHost(host)

+ })

+ legacy, err := m.Start(host, TierDefault, sleepKid)

+ if err != nil {

+ t.Fatal(err)

+ }

+ lk := &kernelKid{cmd: legacy, done: make(chan struct{})}

+ t.Cleanup(func() { _ = legacy.Process.Kill(); <-lk.reaped() })

+ staysUp(t, "flat", lk, parent)

+

+ built := false

+ cmd, err := m.StartChild(host, GenChild(1), TierDefault, TierDefault, func() *exec.Cmd { built = true; return sleepKid() })

+ if !errors.Is(err, ErrLegacyParentPopulated) || cmd != nil || built {

+ t.Fatalf("StartChild beside a flat member = %v, %v (built %v): want ErrLegacyParentPopulated, nothing built", cmd, err, built)

+ }

+ if _, err := os.Stat(filepath.Join(parent, GenChild(1))); !os.IsNotExist(err) {

+ t.Fatalf("the refused start made g1: %v", err)

+ }

+

+ _ = legacy.Process.Kill()

+ <-lk.reaped()

+ for deadline := time.Now().Add(5 * time.Second); strings.TrimSpace(readFixture(t, filepath.Join(parent, "cgroup.procs"))) != ""; time.Sleep(20 * time.Millisecond) {

+ if time.Now().After(deadline) {

+ t.Fatal("the parent still holds the killed flat process")

+ }

+ g1, err := m.StartChild(host, GenChild(1), TierDefault, TierDefault, sleepKid)

+ if err != nil {

+ t.Fatalf("g1 once the flat member exited: %v", err)

+ }

+ gk := reapOnCleanup(t, m, host, GenChild(1), g1)

+ staysUp(t, "g1", gk, filepath.Join(parent, GenChild(1)))

+ if got := strings.TrimSpace(readFixture(t, filepath.Join(parent, GenChild(1), "memory.max"))); got != strconv.FormatInt(TierDefault.MemoryMax, 10) {

+ t.Errorf("g1 memory.max = %q, want the tier", got)

+ }

+ if got := strings.TrimSpace(readFixture(t, filepath.Join(parent, "cgroup.procs"))); got != "" {

+ t.Errorf("the parent holds processes directly: %q", got)

+ }

+}

+

+// Review focus 4 on a real kernel: a generation's shim held before its

+// placement is registered in flight and still in the agent's own cgroup, so

+// the kernel says the host is empty. The host's cleanup must not: Populated

+// is true, Members is ErrPlacementInFlight, and KillAndEmpty waits, killing

+// nothing, until the placement resolves (here: the shim is SIGKILLed, so

+// the start is refused as a placement failure) and only then proves the

+// host empty.

+func TestGenKernelCleanupWaitsForAPlacementInFlight(t *testing.T) {

+ m := genKernelManager(t)

+ const host = "inflight.example"

+ parent := m.hostDir(host)

+ t.Cleanup(func() {

+ m.RemoveChild(host, GenChild(1))

+ m.CleanupHost(host)

+ })

+ setShimVar(t, &shimTestHold, 1)

+ held := make(chan int, 1)

+ orig := startCmd

+ t.Cleanup(func() { startCmd = orig })

+ startCmd = func(cmd *exec.Cmd) error {

+ if err := orig(cmd); err != nil {

+ return err

+ }

+ pid, specFD := cmd.Process.Pid, len(cmd.ExtraFiles) // the shim's spec fd: see TestShimKernelSignalBeforePlacement

+ go func() {

+ if waitShimHeld(pid, specFD) {

+ held <- pid

+ } else {

+ held <- 0

+ }

+ }()

+ return nil

+ }

+ started := make(chan error, 1)

+ go func() {

+ _, err := m.StartChild(host, GenChild(1), TierDefault, TierDefault, sleepKid)

+ started <- err

+ }()

+ pid := <-held

+ if pid == 0 {

+ t.Fatal("the shim never reached its hold before placement")

+ }

+ t.Cleanup(func() { _ = syscall.Kill(pid, syscall.SIGKILL) })

+

+ c, err := m.OpenCleanup(host)

+ if err != nil {

+ t.Fatal(err)

+ }

+ t.Cleanup(func() { _ = c.Close() })

+ events := readFixture(t, filepath.Join(parent, "cgroup.events"))

+ if !strings.Contains(events, "populated 0") {

+ t.Fatalf("the parent's cgroup.events = %q: want the kernel to see it empty, or this proves nothing", events)

+ }

+ if populated, err := c.Populated(); err != nil || !populated {

+ t.Fatalf("Populated = %v, %v with a shim held before placement: want true", populated, err)

+ }

+ if _, err := c.Members(); !errors.Is(err, ErrPlacementInFlight) {

+ t.Fatalf("Members = %v, want ErrPlacementInFlight", err)

+ }

+ ctx, cancel := context.WithTimeout(t.Context(), 200*time.Millisecond)

+ err = c.KillAndEmpty(ctx)

+ cancel()

+ if !errors.Is(err, ErrPlacementInFlight) || !errors.Is(err, context.DeadlineExceeded) {

+ t.Fatalf("KillAndEmpty with a placement in flight = %v: want ErrPlacementInFlight, unproven", err)

+ }

+ if procGone(pid) {

+ t.Fatal("the held shim died: the unproven proof killed it, or the hold did not hold")

+ }

+

+ proof := make(chan error, 1)

+ go func() {

+ ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)

+ defer cancel()

+ proof <- c.KillAndEmpty(ctx)

+ }()

+ select {

+ case err := <-proof:

+ t.Fatalf("KillAndEmpty returned %v while the placement was still in flight", err)

+ case <-time.After(200 * time.Millisecond):

+ }

+ if err := syscall.Kill(pid, syscall.SIGKILL); err != nil {

+ t.Fatal(err)

+ }

+ if err := <-started; !errors.Is(err, ErrLimitNotApplied) || !isPlacementError(err) {

+ t.Fatalf("the generation start = %v, want ErrLimitNotApplied from the placement failure", err)

+ }

+ if err := <-proof; err != nil {

+ t.Fatalf("KillAndEmpty once the placement resolved = %v, want the proof", err)

+ }

+ assertNothingInFlight(t, m)

+}

+

+// A generation start whose exec.CommandContext context ends while its shim

+// is held before placement: exec.Cmd kills the shim, nothing runs, the

+// start is not the box's condition (not ErrLimitNotApplied), the empty

+// child is removed, and nothing is left in flight.

+func TestGenKernelCancelledGenerationStartIsNotABoxCondition(t *testing.T) {

+ m := genKernelManager(t)

+ const host = "cancel.example"

+ parent := m.hostDir(host)

+ t.Cleanup(func() {

+ m.RemoveChild(host, GenChild(1))

+ m.CleanupHost(host)

+ })

+ setShimVar(t, &shimTestHold, 1)

+ ctx, cancel := context.WithCancel(t.Context())

+ defer cancel()

+ held := make(chan bool, 1)

+ orig := startCmd

+ t.Cleanup(func() { startCmd = orig })

+ startCmd = func(cmd *exec.Cmd) error {

+ if err := orig(cmd); err != nil {

+ return err

+ }

+ pid, specFD := cmd.Process.Pid, len(cmd.ExtraFiles)

+ go func() {

+ ok := waitShimHeld(pid, specFD)

+ held <- ok

+ cancel()

+ }()

+ return nil

+ }

+ marker := filepath.Join(t.TempDir(), "ran")

+ var built *exec.Cmd

+ cmd, err := m.StartChild(host, GenChild(1), TierDefault, TierDefault, func() *exec.Cmd {

+ built = exec.CommandContext(ctx, "/bin/sh", "-c", `touch "$0"; sleep 60`, marker)

+ return built

+ })

+ if !<-held {

+ t.Fatal("the shim never reached its hold before placement")

+ }

+ if err == nil || cmd != nil || errors.Is(err, ErrLimitNotApplied) {

+ t.Fatalf("StartChild = %v, %v: want a failure that is not ErrLimitNotApplied", cmd, err)

+ }

+ t.Logf("cancelled generation start: %v", err)

+ assertReaped(t, built)

+ assertNoMarker(t, marker)

+ assertNothingInFlight(t, m)

+ if _, err := os.Stat(filepath.Join(parent, GenChild(1))); !os.IsNotExist(err) {

+ t.Fatalf("the empty g1 survived: %v", err)

+ }

+}

diff --git a/internal/cgroup/gen_test.go b/internal/cgroup/gen_test.go

index 90669225e..7bfdb716d 100644

--- a/internal/cgroup/gen_test.go

+++ b/internal/cgroup/gen_test.go

@@ -1,6 +1,7 @@

package cgroup

import (

+ "context"

"errors"

"fmt"

"os"

@@ -40,8 +41,18 @@ func readFixture(t *testing.T, path string) string {

}

// genManager is a delegated fixture root whose host dir already lists the

-// four controllers, as a real child of the delegated root would.

+// four controllers, as a real child of the delegated root would. A temp

+// directory is no cgroupfs, so no shim can place a process there: the

+// placement is faked (FakePlacementForTest), as if the kernel had placed

+// it. genFixture is the same root with the real placement.

func genManager(t *testing.T, host string) (*Manager, string) {

+ t.Helper()

+ m, parent := genFixture(t, host)

+ FakePlacementForTest(t)

+ return m, parent

+}

+

+func genFixture(t *testing.T, host string) (*Manager, string) {

t.Helper()

quietOOM(t)

m, err := SetupAt(newDelegatedRoot(t), t.Logf)

@@ -707,16 +718,16 @@ func TestARefusedGenerationsLimitFailureLogsTheRefusalOnly(t *testing.T) {

}

-// G: with the clone3 CLONE_INTO_CGROUP placement unavailable, a generation

-// could only be started and then moved by PID: running, for that window,

-// in the agent's cgroup with no limit, and for good if the move fails.

-// A latched fallback refuses before the child is created or build() runs

-// (ensureParent has already run). The flat start keeps the PID-move

-// fallback.

-func TestAGenerationIsNeverStartedThenMovedWhenTheFallbackIsLatched(t *testing.T) {

+// G: with no shim in this process (a binary whose main lacks

+// RunShimIfRequested) a generation could only be started and then moved by

+// PID: running, for that window, in the agent's cgroup with no limit, and

+// for good if the move fails. It is refused before the child is created

+// or build() runs (ensureParent has already run). The flat start keeps the

+// PID-move start.

+func TestGenerationRefusedWithoutTheShim(t *testing.T) {

const host = "h.example.com"

- m, parent := genManager(t, host)

- m.fallback.Store(true)

+ m, parent := genFixture(t, host)

+ noShim(t)

for _, child := range []string{GenChild(1), RoleChild("seed")} {

own := Limit{}

if child == GenChild(1) {

@@ -727,30 +738,166 @@ func TestAGenerationIsNeverStartedThenMovedWhenTheFallbackIsLatched(t *testing.T

}

calls := fakeStart(t, func(*exec.Cmd) error { return nil })

if cmd, err := m.Start("flat.example.com", TierDefault, buildTrue); err != nil || cmd == nil || len(*calls) != 1 {

- t.Fatalf("flag-off Start with the latch = %v, %v after %d starts: want the PID-move start", cmd, err, len(*calls))

+ t.Fatalf("flag-off Start without the shim = %v, %v after %d starts: want the PID-move start", cmd, err, len(*calls))

}

if _, err := os.Stat(filepath.Join(m.hostDir("flat.example.com"), "cgroup.procs")); err != nil {

t.Fatalf("flag-off Start did not move the PID: %v", err)

}

-// G: the child cgroup cannot be opened for the clone. A flat start would

-// fall back to start-and-move for this spawn; a generation start refuses

-// before it builds the command, and drops the child it made.

-func TestAGenerationWhoseCgroupCannotBeOpenedIsRefused(t *testing.T) {

- if !intoCgroupSupported {

- t.Skip("no clone3 placement on this platform; every generation start refuses")

+// noShim makes this test binary one without RunShimIfRequested until t's

+// cleanup.

+func noShim(t *testing.T) {

+ t.Helper()

+ hooked, forced := shimHooked.Swap(false), shimForcedForTest.Swap(false)

+ t.Cleanup(func() {

+ shimHooked.Store(hooked)

+ shimForcedForTest.Store(forced)

+ })

+}

+

+// stubPlacement swaps the placement seam until t's cleanup.

+func stubPlacement(t *testing.T, f func(m *Manager, dir string, cmd *exec.Cmd, before func() error) (bool, error)) {

+ t.Helper()

+ orig := placeStart

+ placeStart = f

+ t.Cleanup(func() { placeStart = orig })

+}

+

+// The fixture's limit files are ordinary files, which keep a refused

+// child's rmdir from succeeding (CleanupHost's comment). The removal tests

+// below start their generation with no limit of its own, so the child is

+// empty when it should go.

+

+// Review focus 5: a missing binary is the command's own failure. The shim

+// answers fork/exec's own error; the start returns it as is, and it is not

+// the box's condition, so it is not ErrLimitNotApplied. The empty child goes.

+func TestGenerationMissingBinaryIsNotABoxCondition(t *testing.T) {

+ const host = "h.example.com"

+ m, parent := genManager(t, host)

+ missing := &os.PathError{Op: "fork/exec", Path: "/x", Err: syscall.ENOENT}

+ stubPlacement(t, func(*Manager, string, *exec.Cmd, func() error) (bool, error) { return false, missing })

+ builds := 0

+ cmd, err := m.StartChild(host, GenChild(1), TierDefault, Limit{}, func() *exec.Cmd { builds++; return buildTrue() })

+ if err != missing || cmd != nil {

+ t.Fatalf("StartChild = %v, %v: want the command's own error, unwrapped, and no command", cmd, err)

+ }

+ if errors.Is(err, ErrLimitNotApplied) {

+ t.Fatal("a missing binary read as the box's condition")

}

- if os.Geteuid() == 0 {

- t.Skip("root opens a mode-0 directory")

+ if builds != 1 {

+ t.Fatalf("built %d times, want once: the command's own failure is never retried", builds)

}

+ if _, err := os.Stat(filepath.Join(parent, "g1")); !os.IsNotExist(err) {

+ t.Fatalf("the empty child survived a failed start: %v", err)

+ }

+}

+

+// A placement failure is the box's condition: ErrLimitNotApplied, wrapping

+// the shim's answer, with no retry (a generation is never started then

+// moved) and the empty child removed.

+func TestGenerationPlacementFailureIsLimitNotApplied(t *testing.T) {

+ const host = "h.example.com"

+ m, parent, lines := legacyParent(t, host, "")

+ failed := &placementError{errors.New("the shim exited before placement")}

+ stubPlacement(t, func(*Manager, string, *exec.Cmd, func() error) (bool, error) { return false, failed })

+ calls := fakeStart(t, func(*exec.Cmd) error { return nil })

+ builds := 0

+ cmd, err := m.StartChild(host, GenChild(1), TierDefault, Limit{}, func() *exec.Cmd { builds++; return buildTrue() })

+ if !errors.Is(err, ErrLimitNotApplied) || !errors.Is(err, failed) || cmd != nil {

+ t.Fatalf("StartChild = %v, %v: want ErrLimitNotApplied wrapping the placement error, and no command", cmd, err)

+ }

+ if builds != 1 || len(*calls) != 0 {

+ t.Fatalf("built %d, started %d: want one build and no start-and-move", builds, len(*calls))

+ }

+ if _, err := os.Stat(filepath.Join(parent, "g1")); !os.IsNotExist(err) {

+ t.Fatalf("the empty child survived a failed placement: %v", err)

+ }

+ if !slices.ContainsFunc(*lines, func(l string) bool {

+ return strings.Contains(l, "placement before exec failed") && strings.Contains(l, "generation start refused")

+ }) {

+ t.Errorf("log lines = %q, want the refusal line", *lines)

+ }

+}

+

+// Ruling 6: a sibling generation being placed is something alive under the

+// parent, so a cold-parent reset (enable the controllers, write the tier)

+// must not run: a swap may have raised that parent on purpose.

+func TestEnsureParentCountsAPlacementBelowTheParent(t *testing.T) {

const host = "h.example.com"

m, parent := genManager(t, host)

- seed := filepath.Join(parent, RoleChild("seed"))

- if err := os.Mkdir(seed, 0o000); err != nil { // open: EACCES

- t.Fatal(err)

+ fakeStart(t, func(*exec.Cmd) error { return nil })

+ writeFixture(t, filepath.Join(parent, "cgroup.events"), "populated 0\nfrozen 0\n")

+ writeFixture(t, filepath.Join(parent, "cgroup.subtree_control"), "cpu memory io pids\n")

+ writeFixture(t, filepath.Join(parent, "memory.max"), "1073741824")

+ done := m.beginPlacement(filepath.Join(parent, GenChild(1)))

+ defer done()

+ if _, err := m.StartChildGated(host, GenChild(2), TierDefault, TierDefault, buildTrue, nil); err != nil {

+ t.Fatalf("StartChildGated beside a generation being placed: %v", err)

+ }

+ if got := readFixture(t, filepath.Join(parent, "memory.max")); got != "1073741824" {

+ t.Errorf("the parent's memory.max was rewritten to %q: a placement in flight was taken for a cold parent", got)

+ }

+ if got := readFixture(t, filepath.Join(parent, "cgroup.subtree_control")); got != "cpu memory io pids\n" {

+ t.Errorf("the parent's subtree_control was rewritten to %q", got)

+ }

+}

+

+// Ruling 6: a flat start being placed into the host directory itself is a

+// direct process of the parent, so a generation start is refused as for a

+// legacy parent, before anything is built.

+func TestEnsureParentCountsAPlacementAtTheParent(t *testing.T) {

+ const host = "h.example.com"

+ m, parent := genManager(t, host)

+ done := m.beginPlacement(parent)

+ defer done()

+ err := refusedChild(t, m, host, GenChild(1), TierDefault, ErrLegacyParentPopulated)

+ if !strings.Contains(err.Error(), "1 process") {

+ t.Errorf("refusal %q, want it to count the placement as one process", err)

}

- t.Cleanup(func() { _ = os.Chmod(seed, 0o755) })

- refusedChild(t, m, host, RoleChild("seed"), Limit{}, ErrLimitNotApplied)

noGenerationChild(t, parent)

}

+

+// A generation start whose exec.CommandContext context ends during the

+// placement (a seed's wake timing out, the agent stopping) is the caller's

+// condition, not the box's: the shim reports it as a placement failure,

+// but the start must not come back as ErrLimitNotApplied. Two ways in:

+// the context had ended before the shim could start (Start answers its

+// error, which the placement error wraps), and it ends while the shim is

+// being placed (exec.Cmd's Cancel kills the shim, whose answer names no

+// context).

+func TestGenerationCancelledStartIsNotABoxCondition(t *testing.T) {

+ const host = "h.example.com"

+ for name, place := range map[string]func(*Manager, string, *exec.Cmd, func() error) (bool, error){

+ "ended before the shim started": func(*Manager, string, *exec.Cmd, func() error) (bool, error) {

+ return false, &placementError{fmt.Errorf("start the shim: %w", context.Canceled)}

+ },

+ "ended during the placement": func(_ *Manager, _ string, cmd *exec.Cmd, _ func() error) (bool, error) {

+ _ = cmd.Cancel() // what exec.Cmd's context watcher does

+ return false, &placementError{errors.New("the shim exited before placement")}

+ },

+ } {

+ t.Run(name, func(t *testing.T) {

+ m, parent, lines := legacyParent(t, host, "")

+ stubPlacement(t, place)

+ ctx, cancel := context.WithCancel(t.Context())

+ cancel()

+ cmd, err := m.StartChild(host, GenChild(1), TierDefault, Limit{}, func() *exec.Cmd {

+ c := exec.CommandContext(ctx, "/bin/true")

+ c.Cancel = func() error { return nil } // no process to kill here

+ return c

+ })

+ if err == nil || cmd != nil || errors.Is(err, ErrLimitNotApplied) {

+ t.Fatalf("StartChild = %v, %v: want a failure that is not ErrLimitNotApplied", cmd, err)

+ }

+ if _, serr := os.Stat(filepath.Join(parent, "g1")); !os.IsNotExist(serr) {

+ t.Fatalf("the empty child survived: %v", serr)

+ }

+ for _, l := range *lines {

+ if strings.Contains(l, "generation start refused") {

+ t.Errorf("a cancelled start logged a box condition: %q", l)

+ }

+ })

+ }

+}

diff --git a/internal/cgroup/inflight.go b/internal/cgroup/inflight.go

index fa955bf90..f86596b96 100644

--- a/internal/cgroup/inflight.go

+++ b/internal/cgroup/inflight.go

@@ -11,7 +11,10 @@ import (

// placed into, from just before its fork until it answers placed or the

// start fails. With clone3 a child appeared in its cgroup atomically; a

// shim spends a few milliseconds in the agent's cgroup first, so a proof

-// that a cgroup is empty must count these too (Ruling 6, Task 2).

+// that a cgroup is empty must count these too (Ruling 6, Task 2). The flat

+// PID-move fallback (startAndMoveIn) registers its target as well, from

+// its start until its PID write has returned. Readers: Cleanup's

+// population and members (cleanup_linux.go) and ensureParent (gen.go).

// beginPlacement registers one placement into dir. done ends it, and is

// idempotent: a second call does nothing. A nil manager registers nothing.

diff --git a/internal/cgroup/recreate_linux.go b/internal/cgroup/recreate_linux.go

index ec730ff40..a9c0b2165 100644

--- a/internal/cgroup/recreate_linux.go

+++ b/internal/cgroup/recreate_linux.go

@@ -17,7 +17,11 @@ import (

// Found on the dev box (arm64, kernel 6.12.105, systemd 257): once

// cgroup.kill has been written to a cgroup, every later

// clone3(CLONE_INTO_CGROUP) into that same cgroup is SIGKILLed at once,

-// while a cgroup made fresh at the same path is not. Generation kills

+// while a cgroup made fresh at the same path is not. The Manager's starts

+// now place through the exec-shim, a cgroup.procs write rather than a

+// clone3, but the strict claims path still clones, and a fresh cgroup is

+// also the one place a kept cgroup's limits are written again, so every

+// start still makes a killed cgroup fresh first. Generation kills

// (KillGen, CloseRemove) remove their child and never meet it. The kills

// that keep their cgroup are a worker's host cgroup (KillHost) and the

// legacy sidecar's role child (KillSidecar); a strict group's own kill

@@ -115,11 +119,11 @@ func pathIdentity(path string) (id strictIdentity, ok bool, err error) {

// the host's start lock (hostLock), so two starts of one host never both

// remove: the first removes the killed cgroup, and the second finds the

// fresh one, whose identity took no kill, and keeps it. The lock is held

-// only for the check and the removal, not across the start's clone3: a

+// only for the check and the removal, not across the start's placement: a

// start's own directory is never removed by another start, because only a

// killed identity is removed and every start checks under this lock first.

// What it does not order is a kill landing while a start is between its

-// check and its clone3; the activator never does that (a kept cgroup is

+// check and its placement; the activator never does that (a kept cgroup is

// killed only once its leader has exited, and nothing of that host or role

// is started until the proof returns).

func (m *Manager) freshen(host, dir string) error {

diff --git a/internal/cgroup/recreate_linux_test.go b/internal/cgroup/recreate_linux_test.go

index 69707f6ee..44e69365f 100644

--- a/internal/cgroup/recreate_linux_test.go

+++ b/internal/cgroup/recreate_linux_test.go

@@ -483,6 +483,7 @@ func TestARetainingSetupMakesEmptyLeftoversFresh(t *testing.T) {

}

removed := fakeCgroupRmdir(t, nil)

fakeStart(t, func(*exec.Cmd) error { return nil })

+ FakePlacementForTest(t) // a temp dir is no cgroupfs, and a role child is never moved by PID

if _, err := m.Start("worker.example.com", TierDefault, buildTrue); err != nil {

t.Fatal(err)

diff --git a/internal/cgroup/shim.go b/internal/cgroup/shim.go

index a02bca75c..ce7efb1aa 100644

--- a/internal/cgroup/shim.go

+++ b/internal/cgroup/shim.go

@@ -26,6 +26,12 @@ const ProbeVerb = "__cgprobe"

var shimHooked atomic.Bool

+// shimForcedForTest makes shimAvailable answer true while

+// FakePlacementForTest has swapped the placement for one that runs no shim.

+// startViaShim itself still asks shimHooked, so the override can never make

+// a test binary without the hook start itself as a shim.

+var shimForcedForTest atomic.Bool

+

// RunShimIfRequested must be the first statement of main(), and of TestMain

// in a test binary that places processes. It marks this process as able to

// serve the shim (shimAvailable), and when this process IS a shim or a

diff --git a/internal/cgroup/shim_linux.go b/internal/cgroup/shim_linux.go

index 3fac628a9..1dd8cad71 100644

--- a/internal/cgroup/shim_linux.go

+++ b/internal/cgroup/shim_linux.go

@@ -24,8 +24,9 @@ import (

)

// shimAvailable reports that RunShimIfRequested ran in this process, so

-// /proc/self/exe will serve ShimVerb.

-func shimAvailable() bool { return shimHooked.Load() }

+// /proc/self/exe will serve ShimVerb, or that FakePlacementForTest stands

+// in for the shim.

+func shimAvailable() bool { return shimHooked.Load() || shimForcedForTest.Load() }

// startViaShim starts cmd so that its process is in dir before any tenant

// code runs: the child starts as this binary (shimExe ShimVerb), writes its

@@ -44,7 +45,9 @@ func shimAvailable() bool { return shimHooked.Load() }

// the status stream proves no R; a start that may have exec'd is reported

// started, so its caller owns the process and nothing runs a tenant twice.

func (m *Manager) startViaShim(dir string, cmd *exec.Cmd, before func() error) (refused bool, err error) {

- if !shimAvailable() {

+ // The hook itself, not shimAvailable: FakePlacementForTest's override

+ // must never send a binary without the hook to start itself as a shim.

+ if !shimHooked.Load() {

return false, &placementError{errors.New("the placement shim is not available in this process")}

}

if cmd.Err != nil {

diff --git a/internal/cgroup/shim_other.go b/internal/cgroup/shim_other.go

index a1842c134..7e8ed7329 100644

--- a/internal/cgroup/shim_other.go

+++ b/internal/cgroup/shim_other.go

@@ -9,7 +9,9 @@ import (

// The placement shim needs cgroup v2, so it is linux only.

-func shimAvailable() bool { return false }

+// shimAvailable is true here only while FakePlacementForTest stands in for

+// the shim, which it never runs.

+func shimAvailable() bool { return shimForcedForTest.Load() }

func runShim([]string) int { return 127 }

carlos / platform Public

cgroup: every Manager start places through the shim; placements in flight count as members (#272)