Skip to content

Commit aad9cac

Browse files
okwasniewskiokwasniewski
committed
fix(daemon-client): adopt a start-race winner only as a reusable daemon
The winner of a start race goes through the same takeover decision as any existing daemon: compatible is reused, newer is refused, older or mismatched is replaced. This client's own daemon is recognized by pid and process start time, so a reused pid is never taken for it.
1 parent 7c5d9b4 commit aad9cac

2 files changed

Lines changed: 98 additions & 17 deletions

File tree

‎src/daemon-client/__tests__/daemon-client-startup-race.test.ts‎

Lines changed: 74 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import assert from 'node:assert/strict';
22
import fs from 'node:fs';
3+
import path from 'node:path';
34
import { afterEach, test, vi } from 'vitest';
45
import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts';
56

@@ -11,13 +12,19 @@ vi.mock('@agent-device/host-kit/retry', async (importOriginal) => ({
1112
...(await importOriginal<typeof import('@agent-device/host-kit/retry')>()),
1213
sleep: vi.fn(async () => {}),
1314
}));
15+
const winner = vi.hoisted(() => ({ pid: 43_300, alive: true }));
1416
vi.mock('../../daemon-process.ts', async (importOriginal) => {
1517
const actual = await importOriginal<typeof import('../../daemon-process.ts')>();
1618
return {
1719
...actual,
18-
isAgentDeviceDaemonProcess: vi.fn(
19-
(pid: number, startTime: string | undefined) =>
20-
pid === WINNER_PID || actual.isAgentDeviceDaemonProcess(pid, startTime),
20+
isAgentDeviceDaemonProcess: vi.fn((pid: number, startTime: string | undefined) =>
21+
pid === winner.pid ? winner.alive : actual.isAgentDeviceDaemonProcess(pid, startTime),
22+
),
23+
stopProcessForTakeover: vi.fn(
24+
async (pid: number, options: Parameters<typeof actual.stopProcessForTakeover>[1]) => {
25+
if (pid !== winner.pid) return await actual.stopProcessForTakeover(pid, options);
26+
winner.alive = false;
27+
},
2128
),
2229
};
2330
});
@@ -26,7 +33,8 @@ import { resolveDaemonPaths, type DaemonPaths } from '../../daemon-resolution.ts
2633
import { sendToDaemon } from '../daemon-client.ts';
2734
import { runCmdDetachedMonitored, type ExecDetachedExit } from '@agent-device/host-kit/command';
2835
import { sleep } from '@agent-device/host-kit/retry';
29-
import { readVersion } from '@agent-device/host-kit/version';
36+
import { findProjectRoot, readVersion } from '@agent-device/host-kit/version';
37+
import { computeDaemonCodeSignature } from '@agent-device/host-kit/code-signature';
3038
import {
3139
startHttpDaemonFixture,
3240
type HttpDaemonFixture,
@@ -36,21 +44,38 @@ import { closeLoopbackServer, supportsLoopbackBind } from '../../__tests__/test-
3644
// Two clients that find no daemon both launch one; the daemon that loses the startup lock exits
3745
// cleanly. These pin that the losing client adopts the winner instead of tearing it down.
3846

39-
const WINNER_PID = 43_300;
47+
const WINNER_PID = winner.pid;
4048
const LOSER_PID = 43_301;
4149

4250
const mockRunCmdDetached = vi.mocked(runCmdDetachedMonitored);
4351
const mockSleep = vi.mocked(sleep);
4452

4553
afterEach(() => {
54+
winner.alive = true;
4655
mockRunCmdDetached.mockReset();
4756
mockSleep.mockReset();
4857
mockSleep.mockImplementation(async () => {});
4958
vi.unstubAllEnvs();
5059
});
5160

61+
/** The code signature this client expects of a daemon it may reuse. */
62+
function currentDaemonCodeSignature(): string {
63+
const root = findProjectRoot();
64+
const distPath = path.join(root, 'dist', 'src', 'internal', 'daemon.js');
65+
const entryPath =
66+
process.execArgv.includes('--experimental-strip-types') || !fs.existsSync(distPath)
67+
? path.join(root, 'src', 'daemon.ts')
68+
: distPath;
69+
return computeDaemonCodeSignature(entryPath, root);
70+
}
71+
5272
/** Records the winning daemon the way it would: the startup lock, then its reachable metadata. */
53-
function writeWinner(paths: DaemonPaths, fixture: HttpDaemonFixture, parts: 'lock' | 'all'): void {
73+
function writeWinner(
74+
paths: DaemonPaths,
75+
fixture: HttpDaemonFixture,
76+
parts: 'lock' | 'all',
77+
version = readVersion(),
78+
): void {
5479
fs.mkdirSync(paths.baseDir, { recursive: true });
5580
fs.writeFileSync(
5681
paths.lockPath,
@@ -62,7 +87,8 @@ function writeWinner(paths: DaemonPaths, fixture: HttpDaemonFixture, parts: 'loc
6287
JSON.stringify({
6388
token: 'winner-secret',
6489
pid: WINNER_PID,
65-
version: readVersion(),
90+
version,
91+
codeSignature: currentDaemonCodeSignature(),
6692
processStartTime: 'winner',
6793
httpPort: fixture.port,
6894
transport: 'http',
@@ -140,3 +166,44 @@ test('a one-shot test run leaves a daemon another client started running', async
140166
fs.rmSync(stateDir, { recursive: true, force: true });
141167
}
142168
});
169+
170+
test('a start race won by an older daemon replaces it instead of adopting it', async (t) => {
171+
if (!(await supportsLoopbackBind())) {
172+
t.skip('loopback listeners are not permitted in this environment');
173+
return;
174+
}
175+
const stateDir = mkdtempForTestSync('agent-device-daemon-start-race-older-');
176+
const paths = resolveDaemonPaths(stateDir);
177+
vi.stubEnv('AGENT_DEVICE_STATE_DIR', stateDir);
178+
const fixture = await startHttpDaemonFixture({ devices: [] });
179+
let launches = 0;
180+
mockRunCmdDetached.mockImplementation(() => {
181+
launches += 1;
182+
if (launches === 1) writeWinner(paths, fixture, 'all', '0.0.1');
183+
const exit: ExecDetachedExit = { pid: LOSER_PID, exitCode: 0 };
184+
return { pid: LOSER_PID, exited: Promise.resolve(exit) };
185+
});
186+
const stderr = vi.spyOn(process.stderr, 'write').mockImplementation(() => true);
187+
188+
try {
189+
await assert.rejects(
190+
sendToDaemon({
191+
session: 'default',
192+
command: 'devices',
193+
positionals: [],
194+
flags: { stateDir },
195+
meta: { requestId: 'req-start-race-older' },
196+
}),
197+
);
198+
assert.equal(fixture.rpcRequests.length, 0);
199+
assert.equal(launches, 2);
200+
assert.match(
201+
String(stderr.mock.calls.flat().join('')),
202+
/Replacing daemon \(pid 43300, v0\.0\.1\)/,
203+
);
204+
} finally {
205+
stderr.mockRestore();
206+
await closeLoopbackServer(fixture.server);
207+
fs.rmSync(stateDir, { recursive: true, force: true });
208+
}
209+
});

‎src/daemon-client/daemon-client-lifecycle.ts‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { DaemonRequest, DaemonResponse } from '../daemon/daemon-request.ts'
88
import { runCmdDetachedMonitored, type ExecDetachedExit } from '@agent-device/host-kit/command';
99
import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell';
1010
import { emitDiagnostic } from '@agent-device/host-kit/diagnostics';
11+
import { readProcessStartTime } from '@agent-device/host-kit/process';
1112
import { sleep } from '@agent-device/host-kit/retry';
1213

1314
import { findUnrecoveredRepairCommitFailure } from '../session-repair-tombstone.ts';
@@ -64,11 +65,13 @@ export type EnsuredDaemon = {
6465

6566
type DaemonStartupLaunch = {
6667
pid: number;
68+
/** The launched process's start time, so a reused pid is never taken for it. */
69+
startTime?: string;
6770
exited: Promise<ExecDetachedExit>;
6871
};
6972

7073
type DaemonStartupWaitResult =
71-
| { kind: 'ready'; info: DaemonInfo; startedByClient: boolean }
74+
| { kind: 'ready'; daemon: EnsuredDaemon }
7275
| { kind: 'early_exit'; exit: ExecDetachedExit }
7376
| { kind: 'timeout' };
7477

@@ -275,9 +278,7 @@ async function startLocalDaemon(settings: DaemonClientSettings): Promise<Ensured
275278
}
276279

277280
const startup = await waitForDaemonStartup(DAEMON_STARTUP_TIMEOUT_MS, settings, launch);
278-
if (startup.kind === 'ready') {
279-
return { info: startup.info, startedByClient: startup.startedByClient };
280-
}
281+
if (startup.kind === 'ready') return startup.daemon;
281282
if (startup.kind === 'early_exit') {
282283
daemonProcess = startup.exit;
283284
startError = describeDaemonEarlyExit(startup.exit);
@@ -302,9 +303,7 @@ async function startLocalDaemon(settings: DaemonClientSettings): Promise<Ensured
302303
cleanupResults.push(cleanup);
303304
if (cleanup.retainedInfoProcess || cleanup.retainedLockProcess) {
304305
const extended = await waitForDaemonStartup(DAEMON_STARTUP_TIMEOUT_MS, settings, launch);
305-
if (extended.kind === 'ready') {
306-
return { info: extended.info, startedByClient: extended.startedByClient };
307-
}
306+
if (extended.kind === 'ready') return extended.daemon;
308307
if (extended.kind === 'early_exit') {
309308
daemonProcess = extended.exit;
310309
startError = describeDaemonEarlyExit(extended.exit);
@@ -596,8 +595,13 @@ async function waitForDaemonStartup(
596595
while (Date.now() - start < timeoutMs) {
597596
const info = readDaemonInfo(settings.paths.infoPath);
598597
if (info && (await canConnect(info, settings.transportPreference))) {
599-
// Another client's daemon may have won the start; only the launched one is this client's.
600-
return { kind: 'ready', info, startedByClient: info.pid === launch.pid };
598+
if (isLaunchedDaemon(info, launch)) {
599+
return { kind: 'ready', daemon: { info, startedByClient: true } };
600+
}
601+
// Another client's daemon won the start: adopt it only as a reusable daemon would be. An
602+
// incompatible one is replaced, and this wait then sees its own daemon's early exit.
603+
const winner = await readReusableLocalDaemon(settings);
604+
if (winner) return { kind: 'ready', daemon: { info: winner, startedByClient: false } };
601605
}
602606
// A daemon that lost the startup lock exits cleanly; the daemon that won it is still starting.
603607
if (earlyExit && !isDaemonLockHeldByAnotherDaemon(settings.paths, earlyExit.pid)) {
@@ -608,6 +612,15 @@ async function waitForDaemonStartup(
608612
return { kind: 'timeout' };
609613
}
610614

615+
/** Whether `info` names the daemon process this client launched: same pid and start time. */
616+
function isLaunchedDaemon(info: DaemonInfo, launch: DaemonStartupLaunch): boolean {
617+
return (
618+
info.pid === launch.pid &&
619+
launch.startTime !== undefined &&
620+
info.processStartTime === launch.startTime
621+
);
622+
}
623+
611624
function startDaemon(settings: DaemonClientSettings): DaemonStartupLaunch {
612625
const launchSpec = resolveDaemonLaunchSpec();
613626
const args = launchSpec.useSrc
@@ -623,10 +636,11 @@ function startDaemon(settings: DaemonClientSettings): DaemonStartupLaunch {
623636
const stdoutFd = fs.openSync(settings.paths.logPath, 'a');
624637
const stderrFd = fs.openSync(settings.paths.logPath, 'a');
625638
try {
626-
return runCmdDetachedMonitored(process.execPath, args, {
639+
const launched = runCmdDetachedMonitored(process.execPath, args, {
627640
env,
628641
stdio: ['ignore', stdoutFd, stderrFd],
629642
});
643+
return { ...launched, startTime: readProcessStartTime(launched.pid) ?? undefined };
630644
} finally {
631645
fs.closeSync(stdoutFd);
632646
fs.closeSync(stderrFd);

0 commit comments

Comments
 (0)