Memory Leak Fixer

Memory Leak Fixer

Proactively find and fix memory leaks in SkiaSharp — a thin managed wrapper over native Skia, so its recurring, high-impact class of leaks is native ownership / disposal correctness — the C# binding failing to dispose, own, pin, or root a native object correctly — not the managed view-retention leaks a pure-managed app framework worries about. This skill hunts that class of leaks and produces a validated fix.

Scope: managed C# only. Work only in the code SkiaSharp owns — the C# bindings (binding/**) and view layers (source/**). Everything under externals/skia/** is upstream Skia (including our C shim): out of scope, not buildable on a standard runner, and handled by a separate process. Every candidate must be provable and fixable from C#.

Read documentation/dev/memory-management.md first — it is the authoritative model (pointer types, owns: flag, ref-count rules, the same-instance-return contract). This skill assumes that model.

The leak catalogue is split into 11 focused references, each with a description, why it's bad, a leak→fix code example, and a leak-specific anti-pattern. Phase 1 maps the selected focus area directly to its one reference file; consult that same file when writing a fix (Phase 3).

Code samples are illustrative and trimmed to the essential lines; real wrappers must retain normal argument validation and use GC.KeepAlive where lifetime requires it.

Golden rules (non-negotiable)

1. One leak per run. Pick the single strongest candidate; do not batch.
2. Empirical confirmation before any fix. A fix is only valid if a regression test FAILS on the current tree without the fix and PASSES with it (red→green, both directions). No demonstrated red→green ⇒ no PR.
3. Never weaken, skip, mute, [Obsolete]-hide, or delete a test to make it pass. If the only thing that turns green is a mute, the fix is wrong — reject it.
4. Never edit generated files or upstream Skia. *.generated.cs and everything under externals/skia/** (including our C shim) are off-limits — this skill is managed-C# only. Every fix lives in binding/** or source/**.
5. ABI stability. Add overloads / methods; never change or remove a public signature.
6. Honest scope note. In every issue/PR, say whether it is a clear framework bug or a usage footgun the framework could harden — and what is empirically proven vs statically reasoned.
7. Finding nothing is the expected outcome. SkiaSharp is mature and heavily hardened; there is no planted/seeded bug waiting to be found. Most runs should end with no candidate. Never invent a leak, never rationalize deliberately-hardened, well-documented code as "decoys," and never lower the evidence bar to force a result. A quiet run is a first-class success — report it and emit a single noop (see Phase 5).

Mode selection

Run the phases in order. The skill has two entry points:

Phase 0 — Prepare the scan (no native download)

CI runner reality: this skill fixes managed C# only (binding/**, source/**). The native library is consumed as a pre-built package (externals-download) — you never build native code, so every candidate must be provable and fixable from C#. A leak whose only fix is in native / upstream Skia is out of scope: file an issue (Phase 4), never open a PR you cannot validate.

1. Confirm the SDK: dotnet --version.
2. Read the ownership model and select the focus-area entry. Do not restore local tools or download pre-built natives during setup, source scanning, or de-duplication. A quiet or duplicate run must end without either operation.

Phase 1 — Scan (find ONE candidate)

1.1 Choose a focus area (round-robin across runs)

A full 11-area sweep every run is wasteful and the surface is mostly hardened, so start from ONE focus area and widen only if it's exhausted.

Override first. If the run supplies an explicit focus area (a bare number 0–10 — e.g. a maintainer testing one area on demand), use that number directly as FOCUS and skip the rotation below. Otherwise rotate the starting area on a time-based round-robin so consecutive runs cover different areas. This needs only date, so it behaves identically locally and in CI — no $GITHUB_RUN_NUMBER / $RANDOM (which don't exist or aren't deterministic outside GitHub Actions):

# Round-robin: advance one focus area every hour, cycling through all 11.
DOY=$(date -u +%j); HOUR=$(date -u +%H)         # day-of-year + hour, both zero-padded
FOCUS=$(( (10#$DOY * 24 + 10#$HOUR) % 11 ))      # 10# forces base-10
echo "focus area: $FOCUS"

The 10# prefix is required: date zero-pads %j/%H, and $(( 08 )) is an invalid-octal error without it. For a targeted local run, skip the rotation and just name the focus area you want.

Every focus area is drawn from a real, historical SkiaSharp leak fix. Open the one file mapped to FOCUS, then read its Where to look line, description, why-it's-bad, leak→fix example, and per-area anti-pattern before scanning:

If the selected focus area is exhausted (its leaks are already open issues/PRs — see 1.3), advance to the next index and load only its mapped reference file.

1.2 Establish the retention/ownership path

For each candidate write the precise path with file:line citations:

• Native-handle leaks: creation site → escape path → missing Dispose/unref.
• owns: bugs: the P/Invoke name that produced the handle (_new_/_create returns an owned object; _get_/property-style returns a borrowed pointer) vs the owns: value the C# wrapper passed.
• Views retention: long-lived root → subscription/handler → transient view, and the unload path that should have detached but doesn't.

Skip if already weak/correct: uses WeakEventHandler/WeakReference/ ConditionalWeakTable, or the ownership already matches the memory-management rules.

1.3 De-dup against this project's own open issues/PRs

Before confirming, fetch and skip anything already covered. Search two ways — real SkiaSharp leak fixes are usually filed as [BUG] … (not [memory-leak] …), so the title-prefix search alone will miss them. Also search by the specific type/API name:

# 1. Prior runs of THIS workflow (our own prefix):
gh issue list --repo "$GITHUB_REPOSITORY" --search '"[memory-leak]" in:title' \
  --state open --limit 100 --json number,title,body
gh pr list --repo "$GITHUB_REPOSITORY" --search '"[memory-leak]" in:title' \
  --state open --limit 100 --json number,title,body

# 2. Human-reported coverage of the SAME api/type (the important check):
#    e.g. for the Blob.FromStream candidate below, this surfaces open PR #3473.
gh issue list --repo "$GITHUB_REPOSITORY" --search 'Blob.FromStream in:title,body' --state open --json number,title
gh pr    list --repo "$GITHUB_REPOSITORY" --search 'Blob.FromStream in:title,body' --state open --json number,title

A candidate is OUT only if an open issue/PR already covers the same handle / ownership path (by our prefix OR by the api/type name). A candidate whose only prior item is CLOSED may be re-filed. Worked example: the HarfBuzzSharp.Blob.FromStream fixed-pointer leak (area 4) is a genuine, still-present bug — but open PR #3473 "Make Blob.FromStream GC safe" already fixes it, so it is OUT: stand down, do not open a duplicate PR, emit a noop.

Pick the ONE strongest candidate. If none is convincing, stop — a quiet run is a success (the surface is hardened; the value is catching new leaks as code lands). Do not keep digging past a reasonable single pass hoping to manufacture a finding: report the quiet result and emit a noop (Phase 5).

1.4 Bootstrap one qualified candidate

Only after one managed-C# candidate has a citable ownership path and clears the Phase 1.3 open-item de-dup gate, prepare for empirical proof. Run this exact command once per run:

dotnet tool restore && dotnet cake --target=externals-download

This is the mandatory bootstrap before any source build or test, not a scan prerequisite. Do not run either command for a quiet/duplicate candidate, and do not repeat either command in later phases.

Phase 2 — Prove (empirical confirmation)

Every in-scope leak is observable from managed code, so prove it with a WeakReference + forced-GC probe that mirrors the existing memory tests. Prove it against the shipped SkiaSharp NuGet in a throwaway project — no source build, no display:

mkdir -p /tmp/leakprobe && cd /tmp/leakprobe

leakprobe.csproj referencing <PackageReference Include="SkiaSharp" Version="*" /> — the floating * resolves to the latest stable SkiaSharp on nuget.org automatically (use Version="*-*" to include the latest preview) — plus xunit + Microsoft.NET.Test.Sdk, then a single [Fact] that:

1. runs Control (correct usage), Leaky (the suspect path), Mitigation (the proposed workaround), each allocating N subjects tracked by WeakReference;
2. forces GC (for (i=0;i<6;i++){ GC.Collect(); GC.WaitForPendingFinalizers(); GC.Collect(); });
3. asserts the leaky subjects stay alive while control + mitigation are collected.

cd /tmp/leakprobe && dotnet test --logger "console;verbosity=normal"

• Passes ⇒ leak confirmed. Failed to build / assertions don't hold ⇒ hypothesis wrong; iterate once on another candidate or stop.
• For undisposed-handle leaks, prefer proving the wrapper is not collected / Dispose is never reached; the in-repo equivalent uses SKObject.GetInstance<T>(handle, out _) + CollectGarbage() (see tests/Tests/SkiaSharp/SKObjectTest.cs).

Phase 3 — Fix (red→green regression test + minimal change)