fix: clear completed PR request generations
This commit is contained in:
parent
cab52d1635
commit
41cbdb1d1d
|
|
@ -0,0 +1,44 @@
|
|||
# Memory Leak Audit Pass 3
|
||||
|
||||
Started: 2026-05-31 PDT
|
||||
|
||||
Objective: continue the repository-wide leak audit from pass 2 on current
|
||||
`origin/main`, with special attention to code changed after
|
||||
`a85e4e8d88` (`docs: record memory leak audit pass 2`).
|
||||
|
||||
## Delta Inventory
|
||||
|
||||
- 2026-05-31: Fast-forwarded the audit worktree to current `origin/main`.
|
||||
- 2026-05-31: Counted 1431 changed code files since pass 2 (`*.ts`,
|
||||
`*.tsx`, `*.js`, `*.jsx`, `*.mjs`, `*.cjs`, `*.swift`).
|
||||
- 2026-05-31: Re-ran delta heuristics for DOM/RN listeners, timers,
|
||||
animation frames, observers, EventEmitter subscriptions, runtime
|
||||
subscriptions, workers, WebSockets, abort controllers, streams, and
|
||||
module-scope `Map`/`Set` caches.
|
||||
- 2026-05-31: Manually followed up high-risk delta hits in GitHub renderer
|
||||
state, PR diff caches, combined diff caches, mobile browser frame caches,
|
||||
main subprocess listeners, runtime file watchers, browser/webContents
|
||||
listeners, notification lifetimes, and renderer UI timers.
|
||||
|
||||
## Finding
|
||||
|
||||
- `src/renderer/src/store/slices/github.ts`: `prRequestGenerations` was a
|
||||
module-scoped map keyed by PR cache key. Each unique PR lookup inserted a
|
||||
generation entry, but completed requests only removed `inflightPRRequests`;
|
||||
the generation entry remained for the lifetime of the renderer. Fixed by
|
||||
deleting the generation key when the request that owns the current
|
||||
generation is also the active in-flight request. Overlapping forced refreshes
|
||||
still keep the stale-response guard because older requests cannot delete a
|
||||
newer generation. Risk: low.
|
||||
|
||||
## Validation
|
||||
|
||||
- `pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/store/slices/github.test.ts`
|
||||
- `pnpm exec oxlint src/renderer/src/store/slices/github.ts src/renderer/src/store/slices/github.test.ts`
|
||||
- `pnpm exec tsgo --noEmit -p config/tsconfig.tc.web.json`
|
||||
- `git diff --check`
|
||||
|
||||
## Remaining Work
|
||||
|
||||
- Continue the pass-3 delta audit beyond the confirmed GitHub PR generation
|
||||
leak. Current evidence does not prove the full repository is leak-free.
|
||||
|
|
@ -4,6 +4,7 @@ GitHub slice's cross-cutting invariants verifiable in one place. */
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { create } from 'zustand'
|
||||
import {
|
||||
_getGitHubPRRequestGenerationCountForTest,
|
||||
createGitHubSlice,
|
||||
mergePRCommentIntoList,
|
||||
prChecksCacheSuffix,
|
||||
|
|
@ -1015,6 +1016,25 @@ describe('createGitHubSlice.fetchPRForBranch', () => {
|
|||
}
|
||||
})
|
||||
|
||||
it('does not retain PR request generation keys after the active request settles', async () => {
|
||||
const store = createTestStore()
|
||||
const repoPath = '/repo'
|
||||
const branch = 'feature/no-generation-leak'
|
||||
const beforeCount = _getGitHubPRRequestGenerationCountForTest()
|
||||
const refreshPRNow = mockApi.gh.refreshPRNow
|
||||
;(mockApi.gh as unknown as { refreshPRNow?: typeof refreshPRNow }).refreshPRNow = undefined
|
||||
mockApi.gh.prForBranch.mockResolvedValueOnce(makePR({ number: 31 }))
|
||||
|
||||
try {
|
||||
await expect(
|
||||
store.getState().fetchPRForBranch(repoPath, branch, { force: true })
|
||||
).resolves.toMatchObject({ number: 31 })
|
||||
expect(_getGitHubPRRequestGenerationCountForTest()).toBe(beforeCount)
|
||||
} finally {
|
||||
mockApi.gh.refreshPRNow = refreshPRNow
|
||||
}
|
||||
})
|
||||
|
||||
it('passes SSH connection identity to GitHub refresh IPC for SSH-backed repos', async () => {
|
||||
const store = createTestStore()
|
||||
const repoPath = '/repo'
|
||||
|
|
|
|||
|
|
@ -365,6 +365,11 @@ const prRefreshStartedHostedReviewEntries = new Map<
|
|||
AppState['hostedReviewCache'][string] | undefined
|
||||
>()
|
||||
|
||||
/** @internal - exposed for leak-regression tests only */
|
||||
export function _getGitHubPRRequestGenerationCountForTest(): number {
|
||||
return prRequestGenerations.size
|
||||
}
|
||||
|
||||
// Why: cap in-flight cross-repo fan-out and hover-prefetches at the renderer
|
||||
// boundary — the main-side gate is behind the IPC queue, so it can't see a
|
||||
// stampede until the calls are already mid-flight. 8 balances responsiveness
|
||||
|
|
@ -2067,6 +2072,9 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
|||
const activeRequest = inflightPRRequests.get(cacheKey)
|
||||
if (activeRequest?.generation === generation) {
|
||||
inflightPRRequests.delete(cacheKey)
|
||||
if (prRequestGenerations.get(cacheKey) === generation) {
|
||||
prRequestGenerations.delete(cacheKey)
|
||||
}
|
||||
}
|
||||
}
|
||||
})()
|
||||
|
|
@ -2741,13 +2749,9 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
|||
issueCache: evictStaleEntries(s.issueCache)
|
||||
}))
|
||||
|
||||
// Why: prRequestGenerations tracks generation counters for inflight
|
||||
// fetch deduplication. Pruning keys that were just evicted from prCache
|
||||
// would race with inflight requests — their generation check would fail
|
||||
// and silently discard valid responses. Since each entry is just a number,
|
||||
// the memory overhead is negligible; let it shrink naturally as keys stop
|
||||
// being fetched. The eviction on prCache/issueCache above is sufficient
|
||||
// to bound the dominant source of growth.
|
||||
// Why: prRequestGenerations tracks only live inflight fetches and is
|
||||
// cleared when the active request settles. Do not prune it here; deleting
|
||||
// a live generation would make the corresponding response look stale.
|
||||
|
||||
// Only re-fetch PR/issue entries that are already stale — skip fresh ones
|
||||
const state = get()
|
||||
|
|
|
|||
Loading…
Reference in New Issue