Skip to content

Commit 79918a2

Browse files
committed
fix: address lifecycle review findings
1 parent c43be1d commit 79918a2

4 files changed

Lines changed: 73 additions & 49 deletions

File tree

src/daemon-client.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ async function sendRequest(info: DaemonInfo, req: DaemonRequest): Promise<Daemon
164164
}
165165

166166
export function resolveDaemonRequestTimeoutMs(raw: string | undefined = process.env.AGENT_DEVICE_DAEMON_TIMEOUT_MS): number {
167+
// iOS physical-device runner startup/build can exceed 60s, so use a safer default for daemon RPCs.
167168
if (!raw) return 180000;
168169
const parsed = Number(raw);
169170
if (!Number.isFinite(parsed)) return 180000;

src/daemon.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,8 @@ function acquireDaemonLock(): boolean {
187187
) {
188188
return false;
189189
}
190+
// Best-effort stale-lock cleanup: another process may win the race between unlink and re-create.
191+
// We rely on the subsequent write with `wx` to enforce single-writer semantics.
190192
try {
191193
fs.unlinkSync(lockPath);
192194
} catch {
@@ -251,19 +253,27 @@ function start(): void {
251253
});
252254

253255
let shuttingDown = false;
256+
const closeServer = async (): Promise<void> => {
257+
await new Promise<void>((resolve) => {
258+
try {
259+
server.close(() => resolve());
260+
} catch {
261+
resolve();
262+
}
263+
});
264+
};
254265
const shutdown = async () => {
255266
if (shuttingDown) return;
256267
shuttingDown = true;
268+
await closeServer();
257269
const sessionsToStop = sessionStore.toArray();
258270
for (const session of sessionsToStop) {
259271
sessionStore.writeSessionLog(session);
260272
}
261273
await stopAllIosRunnerSessions();
262-
server.close(() => {
263-
removeInfo();
264-
releaseDaemonLock();
265-
process.exit(0);
266-
});
274+
removeInfo();
275+
releaseDaemonLock();
276+
process.exit(0);
267277
};
268278

269279
process.on('SIGINT', () => {

src/daemon/handlers/snapshot.ts

Lines changed: 38 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { dispatchCommand, resolveTargetDevice } from '../../core/dispatch.ts';
22
import { isCommandSupportedOnDevice } from '../../core/capabilities.ts';
3-
import { runIosRunnerCommand } from '../../platforms/ios/runner-client.ts';
4-
import { stopIosRunnerSession } from '../../platforms/ios/runner-client.ts';
3+
import { runIosRunnerCommand, stopIosRunnerSession } from '../../platforms/ios/runner-client.ts';
54
import { snapshotAndroid } from '../../platforms/android/index.ts';
65
import {
76
attachRefs,
@@ -79,8 +78,7 @@ export async function handleSnapshotCommands(params: {
7978
}
8079
snapshotScope = resolved;
8180
}
82-
const shouldCleanupSessionlessIosRunner = !session && device.platform === 'ios';
83-
try {
81+
return await withSessionlessRunnerCleanup(session, device, async () => {
8482
const data = (await dispatchCommand(device, 'snapshot', [], req.flags?.out, {
8583
...contextFromFlags(
8684
logPath,
@@ -120,11 +118,7 @@ export async function handleSnapshotCommands(params: {
120118
appBundleId: nextSession.appBundleId,
121119
},
122120
};
123-
} finally {
124-
if (shouldCleanupSessionlessIosRunner) {
125-
await stopIosRunnerSession(device.id);
126-
}
127-
}
121+
});
128122
}
129123

130124
if (command === 'wait') {
@@ -148,8 +142,7 @@ export async function handleSnapshotCommands(params: {
148142
error: { code: 'UNSUPPORTED_OPERATION', message: 'wait is not supported on this device' },
149143
};
150144
}
151-
const shouldCleanupSessionlessIosRunner = !session && device.platform === 'ios';
152-
try {
145+
return await withSessionlessRunnerCleanup(session, device, async () => {
153146
let text: string;
154147
let timeoutMs: number | null;
155148
if (parsed.kind === 'selector') {
@@ -269,11 +262,7 @@ export async function handleSnapshotCommands(params: {
269262
ok: false,
270263
error: { code: 'COMMAND_FAILED', message: `wait timed out for text: ${text}` },
271264
};
272-
} finally {
273-
if (shouldCleanupSessionlessIosRunner) {
274-
await stopIosRunnerSession(device.id);
275-
}
276-
}
265+
});
277266
}
278267

279268
if (command === 'alert') {
@@ -288,8 +277,7 @@ export async function handleSnapshotCommands(params: {
288277
},
289278
};
290279
}
291-
const shouldCleanupSessionlessIosRunner = !session && device.platform === 'ios';
292-
try {
280+
return await withSessionlessRunnerCleanup(session, device, async () => {
293281
if (action === 'wait') {
294282
const timeout = parseTimeout(req.positionals?.[1]) ?? DEFAULT_TIMEOUT_MS;
295283
const start = Date.now();
@@ -321,11 +309,7 @@ export async function handleSnapshotCommands(params: {
321309
);
322310
recordIfSession(sessionStore, session, req, data as Record<string, unknown>);
323311
return { ok: true, data };
324-
} finally {
325-
if (shouldCleanupSessionlessIosRunner) {
326-
await stopIosRunnerSession(device.id);
327-
}
328-
}
312+
});
329313
}
330314

331315
if (command === 'settings') {
@@ -350,18 +334,20 @@ export async function handleSnapshotCommands(params: {
350334
},
351335
};
352336
}
353-
const appBundleId = session?.appBundleId;
354-
const data = await dispatchCommand(
355-
device,
356-
'settings',
357-
[setting, state, appBundleId ?? ''],
358-
req.flags?.out,
359-
{
360-
...contextFromFlags(logPath, req.flags, appBundleId, session?.trace?.outPath),
361-
},
362-
);
363-
recordIfSession(sessionStore, session, req, data ?? { setting, state });
364-
return { ok: true, data: data ?? { setting, state } };
337+
return await withSessionlessRunnerCleanup(session, device, async () => {
338+
const appBundleId = session?.appBundleId;
339+
const data = await dispatchCommand(
340+
device,
341+
'settings',
342+
[setting, state, appBundleId ?? ''],
343+
req.flags?.out,
344+
{
345+
...contextFromFlags(logPath, req.flags, appBundleId, session?.trace?.outPath),
346+
},
347+
);
348+
recordIfSession(sessionStore, session, req, data ?? { setting, state });
349+
return { ok: true, data: data ?? { setting, state } };
350+
});
365351
}
366352

367353
return null;
@@ -420,6 +406,23 @@ async function resolveSessionDevice(
420406
return { session, device };
421407
}
422408

409+
async function withSessionlessRunnerCleanup<T>(
410+
session: SessionState | undefined,
411+
device: SessionState['device'],
412+
task: () => Promise<T>,
413+
): Promise<T> {
414+
const shouldCleanupSessionlessIosRunner = !session && device.platform === 'ios';
415+
try {
416+
return await task();
417+
} finally {
418+
// Sessionless iOS commands intentionally stop the runner to avoid leaked xcodebuild processes.
419+
// For multi-command flows, keep an active session via `open` so the runner can be reused.
420+
if (shouldCleanupSessionlessIosRunner) {
421+
await stopIosRunnerSession(device.id);
422+
}
423+
}
424+
}
425+
423426
function recordIfSession(
424427
sessionStore: SessionStore,
425428
session: SessionState | undefined,

src/platforms/ios/runner-client.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,9 @@ async function executeRunnerCommand(
140140
command: RunnerCommand,
141141
options: { verbose?: boolean; logPath?: string; traceLogPath?: string } = {},
142142
): Promise<Record<string, unknown>> {
143+
let session: RunnerSession | undefined;
143144
try {
144-
const session = await ensureRunnerSession(device, options);
145+
session = await ensureRunnerSession(device, options);
145146
const timeoutMs = session.ready ? RUNNER_COMMAND_TIMEOUT_MS : RUNNER_STARTUP_TIMEOUT_MS;
146147
return await executeRunnerCommandWithSession(
147148
device,
@@ -157,8 +158,12 @@ async function executeRunnerCommand(
157158
typeof appErr.message === 'string' &&
158159
appErr.message.includes('Runner did not accept connection')
159160
) {
160-
await stopIosRunnerSession(device.id);
161-
const session = await ensureRunnerSession(device, options);
161+
if (session) {
162+
await stopRunnerSession(session);
163+
} else {
164+
await stopIosRunnerSession(device.id);
165+
}
166+
session = await ensureRunnerSession(device, options);
162167
const response = await waitForRunner(
163168
session.device,
164169
session.port,
@@ -217,12 +222,17 @@ export async function stopIosRunnerSession(deviceId: string): Promise<void> {
217222
}
218223

219224
export async function stopAllIosRunnerSessions(): Promise<void> {
220-
while (runnerSessions.size > 0) {
221-
const pending = Array.from(runnerSessions.keys());
222-
await Promise.allSettled(pending.map(async (deviceId) => {
223-
await stopIosRunnerSession(deviceId);
224-
}));
225-
}
225+
// Shutdown cleanup drains the sessions known at invocation time; daemon shutdown closes intake.
226+
const pending = Array.from(runnerSessions.keys());
227+
await Promise.allSettled(pending.map(async (deviceId) => {
228+
await stopIosRunnerSession(deviceId);
229+
}));
230+
}
231+
232+
async function stopRunnerSession(session: RunnerSession): Promise<void> {
233+
await withRunnerSessionLock(session.deviceId, async () => {
234+
await stopRunnerSessionInternal(session.deviceId, session);
235+
});
226236
}
227237

228238
async function stopRunnerSessionInternal(deviceId: string, sessionOverride?: RunnerSession): Promise<void> {

0 commit comments

Comments
 (0)