amadan

carlos / platform Public

zdd plan 3 task 2 fix: Shutdown waits for retained unproven wakes

A failed wake whose generation is not proven stopped is retained in
unprovenWakes with its lease (P2-13), but it has no entry, so Shutdown
saw nothing to wait on and returned at once: the generation outlived
Shutdown (spec §12 I2) and its lease stayed held until the TTL even when
the process exited moments later. Shutdown now polls the reaper for
retained wakes too, with the same ticker and budget it uses for pending
stops, and logs the wakes still retained when the budget runs out.

Witness: TestSwapWitnessShutdownWaitsForAnUnprovenWake. Found by the
task 13 sweep's strengthened model.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 502eb11906330b22066ac4ae11bb18d0e3c09d0b parent 5e8db73
2 files changed, +148 −0
  • internal/activator/activator.go +32 −0
  • internal/activator/opset_test.go +116 −0
diff --git a/internal/activator/activator.go b/internal/activator/activator.go
index f009447a3..cf344a66a 100644
--- a/internal/activator/activator.go
+++ b/internal/activator/activator.go
@@ -3645,6 +3645,38 @@ func (a *Service) Shutdown(ctx context.Context) {
}
}
}
+ // Retained failed wakes (zero-downtime spec §7.4, P2-13) have no entry
+ // for the loop above to wait on, so they get the same polling here: the
+ // reaper (reapUnprovenWakesLocked, inside finishPendingStops) retries
+ // each one's stop and proof and gives its lease back once it holds. A
+ // wake still unproven when the budget runs out keeps its lease; the
+ // lease's TTL frees it. With generations off this map fills only from
+ // GenContainment's sentinels (genStartUnproven), so it is empty on a box
+ // without containment and this returns at once.
+ for {
+ a.mu.Lock()
+ left := len(a.unprovenWakes)
+ if left > 0 {
+ a.finishPendingStops()
+ }
+ a.mu.Unlock()
+ if left == 0 {
+ return
+ }
+ select {
+ case <-poll.C:
+ case <-ctx.Done():
+ a.mu.Lock()
+ var held []string
+ for host, u := range a.unprovenWakes {
+ held = append(held, fmt.Sprintf("%s (generation %d)", host, u.gen.N))
+ }
+ a.mu.Unlock()
+ slices.Sort(held)
+ log.Printf("activator: shutdown: out of time with %d failed wake(s) not proven stopped, their leases still held: %s", len(held), strings.Join(held, ", "))
+ return
+ }
+ }
}
// ServeHTTP answers one proxied request for a hibernating instance: wake (or
diff --git a/internal/activator/opset_test.go b/internal/activator/opset_test.go
index ef9ca4e7f..52d19633f 100644
--- a/internal/activator/opset_test.go
+++ b/internal/activator/opset_test.go
@@ -4,8 +4,11 @@ import (
"context"
"errors"
"fmt"
+ "log"
+ "os"
"os/exec"
"slices"
+ "strings"
"sync"
"sync/atomic"
"testing"
@@ -863,3 +866,116 @@ func TestSwapWitnessUnprovenWakeKeepsTheLease(t *testing.T) {
})
}
}
+
+// syncLog is a log destination that goroutines may write concurrently (the
+// reapers log off the test's goroutine).
+type syncLog struct {
+ mu sync.Mutex
+ b strings.Builder
+}
+
+func (s *syncLog) Write(p []byte) (int, error) {
+ s.mu.Lock()
+ defer s.mu.Unlock()
+ return s.b.Write(p)
+}
+
+func (s *syncLog) String() string {
+ s.mu.Lock()
+ defer s.mu.Unlock()
+ return s.b.String()
+}
+
+// Shutdown covers a retained failed wake (spec §7.4, I2: nothing runs after
+// Shutdown returns). No entry waits for one, so Shutdown polls its reaper
+// until the generation is proven stopped and the lease given back, or the
+// budget runs out, which it logs with the lease still held. Before, Shutdown
+// saw no entries and returned at once, leaving the generation running and
+// its lease held until the TTL. The retention has no generations gate (the
+// sentinel exists only under GenContainment), so neither has the wait.
+func TestSwapWitnessShutdownWaitsForAnUnprovenWake(t *testing.T) {
+ for _, row := range []struct {
+ name string
+ gens bool
+ budget time.Duration
+ heal bool
+ }{
+ {"healed within the budget", true, 10 * time.Second, true},
+ {"healed within the budget, generations off", false, 10 * time.Second, true},
+ {"the budget runs out", true, 300 * time.Millisecond, false},
+ } {
+ t.Run(row.name, func(t *testing.T) {
+ a, b, _, _ := testActivator(t, time.Hour)
+ a.gens = row.gens
+ kb := &killGenBackend{fakeBackend: b, err: errors.New("the generation's child still holds a process")}
+ a.backend = &unprovenStartBackend{killGenBackend: kb, fail: func(uint64) (Proc, error) {
+ return nil, fmt.Errorf("start unit: health wait failed; %w: kill timed out", errGenUnproven)
+ }}
+ host := "unproven.example.com"
+ retained := func() bool {
+ a.mu.Lock()
+ defer a.mu.Unlock()
+ return a.unprovenWakes[host] != nil
+ }
+ if _, err := a.beginRequest(context.Background(), testRoute(host)); err == nil {
+ t.Fatal("the wake should have failed")
+ }
+ if !retained() || b.releaseCount() != 0 {
+ t.Fatalf("retained %v, lease releases %d: want the failed wake retained with its lease", retained(), b.releaseCount())
+ }
+ logs := &syncLog{}
+ if !row.heal {
+ flags := log.Flags()
+ log.SetOutput(logs)
+ t.Cleanup(func() { log.SetOutput(os.Stderr); log.SetFlags(flags) })
+ }
+ kills := len(kb.killed())
+
+ shut := make(chan struct{})
+ go func() {
+ ctx, cancel := context.WithTimeout(context.Background(), row.budget)
+ defer cancel()
+ a.Shutdown(ctx)
+ close(shut)
+ }()
+ returned := func() bool {
+ select {
+ case <-shut:
+ return true
+ default:
+ return false
+ }
+ }
+ // Two more failed proofs since Shutdown began: it is still
+ // polling the reaper, so it has not returned while the wake is
+ // retained.
+ waitFor(t, "two failed proofs during Shutdown, or its return", func() bool {
+ return len(kb.killed()) >= kills+2 || returned()
+ })
+ if returned() {
+ t.Fatalf("Shutdown returned while the failed wake was retained (KillGen calls during it: %d)", len(kb.killed())-kills)
+ }
+
+ if row.heal {
+ kb.setErr(nil)
+ }
+ select {
+ case <-shut:
+ case <-time.After(5 * time.Second):
+ t.Fatal("Shutdown did not return")
+ }
+ if row.heal {
+ if retained() || b.releaseCount() != 1 {
+ t.Fatalf("after Shutdown: retained %v, lease releases %d; want the wake proven and its lease given back once", retained(), b.releaseCount())
+ }
+ return
+ }
+ if !retained() || b.releaseCount() != 0 {
+ t.Fatalf("after the budget ran out: retained %v, lease releases %d; want the lease kept", retained(), b.releaseCount())
+ }
+ if got := logs.String(); !strings.Contains(got, host) || !strings.Contains(got, "shutdown") || !strings.Contains(got, "not proven stopped") {
+ t.Fatalf("Shutdown's log does not name the retained wake:\n%s", got)
+ }
+ })
+ }
+}