From 2ddb976f8a4c3c4659653716fc147202eb1714db Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 11 Jul 2026 01:26:53 -0700 Subject: [PATCH] Fix local preflight context cache leak (#7669) --- .../lib/local-preflight-context-cache.test.ts | 81 +++++++++++++ .../src/lib/local-preflight-context-cache.ts | 106 ++++++++++++++++++ .../src/lib/local-preflight-context.ts | 70 +++--------- 3 files changed, 200 insertions(+), 57 deletions(-) create mode 100644 src/renderer/src/lib/local-preflight-context-cache.test.ts create mode 100644 src/renderer/src/lib/local-preflight-context-cache.ts diff --git a/src/renderer/src/lib/local-preflight-context-cache.test.ts b/src/renderer/src/lib/local-preflight-context-cache.test.ts new file mode 100644 index 00000000000..602aa0f302e --- /dev/null +++ b/src/renderer/src/lib/local-preflight-context-cache.test.ts @@ -0,0 +1,81 @@ +import { afterEach, describe, expect, it } from 'vitest' +import type { AppState } from '@/store/types' +import { + _getProjectRuntimePreflightContextCacheSizeForTest, + _getWslPreflightContextCacheSizeForTest, + _hasProjectRuntimePreflightContextCacheEntryForTest, + _hasWslPreflightContextCacheEntryForTest, + getLocalPreflightContext, + resetLocalPreflightContextCachesForTests +} from './local-preflight-context' + +const WSL_CACHE_LIMIT = 128 +const PROJECT_RUNTIME_CACHE_LIMIT = 2048 + +afterEach(() => { + resetLocalPreflightContextCachesForTests() +}) + +function makeWslState(distro: string): AppState { + return { + activeRepoId: 'repo-1', + activeWorktreeId: null, + repos: [{ id: 'repo-1', path: `\\\\wsl.localhost\\${distro}\\home\\alice\\repo` }], + worktreesByRepo: {} + } as AppState +} + +function makeWindowsProjectState(projectId: string): AppState { + return { + activeRepoId: projectId, + activeWorktreeId: null, + repos: [{ id: projectId, path: `C:\\Users\\alice\\${projectId}` }], + settings: {}, + worktreesByRepo: {} + } as AppState +} + +describe('local preflight context caches', () => { + it('releases an old WSL selector snapshot after sustained context churn', () => { + const first = getLocalPreflightContext(makeWslState('Distro0'), 'darwin') + + for (let index = 1; index <= WSL_CACHE_LIMIT; index += 1) { + getLocalPreflightContext(makeWslState(`Distro${index}`), 'darwin') + } + + expect(getLocalPreflightContext(makeWslState('Distro0'), 'darwin')).not.toBe(first) + }) + + it('caps cached WSL distro snapshots', () => { + for (let index = 0; index < WSL_CACHE_LIMIT + 1; index++) { + const distro = `Distro${index}` + expect(getLocalPreflightContext(makeWslState(distro), 'darwin')).toEqual({ + wslDistro: distro + }) + } + + expect(_getWslPreflightContextCacheSizeForTest()).toBe(WSL_CACHE_LIMIT) + expect(_hasWslPreflightContextCacheEntryForTest('Distro0')).toBe(false) + expect(_hasWslPreflightContextCacheEntryForTest(`Distro${WSL_CACHE_LIMIT}`)).toBe(true) + }) + + it('caps cached project runtime snapshots without mutating order on reads', () => { + for (let index = 0; index < PROJECT_RUNTIME_CACHE_LIMIT; index++) { + getLocalPreflightContext(makeWindowsProjectState(`project-${index}`), 'win32') + } + + getLocalPreflightContext(makeWindowsProjectState('project-0'), 'win32') + getLocalPreflightContext( + makeWindowsProjectState(`project-${PROJECT_RUNTIME_CACHE_LIMIT}`), + 'win32' + ) + + expect(_getProjectRuntimePreflightContextCacheSizeForTest()).toBe(PROJECT_RUNTIME_CACHE_LIMIT) + expect( + _hasProjectRuntimePreflightContextCacheEntryForTest('project-0:windows-host:global-default') + ).toBe(false) + expect( + _hasProjectRuntimePreflightContextCacheEntryForTest('project-1:windows-host:global-default') + ).toBe(true) + }) +}) diff --git a/src/renderer/src/lib/local-preflight-context-cache.ts b/src/renderer/src/lib/local-preflight-context-cache.ts new file mode 100644 index 00000000000..ec65a4fb9be --- /dev/null +++ b/src/renderer/src/lib/local-preflight-context-cache.ts @@ -0,0 +1,106 @@ +import type { ProjectExecutionRuntimeResolution } from '../../../shared/project-execution-runtime' + +export type LocalPreflightContext = + | { + wslDistro?: string | null + wslDefault?: boolean + runtimeContextKey?: string + projectRuntime?: ProjectExecutionRuntimeResolution + } + | undefined + +// Why: selector snapshots must be reference-stable, but project/runtime ids can +// churn as repos are added and removed during a long-lived renderer session. +const WSL_PREFLIGHT_CONTEXT_CACHE_MAX = 128 +const PROJECT_RUNTIME_PREFLIGHT_CONTEXT_CACHE_MAX = 2048 + +// Why: these reads run inside broad store selectors. Insertion-order eviction +// keeps cache hits read-only instead of adding Map mutations to every store update. +const wslPreflightContextsByDistro = new Map>() +const projectRuntimePreflightContextsByKey = new Map>() + +export function resetLocalPreflightContextCachesForTests(): void { + wslPreflightContextsByDistro.clear() + projectRuntimePreflightContextsByKey.clear() +} + +export function _getWslPreflightContextCacheSizeForTest(): number { + return wslPreflightContextsByDistro.size +} + +export function _hasWslPreflightContextCacheEntryForTest(wslDistro: string): boolean { + return wslPreflightContextsByDistro.has(wslDistro) +} + +export function _getProjectRuntimePreflightContextCacheSizeForTest(): number { + return projectRuntimePreflightContextsByKey.size +} + +export function _hasProjectRuntimePreflightContextCacheEntryForTest(cacheKey: string): boolean { + return projectRuntimePreflightContextsByKey.has(cacheKey) +} + +export function getWslPreflightContext(wslDistro: string): NonNullable { + const cached = wslPreflightContextsByDistro.get(wslDistro) + if (cached) { + return cached + } + + // Why: React/Zustand selectors must return a cached snapshot. A fresh object + // here triggers a useSyncExternalStore loop when Settings observes WSL repos. + const context = Object.freeze({ wslDistro }) + storeCacheEntry(wslPreflightContextsByDistro, wslDistro, context, WSL_PREFLIGHT_CONTEXT_CACHE_MAX) + return context +} + +export function getProjectRuntimePreflightContext( + resolution: ProjectExecutionRuntimeResolution +): NonNullable { + const cacheKey = getProjectRuntimeContextObjectCacheKey(resolution) + const cached = projectRuntimePreflightContextsByKey.get(cacheKey) + if (cached) { + return cached + } + + const wslDistro = + resolution.status === 'resolved' && resolution.runtime.kind === 'wsl' + ? resolution.runtime.distro + : undefined + // Why: selectors compare by reference; cache each resolved runtime context so + // adding projectRuntime does not reintroduce useSyncExternalStore churn. + const context = Object.freeze({ + ...(wslDistro ? { wslDistro } : {}), + projectRuntime: resolution + }) + storeCacheEntry( + projectRuntimePreflightContextsByKey, + cacheKey, + context, + PROJECT_RUNTIME_PREFLIGHT_CONTEXT_CACHE_MAX + ) + return context +} + +function getProjectRuntimeContextObjectCacheKey( + resolution: ProjectExecutionRuntimeResolution +): string { + if (resolution.status === 'resolved') { + return `${resolution.runtime.cacheKey}:${resolution.runtime.reason}` + } + return `${resolution.repair.cacheKey}:${resolution.repair.source}` +} + +function storeCacheEntry( + cache: Map, + key: string, + value: T, + maxEntries: number +): void { + cache.set(key, value) + if (cache.size > maxEntries) { + const oldest = cache.keys().next() + if (!oldest.done) { + cache.delete(oldest.value) + } + } +} diff --git a/src/renderer/src/lib/local-preflight-context.ts b/src/renderer/src/lib/local-preflight-context.ts index ee275f74fb9..ef4a0bc0469 100644 --- a/src/renderer/src/lib/local-preflight-context.ts +++ b/src/renderer/src/lib/local-preflight-context.ts @@ -13,8 +13,21 @@ import { getCachedWindowsTerminalCapabilities, hasCachedWindowsTerminalCapabilities } from './windows-terminal-capabilities' +import { + getProjectRuntimePreflightContext, + getWslPreflightContext, + type LocalPreflightContext +} from './local-preflight-context-cache' export { localPreflightContextKey } from './local-preflight-context-key' +export type { LocalPreflightContext } from './local-preflight-context-cache' +export { + _getProjectRuntimePreflightContextCacheSizeForTest, + _getWslPreflightContextCacheSizeForTest, + _hasProjectRuntimePreflightContextCacheEntryForTest, + _hasWslPreflightContextCacheEntryForTest, + resetLocalPreflightContextCachesForTests +} from './local-preflight-context-cache' type LocalProjectRuntimeState = Pick< AppState, @@ -26,67 +39,10 @@ type LocalProjectRuntimeWslContext = { availableWslDistros?: readonly string[] | null } -export type LocalPreflightContext = - | { - wslDistro?: string | null - wslDefault?: boolean - runtimeContextKey?: string - projectRuntime?: ProjectExecutionRuntimeResolution - } - | undefined - -const wslPreflightContextsByDistro = new Map>() -const projectRuntimePreflightContextsByKey = new Map>() - export function getWslDistroFromPath(path?: string | null): string | null { return path ? (parseWslUncPath(path)?.distro ?? null) : null } -function getWslPreflightContext(wslDistro: string): NonNullable { - const cached = wslPreflightContextsByDistro.get(wslDistro) - if (cached) { - return cached - } - - // Why: React/Zustand selectors must return a cached snapshot. A fresh object - // here triggers a useSyncExternalStore loop when Settings observes WSL repos. - const context = Object.freeze({ wslDistro }) - wslPreflightContextsByDistro.set(wslDistro, context) - return context -} - -function getProjectRuntimeContextObjectCacheKey( - resolution: ProjectExecutionRuntimeResolution -): string { - if (resolution.status === 'resolved') { - return `${resolution.runtime.cacheKey}:${resolution.runtime.reason}` - } - return `${resolution.repair.cacheKey}:${resolution.repair.source}` -} - -function getProjectRuntimePreflightContext( - resolution: ProjectExecutionRuntimeResolution -): NonNullable { - const cacheKey = getProjectRuntimeContextObjectCacheKey(resolution) - const cached = projectRuntimePreflightContextsByKey.get(cacheKey) - if (cached) { - return cached - } - - const wslDistro = - resolution.status === 'resolved' && resolution.runtime.kind === 'wsl' - ? resolution.runtime.distro - : undefined - // Why: selectors compare by reference; cache each resolved runtime context so - // adding projectRuntime does not reintroduce useSyncExternalStore churn. - const context = Object.freeze({ - ...(wslDistro ? { wslDistro } : {}), - projectRuntime: resolution - }) - projectRuntimePreflightContextsByKey.set(cacheKey, context) - return context -} - export function getLocalProjectExecutionRuntimeContext( state: LocalProjectRuntimeState, worktreeId?: string | null,