Skip to content

Commit 4d8d14a

Browse files
committed
fix(lease): release held leases orphaned by a daemon restart
A held lease's liveness is its daemon connection, so any held lease still persisted at startup is orphaned by definition -- yet StartupConverger.converge() re-armed its TTL timer anyway, parking the device for up to a full heldTtlBackstopMs with no holder. StartupConverger now releases every held lease (reason "orphaned") through the normal release path before timers are restored and before running-capacity convergence, so the freed device is reclaimed and visible to both. Detached leases are untouched -- their liveness is the TTL, not a connection -- and keep today's restored timer. The release capability is injected as a narrow OrphanedLeaseRelease port, wired in LeaseEngine to the existing LeaseReleaseCoordinator, in the same style as the converger's existing LeaseTimerRestorer and InterruptedReclaimRecovery neighbours. Closes #13
1 parent b469e73 commit 4d8d14a

10 files changed

Lines changed: 188 additions & 37 deletions

‎docs/ARCHITECTURE.md‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,19 @@ commits the resulting running or non-running state. Global and platform limits
7575
are checked atomically; no driver-specific runtime details participate in this
7676
decision.
7777

78-
At startup, `StartupConverger` restores persisted lease TTL timers, recovers
79-
unleased interrupted reclaims through the warm-pool recovery port, then
80-
deterministically shuts down excess unleased, unclaimed `ready` registry
81-
devices through `CleanupActionExecutor`. Leased devices are never touched, so
82-
a lowered limit may remain visibly over-limit until leases naturally release.
78+
At startup, `StartupConverger` first releases every persisted `held` lease
79+
(reason `orphaned`) through the normal release path — a held lease's liveness
80+
is its daemon connection, so it cannot have a live holder across a restart,
81+
and this runs before timers are restored so an orphaned lease's timer is
82+
never re-armed. It then restores persisted TTL timers for the remaining
83+
(`detached`) leases, whose liveness is the TTL rather than a connection,
84+
recovers unleased interrupted reclaims through the warm-pool recovery port,
85+
and finally deterministically shuts down excess unleased, unclaimed `ready`
86+
registry devices through `CleanupActionExecutor`. Running this release step
87+
before timer restoration and capacity convergence means the devices it frees
88+
are visible to both. Leased devices that survive the orphan sweep are never
89+
touched, so a lowered limit may remain visibly over-limit until leases
90+
naturally release.
8391

8492
## Device state machine
8593

@@ -183,9 +191,10 @@ capacity coordinator into these direct transactional call chains:
183191
`CleanupActionExecutor`; the executor revalidates registry ownership,
184192
lease/state safety, and delegates the driver operation to
185193
`ManagedDeviceLifecycle`.
186-
- `StartupConverger` runs timer restoration, interrupted-reclaim recovery, and
187-
running-capacity convergence in that order. `NukeService` coordinates lease
188-
release, pending-request cancellation, and registry-scoped reset operations.
194+
- `StartupConverger` runs orphaned held-lease release, timer restoration,
195+
interrupted-reclaim recovery, and running-capacity convergence in that
196+
order. `NukeService` coordinates lease release, pending-request
197+
cancellation, and registry-scoped reset operations.
189198

190199
The serialized decision gate protects only short read-decide-commit sections.
191200
Driver work remains outside it. Component boundaries use direct calls for

‎docs/EVENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ in short: `subject.past-tense-fact`, emitted post-commit, facts not commands.
1515
| `lease.queued` | request id, queue position | no capacity; request entered the wait queue | LeaseAcquisitionCoordinator | implemented |
1616
| `lease.granted` | lease id, device id, requester, mode (held/detached) | a device was assigned and handed out | LeaseLifecycle | implemented |
1717
| `lease.renewed` | lease id, new deadline | detached-mode renew succeeded, **or** a held-mode connection that declared the `heartbeat` capability answered a `lease.heartbeat` push (fires once per lease per `lease.heartbeatIntervalMs` while the holder stays alive) | LeaseLifecycle | implemented |
18-
| `lease.released` | lease id, device id, reason (closed/explicit/killed) | holder connection closed or explicit release | LeaseLifecycle | implemented |
18+
| `lease.released` | lease id, device id, reason (closed/explicit/killed/orphaned) | holder connection closed, explicit release, or (orphaned) a `held` lease found still persisted at daemon startup, which cannot have a live holder across a restart | LeaseLifecycle | implemented |
1919
| `lease.expired` | lease id, device id | TTL backstop fired without a heartbeat sliding it first — for a capability-declaring holder this means it stopped ponging (crashed, hung, or lost its socket); for one that never declared the capability it means the grant-time TTL (or the last explicit `pitlane lease renew`) simply ran out, exactly as before this change | LeaseLifecycle | implemented |
2020
| `lease.rejected` | request spec, reason (timeout/no-wait/unresolvable-spec/already-leased/boot-timeout/killed) | a request ended without a grant | LeaseAcquisitionCoordinator / WaitQueue | implemented |
2121

‎src/bus/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export interface EventMap {
1717
"lease.released": {
1818
readonly leaseId: string;
1919
readonly deviceId: string;
20-
readonly reason: "closed" | "explicit" | "killed";
20+
readonly reason: "closed" | "explicit" | "killed" | "orphaned";
2121
};
2222
"lease.expired": { readonly leaseId: string; readonly deviceId: string };
2323
"lease.rejected": {

‎src/core/lease-engine.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ describe("LeaseEngine", () => {
353353
const unleasedDevice = await seedReady(harness);
354354
await harness.registry.createLease({
355355
deviceId: leasedDevice.id,
356-
mode: "held",
356+
mode: "detached",
357357
requesterId: "active",
358358
ttlDeadline: 2_000,
359359
});
@@ -425,7 +425,7 @@ describe("LeaseEngine", () => {
425425
const device = await seedReady(harness);
426426
await harness.registry.createLease({
427427
deviceId: device.id,
428-
mode: "held",
428+
mode: "detached",
429429
requesterId,
430430
ttlDeadline: 2_000,
431431
});

‎src/core/lease-engine.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,11 @@ export class LeaseEngine {
164164
},
165165
},
166166
registry: options.registry,
167+
releases: {
168+
releaseOrphaned: async (leaseId) => {
169+
await this.#releaseCoordinator.release(leaseId, "orphaned");
170+
},
171+
},
167172
timers: this.#leases,
168173
});
169174
}

‎src/core/lease-lifecycle.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ export class LeaseLifecycle {
124124

125125
async beginRelease(
126126
leaseId: string,
127-
reason: "closed" | "explicit" | "killed" | "expired",
127+
reason: "closed" | "explicit" | "killed" | "orphaned" | "expired",
128128
): Promise<ReleasedLease> {
129129
const released = await this.options.registry.beginRelease(leaseId);
130130
this.options.expiryScheduler.cancel(leaseId);

‎src/core/lease-release-coordinator.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { ReleasedLease } from "./registry.js";
33
import type { SerializedDecision } from "./serialized-decision.js";
44
import type { WarmPoolCoordinator } from "./warm-pool-coordinator.js";
55

6-
export type LeaseReleaseReason = "closed" | "explicit" | "killed";
6+
export type LeaseReleaseReason = "closed" | "explicit" | "killed" | "orphaned";
77

88
export interface LeaseReleaseCommands {
99
release(leaseId: string, reason: LeaseReleaseReason): Promise<void>;

‎src/core/startup-converger.test.ts‎

Lines changed: 106 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,12 @@ function createHarness(
5454
const order: string[] = [];
5555
const claimed = new Set<string>();
5656
const cleanupCalls: string[] = [];
57+
const releasedLeaseIds: string[] = [];
58+
let leaseIdsAtTimerRestore: string[] | undefined;
5759
const timers = {
5860
restoreExpiryTimers: vi.fn(async () => {
5961
order.push("timers");
62+
leaseIdsAtTimerRestore = leases.map((lease) => lease.id);
6063
}),
6164
};
6265
const recovery = {
@@ -75,6 +78,17 @@ function createHarness(
7578
return true;
7679
}),
7780
};
81+
const releases = {
82+
releaseOrphaned: vi.fn(async (leaseId: string) => {
83+
order.push(`release:${leaseId}`);
84+
releasedLeaseIds.push(leaseId);
85+
const leaseIndex = leases.findIndex((lease) => lease.id === leaseId);
86+
const lease = leases[leaseIndex];
87+
if (lease === undefined) return;
88+
leases.splice(leaseIndex, 1);
89+
updateState(lease.deviceId, "ready");
90+
}),
91+
};
7892
const converger = new StartupConverger({
7993
capacity: {
8094
get runningCapacity() {
@@ -90,6 +104,7 @@ function createHarness(
90104
return { devices, leases };
91105
},
92106
},
107+
releases,
93108
timers,
94109
});
95110

@@ -99,7 +114,22 @@ function createHarness(
99114
if (current !== undefined) devices[index] = { ...current, state };
100115
}
101116

102-
return { claimed, cleanup, cleanupCalls, converger, devices, order, recovery, timers };
117+
return {
118+
claimed,
119+
cleanup,
120+
cleanupCalls,
121+
converger,
122+
devices,
123+
get leaseIdsAtTimerRestore() {
124+
return leaseIdsAtTimerRestore;
125+
},
126+
leases,
127+
order,
128+
recovery,
129+
releasedLeaseIds,
130+
releases,
131+
timers,
132+
};
103133
}
104134

105135
describe("StartupConverger", () => {
@@ -151,15 +181,15 @@ describe("StartupConverger", () => {
151181
deviceId: first.id,
152182
grantedAt: 0,
153183
id: "lease-1",
154-
mode: "held" as const,
184+
mode: "detached" as const,
155185
requesterId: "a",
156186
ttlDeadline: 10,
157187
},
158188
{
159189
deviceId: second.id,
160190
grantedAt: 0,
161191
id: "lease-2",
162-
mode: "held" as const,
192+
mode: "detached" as const,
163193
requesterId: "b",
164194
ttlDeadline: 10,
165195
},
@@ -201,4 +231,77 @@ describe("StartupConverger", () => {
201231
expect(harness.cleanupCalls).toEqual(["ready"]);
202232
expect(harness.timers.restoreExpiryTimers).toHaveBeenCalledTimes(2);
203233
});
234+
235+
it("releases an orphaned held lease before timers are restored, freeing its device", async () => {
236+
const held = device("held-device", "ios", "leased", 1);
237+
const leases = [
238+
{
239+
deviceId: held.id,
240+
grantedAt: 0,
241+
id: "lease-held",
242+
mode: "held" as const,
243+
requesterId: "a",
244+
ttlDeadline: 1000,
245+
},
246+
];
247+
const harness = createHarness([held], leases, { android: 1, global: 1, ios: 1 });
248+
249+
await harness.converger.converge();
250+
251+
expect(harness.releases.releaseOrphaned).toHaveBeenCalledTimes(1);
252+
expect(harness.releases.releaseOrphaned).toHaveBeenCalledWith("lease-held");
253+
expect(harness.order.indexOf("release:lease-held")).toBeLessThan(
254+
harness.order.indexOf("timers"),
255+
);
256+
// The orphaned lease is gone from the registry by the time timers are restored, so its
257+
// timer is never re-armed.
258+
expect(harness.leaseIdsAtTimerRestore).toEqual([]);
259+
expect(harness.devices.find((item) => item.id === held.id)?.state).toBe("ready");
260+
});
261+
262+
it("keeps a detached lease's timer restoration untouched", async () => {
263+
const detachedDevice = device("detached-device", "ios", "leased", 1);
264+
const leases = [
265+
{
266+
deviceId: detachedDevice.id,
267+
grantedAt: 0,
268+
id: "lease-detached",
269+
mode: "detached" as const,
270+
requesterId: "a",
271+
ttlDeadline: 1000,
272+
},
273+
];
274+
const harness = createHarness([detachedDevice], leases, { android: 1, global: 1, ios: 1 });
275+
276+
await harness.converger.converge();
277+
278+
expect(harness.releases.releaseOrphaned).not.toHaveBeenCalled();
279+
expect(harness.leaseIdsAtTimerRestore).toEqual(["lease-detached"]);
280+
expect(harness.devices.find((item) => item.id === detachedDevice.id)?.state).toBe("leased");
281+
});
282+
283+
it("frees the orphaned lease's device before running-capacity convergence, making it a shutdown candidate", async () => {
284+
const held = device("held-device", "ios", "leased", 1);
285+
const leases = [
286+
{
287+
deviceId: held.id,
288+
grantedAt: 0,
289+
id: "lease-held",
290+
mode: "held" as const,
291+
requesterId: "a",
292+
ttlDeadline: 1000,
293+
},
294+
];
295+
// maxRunning for ios is 0, so once the device is freed to "ready" it is over capacity
296+
// and must be visible to the excess-capacity sweep that follows.
297+
const harness = createHarness([held], leases, { android: 1, global: 0, ios: 0 });
298+
299+
await harness.converger.converge();
300+
301+
const releaseIndex = harness.order.indexOf("release:lease-held");
302+
const cleanupIndex = harness.order.indexOf("cleanup:held-device");
303+
expect(releaseIndex).toBeGreaterThanOrEqual(0);
304+
expect(cleanupIndex).toBeGreaterThan(releaseIndex);
305+
expect(harness.cleanupCalls).toEqual(["held-device"]);
306+
});
204307
});

‎src/core/startup-converger.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,16 @@ export interface InterruptedReclaimRecovery {
2121
recoverInterruptedReclaim(device: DeviceRecord): Promise<void>;
2222
}
2323

24+
/**
25+
* Releases a lease orphaned by a daemon restart. A held lease's liveness is
26+
* its daemon connection, so any held lease found at startup has no holder by
27+
* definition; this port drives it through the normal release path (reason
28+
* `orphaned`) so the device is reclaimed and `lease.released` is emitted.
29+
*/
30+
export interface OrphanedLeaseRelease {
31+
releaseOrphaned(leaseId: string): Promise<void>;
32+
}
33+
2434
/** Read-only operation claim view used to avoid an in-flight device operation. */
2535
export interface DeviceClaimReader {
2636
isClaimed(deviceId: string): boolean;
@@ -33,6 +43,7 @@ export interface StartupConvergerOptions {
3343
readonly decisions: SerializedDecision;
3444
readonly interruptedReclaimRecovery: InterruptedReclaimRecovery;
3545
readonly registry: StartupRegistry;
46+
readonly releases: OrphanedLeaseRelease;
3647
readonly timers: LeaseTimerRestorer;
3748
}
3849

@@ -44,6 +55,7 @@ export class StartupConverger {
4455
constructor(private readonly options: StartupConvergerOptions) {}
4556

4657
async converge(): Promise<void> {
58+
await this.#releaseOrphanedHeldLeases();
4759
await this.options.timers.restoreExpiryTimers();
4860
await this.#recoverInterruptedReclaims();
4961

@@ -62,6 +74,25 @@ export class StartupConverger {
6274
}
6375
}
6476

77+
/**
78+
* A held lease's liveness is its daemon connection, so any lease still in
79+
* `held` mode at startup is orphaned by definition — it cannot have a live
80+
* holder across a restart. Release it (reason `orphaned`) before timers are
81+
* restored, so its timer is never re-armed, and before capacity
82+
* convergence, so the freed device is visible to it. Detached leases are
83+
* untouched here; their liveness is the TTL, not a connection.
84+
*/
85+
async #releaseOrphanedHeldLeases(): Promise<void> {
86+
const orphanedLeaseIds = await this.options.decisions.run(() =>
87+
this.options.registry.snapshot.leases
88+
.filter((lease) => lease.mode === "held")
89+
.map((lease) => lease.id),
90+
);
91+
for (const leaseId of orphanedLeaseIds) {
92+
await this.options.releases.releaseOrphaned(leaseId);
93+
}
94+
}
95+
6596
async #recoverInterruptedReclaims(): Promise<void> {
6697
const interrupted = await this.options.decisions.run(() => {
6798
const snapshot = this.options.registry.snapshot;

0 commit comments

Comments
 (0)