From 8caece7c2b08b88888a65c8bbb3befed16dd837c Mon Sep 17 00:00:00 2001 From: Vitaliy Ryaboy Date: Sun, 8 Nov 2026 22:47:08 +0100 Subject: [PATCH 1/2] fix(query-core): clean up GC timers for removed cache entries --- .changeset/large-dancers-give.md | 5 + .../query-core/src/__tests__/cacheGc.test.tsx | 241 ++++++++++++++++++ .../src/__tests__/mutationCache.test-d.tsx | 1 + packages/query-core/src/mutation.ts | 7 +- packages/query-core/src/mutationCache.ts | 17 +- packages/query-core/src/query.ts | 20 +- 6 files changed, 283 insertions(+), 8 deletions(-) create mode 100644 .changeset/large-dancers-give.md create mode 100644 packages/query-core/src/__tests__/cacheGc.test.tsx diff --git a/.changeset/large-dancers-give.md b/.changeset/large-dancers-give.md new file mode 100644 index 00000000000..e087613b2a3 --- /dev/null +++ b/.changeset/large-dancers-give.md @@ -0,0 +1,5 @@ +--- +'@tanstack/query-core': patch +--- + +Release garbage collection timers when cache entries are removed, and avoid scheduling new timers for removed queries or mutations after cancellation or observer cleanup. Pending mutations continue to execute normally. diff --git a/packages/query-core/src/__tests__/cacheGc.test.tsx b/packages/query-core/src/__tests__/cacheGc.test.tsx new file mode 100644 index 00000000000..e71a298ff63 --- /dev/null +++ b/packages/query-core/src/__tests__/cacheGc.test.tsx @@ -0,0 +1,241 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createKey } from '@tanstack/query-test-utils' +import { MutationObserver, QueryClient, QueryObserver, noop } from '..' + +const removalOperations: Record< + 'clear' | 'remove', + (clear: () => void, remove: () => void) => void +> = { + clear: (clear) => clear(), + remove: (_clear, remove) => remove(), +} + +describe('garbage collection after cache removal', () => { + let client: QueryClient + + beforeEach(() => { + vi.useFakeTimers() + client = new QueryClient() + }) + + afterEach(() => { + client.clear() + vi.clearAllTimers() + vi.useRealTimers() + }) + + it.each(['clear', 'remove'] as const)( + 'cancels a mutation timer on %s', + (operation) => { + const cache = client.getMutationCache() + const mutation = cache.build(client, { gcTime: 1000 }) + expect(vi.getTimerCount()).toBe(1) + + removalOperations[operation]( + () => client.clear(), + () => cache.remove(mutation), + ) + + expect(cache.getAll()).toStrictEqual([]) + expect(vi.getTimerCount()).toBe(0) + }, + ) + + it.each(['clear', 'remove'] as const)( + 'does not schedule a removed mutation when its observer resets after %s', + async (operation) => { + const cache = client.getMutationCache() + const observer = new MutationObserver(client, { + gcTime: 1000, + mutationFn: () => Promise.resolve('done'), + }) + const unsubscribe = observer.subscribe(noop) + await observer.mutate() + const mutation = cache.getAll()[0]! + + removalOperations[operation]( + () => client.clear(), + () => cache.remove(mutation), + ) + observer.reset() + + expect(cache.getAll()).toStrictEqual([]) + expect(vi.getTimerCount()).toBe(0) + unsubscribe() + }, + ) + + it('continues a removed pending mutation including retries and callbacks', async () => { + const onSuccess = vi.fn<(data: string) => void>() + const mutationFn = vi + .fn<() => Promise>() + .mockRejectedValueOnce(new Error('retry')) + .mockResolvedValueOnce('done') + const observer = new MutationObserver(client, { + gcTime: 1000, + retry: 1, + retryDelay: 20, + mutationFn, + onSuccess, + }) + const unsubscribe = observer.subscribe(noop) + const result = observer.mutate(undefined) + await vi.advanceTimersByTimeAsync(0) + client.clear() + observer.reset() + await vi.advanceTimersByTimeAsync(20) + + await expect(result).resolves.toBe('done') + expect(mutationFn).toHaveBeenCalledTimes(2) + expect(onSuccess).toHaveBeenCalledTimes(1) + expect(client.getMutationCache().getAll()).toStrictEqual([]) + expect(vi.getTimerCount()).toBe(0) + unsubscribe() + }) + + it.each(['clear', 'remove'] as const)( + 'does not reschedule a cancelled query after %s', + async (operation) => { + const cache = client.getQueryCache() + const query = cache.build(client, { + queryKey: createKey(), + gcTime: 1000, + queryFn: () => new Promise(noop), + }) + const result = query.fetch().catch(noop) + expect(vi.getTimerCount()).toBe(1) + + removalOperations[operation]( + () => client.clear(), + () => cache.remove(query), + ) + await result + + expect(cache.getAll()).toStrictEqual([]) + expect(vi.getTimerCount()).toBe(0) + }, + ) + + it.each(['clear', 'remove'] as const)( + 'does not reschedule a removed active query after abort and unsubscribe on %s', + async (operation) => { + const cache = client.getQueryCache() + const key = createKey() + const observer = new QueryObserver(client, { + queryKey: key, + gcTime: 1000, + queryFn: ({ signal }) => { + signal.addEventListener('abort', noop) + return new Promise(noop) + }, + }) + const unsubscribe = observer.subscribe(noop) + const query = cache.find({ queryKey: key })! + + removalOperations[operation]( + () => client.clear(), + () => cache.remove(query), + ) + await vi.advanceTimersByTimeAsync(0) + unsubscribe() + + expect(cache.getAll()).toStrictEqual([]) + expect(vi.getTimerCount()).toBe(0) + }, + ) + + it('does not schedule an old query on unsubscribe when a replacement has the same hash', async () => { + const key = createKey() + const cache = client.getQueryCache() + const observer = new QueryObserver(client, { + queryKey: key, + queryFn: () => Promise.resolve('data'), + gcTime: 1000, + enabled: false, + }) + const unsubscribe = observer.subscribe(noop) + const old = cache.find({ queryKey: key })! + cache.remove(old) + const replacement = cache.build(client, { queryKey: key, gcTime: 1000 }) + await vi.advanceTimersByTimeAsync(0) + unsubscribe() + + expect(cache.get(old.queryHash)).toBe(replacement) + expect(vi.getTimerCount()).toBe(1) + await vi.advanceTimersByTimeAsync(1000) + expect(cache.get(old.queryHash)).toBeUndefined() + expect(vi.getTimerCount()).toBe(0) + }) + + it('does not schedule a cancelled query when a replacement has the same hash', async () => { + const cache = client.getQueryCache() + const key = createKey() + const old = cache.build(client, { + queryKey: key, + queryFn: () => new Promise(noop), + gcTime: 1000, + }) + const result = old.fetch().catch(noop) + cache.remove(old) + const replacement = cache.build(client, { queryKey: key, gcTime: 1000 }) + await result + + expect(cache.get(old.queryHash)).toBe(replacement) + expect(vi.getTimerCount()).toBe(1) + await vi.advanceTimersByTimeAsync(1000) + expect(cache.get(old.queryHash)).toBeUndefined() + }) + + it('still collects an observed query after it is reset and unsubscribed', async () => { + const key = createKey() + const observer = new QueryObserver(client, { + queryKey: key, + queryFn: () => Promise.resolve('data'), + gcTime: 1000, + }) + const unsubscribe = observer.subscribe(noop) + await observer.refetch() + const query = client.getQueryCache().find({ queryKey: key })! + query.reset() + await observer.refetch() + unsubscribe() + + await vi.advanceTimersByTimeAsync(1000) + expect(client.getQueryCache().find({ queryKey: key })).toBeUndefined() + }) + + it('still collects a removed query re-added and used by an observer', async () => { + const key = createKey() + const cache = client.getQueryCache() + const query = cache.build(client, { + queryKey: key, + queryFn: () => Promise.resolve('data'), + gcTime: 1000, + }) + cache.remove(query) + cache.add(query) + const observer = new QueryObserver(client, { + ...query.options, + queryKey: key, + }) + const unsubscribe = observer.subscribe(noop) + await observer.refetch() + unsubscribe() + + await vi.advanceTimersByTimeAsync(1000) + expect(cache.find({ queryKey: key })).toBeUndefined() + }) + + it('still collects a removed mutation re-added and used by an observer', async () => { + const cache = client.getMutationCache() + const mutation = cache.build(client, { gcTime: 1000 }) + cache.remove(mutation) + cache.add(mutation) + const observer = new MutationObserver(client, { gcTime: 1000 }) + mutation.addObserver(observer) + mutation.removeObserver(observer) + + await vi.advanceTimersByTimeAsync(1000) + expect(cache.getAll()).toStrictEqual([]) + }) +}) diff --git a/packages/query-core/src/__tests__/mutationCache.test-d.tsx b/packages/query-core/src/__tests__/mutationCache.test-d.tsx index e0f96b548ce..d1a2bc586cd 100644 --- a/packages/query-core/src/__tests__/mutationCache.test-d.tsx +++ b/packages/query-core/src/__tests__/mutationCache.test-d.tsx @@ -38,6 +38,7 @@ describe('mutationCache', () => { | 'build' | 'add' | 'remove' + | 'has' | 'canRun' | 'runNext' | 'clear' diff --git a/packages/query-core/src/mutation.ts b/packages/query-core/src/mutation.ts index 90b7e48ae03..d2c174058bb 100644 --- a/packages/query-core/src/mutation.ts +++ b/packages/query-core/src/mutation.ts @@ -212,12 +212,15 @@ export class Mutation< /** * Unsubscribes an observer from the mutation. * @internal - * @param observer - The observer to remove. Garbage collection is scheduled afterwards. + * @param observer - The observer to remove. Garbage collection is scheduled afterwards if + * the mutation is still cached. */ removeObserver(observer: MutationObserver): void { this.#observers = this.#observers.filter((x) => x !== observer) - this.scheduleGc() + if (this.#mutationCache.has(this)) { + this.scheduleGc() + } this.#mutationCache.notify({ type: 'observerRemoved', diff --git a/packages/query-core/src/mutationCache.ts b/packages/query-core/src/mutationCache.ts index d709fe154f4..bc065cb28f6 100644 --- a/packages/query-core/src/mutationCache.ts +++ b/packages/query-core/src/mutationCache.ts @@ -179,12 +179,13 @@ export class MutationCache extends Subscribable { } /** - * Removes a mutation from the cache. + * Removes a mutation from the cache and clears its garbage collection timer. * @internal * @param mutation - The mutation to remove, also from its scope's queue. */ remove(mutation: Mutation): void { if (this.#mutations.delete(mutation)) { + mutation.destroy() const scope = scopeFor(mutation) if (typeof scope === 'string') { const scopedMutations = this.#scopes.get(scope) @@ -206,6 +207,16 @@ export class MutationCache extends Subscribable { this.notify({ type: 'removed', mutation }) } + /** + * Checks whether a mutation is still stored in this cache. + * @internal + * @param mutation - The mutation to check. + * @returns Whether the mutation belongs to this cache. + */ + has(mutation: Mutation): boolean { + return this.#mutations.has(mutation) + } + /** * Checks whether a mutation can run now, given the other mutations in its `scope`. * @internal @@ -251,7 +262,8 @@ export class MutationCache extends Subscribable { } /** - * Removes all mutations from the cache. + * Removes all mutations from the cache and clears their garbage collection timers. + * Mutations already executing continue to run. * @example * ```ts * const mutationCache = queryClient.getMutationCache() @@ -264,6 +276,7 @@ export class MutationCache extends Subscribable { const mutations = this.getAll() this.#mutations.clear() this.#scopes.clear() + mutations.forEach((mutation) => mutation.destroy()) mutations.forEach((mutation) => { this.notify({ type: 'removed', mutation }) }) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index c5e97c49388..03080d5e0e8 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -631,7 +631,7 @@ export class Query< * @internal * @param observer - The observer to remove. When the last observer is removed, the in-flight * fetch is cancelled if its abort signal was consumed (otherwise only its retries are stopped), - * and garbage collection is scheduled. + * and garbage collection is scheduled if the query is still cached. */ removeObserver(observer: QueryObserver): void { const index = this.observers.indexOf(observer) @@ -653,7 +653,13 @@ export class Query< } } - this.scheduleGc() + if ( + this.#cache.get( + this.queryHash, + ) === this + ) { + this.scheduleGc() + } } this.#cache.notify({ type: 'observerRemoved', query: this, observer }) @@ -940,8 +946,14 @@ export class Query< this.#retryer = undefined this.#fetchPromise = undefined } - // Schedule query gc after fetching - this.scheduleGc() + // A removed query can finish cancellation after its timer was cleared. + if ( + this.#cache.get( + this.queryHash, + ) === this + ) { + this.scheduleGc() + } } })() this.#fetchPromise = fetchPromise From 65e911ae9035bda28b31c3401e24bf540b4fa5ca Mon Sep 17 00:00:00 2001 From: Vitaliy Ryaboy Date: Sun, 8 Nov 2026 23:31:15 +0100 Subject: [PATCH 2/2] fix(query-core): clear GC timers after synchronous abort cleanup --- .../query-core/src/__tests__/cacheGc.test.tsx | 39 +++++++++++++++++++ packages/query-core/src/query.ts | 11 +++--- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/packages/query-core/src/__tests__/cacheGc.test.tsx b/packages/query-core/src/__tests__/cacheGc.test.tsx index e71a298ff63..26ef2b2e6b9 100644 --- a/packages/query-core/src/__tests__/cacheGc.test.tsx +++ b/packages/query-core/src/__tests__/cacheGc.test.tsx @@ -144,6 +144,45 @@ describe('garbage collection after cache removal', () => { }, ) + it.each(['clear', 'remove'] as const)( + 'clears a GC timer when the abort listener synchronously removes the last observer during %s', + async (operation) => { + const cache = client.getQueryCache() + const key = createKey() + let unsubscribe: () => void = noop + let queryDuringAbort: unknown + const onAbort = vi.fn<() => void>(() => { + queryDuringAbort = cache.find({ queryKey: key }) + unsubscribe() + }) + const observer = new QueryObserver(client, { + queryKey: key, + gcTime: 1000, + queryFn: ({ signal }) => { + signal.addEventListener('abort', onAbort) + return new Promise(noop) + }, + }) + unsubscribe = observer.subscribe(noop) + const query = observer.getCurrentQuery() + + removalOperations[operation]( + () => client.clear(), + () => cache.remove(query), + ) + + expect(onAbort).toHaveBeenCalledTimes(1) + expect(queryDuringAbort).toBe(query) + expect(query.getObserversCount()).toBe(0) + expect(cache.getAll()).toStrictEqual([]) + expect(vi.getTimerCount()).toBe(0) + + await vi.advanceTimersByTimeAsync(0) + expect(cache.getAll()).toStrictEqual([]) + expect(vi.getTimerCount()).toBe(0) + }, + ) + it('does not schedule an old query on unsubscribe when a replacement has the same hash', async () => { const key = createKey() const cache = client.getQueryCache() diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index 03080d5e0e8..9a059d25bcd 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -433,15 +433,16 @@ export class Query< } /** - * Clears the query's garbage collection timeout and silently cancels any - * in-flight fetch. Called by `QueryCache` when the query is removed from - * the cache. + * Silently cancels any in-flight fetch, then clears the query's garbage + * collection timeout. Called by `QueryCache` when the query is removed + * from the cache. * @see {@link Query#cancel} */ override destroy(): void { - super.destroy() - this.cancel({ silent: true }) + + // Abort listeners can synchronously remove observers and schedule GC. + super.destroy() } /**