| index 10f2ede05..30652589e 100644 |
| --- a/internal/activator/activator_test.go |
| +++ b/internal/activator/activator_test.go |
| @@ -41,6 +41,7 @@ type fakeRep struct { |
| mu sync.Mutex |
| err error |
| closed bool |
| + closes int |
| } |
| |
| func (r *fakeRep) Err() error { |
| @@ -57,8 +58,15 @@ func (r *fakeRep) Close(context.Context) error { |
| r.mu.Lock() |
| defer r.mu.Unlock() |
| r.closed = true |
| + r.closes++ // Close IS the lease release: exactly once, on every path |
| return nil |
| } |
| + |
| +func (r *fakeRep) closeCount() int { |
| + r.mu.Lock() |
| + defer r.mu.Unlock() |
| + return r.closes |
| +} |
| func (r *fakeRep) isClosed() bool { |
| r.mu.Lock() |
| defer r.mu.Unlock() |
| @@ -323,13 +331,21 @@ func (b *fakeBackend) setSidecarErr(err error) { |
| b.sidecarErr = err |
| } |
| |
| -func (b *fakeBackend) StartWorker(_ context.Context, rt registry.Route, env map[string]string) (Proc, string, error) { |
| +func (b *fakeBackend) StartWorker(ctx context.Context, rt registry.Route, env map[string]string) (Proc, string, error) { |
| b.mu.Lock() |
| delay := b.workerStartDelay |
| b.workerEnterN++ // counted BEFORE the delay: "the exec has begun" |
| b.mu.Unlock() |
| if delay > 0 { |
| - time.Sleep(delay) // outside the fake's own lock: a slow exec, not a stuck fake |
| + // A slow exec — ExecBackend materializes the build from the bucket |
| + // when live/<host> dangles — and, like the real one, abandonable: a |
| + // cancelled context produces NO child, which is what makes a shutdown |
| + // that cancels distinguishable from one that merely outran the spawn. |
| + select { |
| + case <-time.After(delay): |
| + case <-ctx.Done(): |
| + return nil, "", ctx.Err() |
| + } |
| } |
| b.mu.Lock() |
| defer b.mu.Unlock() |
| @@ -2824,10 +2840,70 @@ func TestShutdownStopsARespawnFromOutlivingTheLease(t *testing.T) { |
| } |
| } |
| |
| +// Shutdown CANCELS an in-flight worker spawn rather than waiting it out. The |
| +// wait is right everywhere else, but a shutdown answers to a deadline it does |
| +// not set: systemd's TimeoutStopSec defaults to 90s and workerSpawnTimeout |
| +// alone is 2 minutes, so blocking on one host's bucket materialize earns a |
| +// SIGKILL that strands leases and sockets for EVERY host on the box. |
| +func TestShutdownCancelsAnInFlightRespawnInsteadOfWaiting(t *testing.T) { |
| + a, b, clk, ver := shippingActivator(t, time.Hour) |
| + if err := a.WakeWorker(alwaysOnWorkerRoute("bot.x.internal")); err != nil { |
| + t.Fatalf("WakeWorker: %v", err) |
| + } |
| + rep := b.lastRep() |
| + b.mu.Lock() |
| + b.workerStartDelay = 5 * time.Second // a materialize nobody should wait for |
| + b.mu.Unlock() |
| + |
| + ver.Store("v2") |
| + a.sweepOnce() |
| + clk.advance(ConvergeGrace + time.Minute) |
| + a.sweepOnce() |
| + // Inside the exec: past the pre-spawn check, so only cancellation — not |
| + // the shutdown announcement — can end this promptly. |
| + waitFor(t, "the respawn's exec to begin", func() bool { return b.workerEnters() == 2 }) |
| + |
| + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) |
| + defer cancel() |
| + start := time.Now() |
| + a.Shutdown(ctx) |
| + if elapsed := time.Since(start); elapsed > 2*time.Second { |
| + t.Fatalf("Shutdown took %s — it waited out the spawn instead of cancelling it, which is how an agent gets SIGKILLed mid-shutdown", elapsed) |
| + } |
| + |
| + // (c) nothing left wedged, and no child running. |
| + if a.entryCount() != 0 { |
| + t.Fatal("Shutdown returned with an entry still live") |
| + } |
| + if n := b.workerStarts(); n != 1 { |
| + t.Fatalf("workerStarts = %d — a cancelled exec must produce no child", n) |
| + } |
| + for i, p := range b.workers { |
| + if !p.isStopped() { |
| + t.Fatalf("worker child %d was left running by Shutdown", i) |
| + } |
| + } |
| + // The lease: released exactly once, by stop()'s rep.Close — the respawn |
| + // never owned it, so cancelling one can neither skip nor double the |
| + // release. |
| + if n := rep.closeCount(); n != 1 { |
| + t.Fatalf("lease closes = %d, want exactly 1 on the cancelled-respawn shutdown path", n) |
| + } |
| + if n := b.releaseCount(); n != 0 { |
| + t.Fatalf("bare lease releases = %d, want 0 — the lease was traded for replication at wake", n) |
| + } |
| + // And the abandoned goroutine really is gone: nothing appears later. |
| + time.Sleep(300 * time.Millisecond) |
| + if n := b.workerStarts(); n != 1 { |
| + t.Fatalf("workerStarts = %d after shutdown — the cancelled respawn spawned anyway", n) |
| + } |
| +} |
| + |
| // A stop that lands AFTER a respawn has committed to spawning must not release |
| // the lease under it: the child appearing moments later would write the |
| // tenant's DB with no single-writer claim behind it. Bounce is the case |
| -// Shutdown's announcement does not cover. |
| +// Shutdown's announcement does not cover — and, unlike a shutdown, it has no |
| +// deadline, so here the stop WAITS the spawn out rather than cancelling it. |
| func TestStopWaitsOutAnInFlightRespawn(t *testing.T) { |
| a, b, clk, ver := shippingActivator(t, time.Hour) |
| if err := a.WakeWorker(alwaysOnWorkerRoute("bot.x.internal")); err != nil { |
amadan