Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .nx/version-plans/version-plan-1784556610341.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
__default__: patch
---

Collapses the two competing subprocess shutdown mechanisms (the global process-level SIGINT/SIGTERM net in packages/tools/src/spawn.ts and the session's own abort-driven dispose) into one session-lifetime abort signal. Long-lived child processes (Android logcat, iOS launch process and XCTest agent, web browser, Vega app) now abort on session teardown through a single signal instead of racing a separate raw-kill handler.
6 changes: 6 additions & 0 deletions packages/cli/src/wizard/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
isProject,
cancelPromptAndExit,
promptConfirm,
ensureSignalsDeliverable,
} from '@react-native-harness/tools';
import { getProjectConfig } from './projectType.js';
import { installPlatforms } from './platforms.js';
Expand Down Expand Up @@ -36,6 +37,11 @@ const checkForExistingConfig = async (projectRoot: string) => {
};

export const runInitWizard = async () => {
// The wizard spawns subprocesses (e.g. platform installers) while a @clack
// spinner may be active, which puts stdin in raw mode and would otherwise
// swallow SIGINT until a prompt reads it. See spawn.ts's doc comment.
ensureSignalsDeliverable();

const projectRoot = process.cwd();

if (!isProject(projectRoot)) {
Expand Down
64 changes: 63 additions & 1 deletion packages/jest/src/__tests__/harness-session.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import { describe, expect, it, vi } from 'vitest';
import { afterEach, describe, expect, it, vi } from 'vitest';
import type { AppConnection } from '@react-native-harness/bridge/server';
import {
getSignalExitCodeForRunState,
waitForBridgeDisconnectOrTimeout,
waitForNextConnected,
waitForStartupCrash,
withPlatformReadyTimeout,
type HarnessRunState,
} from '../harness-session.js';
import type { CrashMonitor } from '../crash-monitor.js';
import { PlatformReadyTimeoutError } from '../errors.js';

const createConnection = (): AppConnection => ({
device: {
Expand Down Expand Up @@ -155,6 +157,66 @@ describe('waitForStartupCrash', () => {
});
});

describe('withPlatformReadyTimeout', () => {
afterEach(() => {
vi.useRealTimers();
});

it('passes the real session signal to work, unmodified by the timeout', async () => {
const controller = new AbortController();
const work = vi.fn(async (signal: AbortSignal) => {
expect(signal).toBe(controller.signal);
return 'ready';
});

await expect(
withPlatformReadyTimeout({
timeout: 1_000,
signal: controller.signal,
work,
}),
).resolves.toBe('ready');
expect(work).toHaveBeenCalledTimes(1);
});

it('throws PlatformReadyTimeoutError on timeout without aborting the session signal', async () => {
vi.useFakeTimers();
const controller = new AbortController();
// Never resolves within the test; simulates a hung platform init.
const work = () => new Promise<void>(() => undefined);

const resultPromise = withPlatformReadyTimeout({
timeout: 1_000,
signal: controller.signal,
work,
});

await vi.advanceTimersByTimeAsync(1_000);

await expect(resultPromise).rejects.toBeInstanceOf(PlatformReadyTimeoutError);
expect(controller.signal.aborted).toBe(false);
});

it('propagates a genuine abort from the session signal as-is', async () => {
const controller = new AbortController();
const abortReason = new DOMException('Aborted', 'AbortError');
const work = (signal: AbortSignal) =>
new Promise<never>((_, reject) => {
signal.addEventListener('abort', () => reject(signal.reason));
});

const resultPromise = withPlatformReadyTimeout({
timeout: 1_000,
signal: controller.signal,
work,
});

controller.abort(abortReason);

await expect(resultPromise).rejects.toBe(abortReason);
});
});

describe('getSignalExitCodeForRunState', () => {
const createRunState = (
overrides: Partial<HarnessRunState> = {}
Expand Down
59 changes: 31 additions & 28 deletions packages/jest/src/harness-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import {
createDiagnostics,
logger,
getTimeoutSignal,
raceAbortSignals,
ensureSignalsDeliverable,
type Diagnostics,
} from '@react-native-harness/tools';
import {
Expand Down Expand Up @@ -220,26 +220,24 @@ const waitForAbort = (signal: AbortSignal): Promise<never> => {
});
};

const withPlatformReadyTimeout = async <T>(options: {
// 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.
// On timeout this throws PlatformReadyTimeoutError without cancelling `work`
// itself — the caller is expected to abort the session signal as part of
// tearing down, which then cancels the still-inflight call cooperatively.
export const withPlatformReadyTimeout = async <T>(options: {
timeout: number;
signal: AbortSignal;
work: (signal: AbortSignal) => Promise<T>;
}): Promise<T> => {
const timeoutSignal = getTimeoutSignal(options.timeout);
const combinedSignal = raceAbortSignals([options.signal, timeoutSignal]);
try {
return await options.work(combinedSignal);
} catch (error) {
if (
error instanceof DOMException &&
error.name === 'AbortError' &&
timeoutSignal.aborted &&
!options.signal.aborted
) {
return await Promise.race([
options.work(options.signal),
waitForAbort(timeoutSignal).catch(() => {
throw new PlatformReadyTimeoutError(options.timeout);
}
throw error;
}
}),
]);
};

type AppReadyOptions = {
Expand Down Expand Up @@ -490,11 +488,14 @@ export const createHarnessSession = async (
platform.platformId,
);

// Single AbortController for the entire setup phase. Registered signal
// handlers abort this so that slow inflight operations (Metro init, platform
// runner startup) are cancelled promptly on SIGTERM/SIGINT.
const setupController = new AbortController();
const onEarlySignal = () => setupController.abort();
// Single, session-lifetime AbortController: it aborts on early SIGTERM/SIGINT
// during setup (below) and, once the session is up, in dispose()'s finally
// (unchanged lifetime). It is the one signal the plugin manager and platform
// runner see for the entire session, so its abort reaches every long-lived
// child the runner threads it into.
ensureSignalsDeliverable();
const sessionController = new AbortController();
const onEarlySignal = () => sessionController.abort();
process.once('SIGTERM', onEarlySignal);
process.once('SIGINT', onEarlySignal);

Expand All @@ -508,7 +509,7 @@ export const createHarnessSession = async (
'session.lock.wait',
() =>
lockManager.acquire(resourceLockKey, {
signal: setupController.signal,
signal: sessionController.signal,
onWait: () => {
didWaitForResourceLock = true;
logRunnerWaitingInQueue(platform);
Expand Down Expand Up @@ -539,7 +540,7 @@ export const createHarnessSession = async (
config: harnessConfig,
platform,
resourceLockManager: lockManager,
signal: setupController.signal,
signal: sessionController.signal,
});
} catch (error) {
portResolveSpan.fail(error);
Expand All @@ -560,13 +561,12 @@ export const createHarnessSession = async (
platform,
});

const pluginAbortController = new AbortController();
const pluginManager = createHarnessPluginManager<HarnessConfig, HarnessPlatform>({
plugins: (runtimeConfig.plugins ?? []) as Array<HarnessPlugin<object, HarnessConfig, HarnessPlatform>>,
projectRoot,
config: runtimeConfig,
runner: platform,
abortSignal: pluginAbortController.signal,
abortSignal: sessionController.signal,
});

const hooks = createHookQueue();
Expand Down Expand Up @@ -606,7 +606,7 @@ export const createHarnessSession = async (
[HARNESS_BRIDGE_PATH]: bridge.ws as unknown as MetroWebSocketEndpoint,
},
},
setupController.signal,
sessionController.signal,
).then((instance) => {
sessionLogger.debug('Metro initialized');
// Attach the Metro diagnostics deriver before the eager prewarm
Expand All @@ -621,7 +621,7 @@ export const createHarnessSession = async (
instance
.prewarm({
platform: platform.platformId,
signal: setupController.signal,
signal: sessionController.signal,
})
.then((completed) => prewarmSpan.end({ completed }))
// Swallow AbortError on teardown so it never becomes an
Expand All @@ -636,7 +636,7 @@ export const createHarnessSession = async (
() =>
withPlatformReadyTimeout({
timeout: runtimeConfig.platformReadyTimeout,
signal: setupController.signal,
signal: sessionController.signal,
work: async (signal) => {
return await import(platform.runner).then((module) =>
module.default(platform.config, runtimeConfig, {
Expand Down Expand Up @@ -834,7 +834,7 @@ export const createHarnessSession = async (
cleanupError = error;
} finally {
await resourceLease.release();
pluginAbortController.abort();
sessionController.abort();
}

sessionLogger.debug('session resources disposed');
Expand Down Expand Up @@ -1105,6 +1105,9 @@ export const createHarnessSession = async (
} catch (error) {
process.off('SIGTERM', onEarlySignal);
process.off('SIGINT', onEarlySignal);
// Cancels any inflight init work still holding sessionController.signal
// (e.g. a platform init call abandoned after a readiness timeout race).
sessionController.abort();
await Promise.allSettled([resourceLease.release(), metroPortLease?.release()]);
throw error;
}
Expand Down
76 changes: 76 additions & 0 deletions packages/platform-android/src/__tests__/app-session.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { describe, expect, it } from 'vitest';
import type { Subprocess } from '@react-native-harness/tools';
import { createAndroidAppSession } from '../app-session.js';

// Mimics nano-spawn: the async iterator only ends once its abort signal
// fires, matching how startLogcat's real subprocess reacts to abort.
const createAbortableLogcatProcess = (signal: AbortSignal): Subprocess =>
({
[Symbol.asyncIterator]: () => ({
next: () =>
new Promise<{ done: true; value: undefined }>((resolve) => {
if (signal.aborted) {
resolve({ done: true, value: undefined });
return;
}
signal.addEventListener(
'abort',
() => resolve({ done: true, value: undefined }),
{ once: true }
);
}),
}),
catch: () => undefined,
}) as unknown as Subprocess;

describe('createAndroidAppSession', () => {
it('combines the session-lifetime signal with its own logcat abort controller', async () => {
let capturedSignal: AbortSignal | undefined;
const controller = new AbortController();

const session = await createAndroidAppSession({
appUid: 1,
bundleId: 'com.example',
startApp: async () => undefined,
stopApp: async () => undefined,
getAppPid: async () => null,
getLogcatTimestamp: async () => '00:00:00.000',
startLogcat: (_args, options) => {
capturedSignal = options.signal;
return createAbortableLogcatProcess(options.signal);
},
signal: controller.signal,
});

expect(capturedSignal?.aborted).toBe(false);

controller.abort();

expect(capturedSignal?.aborted).toBe(true);

await session.dispose();
});

it('still aborts the logcat signal via dispose() with no external signal provided', async () => {
let capturedSignal: AbortSignal | undefined;

const session = await createAndroidAppSession({
appUid: 1,
bundleId: 'com.example',
startApp: async () => undefined,
stopApp: async () => undefined,
getAppPid: async () => null,
getLogcatTimestamp: async () => '00:00:00.000',
startLogcat: (_args, options) => {
capturedSignal = options.signal;
return createAbortableLogcatProcess(options.signal);
},
});

expect(capturedSignal?.aborted).toBe(false);

await session.dispose();

expect(capturedSignal?.aborted).toBe(true);
});
});
13 changes: 12 additions & 1 deletion packages/platform-android/src/app-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ type CreateAndroidAppSessionOptions = {
getDropboxOutput?: () => Promise<string>;
getExitInfo?: () => Promise<string>;
crashArtifactWriter?: CrashArtifactWriter;
/**
* Session-lifetime abort signal (see HarnessPlatformInitOptions.signal).
* Combined with the app session's own logcatAbortController so that
* aborting the session stops the logcat stream even if dispose() hasn't
* been called yet (e.g. SIGTERM during teardown of other resources).
*/
signal?: AbortSignal;
};

export const createAndroidAppSession = async ({
Expand All @@ -94,6 +101,7 @@ export const createAndroidAppSession = async ({
getDropboxOutput,
getExitInfo,
crashArtifactWriter,
signal,
}: CreateAndroidAppSessionOptions): Promise<AppSession> => {
const emitter = createAppSessionEmitter();
const logBuffer = createBoundedLogBuffer();
Expand Down Expand Up @@ -189,8 +197,11 @@ export const createAndroidAppSession = async ({
const logcatTimestamp = await getLogcatTimestamp();
const sessionStartedAt = Date.now();
const logcatAbortController = new AbortController();
const logcatSignal = signal
? AbortSignal.any([signal, logcatAbortController.signal])
: logcatAbortController.signal;
const logcatProcess = startLogcat(getLogcatArgs(appUid, logcatTimestamp), {
signal: logcatAbortController.signal,
signal: logcatSignal,
});
// Aborting is nano-spawn's own cancellation path, so the resulting
// SubprocessError is settled the same way regardless of which of the
Expand Down
2 changes: 2 additions & 0 deletions packages/platform-android/src/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ export const getAndroidEmulatorPlatformInstance = async (
getDropboxOutput: () => adb.getDropboxPrint(adbId),
getExitInfo: () => adb.getActivityExitInfo(adbId, config.bundleId),
crashArtifactWriter: init.crashArtifactWriter,
signal: init.signal,
});
},
dispose: async () => {
Expand Down Expand Up @@ -374,6 +375,7 @@ export const getAndroidPhysicalDevicePlatformInstance = async (
getDropboxOutput: () => adb.getDropboxPrint(adbId),
getExitInfo: () => adb.getActivityExitInfo(adbId, config.bundleId),
crashArtifactWriter: init?.crashArtifactWriter,
signal: init?.signal,
});
},
dispose: async () => {
Expand Down
Loading
Loading