cesnimda
39266c0935
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>
68 lines
3.6 KiB
Markdown
68 lines
3.6 KiB
Markdown
# 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).
|