amadan
host: the lifecycle property's fake admission refuses after a loss as production does, and renew no longer waits
0aa4789f waited for Lost, but the scoped cleanup that retires the install runs after Lost closes, and a timed-out wait fell through silently. The fake Admit now models hostguard.With: it refuses (ErrChanged) once the operation's controller has lost its lease, and checks the install and opens P under the lock cleanupScoped takes, which now reads the claim under that lock too, so a cleanup and an admission are ordered as under the host lock. A missing install at an admission the controller still holds is still invariant (f). renew is back to its fixed settle, so the mid-pass loss interleavings stay in the property. Merge review fix round 1, finding 7.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+29
−33
internal/host/relocationlifecycle_linux_test.go+29 −33
diff --git a/internal/host/relocationlifecycle_linux_test.go b/internal/host/relocationlifecycle_linux_test.go| index 6dec4c869..5d81ac341 100644 |
| --- a/internal/host/relocationlifecycle_linux_test.go |
| +++ b/internal/host/relocationlifecycle_linux_test.go |
| @@ -206,12 +206,30 @@ func newLifeWorld(t *testing.T) *lifeWorld { |
| return w.installed[o.Claim.Op.ID], nil |
| }, |
| Admit: func(_ context.Context, o hostclaim.Observed, _ bool) (uint64, bool, error) { |
| - c := w.current() |
| + // Production admits under hostguard.With, whose CheckLocal |
| + // refuses once the controller has lost its lease, and the |
| + // scoped cleanup retires the install under the same host |
| + // lock. Here the loss check, the install's presence and the |
| + // admission are one step under the lock cleanupScoped takes, |
| + // so a loss mid-pass refuses the admission as it would in |
| + // production; a missing install is still a violation. |
| + ctl, ok := w.e.Controllers.get(w.host, o.Claim.Op.ID) |
| + if !ok { |
| + return 0, false, hostbox.ErrChanged |
| + } |
| w.mu.Lock() |
| - present := w.installed[c.Op.ID] |
| - w.mu.Unlock() |
| - if !hostclaim.PreparedOpened(c) && !present { |
| - w.violate("(f) P's first admission over a missing local install") |
| + defer w.mu.Unlock() |
| + select { |
| + case <-ctl.Lost(): |
| + return 0, false, hostbox.ErrChanged |
| + default: |
| + } |
| + cur, err := hostclaim.Read(context.Background(), w.inner, w.host) |
| + if err != nil { |
| + return 0, false, err |
| + } |
| + if !hostclaim.PreparedOpened(cur.Claim) && !w.installed[cur.Claim.Op.ID] { |
| + w.violated = append(w.violated, "(f) P's first admission over a missing local install") |
| } |
| // The admission opens P (the claim journals it). |
| if _, err := hostclaim.Update(context.Background(), w.inner, o, func(c *hostclaim.Claim) error { |
| @@ -299,9 +317,11 @@ func (w *lifeWorld) cleanupScoped(ctx context.Context, op string, counts bool, g |
| if fail { |
| return errors.New("cleanup: a partial install would not go") |
| } |
| + // The claim is read under the lock the fake admission holds, so a |
| + // cleanup and an admission are ordered (production: the host lock). |
| + w.mu.Lock() |
| c, err := hostclaim.Read(context.Background(), w.inner, w.host) |
| opened := err == nil && !c.Absent && c.Claim.Op.ID == op && hostclaim.PreparedOpened(c.Claim) |
| - w.mu.Lock() |
| if !opened { |
| w.installed[op] = false |
| } |
| @@ -465,38 +485,14 @@ func (w *lifeWorld) violate(s string) { |
| w.mu.Unlock() |
| } |
| -// renew fires hostexec's timers so every controller renews (or loses), and |
| -// waits, bounded, until each open controller has done one or the other. A |
| -// renewal's lease write is a durable store write (main's DirStore fsyncs |
| -// the file and its directory), so a fixed short sleep let the next pass |
| -// race a loss still in flight: its asynchronous cleanup could retire the |
| -// local install between the pass's check and its admission. |
| +// renew fires hostexec's timers so every controller renews (or loses). A |
| +// renewal or a loss may still be in flight when the next event runs; the |
| +// fake admission refuses after a loss as production does. |
| func (w *lifeWorld) renew() { |
| w.clock.tick() |
| - wall, _, _ := w.clock.Now() |
| - deadline := time.Now().Add(time.Second) |
| - for _, c := range w.open() { |
| - for !renewedOrLost(w.ctx, c, wall) && time.Now().Before(deadline) { |
| - time.Sleep(5 * time.Millisecond) |
| - } |
| - } |
| time.Sleep(20 * time.Millisecond) |
| } |
| -// renewedOrLost reports whether c has lost its lease, or renewed it at the |
| -// tick to wall. |
| -func renewedOrLost(ctx context.Context, c RelocationController, wall time.Time) bool { |
| - select { |
| - case <-c.Lost(): |
| - return true |
| - default: |
| - } |
| - ctx, cancel := context.WithTimeout(ctx, 10*time.Millisecond) |
| - defer cancel() |
| - st, err := c.Ready(ctx) |
| - return err == nil && !st.Expires.Before(wall.Add(execctl.TTL)) |
| -} |
| - |
| func (w *lifeWorld) record() (execctl.State, bool, bool) { |
| raw, err := os.ReadFile(filepath.Join(w.dataDir, ".hostexec", w.host+".json")) |
| if errors.Is(err, os.ErrNotExist) { |