From 24fb9b70cf1dc7790db6774f114c072c9accb7d0 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Sun, 25 Oct 2026 12:20:25 +0100 Subject: [PATCH 1/3] =?UTF-8?q?test:=20reproduce=20#1000=20=E2=80=94=20a?= =?UTF-8?q?=20daemon=20stop=20leaves=20a=20health=20recovery=20reboot=20ru?= =?UTF-8?q?nning=20after=20the=20drivers=20are=20disposed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/daemon/main.test.ts | 118 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/src/daemon/main.test.ts b/src/daemon/main.test.ts index a7b2e7924..a4fe94a51 100644 --- a/src/daemon/main.test.ts +++ b/src/daemon/main.test.ts @@ -1641,6 +1641,124 @@ describe("startDaemon stop with a cleanup run in flight (#985)", () => { }); }); +describe("startDaemon stop with a health recovery in flight (#1000)", () => { + const turnOfTheEventLoop = () => new Promise((resolve) => setImmediate(resolve)); + const turns = async (count: number) => { + for (let turn = 0; turn < count; turn += 1) await turnOfTheEventLoop(); + }; + const recovered = ["recover started", "recover finished", "dispose"]; + + it.each([ + { name: "started before the stop", startsDuringStop: false, accepted: [recovered] }, + // why: a stop may either wait for this recovery or keep it from starting; it may not leave it running. + { + name: "due while the stop waits for a cleanup run", + startsDuringStop: true, + accepted: [recovered, ["dispose"]], + }, + ])( + "leaves no recovery reboot $name running once the drivers are disposed", + async ({ startsDuringStop, accepted }) => { + // why: records the order of the driver's recovery reboot and disposal, and holds the reboot mid-call. + class HeldRecoveryDriver extends FakeDriver { + readonly order: string[] = []; + readonly held: (() => void)[] = []; + override async makeReady( + device: Parameters[0], + options: Parameters[1], + ): ReturnType { + if (options.purpose !== "recover") return super.makeReady(device, options); + this.order.push("recover started"); + await new Promise((resolve) => this.held.push(resolve)); + const ready = await super.makeReady(device, options); + this.order.push("recover finished"); + return ready; + } + async dispose(): Promise { + this.order.push("dispose"); + } + } + // why: holds a cleanup run on its disk read once asked to, so the stop waits on it. + class HeldDiskFree extends MemoryFilesystem { + holding = false; + readonly held: (() => void)[] = []; + override async diskFree(path: string): Promise { + if (this.holding) await new Promise((resolve) => this.held.push(resolve)); + return super.diskFree(path); + } + } + const clock = new FakeClock(1_000); + const driver = new HeldRecoveryDriver({ + availableOsVersions: ["26.5"], + clock, + platform: "ios", + }); + const filesystem = new HeldDiskFree(); + const { daemon } = await start({ clock, drivers: [driver], filesystem }); + const agent = { + manageEventSubscription: () => undefined, + via: "socket", + principal: "agent", + role: "agent", + } as const; + await daemon.dispatch( + "lease.request", + { model: "iPhone 16", osVersion: "26.5", platform: "ios", requesterId: "agent-1" }, + agent, + ); + // why: the leased simulator dies outside simlock; the monitor recovers it on its second stopped reading, one 30 s tick apart. + const { devices } = await driver.listManaged(); + expect(devices).toHaveLength(1); + driver.setManagedReality({ + devices: devices.map((device) => ({ ...device, runState: "stopped" as const })), + processes: [], + }); + clock.advance(30_000); + await turns(20); + + if (startsDuringStop) { + filesystem.holding = true; + void daemon + .dispatch( + "cleanup.run", + { dryRun: true }, + { + manageEventSubscription: () => undefined, + via: "socket", + principal: "admin", + role: "admin", + }, + ) + .catch(() => undefined); + await vi.waitFor(() => expect(filesystem.held).toHaveLength(1)); + } else { + clock.advance(30_000); + await vi.waitFor(() => expect(driver.order).toEqual(["recover started"])); + } + + const stopped = daemon.stop("test-stop"); + let stopEnded = false; + void stopped.then(() => { + stopEnded = true; + }); + if (startsDuringStop) { + await turns(20); + expect(stopEnded).toBe(false); + clock.advance(30_000); + await turns(50); + for (const release of filesystem.held.splice(0)) release(); + } + for (let turn = 0; turn < 200 && !stopEnded; turn += 1) await turnOfTheEventLoop(); + for (const release of driver.held.splice(0)) release(); + await stopped; + // why: a reboot that outlived the stop records its end a few turns after it is released. + await turns(50); + + expect(accepted).toContainEqual(driver.order); + }, + ); +}); + describe("startDaemon stop with a warm target", () => { it("creates no further device for a target once a stop is asked for, though the creation in flight ends while the gateway stops", async () => { const directory = await mkdtemp(join(tmpdir(), "simlock-main-stop-target-")); From 9ad26e55e7725bfe2df75d21c2084d420d75f770 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Sun, 25 Oct 2026 13:31:39 +0100 Subject: [PATCH 2/3] fix(daemon): a stop waits for health ticks and recoveries in flight (#1000) --- docs/internal/flows/startup.md | 6 ++-- src/daemon/server.ts | 4 ++- src/leasing/lease-health-monitor.ts | 51 +++++++++++++++++++++-------- 3 files changed, 43 insertions(+), 18 deletions(-) diff --git a/docs/internal/flows/startup.md b/docs/internal/flows/startup.md index 9cab62262..71b40ea41 100644 --- a/docs/internal/flows/startup.md +++ b/docs/internal/flows/startup.md @@ -119,9 +119,9 @@ platform runs the deferred reclaim (step 21.2). 2. Stop auxiliary fronts: agent-device edge (cancel its waiting allocates), HTTP (after its start settles), the uplink and its redial loop. 3. Log, emit `daemon.stopping`. -4. Unsubscribe pushes; dispose host load, dispatcher, reaper, then wait for every - cleanup run in flight (scheduled, dry-run or rule-scoped) to end; dispose health - monitor, physical watcher. +4. Unsubscribe pushes; dispose host load, dispatcher, reaper, health monitor, then wait for every + cleanup run in flight (scheduled, dry-run or rule-scoped) and every health tick or + recovery reboot in flight to end; dispose physical watcher. 5. Settle — later converge steps are skipped; leasing drains the physical watcher and background reclaims; core drains the warm pool. 6. Dispose — core and leasing timers, `ComponentInstaller.close`, agent-device diff --git a/src/daemon/server.ts b/src/daemon/server.ts index cce24fca8..2bb4ea7f5 100644 --- a/src/daemon/server.ts +++ b/src/daemon/server.ts @@ -637,10 +637,12 @@ export class DaemonServer { this.options.hostLoad?.dispose(); this.#workerDispatcher?.dispose(); this.#engine?.reaper.dispose(); + this.#engine?.healthMonitor?.dispose(); // why: every cleanup run in flight (scheduled, dry-run or rule-scoped) still reads the registry // and calls drivers; each ends before anything below closes them. await this.#engine?.reaper.settle(); - this.#engine?.healthMonitor?.dispose(); + // why: a health tick or recovery reboot already running still acts on a device; it ends before the drivers are disposed. + await this.#engine?.healthMonitor?.settle(); this.#engine?.physicalDeviceWatcher?.dispose(); // why: a stop releases nothing; leases persist with their deadline. Only work already in // flight is drained, so `daemon stop` finishes on a settled pool. Disposal follows rather than diff --git a/src/leasing/lease-health-monitor.ts b/src/leasing/lease-health-monitor.ts index 1a6672bf1..29c7f1fe5 100644 --- a/src/leasing/lease-health-monitor.ts +++ b/src/leasing/lease-health-monitor.ts @@ -54,6 +54,7 @@ export class LeaseHealthMonitor { readonly #inFlight = new Set(); readonly #logger: Logger; readonly #pendingBackoffs = new Map void>(); + readonly #running = new Set>(); #disposed = false; #tickTimer: TimerHandle | undefined; @@ -90,6 +91,21 @@ export class LeaseHealthMonitor { this.#pendingBackoffs.clear(); } + /** + * why: Resolves once the running tick and every recovery have ended, including work a finishing + * one starts. Call after `dispose()`: a disposed monitor starts no recovery, so this ends. + */ + async settle(): Promise { + while (this.#running.size > 0) { + await Promise.allSettled(this.#running); + } + } + + #track(work: Promise): void { + this.#running.add(work); + void work.finally(() => this.#running.delete(work)); + } + #armTick(): void { if (this.#disposed) { return; @@ -98,12 +114,14 @@ export class LeaseHealthMonitor { this.options.config.health.probeIntervalMs, () => { this.#tickTimer = undefined; - void this.#runTick().then( - () => this.#armTick(), - (error: unknown) => { - this.#logger.error("health monitor tick failed", { error: summarizeError(error) }); - this.#armTick(); - }, + this.#track( + this.#runTick().then( + () => this.#armTick(), + (error: unknown) => { + this.#logger.error("health monitor tick failed", { error: summarizeError(error) }); + this.#armTick(); + }, + ), ); }, ); @@ -249,19 +267,24 @@ export class LeaseHealthMonitor { } #dispatchRecovery(device: VirtualDevice, lease: LeaseRecord): void { + if (this.#disposed) { + return; + } if (this.#inFlight.size >= this.options.config.health.maxConcurrentRecoveries) { // Cap reached; this device's counters stay elevated and it is retried next tick. return; } this.#inFlight.add(device.id); - void this.#recover(device, lease) - .catch((error: unknown) => { - this.#logger.error("unexpected error recovering device", { - deviceId: device.id, - error: summarizeError(error), - }); - }) - .finally(() => this.#inFlight.delete(device.id)); + this.#track( + this.#recover(device, lease) + .catch((error: unknown) => { + this.#logger.error("unexpected error recovering device", { + deviceId: device.id, + error: summarizeError(error), + }); + }) + .finally(() => this.#inFlight.delete(device.id)), + ); } async #recover(device: VirtualDevice, lease: LeaseRecord): Promise { From e6f886f7fbcf19d1039a175a942663b097fc3972 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Sun, 25 Oct 2026 13:34:46 +0100 Subject: [PATCH 3/3] test(leasing): settle waits for a health tick running at dispose and no recovery starts after it (#1000) --- src/leasing/lease-health-monitor.test.ts | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/leasing/lease-health-monitor.test.ts b/src/leasing/lease-health-monitor.test.ts index 9b3edd984..257da2448 100644 --- a/src/leasing/lease-health-monitor.test.ts +++ b/src/leasing/lease-health-monitor.test.ts @@ -713,4 +713,40 @@ describe("LeaseHealthMonitor", () => { await flush(); expect(makeReadyCalls(harness.driver)).toBe(1); }); + + it("settle() waits for a tick running when the monitor is disposed, and that tick starts no recovery", async () => { + const harness = await createHarness(); + const { device } = await seedLeased(harness); + harness.driver.setManagedReality({ + devices: [observedDevice(device.driverDeviceId, "stopped")], + processes: [], + }); + harness.monitor.start(); + harness.clock.advance(30_000); + await flush(); + // why: holds tick 2 on its observation so the monitor is disposed while that tick runs. + const listManaged = harness.driver.listManaged.bind(harness.driver); + let release: () => void = () => undefined; + harness.driver.listManaged = async () => { + await new Promise((resolve) => { + release = resolve; + }); + return listManaged(); + }; + harness.clock.advance(30_000); + await flush(); + + harness.monitor.dispose(); + let settled = false; + const settling = harness.monitor.settle().then(() => { + settled = true; + }); + await flush(); + expect(settled).toBe(false); + + release(); + await settling; + expect(settled).toBe(true); + expect(makeReadyCalls(harness.driver)).toBe(0); + }); });