fix(android): start a helper instead of writing to a session that already died

Author: thymikee

packages/platform-android/src/__tests__/snapshot-helper-session-lifecycle.test.ts

@@ -174,6 +174,33 @@ test('restarts the helper session when capture options change', async () => {
   );
 });
 
+test('a session whose helper process died is not written to again', async () => {
+  const calls: string[][] = [];
+  const spawnArgs: string[][] = [];
+  const processes: FakeAndroidProcess[] = [];
+  const provider = createSessionProvider({ calls, spawnArgs, processes });
+
+  await captureAndroidSnapshotWithHelperSession({
+    adb: provider.exec,
+    adbProvider: provider,
+    deviceKey: 'android:emulator-5554',
+  });
+  processes[0]!.emitExit(137, null);
+
+  const restarted = await captureAndroidSnapshotWithHelperSession({
+    adb: provider.exec,
+    adbProvider: provider,
+    deviceKey: 'android:emulator-5554',
+  });
+
+  assert.equal(restarted?.metadata.sessionReused, false);
+  assert.equal(spawnArgs.length, 2);
+  assert.equal(
+    calls.some((args) => args[0] === 'forward' && args[1] === '--remove'),
+    true,
+  );
+});
+
 test('a quit acknowledged and followed by process exit skips the force-stop round trip', async () => {
   const calls: string[][] = [];
   const processes: FakeAndroidProcess[] = [];

packages/platform-android/src/snapshot-helper-retirement.ts

@@ -238,7 +238,8 @@ export function observeAndroidSnapshotHelperProcessExit(
   };
 }
 
-function hasAndroidSnapshotHelperProcessEnded(childProcess: AndroidAdbProcess): boolean {
+/** Whether the host side of an instrumentation session is already gone. */
+export function hasAndroidSnapshotHelperProcessEnded(childProcess: AndroidAdbProcess): boolean {
   return childProcess.exitCode != null || childProcess.signalCode != null;
 }
 

packages/platform-android/src/snapshot-helper-session-lifecycle.ts

@@ -35,6 +35,7 @@ import {
   ANDROID_SNAPSHOT_HELPER_DEVICE_RETIREMENT_TIMEOUT_MS,
   ANDROID_SNAPSHOT_HELPER_HOST_PROCESS_EXIT_GRACE_MS,
   getAndroidSnapshotHelperSessionDeviceKey,
+  hasAndroidSnapshotHelperProcessEnded,
   observeAndroidSnapshotHelperProcessExit,
   recoverAndroidSnapshotHelperRetirement,
   recordAndroidSnapshotHelperRelease,
@@ -137,11 +138,16 @@ async function resolveAndroidSnapshotHelperSession(params: {
   resolved: AndroidSnapshotHelperResolvedCaptureOptions;
 }): Promise<AndroidSnapshotHelperSession | undefined> {
   const { deviceKey, identity, options, resolved } = params;
-  let session = sessions.get(deviceKey);
-  if (session && session.identity !== identity) {
-    await stopAndroidSnapshotHelperSession(deviceKey);
-    session = undefined;
+  const cached = sessions.get(deviceKey);
+  const reusable = cached !== undefined && isReusableAndroidSnapshotHelperSession(cached, identity);
+  if (cached && !reusable) {
+    await stopAndroidSnapshotHelperSession(deviceKey, {
+      // A process that already exited cannot answer the forwarded port: the forward is all that is
+      // left of it, so there is nothing to quit gracefully.
+      force: hasAndroidSnapshotHelperProcessEnded(cached.process),
+    });
   }
+  let session = reusable ? cached : undefined;
   if (!session) {
     try {
       session = await startAndroidSnapshotHelperSession({
@@ -167,6 +173,19 @@ async function resolveAndroidSnapshotHelperSession(params: {
   return session;
 }
 
+/**
+ * A cached session is worth writing to only while it belongs to this helper build and its
+ * instrumentation process is still running. The helper binds its session socket inside that
+ * process, so a process that has exited has nobody left to accept on the forwarded port: the
+ * command would die on a dead socket and fall back, instead of starting a helper that can answer.
+ */
+function isReusableAndroidSnapshotHelperSession(
+  session: AndroidSnapshotHelperSession,
+  identity: string,
+): boolean {
+  return session.identity === identity && !hasAndroidSnapshotHelperProcessEnded(session.process);
+}
+
 async function startAndroidSnapshotHelperSession(params: {
   deviceKey: string;
   identity: string;