.agents/skills/memory-leak-fixer/SKILL.md
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 this skill scans against — 11 real focus areas, each with a description,
why it's bad, a leak→fix code example, and a leak-specific anti-pattern — is in
references/types-of-leaks.md. Read it before scanning
(Phase 1) and consult the matching focus area when writing a fix (Phase 3).
[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.*.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/**.noop (see Phase 5).Run the phases in order. The skill has two entry points:
| You were asked to… | Start at | Notes |
|---|---|---|
| Find a leak (scan only) / file an issue | Phase 1 → 2 → (report) | Stop after confirmation; file [memory-leak] issue. |
| Scan and fix (the default, and what the workflow does) | Phase 0 → 1 → 2 → 3 → 4 → 5 | End-to-end: hunt → prove → fix → file the finding as an issue and open a linked draft PR that closes it (Fixes #…). |
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.
Confirm the SDK: dotnet --version.
Restore the pre-built natives so the C# projects build and the tests run:
dotnet cake --target=externals-download # pre-built natives for managed-C# work
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. Now open
references/types-of-leaks.md and load focus area #FOCUS: its
Where to look line gives the path + grep starting points, and the rest of the entry is the
description, why-it's-bad, a leak→fix example, and the per-area anti-pattern. Read that
focus area before scanning. If it's exhausted (its leaks are already open issues/PRs — see 1.3),
advance to the next index and load that focus area.
For each candidate write the precise path with file:line citations:
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.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.
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).
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:
WeakReference;for (i=0;i<6;i++){ GC.Collect(); GC.WaitForPendingFinalizers(); GC.Collect(); });cd /tmp/leakprobe && dotnet test --logger "console;verbosity=normal"
Dispose is never reached; the in-repo equivalent uses SKObject.GetInstance<T>(handle, out _) + CollectGarbage() (see tests/Tests/SkiaSharp/SKObjectTest.cs).Add a focused regression test to the console test project (source lives under tests/Tests/,
run via tests/SkiaSharp.Tests.Console). Model it on existing disposal/leak tests:
AssertEx.EventuallyGC(weakRef, …) — GC-based (tests/Tests/Xunit/AssertEx.cs).SKObject.GetInstance<T>(handle, out inst) + CollectGarbage() — wrapper lifecycle
(tests/Tests/SkiaSharp/SKObjectTest.cs).tests/SkiaSharp.Tests.Devices/Tests/Maui/MemoryLeakTests.cs.Build and confirm the test FAILS on the current tree (proves it catches the leak):
dotnet cake --target=externals-download # pre-built natives
dotnet build binding/SkiaSharp/SkiaSharp.csproj
dotnet test tests/SkiaSharp.Tests.Console/SkiaSharp.Tests.Console.csproj --filter "FullyQualifiedName~<YourTestName>"
If a test you expected to be red is green, your hypothesis is wrong — go back to Phase 1.
Apply the Fix (✓) for the matching focus area in
references/types-of-leaks.md — every focus area has a worked
before/after there. Then re-read that focus area's Watch out (❌ don't): note: it names the
specific wrong fix that turns one leak into another (an unconditional Dispose, flipping
owns: blind, nulling a field before disposing, a pinned GCHandle where a plain field
suffices, …).
Touch only the minimal code, and keep it inside binding/** / source/**. Never change a
public signature to fix ownership — add an overload or fix internals (ABI stability). If the
only correct fix is in native / upstream Skia, stop and file an issue (Phase 4) — this
skill does not open native PRs.
Rebuild and re-run the regression test (now PASSES) plus neighbouring tests:
dotnet test tests/SkiaSharp.Tests.Console/SkiaSharp.Tests.Console.csproj --filter "FullyQualifiedName~<YourTestName>"
# then a wider relevant slice, e.g. the type's test class, to catch regressions
Enforce red→green in both directions: revert the fix ⇒ red; re-apply ⇒ green.
Run this checklist before committing or opening the PR, so a bad attempt is dropped now
instead of pushed and reverted. If any box can't be ticked, fix it or stand down (emit a
noop) — do not open the PR.
[Obsolete]-hidden, skipped, or weakened.binding/** / source/** only — no *.generated.cs, no
externals/skia/**, no native / upstream change.references/types-of-leaks.md does not describe
what you just did (no unconditional same-instance Dispose, no blind owns: flip, no
field nulled before dispose, no pinned GCHandle or hand-rolled keep-alive field/List<T>
where SKObject.Referenced(...) / KeepAliveObjects (or a plain field) suffices, …).All ticked ⇒ proceed to Phase 4 (file the finding, then open the PR). Any unticked ⇒ no PR
(fix it, file the finding as an issue on its own, or noop).
A confirmed, managed-C#-fixable leak produces two linked safe outputs so the finding and the fix are tracked separately and the issue auto-closes when the PR merges.
Labels (both the issue and the PR): a memory leak is a performance concern, so tag both
outputs with tenet/performance (the quality-tenet umbrella) and perf/memory-leak (the
performance sub-type). When this skill runs from the memory-leak-fixer workflow these labels
are applied automatically by its safe-outputs config; when filing by hand, add them yourself.
The perf/* taxonomy is defined in the issue-triage skill
(references/labels.md).
Emit a create_issue that describes the leak, not the fix. Give it a temporary_id
(format aw_ + 3–8 alphanumeric characters — no underscores or other symbols — e.g. aw_leak1)
so the PR can reference it
before its real number exists. Body (markdown):
memory-leak-fixer skill.file:line citations.tenet/performance + perf/memory-leak.Create a feature branch (dev/memory-leak-<short-desc>), commit the test + fix, and open a
draft create_pull_request. Body (markdown):
Fix ✓).dotnet test commands.Fixes #<temporary_id>
— e.g. Fixes #aw_leak1 (the id you gave the issue in 4.1). gh-aw rewrites it to the real issue
number once the issue is created.tenet/performance + perf/memory-leak.If the leak is real but the only correct fix lives under externals/skia/** (incl. the C
shim), do not open a PR. Emit the create_issue from 4.1 alone — finding plus the
proposed native fix — so nothing is lost.
Write a short summary: which focus area, the candidate (file:line), the proof result, and the
resulting issue + PR links. When run from the agentic workflow, append this to the run's step
summary.
End with the right safe output(s):
Fixes #… so merging closes the issue).create-issue alone (finding + proposal).noop carrying
this summary.A noop is the correct "nothing to do / analysis only" signal — never finish with no safe
output, which makes the run look incomplete.