Skip to content

Commit 14a2dea

Browse files
committed
fix(ios-snapshot): remove the stop listener when a detached wait settles
waitForDetachedAttempt added an abort listener to stop but only ever removed the one on signal, so every call with no explicit stop leaked one listener onto the module-level NO_STOP signal per production caller (lifecycle.ts's bridge-connect retry sleep and every bridge request's waitForSimulatorTurn). Make stop optional, remove its listener on every settle path, and delete NO_STOP now that the wait tolerates a missing stop directly.
1 parent 58e61bf commit 14a2dea

4 files changed

Lines changed: 73 additions & 11 deletions

File tree

packages/platform-apple/src/detached-attempt.test.ts

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import assert from 'node:assert/strict';
2-
import { test } from 'vitest';
3-
import { createDetachedAttempts } from './detached-attempt.ts';
2+
import { getEventListeners } from 'node:events';
3+
import { test, vi } from 'vitest';
4+
import { createDetachedAttempts, waitForDetachedAttempt } from './detached-attempt.ts';
45

56
const PENDING = new Error('still-running');
67

@@ -249,6 +250,49 @@ test('a caller answered by the attempt stops the wait it left running', async ()
249250
assert.equal(waitState, 'resolved');
250251
});
251252

253+
test('a wait that settles by its own timeout releases the stop listener it added', async () => {
254+
vi.useFakeTimers();
255+
try {
256+
const stop = new AbortController();
257+
const waiting = waitForDetachedAttempt({
258+
waitMs: 20,
259+
signal: undefined,
260+
stop: stop.signal,
261+
cancelled: () => new Error('unreachable'),
262+
});
263+
264+
await vi.advanceTimersByTimeAsync(20);
265+
await waiting;
266+
267+
assert.equal(getEventListeners(stop.signal, 'abort').length, 0);
268+
} finally {
269+
vi.useRealTimers();
270+
}
271+
});
272+
273+
test('a caller signal already aborted rejects at once, without spending waitMs', async () => {
274+
vi.useFakeTimers();
275+
try {
276+
const controller = new AbortController();
277+
const stop = new AbortController();
278+
const reason = new Error('already-cancelled');
279+
controller.abort(reason);
280+
281+
const waiting = waitForDetachedAttempt({
282+
waitMs: 60_000,
283+
signal: controller.signal,
284+
stop: stop.signal,
285+
cancelled: () => reason,
286+
});
287+
288+
await assert.rejects(waiting, (error) => error === reason);
289+
assert.equal(vi.getTimerCount(), 0);
290+
assert.equal(getEventListeners(stop.signal, 'abort').length, 0);
291+
} finally {
292+
vi.useRealTimers();
293+
}
294+
});
295+
252296
function identity(error: unknown): unknown {
253297
return error;
254298
}

packages/platform-apple/src/detached-attempt.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,16 +68,16 @@ type Attempt<Value> = {
6868
* this module describes: sleeps `waitMs`, resolves when that sleep is spent or when `stop` says the
6969
* answer arrived elsewhere, and rejects only on the caller's own abort so that stays typed.
7070
*
71-
* `stop` needs neither cleanup nor an already-aborted check: `value()` creates it moments before
72-
* calling and aborts it in a `finally`, which releases the `{ once: true }` listener. The caller's
73-
* signal outlives the wait and does have its listener removed.
71+
* Every listener this adds to a signal it did not create is removed once the wait settles, on every
72+
* path (timeout, `stop`, or the caller's own abort) — `stop` is optional for a caller with no signal
73+
* to end the wait early.
7474
*/
7575
export function waitForDetachedAttempt(
7676
params: Readonly<{
7777
waitMs: number;
7878
/** The caller's own deadline signal; a wait inside it keeps a client abort a client abort. */
7979
signal: AbortSignal | undefined;
80-
stop: AbortSignal;
80+
stop: AbortSignal | undefined;
8181
/** The rejection for the caller aborting, so each owner keeps its own error type. */
8282
cancelled: () => unknown;
8383
}>,
@@ -90,11 +90,12 @@ export function waitForDetachedAttempt(
9090
function finish(settle: () => void): void {
9191
clearTimeout(timer);
9292
signal?.removeEventListener('abort', onAbort);
93+
stop?.removeEventListener('abort', onStop);
9394
settle();
9495
}
9596
signal?.addEventListener('abort', onAbort, { once: true });
97+
stop?.addEventListener('abort', onStop, { once: true });
9698
if (signal?.aborted) onAbort();
97-
stop.addEventListener('abort', onStop, { once: true });
9899
});
99100
}
100101

packages/platform-apple/src/snapshot-source/deadline.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { getEventListeners } from 'node:events';
12
import { expect, test, vi } from 'vitest';
23
import { createSnapshotSourceDeadline, waitForSnapshotSourceDelay } from './deadline.ts';
34
import { SnapshotSourceError } from './errors.ts';
@@ -47,3 +48,22 @@ test('an aborted caller signal stays typed cancellation next to a stop', async (
4748
vi.useRealTimers();
4849
}
4950
});
51+
52+
test('a delay called with no stop, the way every lifecycle site sends it, leaves no listener behind', async () => {
53+
vi.useFakeTimers();
54+
try {
55+
const caller = new AbortController();
56+
const deadline = createSnapshotSourceDeadline(60_000, caller.signal);
57+
58+
for (let attempt = 0; attempt < 3; attempt += 1) {
59+
const waiting = waitForSnapshotSourceDelay(deadline, 1_000, WAIT_CODE);
60+
await vi.advanceTimersByTimeAsync(1_000);
61+
await waiting;
62+
}
63+
64+
expect(getEventListeners(caller.signal, 'abort').length).toBe(0);
65+
expect(vi.getTimerCount()).toBe(0);
66+
} finally {
67+
vi.useRealTimers();
68+
}
69+
});

packages/platform-apple/src/snapshot-source/deadline.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,6 @@ export function remainingSnapshotSourceMs(deadline: SnapshotSourceDeadline, code
2525
return Math.max(1, Math.floor(remainingMs));
2626
}
2727

28-
/** A sleep nobody has asked to end early. */
29-
const NO_STOP = new AbortController().signal;
30-
3128
/**
3229
* Sleeps inside the caller's own deadline, so a client abort stays a typed `cancelled` instead of
3330
* arriving as a fresh timeout. `stop` is for a caller that no longer needs the sleep because the work
@@ -43,7 +40,7 @@ export async function waitForSnapshotSourceDelay(
4340
await waitForDetachedAttempt({
4441
waitMs: delayMs,
4542
signal: deadline.signal,
46-
stop: stop ?? NO_STOP,
43+
stop,
4744
cancelled: () => snapshotSourceError('cancelled', 'abort-signal'),
4845
});
4946
}

0 commit comments

Comments
 (0)