Skip to content

Commit 5be101c

Browse files
authored
fix: clean up Android snapshot helper sessions (#862)
1 parent c9748a9 commit 5be101c

7 files changed

Lines changed: 303 additions & 18 deletions

File tree

src/core/interactors/android.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ export function createAndroidInteractor(device: DeviceInfo): Interactor {
7171
depth: options?.depth,
7272
scope: options?.scope,
7373
raw: options?.raw,
74+
// appBundleId is present for app-backed daemon sessions; keep the helper warm there,
75+
// but release it after standalone device snapshots so UiAutomation is not squatted.
76+
helperSessionScope: options?.appBundleId ? 'daemon-session' : 'command',
7477
}),
7578
{ backend: 'android' },
7679
);

src/daemon/handlers/__tests__/session-close-shutdown.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ vi.mock('../../../platforms/android/perf.ts', async (importOriginal) => {
2626
const actual = await importOriginal<typeof import('../../../platforms/android/perf.ts')>();
2727
return { ...actual, cleanupAndroidNativePerfSession: vi.fn(async () => {}) };
2828
});
29+
vi.mock('../../../platforms/android/snapshot-helper.ts', async (importOriginal) => {
30+
const actual =
31+
await importOriginal<typeof import('../../../platforms/android/snapshot-helper.ts')>();
32+
return { ...actual, stopAndroidSnapshotHelperSessionForDevice: vi.fn(async () => {}) };
33+
});
2934
vi.mock('../../../platforms/ios/macos-helper.ts', async (importOriginal) => {
3035
const actual = await importOriginal<typeof import('../../../platforms/ios/macos-helper.ts')>();
3136
return { ...actual, runMacOsAlertAction: vi.fn(async () => {}) };
@@ -50,13 +55,17 @@ import { runCmd } from '../../../utils/exec.ts';
5055
import { dispatchCommand } from '../../../core/dispatch.ts';
5156
import { cleanupAppleXctracePerfCapture } from '../../../platforms/ios/perf-xctrace.ts';
5257
import { cleanupAndroidNativePerfSession } from '../../../platforms/android/perf.ts';
58+
import { stopAndroidSnapshotHelperSessionForDevice } from '../../../platforms/android/snapshot-helper.ts';
5359
import { WEB_DESKTOP_DEVICE } from '../../../__tests__/test-utils/index.ts';
5460

5561
const mockShutdownSimulator = vi.mocked(shutdownSimulator);
5662
const mockRunCmd = vi.mocked(runCmd);
5763
const mockDispatchCommand = vi.mocked(dispatchCommand);
5864
const mockCleanupAppleXctracePerfCapture = vi.mocked(cleanupAppleXctracePerfCapture);
5965
const mockCleanupAndroidNativePerfSession = vi.mocked(cleanupAndroidNativePerfSession);
66+
const mockStopAndroidSnapshotHelperSessionForDevice = vi.mocked(
67+
stopAndroidSnapshotHelperSessionForDevice,
68+
);
6069

6170
const noopInvoke = async (_req: DaemonRequest): Promise<DaemonResponse> => ({ ok: true, data: {} });
6271

@@ -176,6 +185,40 @@ test('close --shutdown calls shutdownAndroidEmulator for Android emulator and in
176185
}
177186
});
178187

188+
test('close stops Android snapshot helper session before deleting session', async () => {
189+
const sessionStore = makeSessionStore();
190+
const sessionName = 'android-snapshot-helper-session';
191+
const device: SessionState['device'] = {
192+
platform: 'android',
193+
id: 'emulator-5554',
194+
name: 'Pixel_9_API_35',
195+
kind: 'emulator',
196+
booted: true,
197+
};
198+
sessionStore.set(sessionName, {
199+
...makeSession(sessionName, device),
200+
appBundleId: 'com.example.app',
201+
});
202+
203+
const response = await handleSessionCommands({
204+
req: {
205+
token: 't',
206+
session: sessionName,
207+
command: 'close',
208+
positionals: [],
209+
flags: {},
210+
},
211+
sessionName,
212+
logPath: path.join(os.tmpdir(), 'daemon.log'),
213+
sessionStore,
214+
invoke: noopInvoke,
215+
});
216+
217+
expect(response?.ok).toBe(true);
218+
expect(mockStopAndroidSnapshotHelperSessionForDevice).toHaveBeenCalledWith(device);
219+
expect(sessionStore.get(sessionName)).toBeUndefined();
220+
});
221+
179222
test('close --shutdown is ignored for non-simulator iOS devices', async () => {
180223
const sessionStore = makeSessionStore();
181224
const sessionName = 'ios-device-shutdown-session';
@@ -411,6 +454,24 @@ test('daemon session teardown stops active Android native perf capture', async (
411454
expect(session.nativePerf?.android).toBeUndefined();
412455
});
413456

457+
test('daemon session teardown stops Android snapshot helper session', async () => {
458+
const sessionName = 'android-snapshot-helper-teardown-session';
459+
const session = {
460+
...makeSession(sessionName, {
461+
platform: 'android',
462+
id: 'emulator-5554',
463+
name: 'Pixel',
464+
kind: 'emulator',
465+
booted: true,
466+
}),
467+
appBundleId: 'com.example.app',
468+
} as SessionState;
469+
470+
await teardownSessionResources(session, sessionName);
471+
472+
expect(mockStopAndroidSnapshotHelperSessionForDevice).toHaveBeenCalledWith(session.device);
473+
});
474+
414475
test('close --shutdown is ignored for Android devices', async () => {
415476
const sessionStore = makeSessionStore();
416477
const sessionName = 'android-device-shutdown-session';

src/daemon/handlers/session-close.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { stopAppLog } from '../app-log.ts';
99
import { stopIosRunnerSession } from '../../platforms/ios/runner-client.ts';
1010
import { cleanupAppleXctracePerfCapture } from '../../platforms/ios/perf-xctrace.ts';
1111
import { cleanupAndroidNativePerfSession } from '../../platforms/android/perf.ts';
12+
import { stopAndroidSnapshotHelperSessionForDevice } from '../../platforms/android/snapshot-helper.ts';
1213
import { clearRuntimeHintsFromApp, hasRuntimeTransportHints } from '../runtime-hints.ts';
1314
import { cleanupRetainedMaterializedPathsForSession } from '../materialized-path-registry.ts';
1415
import {
@@ -80,6 +81,11 @@ async function stopSessionAndroidNativePerfCapture(session: SessionState): Promi
8081
session.nativePerf = { ...(session.nativePerf ?? {}), android: undefined };
8182
}
8283

84+
async function stopSessionAndroidSnapshotHelper(session: SessionState): Promise<void> {
85+
if (session.device.platform !== 'android') return;
86+
await stopAndroidSnapshotHelperSessionForDevice(session.device);
87+
}
88+
8389
export async function teardownSessionResources(
8490
session: SessionState,
8591
sessionName: string,
@@ -89,6 +95,7 @@ export async function teardownSessionResources(
8995
}
9096
await stopSessionApplePerfCapture(session);
9197
await stopSessionAndroidNativePerfCapture(session);
98+
await stopSessionAndroidSnapshotHelper(session);
9299
if (isApplePlatform(session.device.platform)) {
93100
await stopAppleRunnerForClose(session);
94101
}
@@ -111,6 +118,7 @@ export async function handleCloseCommand(params: {
111118
}
112119
await stopSessionApplePerfCapture(session);
113120
await stopSessionAndroidNativePerfCapture(session);
121+
await stopSessionAndroidSnapshotHelper(session);
114122
if (shouldDispatchPlatformClose(req, session)) {
115123
if (shouldStopAppleRunnerBeforeTargetedClose(session)) {
116124
await stopAppleRunnerForClose(session);

src/platforms/android/__tests__/snapshot.test.ts

Lines changed: 197 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1-
import { beforeEach, test, vi } from 'vitest';
1+
import { afterEach, beforeEach, test, vi } from 'vitest';
22
import assert from 'node:assert/strict';
3+
import { EventEmitter } from 'node:events';
34
import { promises as fs } from 'node:fs';
5+
import net from 'node:net';
46
import os from 'node:os';
57
import path from 'node:path';
8+
import { PassThrough } from 'node:stream';
69

710
vi.mock('../../../utils/exec.ts', async (importOriginal) => {
811
const actual = await importOriginal<typeof import('../../../utils/exec.ts')>();
@@ -22,11 +25,16 @@ import { AppError } from '../../../utils/errors.ts';
2225
import { runCmd } from '../../../utils/exec.ts';
2326
import { sleep } from '../adb.ts';
2427
import {
28+
resetAndroidSnapshotHelperSessions,
2529
resetAndroidSnapshotHelperInstallCache,
2630
type AndroidAdbExecutor,
2731
type AndroidSnapshotHelperManifest,
2832
} from '../snapshot-helper.ts';
29-
import { withAndroidAdbProvider, type AndroidAdbProvider } from '../adb-executor.ts';
33+
import {
34+
withAndroidAdbProvider,
35+
type AndroidAdbProcess,
36+
type AndroidAdbProvider,
37+
} from '../adb-executor.ts';
3038

3139
const VALID_PNG = Buffer.from(
3240
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+b9xkAAAAASUVORK5CYII=',
@@ -94,7 +102,123 @@ function createHelperAdb(
94102
};
95103
}
96104

97-
beforeEach(() => {
105+
function createPersistentSnapshotHelperProvider(options: {
106+
calls: string[][];
107+
spawnArgs: string[][];
108+
killedProcesses: FakeAndroidProcess[];
109+
}): AndroidAdbProvider {
110+
return {
111+
exec: async (args) => {
112+
options.calls.push(args);
113+
if (args.includes('--show-versioncode')) return installedHelperProbe;
114+
if (args[0] === 'forward') return { exitCode: 0, stdout: '', stderr: '' };
115+
if (args[0] === 'shell' && args[1] === 'am' && args[2] === 'force-stop') {
116+
return { exitCode: 0, stdout: '', stderr: '' };
117+
}
118+
throw new Error(`unexpected persistent helper adb args: ${args.join(' ')}`);
119+
},
120+
spawn: (args) => {
121+
options.spawnArgs.push(args);
122+
const process = new FakeAndroidProcess();
123+
const port = readSessionPort(args);
124+
let snapshotCount = 0;
125+
const server = net.createServer((socket) => {
126+
socket.once('data', (chunk) => {
127+
const command = chunk.toString('utf8').trim();
128+
const [, requestId = ''] = command.split(/\s+/, 2);
129+
if (command.startsWith('quit')) {
130+
socket.end(sessionResponse({ requestId, body: '' }));
131+
return;
132+
}
133+
snapshotCount += 1;
134+
const body = `<hierarchy><node text="persistent helper snapshot ${snapshotCount}" bounds="[0,0][10,10]" /></hierarchy>`;
135+
socket.end(
136+
sessionResponse({
137+
requestId,
138+
body,
139+
metadata: {
140+
waitForIdleTimeoutMs: '500',
141+
waitForIdleQuietMs: '100',
142+
timeoutMs: '5000',
143+
maxDepth: '128',
144+
maxNodes: '5000',
145+
rootPresent: 'true',
146+
captureMode: 'interactive-windows',
147+
windowCount: '1',
148+
nodeCount: '1',
149+
truncated: 'false',
150+
elapsedMs: '8',
151+
},
152+
}),
153+
);
154+
});
155+
});
156+
server.listen(port, '127.0.0.1', () => {
157+
process.stdout.write(
158+
[
159+
'INSTRUMENTATION_STATUS: agentDeviceProtocol=android-snapshot-helper-v1',
160+
'INSTRUMENTATION_STATUS: sessionReady=true',
161+
'INSTRUMENTATION_STATUS_CODE: 2',
162+
'',
163+
].join('\n'),
164+
);
165+
});
166+
process.onKill = () => {
167+
options.killedProcesses.push(process);
168+
server.close(() => process.emitExit(0, null));
169+
};
170+
return process;
171+
},
172+
};
173+
}
174+
175+
function sessionResponse(params: {
176+
requestId: string;
177+
body: string;
178+
metadata?: Record<string, string>;
179+
}): string {
180+
const headers = {
181+
agentDeviceProtocol: 'android-snapshot-helper-v1',
182+
helperApiVersion: '1',
183+
outputFormat: 'uiautomator-xml',
184+
requestId: params.requestId,
185+
ok: 'true',
186+
byteLength: String(Buffer.byteLength(params.body, 'utf8')),
187+
...params.metadata,
188+
};
189+
return `${Object.entries(headers)
190+
.map(([key, value]) => `${key}=${value}`)
191+
.join('\n')}\n\n${params.body}`;
192+
}
193+
194+
function readSessionPort(args: string[]): number {
195+
const index = args.indexOf('sessionPort');
196+
assert.notEqual(index, -1);
197+
return Number(args[index + 1]);
198+
}
199+
200+
class FakeAndroidProcess extends EventEmitter implements AndroidAdbProcess {
201+
stdin = new PassThrough();
202+
stdout = new PassThrough();
203+
stderr = new PassThrough();
204+
killed = false;
205+
onKill: (() => void) | undefined;
206+
207+
kill(): boolean {
208+
if (this.killed) return true;
209+
this.killed = true;
210+
this.onKill?.();
211+
return true;
212+
}
213+
214+
emitExit(code: number | null, signal: NodeJS.Signals | null): void {
215+
this.emit('exit', code, signal);
216+
this.emit('close', code, signal);
217+
}
218+
}
219+
220+
beforeEach(async () => {
221+
await resetAndroidSnapshotHelperSessions();
98222
resetAndroidSnapshotHelperInstallCache();
99223
mockRunCmd.mockReset();
100224
mockSleep.mockReset();
@@ -107,6 +231,10 @@ beforeEach(() => {
107231
});
108232
});
109233

234+
afterEach(async () => {
235+
await resetAndroidSnapshotHelperSessions();
236+
});
237+
110238
test('screenshotAndroid waits for transient UI to settle before capture', async () => {
111239
const events: string[] = [];
112240
const outPath = path.join(os.tmpdir(), `agent-device-android-screenshot-${Date.now()}.png`);
@@ -496,6 +624,72 @@ test('snapshotAndroid resolves helper adb through scoped provider', async () =>
496624
assert.equal(mockRunCmd.mock.calls.length, 0);
497625
});
498626

627+
test('snapshotAndroid stops command-scoped persistent helper session after capture', async () => {
628+
const adbCalls: string[][] = [];
629+
const spawnArgs: string[][] = [];
630+
const killedProcesses: FakeAndroidProcess[] = [];
631+
const provider = createPersistentSnapshotHelperProvider({
632+
calls: adbCalls,
633+
spawnArgs,
634+
killedProcesses,
635+
});
636+
637+
const result = await snapshotAndroid(device, {
638+
helperAdb: provider,
639+
helperArtifact,
640+
});
641+
642+
assert.equal(result.nodes[0]?.label, 'persistent helper snapshot 1');
643+
assert.equal(result.androidSnapshot.helperTransport, 'persistent-session');
644+
assert.equal(result.androidSnapshot.helperSessionReused, false);
645+
assert.equal(spawnArgs.length, 1);
646+
assert.equal(killedProcesses.length, 1);
647+
assert.equal(
648+
adbCalls.some((args) => args[0] === 'forward' && args[1] === '--remove'),
649+
true,
650+
);
651+
});
652+
653+
test('snapshotAndroid keeps daemon-session helper alive for reuse until session cleanup', async () => {
654+
const adbCalls: string[][] = [];
655+
const spawnArgs: string[][] = [];
656+
const killedProcesses: FakeAndroidProcess[] = [];
657+
const provider = createPersistentSnapshotHelperProvider({
658+
calls: adbCalls,
659+
spawnArgs,
660+
killedProcesses,
661+
});
662+
663+
const first = await snapshotAndroid(device, {
664+
helperAdb: provider,
665+
helperArtifact,
666+
helperSessionScope: 'daemon-session',
667+
});
668+
const second = await snapshotAndroid(device, {
669+
helperAdb: provider,
670+
helperArtifact,
671+
helperSessionScope: 'daemon-session',
672+
});
673+
674+
assert.equal(first.androidSnapshot.helperSessionReused, false);
675+
assert.equal(second.androidSnapshot.helperSessionReused, true);
676+
assert.equal(second.nodes[0]?.label, 'persistent helper snapshot 2');
677+
assert.equal(spawnArgs.length, 1);
678+
assert.equal(killedProcesses.length, 0);
679+
assert.equal(
680+
adbCalls.some((args) => args[0] === 'forward' && args[1] === '--remove'),
681+
false,
682+
);
683+
684+
await resetAndroidSnapshotHelperSessions();
685+
686+
assert.equal(killedProcesses.length, 1);
687+
assert.equal(
688+
adbCalls.some((args) => args[0] === 'forward' && args[1] === '--remove'),
689+
true,
690+
);
691+
});
692+
499693
test('snapshotAndroid falls back to stock uiautomator when helper fails', async () => {
500694
const adbCalls: string[][] = [];
501695
const stockXml =

0 commit comments

Comments
 (0)