---
name: memory-leak-fixer
description: >
  Scan SkiaSharp for native ownership / disposal memory leaks AND fix them with a
  red→green regression test. Two combined modes in one skill: (1) SCAN — hunt the
  SkiaSharp leak signature (undisposed SKObject handle, wrong `owns:` flag,
  same-instance double-dispose, unremoved event/handler subscription,
  `fixed`-pointer lifetime) and empirically confirm it; (2) FIX — write
  a failing regression test, implement the minimal idiomatic fix, prove it goes green,
  and open a PR.

  Triggers: "memory leak", "leak scan", "leak hunter", "find leaks", "disposal bug",
  "not disposed", "undisposed handle", "owns flag", "double free", "AccessViolation on
  dispose", "native memory grows", "handle leak", "GC does not collect SKObject",
  "fix the leak", any request to proactively find or fix SkiaSharp memory/disposal leaks.

  For a user-reported functional bug that is not a leak, use `issue-fix` instead.
---

# 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`](../../../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:

| 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 #…`). |

---

## 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):

```bash
# 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:

| `FOCUS` | Reference |
|---:|---|
| 0 | [`references/leaks/undisposed-native-handle.md`](references/leaks/undisposed-native-handle.md) |
| 1 | [`references/leaks/wrong-owns-flag.md`](references/leaks/wrong-owns-flag.md) |
| 2 | [`references/leaks/same-instance-double-dispose.md`](references/leaks/same-instance-double-dispose.md) |
| 3 | [`references/leaks/managed-retention-views.md`](references/leaks/managed-retention-views.md) |
| 4 | [`references/leaks/fixed-pointer-lifetime.md`](references/leaks/fixed-pointer-lifetime.md) |
| 5 | [`references/leaks/finalizer-collection-ordering.md`](references/leaks/finalizer-collection-ordering.md) |
| 6 | [`references/leaks/clone-copy-double-free.md`](references/leaks/clone-copy-double-free.md) |
| 7 | [`references/leaks/native-statics-singletons.md`](references/leaks/native-statics-singletons.md) |
| 8 | [`references/leaks/field-not-nulled-on-dispose.md`](references/leaks/field-not-nulled-on-dispose.md) |
| 9 | [`references/leaks/stream-callback-proxy-lifetime.md`](references/leaks/stream-callback-proxy-lifetime.md) |
| 10 | [`references/leaks/allocation-failure-path.md`](references/leaks/allocation-failure-path.md) |

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**:

```bash
# 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**:

```bash
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:

```bash
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**.

```bash
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)

### 3.1 Write the failing test FIRST (red)
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`).
- For views: the handler pattern in
  `tests/SkiaSharp.Tests.Devices/Tests/Maui/MemoryLeakTests.cs`.

Using the one bootstrap from Phase 1.4, build and **confirm the test FAILS** on the current tree
(proves it catches the leak):

```bash
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.

### 3.2 Implement the minimal idiomatic fix
Apply the **Fix (✓)** in the selected focus-area reference — every reference has a worked
before/after. Then re-read that reference'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.

### 3.3 Confirm green + no regressions
Rebuild and re-run the regression test (now PASSES) plus neighbouring tests:

```bash
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.

### 3.4 Self-review gate — before you open the PR
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.

- [ ] The test genuinely goes **red without the fix, green with it**, both directions
      (§3.3) — not green because a test was muted, `[Obsolete]`-hidden, skipped, or weakened.
- [ ] The fix is inside `binding/**` / `source/**` only — no `*.generated.cs`, no
      `externals/skia/**`, no native / upstream change.
- [ ] **No public signature changed** — overloads / internals only (ABI stable).
- [ ] The selected reference's **Watch out (❌ don't):** note 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, …).
- [ ] This is a real leak with a citable path — not hardened, documented code rationalised
      as a "decoy," and not a finding manufactured to avoid a quiet run.
- [ ] Not already covered by an open issue/PR (§1.3).

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`).

---

## Phase 4 — File the finding, then open the linked fix PR

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`](../issue-triage/references/labels.md)).

### 4.1 The issue — the finding
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):
- **AI-generated banner** naming this workflow + the `memory-leak-fixer` skill.
- **Focus area**, and the **retention/ownership path** with `file:line` citations.
- **Evidence**: the Phase 2 proof — the probe you ran and its alive/collected counts.
- **Scope note**: framework bug vs footgun; empirically-proven vs statically-reasoned; ABI impact.
- **Labels**: `tenet/performance` + `perf/memory-leak`.

### 4.2 The PR — the fix
Create a feature branch (`dev/memory-leak-<short-desc>`), commit the test + fix, and open a
**draft** `create_pull_request`. Body (markdown):
- **AI-generated banner** naming this workflow + skill.
- **The fix**: what changed and why it is the idiomatic pattern (point at the focus area's `Fix ✓`).
- **Proof (red→green)**: the failing-then-passing test and the exact `dotnet test` commands.
- **A closing keyword on its own line so merging auto-closes the finding:** `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.
- **Labels**: `tenet/performance` + `perf/memory-leak`.


### 4.3 Out of scope (native / upstream only)
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.

---

## Phase 5 — Report

Write a short summary: which focus area, the candidate (`file:line`), the proof result, and the
resulting issue + PR links. Name the actual checked universe and evidence: for an exhaustive claim,
name the bounded query/path and confirm that every returned result was inspected without
truncation; for a sample, say it was representative and name the files or candidates actually
opened. Never infer an exhaustive scan or aggregate count from a few representative reads. When
run from the agentic workflow, append this to the run's step summary.

**End with the right safe output(s):**
- **Confirmed + managed-C# fix** → the **issue + PR pair** from Phase 4 (the PR body carries
  `Fixes #…` so merging closes the issue).
- **Confirmed but native/upstream-only fix** → the **`create-issue`** alone (finding + proposal).
- **Quiet run** (no convincing candidate) **or** a **dry run** → a single **`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.