From f3f2cff8d7758588b1c5f010365e84f10739322d Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 26 Aug 2026 22:56:20 +0000 Subject: [PATCH 1/4] fix(deps): replace heic with fork commit fixing native decode leak Co-authored-by: Florian Rathe --- go.mod | 2 ++ go.sum | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index baad4ff..0e094af 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,8 @@ require ( golang.org/x/mod v0.40.0 ) +replace github.com/gen2brain/heic => github.com/frathe/heic v0.0.0-20260820164529-0ac0a39f8206 + require ( fyne.io/systray v1.12.2 // indirect github.com/BurntSushi/toml v1.6.0 // indirect diff --git a/go.sum b/go.sum index c4d24b5..41f27d3 100644 --- a/go.sum +++ b/go.sum @@ -105,6 +105,8 @@ github.com/felixge/fgprof v0.9.3 h1:VvyZxILNuCiUCSXtPtYmmtGvb65nqXh2QFWc0Wpf2/g= github.com/felixge/fgprof v0.9.3/go.mod h1:RdbpDgzqYVh/T9fPELJyV7EYJuHB55UTEULNun8eiPw= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= +github.com/frathe/heic v0.0.0-20260820164529-0ac0a39f8206 h1:+0q4uGHAATWnRF/IGETH9q5+yVi2/HPwq0O8b0/wFy4= +github.com/frathe/heic v0.0.0-20260820164529-0ac0a39f8206/go.mod h1:ja42wMJc4fpnKsfdUJxeZa2YqqRnes1wS0xqs5+8o5w= github.com/fredbi/uri v1.1.1 h1:xZHJC08GZNIUhbP5ImTHnt5Ya0T8FI2VAwI/37kh2Ko= github.com/fredbi/uri v1.1.1/go.mod h1:4+DZQ5zBjEwQCDmXW5JdIjz0PUA+yJbvtBv+u+adr5o= github.com/fsnotify/fsnotify v1.10.1 h1:b0/UzAf9yR5rhf3RPm9gf3ehBPpf0oZKIjtpKrx59Ho= @@ -119,8 +121,6 @@ github.com/fyne-io/oksvg v0.2.0 h1:mxcGU2dx6nwjJsSA9PCYZDuoAcsZ/OuJlvg/Q9Njfo8= github.com/fyne-io/oksvg v0.2.0/go.mod h1:dJ9oEkPiWhnTFNCmRgEze+YNprJF7YRbpjgpWS4kzoI= github.com/gen2brain/avif v0.6.0 h1:/8WSgcU+IEF0jhKYsUZ/mzlziFuTeJFpIKBj2siTQps= github.com/gen2brain/avif v0.6.0/go.mod h1:QgrYqdVE9y40PCfArK9VakcMIpYeDYpZmCSLkW6C1n8= -github.com/gen2brain/heic v0.7.1 h1:Aha1sZdKEeZeWl5o0xkSg7NBRhhkrlokGVCRri+2Qcc= -github.com/gen2brain/heic v0.7.1/go.mod h1:ja42wMJc4fpnKsfdUJxeZa2YqqRnes1wS0xqs5+8o5w= github.com/go-chi/chi/v5 v5.3.0 h1:halUjDxhshgXHMrao5bB8eNBXo/rnzwr8m5m36glehM= github.com/go-chi/chi/v5 v5.3.0/go.mod h1:R+tYY2hNuVUUjxoPtqUdgBqevM9s9njzkTLutVsOCto= github.com/go-gl/gl v0.0.0-20260331235117-4566fea9a276 h1:IO5P06Pcj9K04d+l4nrf3c2U56+dAotIFG6u4P1wAHI= From 9270610fc56ceff8941cb2a0b040cefd55c815f0 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 26 Aug 2026 22:56:37 +0000 Subject: [PATCH 2/4] docs: document HEIC replace; add optional heicleak RSS test Co-authored-by: Florian Rathe --- AGENTS.md | 1 + internal/imaging/heic_leak_test.go | 51 ++++++++++++++++++++++++++ internal/imaging/rss_heicleak_linux.go | 30 +++++++++++++++ internal/imaging/rss_heicleak_stub.go | 7 ++++ needs_refactoring.md | 2 + 5 files changed, 91 insertions(+) create mode 100644 internal/imaging/heic_leak_test.go create mode 100644 internal/imaging/rss_heicleak_linux.go create mode 100644 internal/imaging/rss_heicleak_stub.go diff --git a/AGENTS.md b/AGENTS.md index 17b16b1..769447f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,6 +40,7 @@ - Use the Makefile: `make run`, `make build`, `make test`, `make fmt`, `make vet`; `make help` lists packaging/security targets. - Match CI before handoff: `make fmt-check` (`goimports -local github.com/frathe/picfetch`), `go vet ./...`, `go build ./...`, then `go test -timeout 20m -race ./...` from the repository root. +- **HEIC decoder pin:** `go.mod` replaces `github.com/gen2brain/heic` with a fork commit containing [gen2brain/heic#16](https://github.com/gen2brain/heic/pull/16) (fixes native memory leak [issue #15](https://github.com/gen2brain/heic/issues/15)). Remove the `replace` and bump to an official release once upstream tags a version that includes that fix. Optional manual RSS check: `PICFETCH_HEIC_LEAK_TEST=1 go test -tags=heicleak -run TestHEICDecode_DoesNotGrowRSSUnbounded ./internal/imaging/...`. - Run focused tests while iterating, e.g. `go test -run TestE2E -v ./internal/ui/...`; the complete suite remains the final check. - Golden screenshots are under `internal/ui/testdata/`. Regenerate only with `make golden` (Docker linux/amd64), inspect `internal/ui/testdata/failed/*.png`, and never commit failed renders. - Packaging uses Fyne/Fyne-cross; macOS is native, while Windows/Linux cross-builds require Docker. `fyne package` may bump `FyneApp.toml`’s build number. diff --git a/internal/imaging/heic_leak_test.go b/internal/imaging/heic_leak_test.go new file mode 100644 index 0000000..7de9b94 --- /dev/null +++ b/internal/imaging/heic_leak_test.go @@ -0,0 +1,51 @@ +//go:build heicleak + +package imaging + +import ( + "os" + "runtime" + "runtime/debug" + "testing" + + "fyne.io/fyne/v2/storage" +) + +func TestHEICDecode_DoesNotGrowRSSUnbounded(t *testing.T) { + if os.Getenv("PICFETCH_HEIC_LEAK_TEST") == "" { + t.Skip("set PICFETCH_HEIC_LEAK_TEST=1 to run native RSS check") + } + + data, err := os.ReadFile("testdata/test_exif.heic") + if err != nil { + t.Fatalf("read fixture: %v", err) + } + path := writeTempFile(t, "leak.heic", data) + + rssBefore, ok := readRSS() + if !ok { + t.Skip("RSS measurement unavailable on this platform") + } + + const iterations = 40 + for i := 0; i < iterations; i++ { + debug.FreeOSMemory() + runtime.GC() + _, err := LoadImage(storage.NewFileURI(path), DefaultImgCacheBytes) + if err != nil { + t.Fatalf("decode %d: %v", i, err) + } + } + debug.FreeOSMemory() + runtime.GC() + + rssAfter, ok := readRSS() + if !ok { + t.Fatal("RSS measurement failed after decode loop") + } + + const maxGrowthMB = 150 // libheif one-time init + headroom; leaky build adds hundreds of MB + if growth := rssAfter - rssBefore; growth > maxGrowthMB*1024*1024 { + t.Fatalf("RSS grew %d bytes over %d decodes; want <= %d MB (leak suspected)", growth, iterations, maxGrowthMB) + } +} diff --git a/internal/imaging/rss_heicleak_linux.go b/internal/imaging/rss_heicleak_linux.go new file mode 100644 index 0000000..81ea8bf --- /dev/null +++ b/internal/imaging/rss_heicleak_linux.go @@ -0,0 +1,30 @@ +//go:build heicleak && linux + +package imaging + +import ( + "os" + "strconv" + "strings" +) + +func readRSS() (uint64, bool) { + data, err := os.ReadFile("/proc/self/status") + if err != nil { + return 0, false + } + for line := range strings.SplitSeq(string(data), "\n") { + if after, ok := strings.CutPrefix(line, "VmRSS:"); ok { + fields := strings.Fields(after) + if len(fields) == 0 { + return 0, false + } + kb, err := strconv.ParseUint(fields[0], 10, 64) + if err != nil { + return 0, false + } + return kb * 1024, true + } + } + return 0, false +} diff --git a/internal/imaging/rss_heicleak_stub.go b/internal/imaging/rss_heicleak_stub.go new file mode 100644 index 0000000..3b9b3d6 --- /dev/null +++ b/internal/imaging/rss_heicleak_stub.go @@ -0,0 +1,7 @@ +//go:build heicleak && !linux + +package imaging + +func readRSS() (uint64, bool) { + return 0, false +} diff --git a/needs_refactoring.md b/needs_refactoring.md index 050a13d..42c4cb4 100644 --- a/needs_refactoring.md +++ b/needs_refactoring.md @@ -33,6 +33,8 @@ dependency, and one upstream library bug. - **Fix**: watch for the upstream release containing PR #16 and bump. If it stalls, add a `replace` to the patched fork — one line, immediately shippable. Either way, add a note in `AGENTS.md` so the pin isn't forgotten. +- **Mitigation (2026-08-26):** `go.mod` replace → `frathe/heic@0ac0a39` until + upstream releases PR #16. Remove replace on bump. ### 2. The duplicate-visibility model lives inside the grid feature From 95714982cf70312fbaaf23a0e6811eb820a8de12 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 26 Aug 2026 22:59:17 +0000 Subject: [PATCH 3/4] test(imaging): warm up HEIC decoder before RSS leak check Co-authored-by: Florian Rathe --- internal/imaging/heic_leak_test.go | 43 +++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/internal/imaging/heic_leak_test.go b/internal/imaging/heic_leak_test.go index 7de9b94..a17816a 100644 --- a/internal/imaging/heic_leak_test.go +++ b/internal/imaging/heic_leak_test.go @@ -22,30 +22,47 @@ func TestHEICDecode_DoesNotGrowRSSUnbounded(t *testing.T) { } path := writeTempFile(t, "leak.heic", data) - rssBefore, ok := readRSS() + // Warm libheif/wazero once so one-time init is not counted as per-decode growth. + if _, err := LoadImage(storage.NewFileURI(path), DefaultImgCacheBytes); err != nil { + t.Fatalf("warmup decode: %v", err) + } + settleRSS() + + const mid = 20 + for i := 0; i < mid; i++ { + if _, err := LoadImage(storage.NewFileURI(path), DefaultImgCacheBytes); err != nil { + t.Fatalf("decode %d: %v", i, err) + } + } + settleRSS() + rssMid, ok := readRSS() if !ok { t.Skip("RSS measurement unavailable on this platform") } - const iterations = 40 + const iterations = 20 for i := 0; i < iterations; i++ { - debug.FreeOSMemory() - runtime.GC() - _, err := LoadImage(storage.NewFileURI(path), DefaultImgCacheBytes) - if err != nil { - t.Fatalf("decode %d: %v", i, err) + if _, err := LoadImage(storage.NewFileURI(path), DefaultImgCacheBytes); err != nil { + t.Fatalf("decode %d: %v", mid+i, err) } } - debug.FreeOSMemory() - runtime.GC() - + settleRSS() rssAfter, ok := readRSS() if !ok { t.Fatal("RSS measurement failed after decode loop") } - const maxGrowthMB = 150 // libheif one-time init + headroom; leaky build adds hundreds of MB - if growth := rssAfter - rssBefore; growth > maxGrowthMB*1024*1024 { - t.Fatalf("RSS grew %d bytes over %d decodes; want <= %d MB (leak suspected)", growth, iterations, maxGrowthMB) + // With the upstream leak, RSS climbs roughly linearly with decode count. + // After PR #16 the second batch should not add much beyond wasm init noise. + const maxGrowthMB = 80 + if growth := rssAfter - rssMid; growth > maxGrowthMB*1024*1024 { + t.Fatalf("RSS grew %d bytes over %d decodes after warmup; want <= %d MB (leak suspected)", growth, iterations, maxGrowthMB) } } + +func settleRSS() { + debug.FreeOSMemory() + runtime.GC() + debug.FreeOSMemory() + runtime.GC() +} From ac7513503b69dc07b056dbb35c7b40f8d1a7d95e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 26 Aug 2026 23:19:38 +0000 Subject: [PATCH 4/4] docs: add HEIC leak fix implementation plan Co-authored-by: Florian Rathe --- plans/2026-08-26-heic-native-memory-leak.md | 402 ++++++++++++++++++++ 1 file changed, 402 insertions(+) create mode 100644 plans/2026-08-26-heic-native-memory-leak.md diff --git a/plans/2026-08-26-heic-native-memory-leak.md b/plans/2026-08-26-heic-native-memory-leak.md new file mode 100644 index 0000000..288039e --- /dev/null +++ b/plans/2026-08-26-heic-native-memory-leak.md @@ -0,0 +1,402 @@ +# HEIC Native Memory Leak Fix Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Stop unbounded native-memory growth when decoding HEIC files by pinning a patched `github.com/gen2brain/heic` (upstream PR #16) until an official release ships, and document the pin so it is not forgotten. + +**Architecture:** The leak is upstream in `decodeDynamic` (`heif_decode_image` without `heif_image_release`). PicFetch consumes HEIC only through blank imports in `internal/imaging/loader.go` and direct EXIF calls in `internal/imaging/exif.go`; no local decode wrapper exists. The fix is a `go.mod` `replace` to the patched fork commit (or a future tagged release), plus AGENTS documentation. Existing functional HEIC tests in `internal/imaging` remain the correctness gate; RSS verification is manual or behind an optional build tag. + +**Tech Stack:** Go modules, `github.com/gen2brain/heic` v0.7.1 (leaky) → patched fork at `frathe/heic@0ac0a39`, wazero WASM libheif bridge, existing `internal/imaging` test fixtures (`testdata/test_exif.heic`). + +## Global Constraints + +- Preserve no-cgo HEIC decoding through `gen2brain/heic` (WASM/wazero); do not add cgo or a parallel decode path in `internal/imaging`. +- Do not change `loader.go` / `exif.go` imports unless upstream module path changes (it does not for a `replace`). +- Do not add mutable package-level test seams. +- Match CI before handoff: `make fmt-check`, `go vet ./...`, `go build ./...`, then `go test -timeout 20m -race ./...` from repo root. +- Open work belongs in `todos.md`; no `TODO`/`FIXME` in source. +- Subagents do **not** `git commit`. After each task the parent reviews the diff, fixes if needed, then commits and pushes. +- Update `ARCHITECTURE.md` only if package map changes (it should not for this dependency pin). + +--- + +## Context (verified 2026-08-26) + +| Fact | Value | +|------|-------| +| Current pin | `github.com/gen2brain/heic v0.7.1` (`go.mod:11`) | +| Latest upstream tag | `v0.7.1` @ `10ce79e` — no newer release | +| Upstream PR | [gen2brain/heic#16](https://github.com/gen2brain/heic/pull/16) — **open**, authored by frathe | +| Fix commit | `0ac0a39f8206d395737a3118d7784ee6fc3bf0ba` on `frathe/heic:fix-decode-image-leak` | +| Diff | One line: `defer heifImageRelease(heifImg)` in `decode_dynamic.go` | +| Commits above v0.7.1 | 2 (`6e6457f` README tweak on upstream main, then the fix) | +| PicFetch import sites | `internal/imaging/loader.go:29` (blank import), `internal/imaging/exif.go:12` | +| Existing HEIC tests | `loader_test.go` (decode + orientation), `exif_test.go` (metadata fallback) | +| Leak signature | Go `HeapAlloc` flat; process RSS grows ~18 MB/decode for 12 MP HEIC | + +--- + +## Decisions + +### Locked (unless you override in open questions) + +1. **Primary fix path:** `go.mod` `replace` to patched fork **now**, not wait for upstream tag — item 1 is Risk 5 / crash-grade for iPhone libraries. +2. **Replace target:** `github.com/frathe/heic` at commit `0ac0a39f8206d395737a3118d7784ee6fc3bf0ba` (PR #16 head). The extra README commit between v0.7.1 and the fix is harmless. +3. **No imaging code changes** unless verification proves the replace did not take effect. +4. **Correctness gate:** existing `internal/imaging` HEIC tests + full `./...` race suite. +5. **AGENTS.md note required** per `needs_refactoring.md` item 1. + +### Open (see “Questions for Florian” — parent must resolve before Task 1) + +- O1: Replace-now vs watch-only until upstream merges +- O2: Optional RSS regression test vs manual verification only +- O3: How to record the pin in `THIRD-PARTY-NOTICES.md` +- O4: What to do with `needs_refactoring.md` entry when done +- O5: Upstream-release follow-up automation + +--- + +## File map + +| File | Responsibility | +|------|----------------| +| `go.mod` | Add `replace github.com/gen2brain/heic => github.com/frathe/heic v0.7.1-leakfix` (pseudo-version) or commit-pinned replace; keep `require` line at `v0.7.1` until upstream tags. | +| `go.sum` | Regenerated by `go mod tidy` after replace. | +| `AGENTS.md` | Short “Dependency pins” note: HEIC replace, link to issue #15 / PR #16, remove when upstream ≥ fix release. | +| `THIRD-PARTY-NOTICES.md` | Optionally annotate heic section with fork source (decision O3). | +| `needs_refactoring.md` | Mark item 1 mitigated or move note (decision O4). | +| `internal/imaging/heic_leak_test.go` | **Optional** (decision O2): build-tagged RSS stress test. | +| `internal/imaging/loader_test.go` | Unchanged; run existing HEIC subtests. | +| `internal/imaging/exif_test.go` | Unchanged; run HEIC metadata tests. | + +No changes to `loader.go`, `exif.go`, or UI packages expected. + +--- + +## Delegation + +Execute **strictly in order**. Parent reviews the full diff after every task and fixes before the next dispatch. One implementer subagent per task. + +| Task | Subagent type | Model | Why | +|------|---------------|-------|-----| +| 0 Decision gate | *(parent + Florian)* | — | Resolve open questions O1–O5 before any code. | +| 1 Pin patched module | `go-expert` | `cursor-grok-4.6-high-fast` | Single-file module edit + tidy; fast model suffices. | +| 2 Document the pin | `generalPurpose` | `composer-2.5` | AGENTS + notices + backlog text; no Go. | +| 3 Verify | `go-expert` | `claude-sonnet-5-thinking-high` | Must run focused HEIC tests and full race suite; confirm replace resolves. | +| 4 RSS regression test *(optional)* | `go-expert` | `claude-opus-5-thinking-high` | Native RSS tests are flaky and platform-sensitive; use Opus only if O2 chooses automated guard. | + +**Escalation:** If Task 1 `go mod tidy` fails (module proxy, missing fork tag), re-dispatch Task 1 on `claude-sonnet-5-thinking-high`. If Task 3 shows HEIC still resolves to unpatched code, escalate to `claude-opus-5-thinking-high`. + +**Parent review checklist (every task):** +- `go list -m github.com/gen2brain/heic` shows `replace` → `frathe/heic` @ `0ac0a39…` +- No edits outside task file map +- AGENTS note names issue #15, PR #16, and removal trigger +- `TestLoadImage/valid heic` and `TestReadMetadata_HEICFallback` pass +- Full `go test -race ./...` green before calling item 1 done + +--- + +### Task 0: Decision gate (parent — block until Florian answers) + +**Files:** none + +- [ ] **Step 1:** Florian confirms O1–O5 (see bottom of plan). +- [ ] **Step 2:** Parent records decisions in the “Decisions (locked)” section above and checks the corresponding boxes before dispatching Task 1. + +--- + +### Task 1: Pin patched `gen2brain/heic` via `replace` + +**Files:** +- Modify: `go.mod` (after the `require (` block closes, before indirect requires) +- Modify: `go.sum` (via tidy) + +**Interfaces:** +- Consumes: decision O1 = ship replace now; replace target `0ac0a39` +- Produces: module graph that builds against patched `decodeDynamic` + +- [ ] **Step 1: Add replace directive** + +Append to `go.mod` (adjust pseudo-version if `go mod tidy` prefers a different form): + +```go +replace github.com/gen2brain/heic => github.com/frathe/heic v0.7.1-0.20260820164529-0ac0a39f8206 +``` + +If pseudo-version resolution fails, use commit form: + +```go +replace github.com/gen2brain/heic => github.com/frathe/heic 0ac0a39f8206d395737a3118d7784ee6fc3bf0ba +``` + +Keep the existing require line: + +```go +github.com/gen2brain/heic v0.7.1 +``` + +- [ ] **Step 2: Tidy and confirm resolution** + +Run: + +```bash +go mod tidy +go list -m -json github.com/gen2brain/heic | jq '{Path, Version, Replace}' +``` + +Expected: `Replace` points at `github.com/frathe/heic` with version/commit containing `0ac0a39`. + +- [ ] **Step 3: Smoke build** + +Run: + +```bash +go build ./... +``` + +Expected: exit 0. + +- [ ] **Step 4: Parent review** — diff is only `go.mod` + `go.sum`. + +--- + +### Task 2: Document the dependency pin + +**Files:** +- Modify: `AGENTS.md` (new bullet under Build and Verification or a short “Dependency pins” subsection) +- Modify: `THIRD-PARTY-NOTICES.md` — **only if** O3 ≠ “leave unchanged” +- Modify: `needs_refactoring.md` — **only if** O4 ≠ “leave unchanged” + +**Interfaces:** +- Consumes: Task 1 merged replace +- Produces: discoverable note for future agents/releases + +- [ ] **Step 1: AGENTS.md note** + +Add under **Build and Verification** (or new **Dependency pins** subsection): + +```markdown +- **HEIC decoder pin:** `go.mod` replaces `github.com/gen2brain/heic` with a fork commit containing [gen2brain/heic#16](https://github.com/gen2brain/heic/pull/16) (fixes native memory leak [issue #15](https://github.com/gen2brain/heic/issues/15)). Remove the `replace` and bump to an official release once upstream tags a version that includes that fix. +``` + +- [ ] **Step 2: THIRD-PARTY-NOTICES (if O3 = annotate)** + +In the `## github.com/gen2brain/heic` section, add after Source line: + +```markdown +Patched via go.mod replace: https://github.com/frathe/heic/commit/0ac0a39f8206d395737a3118d7784ee6fc3bf0ba (pending upstream PR #16). +``` + +- [ ] **Step 3: needs_refactoring.md (if O4 = mark mitigated)** + +Under item 1, add a line: + +```markdown +- **Mitigation (2026-08-26):** `go.mod` replace → `frathe/heic@0ac0a39` until upstream releases PR #16. Remove replace on bump. +``` + +Or move a one-line summary to `finished_refactorings/` if Florian prefers that cadence. + +- [ ] **Step 4: Parent review** — prose is accurate, links live, no duplicate TODOs in source. + +--- + +### Task 3: Verify decode correctness and full suite + +**Files:** +- Test (run only): `internal/imaging/loader_test.go`, `internal/imaging/exif_test.go` + +**Interfaces:** +- Consumes: Tasks 1–2 +- Produces: CI-equivalent green run + +- [ ] **Step 1: Focused HEIC tests** + +Run: + +```bash +go test -race -run 'TestLoadImage/valid_heic|TestReadMetadata_HEIC' ./internal/imaging/... +``` + +Expected: PASS (subtest name may be spaced: `valid heic` — use `-run TestLoadImage` if shorthand misses). + +Safer explicit run: + +```bash +go test -race -run 'TestLoadImage' ./internal/imaging/ -v -count=1 2>&1 | tee /tmp/heic-loader-test.log +go test -race -run 'TestReadMetadata_HEIC' ./internal/imaging/ -v -count=1 +``` + +- [ ] **Step 2: Confirm patched module in test binary** + +Run: + +```bash +go list -f '{{.Replace}}' github.com/gen2brain/heic +``` + +Expected: non-empty replace to frathe/heic. + +- [ ] **Step 3: Format and vet** + +Run: + +```bash +make fmt-check +go vet ./... +``` + +Expected: clean. + +- [ ] **Step 4: Full race suite** + +Run: + +```bash +go test -timeout 20m -race ./... +``` + +Expected: all packages PASS (same as CI). + +- [ ] **Step 5: Manual RSS check (always, even if Task 4 skipped)** + +Document in PR / handoff (not necessarily committed): + +1. Build picfetch: `make build` +2. Open a folder with many 12 MP HEIC files (or loop-decode fixture 60× with repro from issue #15) +3. Watch RSS (`ps` / Activity Monitor / `vmmap` on macOS): should plateau, not grow ~18 MB/decode + +- [ ] **Step 6: Parent review** — logs attached; no test skips added without `-short` justification. + +--- + +### Task 4 (optional): Automated RSS regression guard + +**Skip unless Florian chooses O2 = automated test.** + +**Files:** +- Create: `internal/imaging/heic_leak_test.go` + +**Interfaces:** +- Consumes: Task 1 replace +- Produces: `go test -tags=heicleak ./internal/imaging/...` optional CI job or local-only check + +- [ ] **Step 1: Write build-tagged test** + +```go +//go:build heicleak + +package imaging + +import ( + "os" + "runtime" + "runtime/debug" + "testing" + + "fyne.io/fyne/v2/storage" +) + +func TestHEICDecode_DoesNotGrowRSSUnbounded(t *testing.T) { + if os.Getenv("PICFETCH_HEIC_LEAK_TEST") == "" { + t.Skip("set PICFETCH_HEIC_LEAK_TEST=1 to run native RSS check") + } + data, err := os.ReadFile("testdata/test_exif.heic") + if err != nil { + t.Fatalf("read fixture: %v", err) + } + path := writeTempFile(t, "leak.heic", data) + + rssBefore := readRSS(t) + const iterations = 40 + for i := 0; i < iterations; i++ { + debug.FreeOSMemory() + runtime.GC() + _, err := LoadImage(storage.NewFileURI(path), DefaultImgCacheBytes) + if err != nil { + t.Fatalf("decode %d: %v", i, err) + } + } + debug.FreeOSMemory() + runtime.GC() + rssAfter := readRSS(t) + + const maxGrowthMB = 150 // libheif one-time init + headroom; leaky build adds ~750MB+ at 40 iter + if growth := rssAfter - rssBefore; growth > maxGrowthMB*1024*1024 { + t.Fatalf("RSS grew %d bytes over %d decodes; want <= %d MB (leak suspected)", growth, iterations, maxGrowthMB) + } +} +``` + +Implement `readRSS` for `linux` (read `/proc/self/status` VmRSS) and `darwin` (syscall or skip with message). **Do not** enable in default CI — too flaky on shared runners. + +- [ ] **Step 2: Run locally once with env var set** + +Run: + +```bash +PICFETCH_HEIC_LEAK_TEST=1 go test -tags=heicleak -race -run TestHEICDecode_DoesNotGrowRSSUnbounded ./internal/imaging/ -v -count=1 +``` + +- [ ] **Step 3: Document in AGENTS.md** — one line that `-tags=heicleak` exists for manual native-memory checks. + +- [ ] **Step 4: Parent review** — default CI unchanged; test skips without env var. + +--- + +## Upstream release follow-up (after merge of PR #16) + +When `gen2brain/heic` tags a release containing the fix: + +1. Remove the `replace` line from `go.mod` +2. Bump `require github.com/gen2brain/heic v0.7.x` to the new tag +3. `go mod tidy` +4. Re-run Task 3 verification +5. Remove or shorten AGENTS pin note; update THIRD-PARTY-NOTICES source URL to new tag + +*(Optional automation if O5 = subscribe: use cursor-subscriptions `subscribe_github_pr` on PR #16 or a timer to re-check releases.)* + +--- + +## Questions for Florian + +Please answer these before Task 1 is dispatched: + +**O1 — Ship strategy** + +Which do you want? + +- **A)** Land `replace` → `frathe/heic@0ac0a39` immediately (plan default) +- **B)** Wait for upstream to merge PR #16 and tag; only add AGENTS “watch” note for now +- **C)** Land replace now **and** keep a GitHub subscription / periodic check for the official release + +**O2 — Regression testing** + +- **A)** Manual RSS verification only (Task 3 step 5); rely on existing functional HEIC tests in CI +- **B)** Add optional `-tags=heicleak` RSS test (Task 4), not in default CI +- **C)** Attempt to run RSS test in CI (not recommended — flaky on GitHub runners) + +**O3 — THIRD-PARTY-NOTICES** + +- **A)** Add fork/commit annotation under the heic section +- **B)** Leave unchanged (license text still MIT from same project) + +**O4 — needs_refactoring.md** + +- **A)** Add “Mitigation” sub-bullet under item 1 +- **B)** Move summary to `finished_refactorings/2026-08-26-heic-leak-replace.md` and shorten item 1 +- **C)** Leave backlog unchanged until upstream release + +**O5 — PR #16 follow-up** + +- **A)** You will merge upstream yourself; no automation +- **B)** Subscribe this cloud agent run to PR #16 / release checks when executing +- **C)** Add a one-line reminder in `todos.md` under Internal + +--- + +## Suggested commit messages (parent, after review) + +1. `fix(deps): replace heic with fork commit fixing native decode leak` +2. `docs: document HEIC module replace pending upstream PR #16` +3. *(optional)* `test(imaging): add heicleak build tag for RSS regression check`