diff --git a/.changeset/big-pugs-wish.md b/.changeset/big-pugs-wish.md new file mode 100644 index 000000000..ac61a8ead --- /dev/null +++ b/.changeset/big-pugs-wish.md @@ -0,0 +1,5 @@ +--- +'@solana/transaction-confirmation': patch +--- + +Fix an abort listener leak in `getTimeoutPromise`. The listener registered on the caller's `AbortSignal` was never removed after the promise settled, which caused listeners to accumulate when the same signal was reused across multiple calls. `getTimeoutPromise` now registers the listener with the auto-cleanup `signal` option already used by the other strategies in this package and releases it in a `finally` block. diff --git a/packages/transaction-confirmation/src/__tests__/confirmation-strategy-timeout-test.ts b/packages/transaction-confirmation/src/__tests__/confirmation-strategy-timeout-test.ts index e160ae8d5..305f68e21 100644 --- a/packages/transaction-confirmation/src/__tests__/confirmation-strategy-timeout-test.ts +++ b/packages/transaction-confirmation/src/__tests__/confirmation-strategy-timeout-test.ts @@ -30,4 +30,48 @@ describe('getTimeoutPromise', () => { abortController.abort(); await expect(timeoutPromise).rejects.toThrow(/operation was aborted/); }); + it('registers the caller abort listener with an auto-cleanup signal', () => { + const abortController = new AbortController(); + const addEventListenerSpy = jest.spyOn(abortController.signal, 'addEventListener'); + const timeoutPromise = getTimeoutPromise({ + abortSignal: abortController.signal, + commitment: 'finalized', + }); + // Suppress the eventual rejection so the test does not produce an + // unhandled rejection warning at teardown. + timeoutPromise.catch(() => {}); + // The listener must be registered with the modern `signal` option so that + // it is released automatically when the inner controller aborts in `finally`. + expect(addEventListenerSpy).toHaveBeenCalledWith( + 'abort', + expect.any(Function), + expect.objectContaining({ signal: expect.objectContaining({ aborted: false }) }), + ); + }); + it('aborts the cleanup signal once the timeout elapses', async () => { + expect.assertions(1); + const abortController = new AbortController(); + const addEventListenerSpy = jest.spyOn(abortController.signal, 'addEventListener'); + const settled = getTimeoutPromise({ + abortSignal: abortController.signal, + commitment: 'finalized', + }).catch((e: unknown) => e); + await jest.advanceTimersByTimeAsync(60_000); + await settled; + const cleanupSignal = (addEventListenerSpy.mock.calls[0][2] as { signal: AbortSignal }).signal; + expect(cleanupSignal.aborted).toBe(true); + }); + it('aborts the cleanup signal when the caller aborts', async () => { + expect.assertions(1); + const abortController = new AbortController(); + const addEventListenerSpy = jest.spyOn(abortController.signal, 'addEventListener'); + const settled = getTimeoutPromise({ + abortSignal: abortController.signal, + commitment: 'finalized', + }).catch((e: unknown) => e); + abortController.abort(); + await settled; + const cleanupSignal = (addEventListenerSpy.mock.calls[0][2] as { signal: AbortSignal }).signal; + expect(cleanupSignal.aborted).toBe(true); + }); }); diff --git a/packages/transaction-confirmation/src/confirmation-strategy-timeout.ts b/packages/transaction-confirmation/src/confirmation-strategy-timeout.ts index a0c934735..a08a15962 100644 --- a/packages/transaction-confirmation/src/confirmation-strategy-timeout.ts +++ b/packages/transaction-confirmation/src/confirmation-strategy-timeout.ts @@ -1,3 +1,4 @@ +import { AbortController } from '@solana/event-target-impl'; import type { Commitment } from '@solana/rpc-types'; type Config = Readonly<{ @@ -32,22 +33,29 @@ type Config = Readonly<{ * ``` */ export async function getTimeoutPromise({ abortSignal: callerAbortSignal, commitment }: Config) { - return await new Promise((_, reject) => { - const handleAbort = (e: AbortSignalEventMap['abort']) => { - clearTimeout(timeoutId); - const abortError = new DOMException((e.target as AbortSignal).reason, 'AbortError'); - reject(abortError); - }; - callerAbortSignal.addEventListener('abort', handleAbort); - const timeoutMs = commitment === 'processed' ? 30_000 : 60_000; - const startMs = performance.now(); - const timeoutId = - // We use `setTimeout` instead of `AbortSignal.timeout()` because we want to measure - // elapsed time instead of active time. - // See https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/timeout_static - setTimeout(() => { - const elapsedMs = performance.now() - startMs; - reject(new DOMException(`Timeout elapsed after ${elapsedMs} ms`, 'TimeoutError')); - }, timeoutMs); - }); + const abortController = new AbortController(); + let timeoutId: ReturnType | undefined; + try { + return await new Promise((_, reject) => { + const handleAbort = (e: AbortSignalEventMap['abort']) => { + clearTimeout(timeoutId); + const abortError = new DOMException((e.target as AbortSignal).reason, 'AbortError'); + reject(abortError); + }; + callerAbortSignal.addEventListener('abort', handleAbort, { signal: abortController.signal }); + const timeoutMs = commitment === 'processed' ? 30_000 : 60_000; + const startMs = performance.now(); + timeoutId = + // We use `setTimeout` instead of `AbortSignal.timeout()` because we want to measure + // elapsed time instead of active time. + // See https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/timeout_static + setTimeout(() => { + const elapsedMs = performance.now() - startMs; + reject(new DOMException(`Timeout elapsed after ${elapsedMs} ms`, 'TimeoutError')); + }, timeoutMs); + }); + } finally { + clearTimeout(timeoutId); + abortController.abort(); + } }