fix: cancel uncleared timers and abort listeners left by lost Promise.race branches

.nx/version-plans/fix-cancel-race-timers-and-listeners.md

@@ -0,0 +1,10 @@
+---
+__default__: patch
+---
+
+Harness runs on iOS no longer linger for up to ~25 seconds after tests finish
+printing results. Uncancelled timers and abort listeners left over from
+internal `Promise.race` calls (XCTest agent shutdown, Android/iOS/Vega app
+session polling, and startup crash detection) could keep the process alive
+past test completion; they are now cleaned up as soon as the other side of
+the race settles.

packages/jest/src/__tests__/harness-session.test.ts

@@ -155,6 +155,40 @@ describe('waitForStartupCrash', () => {
     await expect(waitPromise).rejects.toThrow('Aborted');
     expect(watch).not.toHaveBeenCalled();
   });
+
+  it('removes its abort listener on the session signal once the crash watch wins', async () => {
+    let rejectWatch!: (error: unknown) => void;
+    const cancel = vi.fn();
+    const watch = vi.fn(() => ({
+      promise: new Promise<never>((_, reject) => {
+        rejectWatch = reject;
+      }),
+      cancel,
+    }));
+    const crashMonitor = { watch } as unknown as CrashMonitor;
+    const controller = new AbortController();
+    const addSpy = vi.spyOn(controller.signal, 'addEventListener');
+    const removeSpy = vi.spyOn(controller.signal, 'removeEventListener');
+
+    const waitPromise = waitForStartupCrash({
+      crashMonitor,
+      detectNativeCrashes: true,
+      testFilePath: '/test.harness.ts',
+      signal: controller.signal,
+    });
+
+    // The crash watch wins the race, not the session signal.
+    rejectWatch(new Error('native crash detected'));
+
+    await expect(waitPromise).rejects.toThrow('native crash detected');
+    expect(cancel).toHaveBeenCalledTimes(1);
+
+    // Regression: forgetting to cancel the losing waitForAbort() left its
+    // listener attached to this session-lifetime signal for the rest of
+    // the session — one leaked listener per test file.
+    expect(addSpy).toHaveBeenCalledTimes(1);
+    expect(removeSpy).toHaveBeenCalledTimes(1);
+  });
 });
 
 describe('withPlatformReadyTimeout', () => {

packages/jest/src/harness-session.ts

@@ -41,6 +41,7 @@ import {
   logger,
   getTimeoutSignal,
   ensureSignalsDeliverable,
+  waitForAbort,
   type Diagnostics,
 } from '@react-native-harness/tools';
 import {
@@ -204,22 +205,6 @@ export const getSignalExitCodeForRunState = (
 // Internal helpers
 // ---------------------------------------------------------------------------
 
-const createAbortError = () =>
-  new DOMException('The operation was aborted', 'AbortError');
-
-const waitForAbort = (signal: AbortSignal): Promise<never> => {
-  if (signal.aborted) {
-    return Promise.reject(signal.reason ?? createAbortError());
-  }
-  return new Promise((_, reject) => {
-    signal.addEventListener(
-      'abort',
-      () => reject(signal.reason ?? createAbortError()),
-      { once: true },
-    );
-  });
-};
-
 // Bounds only the init *call* with a readiness timeout; `options.signal` (the
 // session-lifetime signal) is passed to `work` unmodified, so a slow init
 // call keeps observing real session teardown, not a synthetic timeout abort.
@@ -232,12 +217,17 @@ export const withPlatformReadyTimeout = async <T>(options: {
   work: (signal: AbortSignal) => Promise<T>;
 }): Promise<T> => {
   const timeoutSignal = getTimeoutSignal(options.timeout);
-  return await Promise.race([
-    options.work(options.signal),
-    waitForAbort(timeoutSignal).catch(() => {
-      throw new PlatformReadyTimeoutError(options.timeout);
-    }),
-  ]);
+  const abortWait = waitForAbort(timeoutSignal);
+  try {
+    return await Promise.race([
+      options.work(options.signal),
+      abortWait.promise.catch(() => {
+        throw new PlatformReadyTimeoutError(options.timeout);
+      }),
+    ]);
+  } finally {
+    abortWait.cancel();
+  }
 };
 
 type AppReadyOptions = {
@@ -264,15 +254,17 @@ export const waitForStartupCrash = async ({
   signal: AbortSignal;
 }) => {
   if (!detectNativeCrashes) {
-    return await waitForAbort(signal);
+    return await waitForAbort(signal).promise;
   }
 
   const watch = crashMonitor.watch(testFilePath, 'startup');
   watch.promise.catch(ignorePromiseRejection); // suppress unhandled-rejection when abort wins race
+  const abortWait = waitForAbort(signal);
   try {
-    return await Promise.race([watch.promise, waitForAbort(signal)]);
+    return await Promise.race([watch.promise, abortWait.promise]);
   } finally {
     watch.cancel();
+    abortWait.cancel();
   }
 };
 

packages/platform-android/src/__tests__/adb-wait.test.ts

@@ -0,0 +1,95 @@
+import { afterEach, describe, expect, it, vi } from 'vitest';
+
+const mocks = vi.hoisted(() => ({
+  spawn: vi.fn(),
+}));
+
+// The global test/setup.ts mocks '../adb.js' wholesale (including
+// waitForEmulator itself), which is right for consumers of this module but
+// defeats the purpose here: this suite exercises the *real* polling loop
+// (and its private waitWithSignal helper) to catch timer/listener leaks.
+// Re-declaring the mock for this file restores the actual implementation,
+// while `spawn` and `getAdbBinaryPath` (genuine cross-module imports) are
+// swapped out to keep `getDeviceIds()` from touching a real adb binary.
+vi.mock('../adb.js', async () => {
+  const actual = await vi.importActual<typeof import('../adb.js')>(
+    '../adb.js'
+  );
+
+  return actual;
+});
+
+vi.mock('../environment.js', async () => {
+  const actual = await vi.importActual<typeof import('../environment.js')>(
+    '../environment.js'
+  );
+
+  return { ...actual, getAdbBinaryPath: () => '/fake/sdk/platform-tools/adb' };
+});
+
+vi.mock('@react-native-harness/tools', async () => {
+  const actual = await vi.importActual<
+    typeof import('@react-native-harness/tools')
+  >('@react-native-harness/tools');
+
+  return { ...actual, spawn: mocks.spawn };
+});
+
+const { waitForEmulator } = await import('../adb.js');
+
+afterEach(() => {
+  vi.useRealTimers();
+  vi.restoreAllMocks();
+});
+
+describe('waitForEmulator polling', () => {
+  it('does not leak an abort listener per poll iteration when the delay wins', async () => {
+    vi.useFakeTimers();
+    // No matching device on any poll: `adb devices` reports an empty list.
+    mocks.spawn.mockResolvedValue({ stdout: 'List of devices attached\n' });
+
+    const controller = new AbortController();
+    const addSpy = vi.spyOn(controller.signal, 'addEventListener');
+    const removeSpy = vi.spyOn(controller.signal, 'removeEventListener');
+
+    const resultPromise = waitForEmulator('Test_AVD', controller.signal);
+    resultPromise.catch(() => undefined);
+
+    for (let i = 0; i < 3; i += 1) {
+      await vi.advanceTimersByTimeAsync(1_000);
+    }
+
+    // Regression: each 1s poll iteration used to add an 'abort' listener
+    // to the local waitForAbort helper that was never removed when the
+    // delay (not the abort) won the race, leaking one listener per second
+    // of polling against this session-lifetime signal.
+    expect(addSpy.mock.calls.length).toBeGreaterThanOrEqual(3);
+    const addCallsBeforeAbort = addSpy.mock.calls.length;
+
+    controller.abort(new Error('teardown'));
+    await expect(resultPromise).rejects.toThrow('teardown');
+
+    // Every listener added while losing the race — including the one still
+    // pending when the abort itself won — must be removed again.
+    expect(removeSpy).toHaveBeenCalledTimes(addCallsBeforeAbort);
+  });
+
+  it('clears the pending 1s poll timer when the signal aborts mid-wait', async () => {
+    vi.useFakeTimers();
+    mocks.spawn.mockResolvedValue({ stdout: 'List of devices attached\n' });
+
+    const controller = new AbortController();
+    const resultPromise = waitForEmulator('Test_AVD', controller.signal);
+    resultPromise.catch(() => undefined);
+
+    // Let the loop enter its 1s wait, then abort before the timer fires.
+    await vi.advanceTimersByTimeAsync(0);
+    controller.abort(new Error('teardown'));
+
+    await expect(resultPromise).rejects.toThrow('teardown');
+    // Regression: the bare setTimeout-backed `wait()` used to have no
+    // handle to clear, so it stayed alive in the event loop for the rest
+    // of the 1s interval even after the abort branch won.
+    expect(vi.getTimerCount()).toBe(0);
+  });
+});

packages/platform-android/src/adb.ts

@@ -1,8 +1,10 @@
 import { type AndroidAppLaunchOptions } from '@react-native-harness/platforms';
 import {
+  delay,
   logger,
   spawn,
   SubprocessError,
+  waitForAbort,
   type Subprocess,
 } from '@react-native-harness/tools';
 import { spawn as nodeSpawn } from 'node:child_process';
@@ -34,28 +36,6 @@ import {
   AdbPermissionGrantError,
 } from './adb-errors.js';
 
-const wait = async (ms: number): Promise<void> => {
-  await new Promise((resolve) => {
-    setTimeout(resolve, ms);
-  });
-};
-
-const waitForAbort = (signal: AbortSignal): Promise<never> => {
-  if (signal.aborted) {
-    return Promise.reject(signal.reason);
-  }
-
-  return new Promise((_, reject) => {
-    signal.addEventListener(
-      'abort',
-      () => {
-        reject(signal.reason);
-      },
-      { once: true }
-    );
-  });
-};
-
 const waitWithSignal = async (
   ms: number,
   signal: AbortSignal
@@ -64,7 +44,18 @@ const waitWithSignal = async (
     throw signal.reason;
   }
 
-  await Promise.race([wait(ms), waitForAbort(signal)]);
+  // Both sides of this race must be torn down explicitly when they lose:
+  // an uncancelled timer keeps a ref'd Timeout alive, and an uncancelled
+  // abort wait leaks a listener on `signal`, which is a session-lifetime
+  // signal here and gets polled against once per second.
+  const timer = delay(ms);
+  const abort = waitForAbort(signal);
+  try {
+    await Promise.race([timer.promise, abort.promise]);
+  } finally {
+    timer.cancel();
+    abort.cancel();
+  }
 };
 
 const EMULATOR_STARTUP_OBSERVATION_TIMEOUT_MS = 5000;
@@ -617,13 +608,19 @@ export const startEmulator = async (
       throw error;
     });
 
-  const observationTimeout = wait(EMULATOR_STARTUP_OBSERVATION_TIMEOUT_MS).then(
+  const observationTimer = delay(EMULATOR_STARTUP_OBSERVATION_TIMEOUT_MS);
+  const observationTimeout = observationTimer.promise.then(
     () => 'timeout' as const
   );
 
   try {
     await Promise.race([earlyExit, observedBoot, observationTimeout]);
   } finally {
+    // If `earlyExit` or `observedBoot` won the race, this timer would
+    // otherwise sit ref'd in the event loop for the rest of its 5s;
+    // cancelling here is a no-op on the happy "proceed detached" path
+    // where the timer itself won.
+    observationTimer.cancel();
     cleanup();
   }
 

packages/platform-ios/src/__tests__/app-session.test.ts

@@ -61,6 +61,39 @@ describe('createIosAppSession', () => {
     }
   });
 
+  it('resolves dispose() without waiting out the pending poll delay', async () => {
+    vi.useFakeTimers();
+
+    try {
+      const launchProcess = createPendingLaunchProcess();
+      const isAppRunning = vi.fn<() => Promise<boolean>>().mockResolvedValue(true);
+      const stopApp = vi.fn(async () => undefined);
+
+      const sessionPromise = createIosAppSession({
+        launch: () => launchProcess,
+        stopApp,
+        isAppRunning,
+      });
+
+      await vi.advanceTimersByTimeAsync(100);
+      const session = await sessionPromise;
+
+      // Let the poll loop enter its 1s wait before tearing down.
+      await vi.advanceTimersByTimeAsync(0);
+
+      // Regression: dispose() used to await the poll task directly, which
+      // was blocked on an uncancelled 1s sleep(). With fake timers and no
+      // advanceTimersByTimeAsync call left below, this await only resolves
+      // if dispose() itself resolves the pending poll delay instead of
+      // waiting for the underlying timer to fire.
+      await session.dispose();
+
+      expect(vi.getTimerCount()).toBe(0);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
   it('disposes the session when the session-lifetime signal aborts', async () => {
     vi.useFakeTimers();
 

packages/platform-ios/src/__tests__/xctest-agent.test.ts

@@ -514,6 +514,34 @@ describe('xctest-agent orchestration', () => {
     expect(mocks.kill).not.toHaveBeenCalled();
   });
 
+  it('clears the graceful-shutdown timers once shutdown succeeds, leaving no pending timer', async () => {
+    vi.useFakeTimers();
+
+    try {
+      const controller = createXCTestAgentController({
+        port: 49154,
+        shutdownTimeoutMs: 30_000,
+        target: {
+          kind: 'simulator',
+          id: 'sim-timeout',
+        },
+      });
+
+      await controller.ensureStarted();
+      await controller.dispose();
+
+      expect(mocks.shutdown).toHaveBeenCalledTimes(1);
+      expect(mocks.kill).not.toHaveBeenCalled();
+      // Regression coverage for the process lingering ~25s after a harness
+      // run completed: waitForShutdown/waitForGracefulShutdown each raced a
+      // 30s delay() against the real shutdown and never cleared it when the
+      // shutdown won, leaving two ref'd Timeouts alive in the event loop.
+      expect(vi.getTimerCount()).toBe(0);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
   it('kills the agent process when graceful shutdown times out', async () => {
     mocks.shutdown.mockResolvedValue(undefined);