From f4557bfef1257c62937e45cd8fbbdd2d2c237a3f Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sat, 26 Sep 2026 12:46:48 -0700 Subject: [PATCH] fix(renderer): release retired store snapshots from selector caches (#23187) Co-authored-by: m4air --- ...rent-pr-checks-projection-selector.test.ts | 33 +++++++++++++++++++ .../parent-pr-checks-projection-selector.ts | 13 ++++++-- .../terminal-pane-host-state-memo.test.ts | 17 ++++++++++ .../terminal-pane/terminal-pane-host-state.ts | 7 ++-- 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.test.ts b/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.test.ts index 7d7e1b1e4ad..6706b5ea0ad 100644 --- a/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.test.ts +++ b/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.test.ts @@ -56,6 +56,39 @@ function review(number: number): HostedReviewInfo { } describe('parent PR checks projection selector', () => { + it.each([false, true])( + 'releases unrelated store data while keeping the projection cached (replacement=%s)', + async (replacement) => { + if (typeof globalThis.gc !== 'function') { + throw new Error('Run with the repository Vitest --expose-gc config') + } + const select = createParentPrChecksProjectionSelector({ + worktrees: [worktree(0)], + repos: [repo()], + settings: null, + refreshOutcomes: new Map() + }) + if (replacement) { + select({ hostedReviewCache: {}, prCache: {}, checksCache: {} }) + } + const caches = { hostedReviewCache: {}, prCache: {}, checksCache: {} } + function selectRetiredState(): WeakRef { + const unrelatedData = { content: new Uint8Array(1024 * 1024) } + const state = { ...caches, unrelatedData } + select(state) + return new WeakRef(unrelatedData) + } + const retired = selectRetiredState() + const projection = select(caches) + for (let round = 0; round < 3; round++) { + await new Promise((resolve) => setImmediate(resolve)) + globalThis.gc() + } + expect(retired.deref()).toBeUndefined() + expect(select(caches)).toBe(projection) + } + ) + it('does not inspect tracked keys when cache map references are unchanged', () => { const cacheRead = vi.fn() const observedCache = new Proxy( diff --git a/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.ts b/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.ts index 42ad55de0a8..4c4f04cb7ac 100644 --- a/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.ts +++ b/src/renderer/src/components/right-sidebar/parent-pr-checks-projection-selector.ts @@ -18,6 +18,15 @@ type CacheDependency = { value: unknown } +function selectCacheReferences(state: ReviewCacheState): ReviewCacheState { + // Pick narrows the type, not the object received from the store. + return { + hostedReviewCache: state.hostedReviewCache, + prCache: state.prCache, + checksCache: state.checksCache + } +} + function trackCacheReads( state: ReviewCacheState, cacheName: K, @@ -74,7 +83,7 @@ export function createParentPrChecksProjectionSelector( } if (dependenciesAreCurrent(state, cached.cacheReferences, cached.dependencies)) { // Why: adopting unrelated replacement maps keeps later store notifications O(1). - cached.cacheReferences = state + cached.cacheReferences = selectCacheReferences(state) return cached.projection } } @@ -88,7 +97,7 @@ export function createParentPrChecksProjectionSelector( prCache: trackCacheReads(state, 'prCache', dependencies), checksCache: trackCacheReads(state, 'checksCache', dependencies) }) - cached = { cacheReferences: state, dependencies, projection } + cached = { cacheReferences: selectCacheReferences(state), dependencies, projection } return projection } } diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-host-state-memo.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-host-state-memo.test.ts index ba9c169295d..ba4bb48f1c3 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-host-state-memo.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-host-state-memo.test.ts @@ -54,6 +54,23 @@ beforeEach(() => { }) describe('selectTerminalPaneHostState memo', () => { + it('releases the last store snapshot when no terminal selects again', async () => { + if (typeof globalThis.gc !== 'function') { + throw new Error('Run with the repository Vitest --expose-gc config') + } + function selectRetiredState(): WeakRef { + const state = makeState() + selectTerminalPaneHostState(state, LOCAL_WORKTREE) + return new WeakRef(state) + } + const retired = selectRetiredState() + for (let round = 0; round < 3; round++) { + await new Promise((resolve) => setImmediate(resolve)) + globalThis.gc() + } + expect(retired.deref()).toBeUndefined() + }) + it('returns the same object across an unrelated store write', () => { const first = makeState() const before = selectTerminalPaneHostState(first, LOCAL_WORKTREE) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-host-state.ts b/src/renderer/src/components/terminal-pane/terminal-pane-host-state.ts index 6b40bdf379c..c0d1615fbeb 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-host-state.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-host-state.ts @@ -78,7 +78,8 @@ function isSameHostState(a: TerminalPaneHostState, b: TerminalPaneHostState): bo ) } -let cachedState: AppState | null = null +// Identity checks must not keep a retired store alive after the last pane unmounts. +let cachedState: WeakRef | null = null let cachedByWorktreeId = new Map() let previousByWorktreeId = new Map() @@ -100,10 +101,10 @@ export function selectTerminalPaneHostState( state: AppState, worktreeId: string ): TerminalPaneHostState { - if (state !== cachedState) { + if (state !== cachedState?.deref()) { previousByWorktreeId = cachedByWorktreeId cachedByWorktreeId = new Map() - cachedState = state + cachedState = new WeakRef(state) } const cached = cachedByWorktreeId.get(worktreeId) if (cached) {