docs(perf): memory-leak investigation reports (no leak found)
Evidence-based investigation across every leak vector (timers, listeners, object
URLs, observers, websockets, static server collections, IMemoryCache, Python
caches). Verdict: no confirmed memory leak — the codebase has disciplined
cleanup. One resource-release correctness bug (over-eager blob-URL revocation in
the CV carousel) was found and fixed (eed9b1f).
Adds docs/performance/: MEMORY_LEAK_REPORT.md, ROOT_CAUSE_ANALYSIS.md,
PERFORMANCE_IMPROVEMENTS.md.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,67 @@
|
||||
# Root Cause Analysis — Job Tracker resource audit
|
||||
|
||||
**Companion to:** [MEMORY_LEAK_REPORT.md](MEMORY_LEAK_REPORT.md)
|
||||
|
||||
## Summary
|
||||
There is **no memory leak** to root-cause. The investigation surfaced exactly one defect — an
|
||||
**over-eager blob-URL revocation** in the CV PDF carousel — which is a *release-too-early* bug, the
|
||||
inverse of a leak. This document root-causes that defect and explains why the "app memory grows" symptom
|
||||
does **not** indicate a leak here.
|
||||
|
||||
## The one defect — over-revoked preview URLs
|
||||
|
||||
### What the code did (before)
|
||||
`job-tracker-ui/src/pages/ProfilePage.tsx`:
|
||||
```ts
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
pdfCarousel.forEach((item) => item.pdfUrl && URL.revokeObjectURL(item.pdfUrl));
|
||||
};
|
||||
}, [pdfCarousel]); // <-- deps on pdfCarousel
|
||||
```
|
||||
A cleanup with `[pdfCarousel]` deps runs its teardown **before every re-run**, i.e. on *every* change to
|
||||
`pdfCarousel`, not just on unmount.
|
||||
|
||||
### Why it broke
|
||||
`buildPdfCarousel()` seeds all templates, then `savePdfToCarousel()` replaces each seed **in place**, one
|
||||
`setPdfCarousel` call at a time (`ProfilePage.tsx:400-410`). Trace with templates A, B, C:
|
||||
|
||||
1. `[A₁, B₀, C₀]` (A built, B/C seeds without URLs) — cleanup revoked prior `[A₀,B₀,C₀]` (no URLs). OK.
|
||||
2. `[A₁, B₁, C₀]` (B built) — cleanup runs on the **previous** array `[A₁,B₀,C₀]` → **revokes `A₁`'s URL**,
|
||||
but `A₁` is still present in the new array and still shown when the user flips the carousel to A.
|
||||
3. `[A₁, B₁, C₁]` (C built) — cleanup revokes `[A₁,B₁,C₀]` → revokes `B₁` too.
|
||||
|
||||
**Result:** after building an N-template deck, every preview except the **last** points at a revoked
|
||||
(broken) blob URL.
|
||||
|
||||
### Root cause
|
||||
Wrong effect dependency scope: a resource that should be released **once, on unmount** was tied to a
|
||||
value-change dependency, so React's "cleanup-before-next-run" semantics turned it into a per-change
|
||||
revoke. Compounded by the fact that legitimate drop paths already revoke explicitly
|
||||
(`savePdfToCarousel` replace at `:402-403`, `resetPdfCarousel` clear at `:378-384`), making the effect's
|
||||
revocation redundant *and* destructive.
|
||||
|
||||
### Why it is not a leak
|
||||
On unmount the effect *did* revoke the current array (deps capture the latest value), so URLs were freed.
|
||||
The bug wastes nothing and retains nothing — it releases too **eagerly**. It is a correctness bug
|
||||
(broken previews), filed here because Phase 3.5 explicitly covers "image/media resources … released".
|
||||
|
||||
### Fix (commit `eed9b1f`)
|
||||
Track the carousel in a ref; revoke **only on unmount** (empty-deps effect). Drop paths keep their
|
||||
explicit revokes. Verified: `profile-page.test.tsx` 5/5.
|
||||
|
||||
## Why the "memory grows" symptom is not a leak here
|
||||
Per the mission's Final Rule, distinguishing the four causes:
|
||||
- **Expected caching** — MUI emotion style cache, `react-scripts` dev tooling, and route component state
|
||||
grow then plateau; not unbounded.
|
||||
- **Delayed GC** — detached nodes from closed dialogs/pages are collected on the next major GC, not
|
||||
instantly; a rising sawtooth is normal.
|
||||
- **Browser behaviour** — bfcache, image decode buffers, and devtools retention inflate numbers in a way
|
||||
unrelated to app code.
|
||||
- **Genuine leak** — would require a retained root (listener, timer, global ref, live connection). None
|
||||
exists in this codebase (see the vector table in the main report).
|
||||
|
||||
## Contributing (non-defect) observations
|
||||
- **Extraction-poll churn** (`ProfilePage.tsx:315-324`): interval recreated every 4s while a run is
|
||||
active because `extractionRuns` is in the deps and mutates each poll. Harmless; optionally stabilise
|
||||
(see improvements doc).
|
||||
Reference in New Issue
Block a user