Skip to content

Commit 39b4e48

Browse files
authored
fix(daemon): reject instead of hanging when the socket claim loses the race (#108)
**Stacked on #107.** A regression this stack introduced, caught by a verification pass over the earlier fixes. `gatewayStarted` is settled only from inside `onSocketClaimed`'s handler, and `DaemonServer.start()` fires that callback only once the socket claim **and** the admin-secret write have succeeded. So a daemon that loses the start race rejects with `DaemonAlreadyRunningError` without the callback ever running — nothing settles `gatewayStarted`, and the `Promise.allSettled` join added by the bind-failure fix waits on it **forever**. `startDaemon()` therefore hung: no rejection, no "Daemon failed to start", no non-zero exit code. Both `main` and the pre-fix stack awaited `daemon.start()` alone and rejected immediately, so this is a regression introduced by that fix, not a pre-existing gap. It is reachable by racing `simlock daemon start`, and by the CLI's own auto-launch — a common path, not an exotic one, whenever HTTP is enabled. The fix releases `gatewayStarted` on the `daemon.start()` failure path. A `socketClaimed` guard keeps it from resolving early while a *claimed* daemon's gateway is still binding, so a genuine bind failure is still reported — the behaviour the previous PR added stays intact. The regression test races two `startDaemon()` calls on one data directory with distinct HTTP ports, so the only thing that can fail is the socket claim, and asserts the second **rejects** rather than hanging. Verified by stashing the fix: without it the test reports `hung` instead of `rejected`.
1 parent a9f6aac commit 39b4e48

2 files changed

Lines changed: 67 additions & 1 deletion

File tree

src/daemon/main.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,58 @@ describe("startDaemon HTTP gateway stop-during-start race (review finding S5)",
324324
});
325325
});
326326

327+
describe("startDaemon socket race with HTTP enabled", () => {
328+
// Review finding V1: `gatewayStarted` is settled only from inside `onSocketClaimed`'s
329+
// handler, and `start()` fires that callback only after the socket claim succeeds. A daemon
330+
// that loses the start race therefore rejected without the callback ever running, so nothing
331+
// settled `gatewayStarted` and `Promise.allSettled` waited on it forever -- `startDaemon()`
332+
// hung instead of reporting the lost race, with no rejection and no non-zero exit code.
333+
// Reachable by racing `simlock daemon start`, or by the CLI's own auto-launch.
334+
it("rejects rather than hanging when the socket is already claimed", async () => {
335+
const directory = await mkdtemp(join(tmpdir(), "simlock-main-socket-race-"));
336+
temporaryDirectories.push(directory);
337+
const filesystem = new MemoryFilesystem();
338+
const statePath = join(directory, "state.json");
339+
const options = (port: number): StartDaemonOptions =>
340+
({
341+
clock: new FakeClock(1_000),
342+
configOverrides: { http: { enabled: true, host: "127.0.0.1", port } },
343+
dataDirectory: directory,
344+
drivers: [
345+
new FakeDriver({
346+
availableOsVersions: ["26.5"],
347+
clock: new FakeClock(1_000),
348+
platform: "ios",
349+
}),
350+
],
351+
filesystem,
352+
logger: new JsonLinesLogger({
353+
clock: new FakeClock(1_000),
354+
level: "debug",
355+
sink: new MemoryLogSink(),
356+
}),
357+
statePath,
358+
version: "1.2.3",
359+
}) as StartDaemonOptions;
360+
361+
const first = await startDaemon(options(47_013));
362+
try {
363+
// A distinct port, so the only thing that can fail is the socket claim itself.
364+
const second = startDaemon(options(47_014));
365+
const outcome = await Promise.race([
366+
second.then(
367+
() => "resolved" as const,
368+
() => "rejected" as const,
369+
),
370+
new Promise<"hung">((resolve) => setTimeout(() => resolve("hung"), 3_000)),
371+
]);
372+
expect(outcome).toBe("rejected");
373+
} finally {
374+
await first.stop("test-cleanup").catch(() => undefined);
375+
}
376+
});
377+
});
378+
327379
describe("startDaemon HTTP gateway bind failure", () => {
328380
// Review finding B6: before this fix, an HTTP bind failure (occupied port) logged and
329381
// stopped the daemon from inside `onSocketClaimed`'s handler without `startDaemon()` itself

src/daemon/main.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,9 @@ export async function startDaemon(options: StartDaemonOptions = {}): Promise<Dae
191191
// wait for, so this resolves immediately.
192192
let resolveGatewayStarted: (() => void) | undefined;
193193
let rejectGatewayStarted: ((error: unknown) => void) | undefined;
194+
/** Whether `onSocketClaimed` ever fired, i.e. whether anything will ever settle
195+
* `gatewayStarted`. See the `finally` on `daemon.start()` below. */
196+
let socketClaimed = false;
194197
const gatewayStarted: Promise<void> = config.http.enabled
195198
? new Promise<void>((resolve, reject) => {
196199
resolveGatewayStarted = resolve;
@@ -263,6 +266,7 @@ export async function startDaemon(options: StartDaemonOptions = {}): Promise<Dae
263266
...(config.http.enabled
264267
? {
265268
onSocketClaimed: () => {
269+
socketClaimed = true;
266270
void startHttpGateway().then(
267271
() => resolveGatewayStarted?.(),
268272
(error: unknown) => {
@@ -309,7 +313,17 @@ export async function startDaemon(options: StartDaemonOptions = {}): Promise<Dae
309313
// finishes starting or fails to. Both are already running concurrently by the time this line
310314
// is reached (the gateway since `onSocketClaimed` fired partway through `daemon.start()`), so
311315
// this changes nothing about when either finishes -- only what `startDaemon()` itself reports.
312-
const [daemonResult, gatewayResult] = await Promise.allSettled([daemon.start(), gatewayStarted]);
316+
// `gatewayStarted` is settled only from inside `onSocketClaimed`'s handler, and `start()`
317+
// fires that callback only once the socket claim (and the admin-secret write) has succeeded.
318+
// A daemon that loses the start race therefore rejects without the callback ever running, so
319+
// nothing would settle `gatewayStarted` and the join below would hang forever rather than
320+
// reporting the failure. Release it here on that path -- resolving an already-settled promise
321+
// is a no-op, so this cannot pre-empt a real bind result, and the `socketClaimed` guard keeps
322+
// it from resolving early while a claimed daemon's gateway is still binding.
323+
const daemonStarted = daemon.start().finally(() => {
324+
if (!socketClaimed) resolveGatewayStarted?.();
325+
});
326+
const [daemonResult, gatewayResult] = await Promise.allSettled([daemonStarted, gatewayStarted]);
313327
if (gatewayResult.status === "rejected" && daemonResult.status === "fulfilled") {
314328
// The daemon itself came all the way up -- convergence succeeded, `daemon.started` was
315329
// already emitted -- but its HTTP gateway never bound. Tear the whole thing down now,

0 commit comments

Comments
 (0)