From 174e9a9cd56ca7f89bad0fc0a5c2f282794da089 Mon Sep 17 00:00:00 2001 From: kamja Date: Tue, 9 Dec 2025 11:21:29 +0900 Subject: [PATCH 1/2] fix(query-core): prevent duplicate abort event listeners in infinite queries - Add listenerAttached flag to prevent multiple event listener registrations - Add { once: true } option for automatic cleanup - Add test to verify no duplicate listeners when signal is accessed multiple times Fixes memory leak when signal property is accessed multiple times during infinite query pagination. --- .changeset/calm-goats-punch.md | 5 ++ .../__tests__/infiniteQueryBehavior.test.tsx | 47 +++++++++++++++++++ .../query-core/src/infiniteQueryBehavior.ts | 14 ++++-- 3 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 .changeset/calm-goats-punch.md diff --git a/.changeset/calm-goats-punch.md b/.changeset/calm-goats-punch.md new file mode 100644 index 0000000000..04e2bc3df8 --- /dev/null +++ b/.changeset/calm-goats-punch.md @@ -0,0 +1,5 @@ +--- +'@tanstack/query-core': patch +--- + +Fix memory leak in infinite query by preventing duplicate abort event listeners diff --git a/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx b/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx index db96ea17da..983b7fc9ce 100644 --- a/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx +++ b/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx @@ -489,4 +489,51 @@ describe('InfiniteQueryBehavior', () => { unsubscribe() }) + + test('should not register duplicate abort event listeners when signal is accessed multiple times', async () => { + const key = queryKey() + let signalAccessCount = 0 + const listenerCounts: Array = [] + + const queryFnSpy = vi.fn().mockImplementation(({ signal }) => { + signalAccessCount++ + + const originalAddEventListener = signal.addEventListener + let currentListenerCount = 0 + signal.addEventListener = vi.fn((...args) => { + currentListenerCount++ + return originalAddEventListener.apply(signal, args) + }) + + // Access signal multiple times to trigger getter + signal + signal + signal + + listenerCounts.push(currentListenerCount) + signal.addEventListener = originalAddEventListener + + return `page-${signalAccessCount}` + }) + + const observer = new InfiniteQueryObserver(queryClient, { + queryKey: key, + queryFn: queryFnSpy, + getNextPageParam: (_lastPage, pages) => { + return pages.length < 3 ? pages.length + 1 : undefined + }, + initialPageParam: 1, + }) + + const unsubscribe = observer.subscribe(() => {}) + + await vi.advanceTimersByTimeAsync(0) + + await observer.fetchNextPage() + await observer.fetchNextPage() + + expect(listenerCounts.every((count) => count <= 1)).toBe(true) + + unsubscribe() + }) }) diff --git a/packages/query-core/src/infiniteQueryBehavior.ts b/packages/query-core/src/infiniteQueryBehavior.ts index 476d90ce15..9e3237c8c2 100644 --- a/packages/query-core/src/infiniteQueryBehavior.ts +++ b/packages/query-core/src/infiniteQueryBehavior.ts @@ -22,16 +22,22 @@ export function infiniteQueryBehavior( const fetchFn = async () => { let cancelled = false + let listenerAttached = false const addSignalProperty = (object: unknown) => { Object.defineProperty(object, 'signal', { enumerable: true, get: () => { if (context.signal.aborted) { cancelled = true - } else { - context.signal.addEventListener('abort', () => { - cancelled = true - }) + } else if (!listenerAttached) { + listenerAttached = true + context.signal.addEventListener( + 'abort', + () => { + cancelled = true + }, + { once: true }, + ) } return context.signal }, From 26cc89332244e9ca66c012b4e5b6ae52c2d9a13c Mon Sep 17 00:00:00 2001 From: kamja Date: Tue, 9 Dec 2025 12:15:55 +0900 Subject: [PATCH 2/2] test(query-core): improve test to properly detect duplicate abort listeners Based on code review feedback, the previous test approach had a flaw: - Signal destructuring ({signal}) invokes the getter before addEventListener could be spied - The test was not actually catching duplicate listener registrations New approach: - Spy on AbortSignal.prototype.addEventListener before query execution - Access signal multiple times within queryFn to trigger getter repeatedly - Verify that only 3 abort listeners are registered (1 per page) instead of 9 (3 per page) This test now properly validates the memory leak fix and will fail if the duplicate listener prevention is removed. --- .../__tests__/infiniteQueryBehavior.test.tsx | 60 ++++++++++--------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx b/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx index 983b7fc9ce..62e4cf34aa 100644 --- a/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx +++ b/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx @@ -492,28 +492,18 @@ describe('InfiniteQueryBehavior', () => { test('should not register duplicate abort event listeners when signal is accessed multiple times', async () => { const key = queryKey() - let signalAccessCount = 0 - const listenerCounts: Array = [] - const queryFnSpy = vi.fn().mockImplementation(({ signal }) => { - signalAccessCount++ + // Track addEventListener calls before the query starts + const addEventListenerSpy = vi.spyOn(AbortSignal.prototype, 'addEventListener') - const originalAddEventListener = signal.addEventListener - let currentListenerCount = 0 - signal.addEventListener = vi.fn((...args) => { - currentListenerCount++ - return originalAddEventListener.apply(signal, args) - }) - - // Access signal multiple times to trigger getter - signal - signal - signal - - listenerCounts.push(currentListenerCount) - signal.addEventListener = originalAddEventListener + const queryFnSpy = vi.fn().mockImplementation((context) => { + // Access signal multiple times to trigger the getter repeatedly + // This simulates code that might reference the signal property multiple times + context.signal + context.signal + context.signal - return `page-${signalAccessCount}` + return 'page' }) const observer = new InfiniteQueryObserver(queryClient, { @@ -527,13 +517,29 @@ describe('InfiniteQueryBehavior', () => { const unsubscribe = observer.subscribe(() => {}) - await vi.advanceTimersByTimeAsync(0) - - await observer.fetchNextPage() - await observer.fetchNextPage() - - expect(listenerCounts.every((count) => count <= 1)).toBe(true) - - unsubscribe() + try { + // Wait for initial page + await vi.advanceTimersByTimeAsync(0) + + // Fetch additional pages + await observer.fetchNextPage() + await observer.fetchNextPage() + + // Sanity check: we fetched 3 pages (initial + 2 next pages) + expect(queryFnSpy).toHaveBeenCalledTimes(3) + + // Count total abort listeners registered + const totalAbortListeners = addEventListenerSpy.mock.calls.filter( + (call) => call[0] === 'abort' + ).length + + // With the fix: Each page registers exactly 1 abort listener despite signal being accessed 3 times + // We fetch 3 pages, so exactly 3 abort listeners + // Without the fix: Each signal access registers a listener = 3 accesses × 3 pages = 9 listeners + expect(totalAbortListeners).toBe(3) + } finally { + addEventListenerSpy.mockRestore() + unsubscribe() + } }) })