Add memory-leak-fixer skill + agentic workflow (self-testing dry-run PR trigger) by mattleibow · Pull Request #4330 · mono/SkiaSharp

mattleibow · 2026-07-03T15:59:56Z

What

Adapts the AI memory-leak scanner+fixer idea from dotnet/maui#36276 to SkiaSharp's real leak family: native ownership / disposal correctness, rather than MAUI's managed view-retention leaks.

Two deliverables:

.agents/skills/memory-leak-fixer/SKILL.md — a combined scan → prove → fix methodology grounded in SkiaSharp's ownership model (documentation/dev/memory-management.md). Leak families it hunts:
- undisposed native SKObject handles (factory/getter/cache escapes)
- wrong owns: flag (double-free or leak)
- C-API ownership mismatch (missing sk_ref_sp/.release(), delete on ref-counted)
- same-instance return disposed twice (Subset/ToRasterImage)
- managed retention in SkiaSharp.Views* (handler/control/event with no teardown)
- fixed-pointer lifetime (HarfBuzz Blob family)
Golden rule: every fix is empirically proven red→green (test fails without the fix, passes with it) before anything ships. Never weaken/mute tests, never edit generated files or upstream Skia.
.github/workflows/memory-leak-fixer.md (+ compiled .lock.yml) — a gh-aw workflow that runs the skill on a 12h schedule / manual dispatch and opens a single draft [memory-leak] PR. Automated PRs are scoped to runner-validatable managed-C# fixes (bootstrapped with pre-built natives via externals-download, validated with dotnet test). If the only viable fix needs a native/C-API source rebuild, it files a [memory-leak] issue instead of an unvalidated PR.

Also registers /memory-leak-fixer in AGENTS.md (slash-command + issue-classification tables).

Self-testing dry-run loop

This PR adds a pull_request trigger scoped to the skill/workflow paths:

pull_request:
  paths:
    - ".github/workflows/memory-leak-fixer.md"
    - ".github/workflows/memory-leak-fixer.lock.yml"
    - ".agents/skills/memory-leak-fixer/**"

Any PR that edits the skill or workflow re-runs the entire pipeline in forced dry-run (Step 2.6 guardrail): it does the full scan/prove/fix locally but must not emit any create-pull-request/create-issue safe output — everything goes to the step summary only. That lets us iterate on the prompt and skill and watch real runs safely, without ever opening a spurious PR/issue. gh-aw auto-adds fork-safety (same-repo head only) to the PR trigger.

Notes

Compiles clean: gh aw compile memory-leak-fixer → 0 errors, 0 warnings; actions-lock.json unchanged.
Draft, [memory-leak] title prefix, one-action-per-run cap, agentic-workflows label, read-only base permissions, network restricted to defaults + dotnet feeds.

🤖 Generated with the help of Copilot. Inspired by dotnet/maui#36276.

Adapt the AI leak scanner+fixer idea from dotnet/maui#36276 to SkiaSharp's real leak family: native ownership / disposal correctness rather than managed view retention. - .agents/skills/memory-leak-fixer/SKILL.md: a combined scan+prove+fix methodology grounded in SkiaSharp's ownership model (undisposed SKObject handles, wrong `owns:` flags, C-API ref-count mismatches, same-instance double-dispose, unremoved view/handler subscriptions, `fixed`-pointer lifetime). Every fix must be empirically proven red->green before shipping. - .github/workflows/memory-leak-fixer.md (+ compiled .lock.yml): a gh-aw workflow that runs the skill on a 12h schedule / manual dispatch and opens a single draft `[memory-leak]` PR (or files an issue when the only fix needs a native source rebuild). PRs are scoped to runner-validatable managed-C# fixes. - A `pull_request` trigger scoped to the skill/workflow paths runs the whole pipeline in forced dry-run (no PR/issue emitted) so we can iterate on the prompt and skill and watch real runs safely. - AGENTS.md: register /memory-leak-fixer in the slash-command and issue-classification tables. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-07-03T16:00:05Z

📦 Try the packages from this PR

Warning

Do not run these scripts without first reviewing the code in this PR.

Step 1 — Download the packages

bash / macOS / Linux:

curl -fsSL https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.sh | bash -s -- 4330

PowerShell / Windows:

iex "& { $(irm https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.ps1) } 4330"

Step 2 — Add the local NuGet source

dotnet nuget add source ~/.skiasharp/hives/pr-4330/packages --name skiasharp-pr-4330

More options

Option	Description
`--successful-only` / `-SuccessfulOnly`	Only use successful builds
`--force` / `-Force`	Overwrite previously downloaded packages
`--list` / `-List`	List available artifacts without downloading
`--build-id ID` / `-BuildId ID`	Download from a specific build

Or download manually from Azure Pipelines — look for the nuget artifact on the build for this PR.

Remove the source when you're done:

dotnet nuget remove source skiasharp-pr-4330

First real dry-run (PR #4330) surfaced three prompt issues; fix all three: - Anti-hallucination: state plainly there is NO seeded/planted bug and that finding nothing is the expected, successful outcome. The first run spent 19m / 4.6M tokens convinced a bug must exist and labelled hardened code "decoys". - Quiet/dry runs must emit a single `noop` (not silence), so gh-aw stops logging the run as "incomplete". Update golden rules, guardrails, Phase 5. - Tell the agent the skia submodule is NOT checked out on the runner: scan the managed C# bindings only; don't treat the missing C-API sources as a blocker. - Timebox the scan to one focused pass; avoid open-ended sub-agent explorations. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

- Rename the workflow display name to "Fixer - Memory Leak" (H1 heading, which gh-aw uses as the workflow name). - checkout: submodules: true — pull the skia fork read-only so the scan phase can verify managed `owns:` flags against the real C ref-count contracts in our C shim (externals/skia/src/c + include/c). Mirrors auto-skia-sync's approach (which uses `recursive` because it builds; we only read, so `true` suffices). - Update the skill + workflow notes: the C shim IS now readable, but native builds still use pre-built packages (externals-download) on the runner. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-07-03T17:03:20Z

📖 Documentation Preview

The documentation for this PR has been deployed and is available at:

🔗 View Staging Site
🔗 View Staging Docs
🔗 View Staging Gallery (Blazor)
🔗 View Staging Gallery (Uno Platform)
🔗 View Staging SkiaFiddle

This preview will be updated automatically when you push new commits to this PR.

This comment is automatically updated by the documentation staging workflow.

Expand the skill's scan taxonomy from 6 to 12 families, each backed by a real historical SkiaSharp leak fix (finalizer/collection ordering #3796/#3291, Clone double-free #2904, disposing native statics #1863/#4080/#1224, field-not-nulled #1256/#1344, stream/callback/delegate-proxy lifetime #3589/#2916/#996, allocation-failure #1784/#1642). Broaden Phase 1.3 de-dup to search by api/type name (real leaks are filed as [BUG], not [memory-leak]) and add the Blob.FromStream / PR #3473 worked example. Extend the Phase 3.2 fix table to cover all 12 families. Document two verified, un-filed family-6 candidates surfaced by a model-diverse scan (SKRegion.SpanIterator missing parent ref; SKPixmap.ExtractSubset/With* not propagating pixelSource). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

…s.md Move the 12-family leak taxonomy out of SKILL.md into a dedicated reference file, matching the sibling-skill references/ convention. Each family now has a description, why-it's-bad, a leaking code example, and the idiomatic fix example (e.g. real Blob.FromStream fixed-pointer leak, SKRegion.SpanIterator missing parent ref, dispose-protected singletons). SKILL.md keeps a compact scan cheat-sheet (where-to-look + grep) and points to the reference; it now instructs the agent to read the catalogue before scanning and consult the matching family when fixing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Native Skia (externals/skia/**, including our C shim) is upstream and cannot be validated red->green on a standard runner, so scope the skill and workflow to managed C# (binding/**, source/**) exclusively. - types-of-leaks.md: drop the C-API-mismatch family, renumber the taxonomy 12->11 (families 0-10), and add a per-family "Watch out" anti-pattern to every family. - SKILL.md: remove native/C-API scan+fix scope; rotation modulo % 12 -> % 11; collapse Phase 2 to a single managed-observable flow; slim 3.2 to point at the catalogue; add a 3.4 self-review gate (relocated anti-patterns) that runs before the PR is opened. - workflow: reframe header/description/guardrails to managed-C#-only and drop `submodules: true` from checkout (skia source no longer needed); keep fetch-depth: 0 for PR branch ops + commit-count rotation. - Recompile lock (name "Fixer - Memory Leak" unchanged). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

The reference named the live, un-fixed SKRegion.SpanIterator (and SKPixmap.ExtractSubset) with exact file locations and fixes, and the SKILL.md cheat-sheet listed them as `live:` targets. The last dry-run "found" SpanIterator by copying that answer key, not by investigating — which doesn't validate the scan as a capability. Replace family 5's worked example with a generic/synthetic parent-cursor before/after, add a detection heuristic (compare sibling wrappers; the one missing a parent field is the suspect), and delete the explicit "un-filed examples this workflow surfaced" list. Every other family only cites already-fixed PRs, which are de-dup-safe teaching references. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

…ger prose - SKILL.md §1.1: replace the recent-files/random seed picker with a time-based round-robin (day-of-year + hour, base-10 guarded) so consecutive runs cover different families identically locally and in CI; stop re-listing families and point at the types catalogue as the single source of truth. - types-of-leaks.md: add a per-family "Where to look" line (managed-C# path + grep) so the family list lives in exactly one place. - workflow .md Step 1 + Guardrail 6: gate mode/dry-run on ${{ github.event_name }} (which always resolves) instead of unreliable input-echo tokens; describe the empty issue_number case without emitting a literal template that fails validation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

…aSharp repo) The workflow already runs as "Fixer - Memory Leak"; strip the leftover "— SkiaSharp" suffix from the banner comment and the SKILL.md H1, and replace gratuitous possessives ("SkiaSharp's managed C#", "code SkiaSharp owns") with repo-relative phrasing. Real paths, the mono/SkiaSharp slug, the SkiaSharp NuGet package name, and substantive framing are kept. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

…NNN) For tracking, a confirmed managed-C#-fixable leak now emits a linked pair instead of a single output: a [memory-leak] issue (the finding — family, retention path, Phase-2 evidence) plus a draft fix PR (the code + red→green test) whose body ends with "Fixes #<temporary_id>". gh-aw's create_pull_request resolves the temporary id to the real issue number in the same run, so merging the PR auto-closes the issue. - SKILL.md Phase 4 rewritten into 4.1 (issue/finding + temporary_id), 4.2 (PR/fix with Fixes #<temp-id>; PR-only + real number in fix-a-known-leak mode), 4.3 (native-only → issue alone). Mode table + Phase 5 + self-review gate updated. - workflow: Guardrail 1 now emits the issue+PR pair; create-issue gains title-prefix "[memory-leak] " so both issues and PRs share the de-dup marker; intro/report/dry-run prose updated. Dry runs still emit only a noop. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

…w validator) The gh-aw create_issue validator enforces /^aw_[A-Za-z0-9]{3,8}$/ (3-8 alphanumeric, no underscore). SKILL.md incorrectly said 3-12 letters/digits/ underscores, which could lead the agent to emit an invalid temporary_id and have create_issue reject it. The example aw_leak1 was already valid. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Simplify to a single behaviour: every run does the full scan → prove → fix → file (issue + linked fix PR) pipeline. The only knob is dry_run (do everything but open nothing; forced on pull_request self-tests). Removed the workflow_dispatch `issue_number` input and the "fix a known/reported leak" mode branch from the workflow prose, guardrails, report section, and the skill's mode table / Phase 4.2 / Phase 5. `temporary_id` stays — it's what links the newly filed finding issue to the newly opened fix PR (Fixes #<temporary_id>) in the always-on pairing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

…ready (#4361) [skills] Add performance-fixer skill + make the benchmark harness AI-ready (#4361) Context: dotnet/aspnetcore optimize-aspnetcore-performance (reference architecture) Related: #4241 (SKMatrix native→managed, up to 24×), #4345 (SKColor alloc-free parse), #4247 (SKSurface.Canvas caching), #4362 (perf/* label taxonomy), #4330 (memory-leak-fixer) SkiaSharp is a thin managed wrapper over native Skia, so its recurring, high-impact performance family is the tax the C# layer adds between the caller and Skia — a P/Invoke transition paid for math that is a few float ops, an allocation on a hot parse/convert path, a native lookup redone on every getter, or per-element marshalling in a loop. We have merged several such wins by hand (#4241, #4345, #4247); this adds an autonomous scanner so more of them surface as code lands, without a human hunting for them. Add a `performance-fixer` skill (sibling to `memory-leak-fixer`) plus a scheduled gh-aw workflow that scans the managed binding for one hot-path opportunity, proves it, fixes it (managed-only, ABI-safe), and files a linked issue + draft PR. The defining discipline is two proofs, which a generic perf pass lacks: every fix must be shown FASTER (a BenchmarkDotNet New-vs-Old measurement) AND behaviour-IDENTICAL (an equivalence test — bit-exact vs SkiaApi.sk_* for numeric ports, across edge inputs, and mutation-checked to prove it catches divergence). A faster answer that differs from Skia is a rendering regression, not a win, so it is rejected. The skill also encodes the traps we learned the hard way: the x86/x87 float-determinism native fallback (#4241), the ARM64 Vector256 regression, statistical benchmark rigor, and full behaviour parity including exceptions, ownership/GC.KeepAlive lifetime, and rendered pixels. ~~ Reference structure ~~ References are split by area (mirroring the aspnetcore skill): references/hot-paths/* (SkiaSharp code areas — geometry-math, color, handles-and-collections, text-and-fonts, pixels-and-images) and references/bcl-patterns/* (the .NET techniques), plus decision-framework.md (impact×complexity rubric + the two-proof gate), measuring.md, signals.md (detection routing), and repo-helpers.md. A lean SKILL.md routes between them. ~~ Benchmark harness readiness ~~ So a scan never wastes time repairing infra: replace TheBenchmark.cs — a trap whose [SimpleJob(Net48/Net70/Net80)] attributes silently skip off-Windows and whose empty bodies taught none of the conventions — with a working TemplateBenchmark.cs (New-vs-Old + [MemoryDiagnoser] + [Params] + return-sink that runs out of the box), and add benchmarks/README.md documenting the add/run commands, --list flat / --job short for fast iteration, the single-TFM rule, and the InternalsVisibleTo native-oracle pattern. ~~ Labels ~~ Findings are labelled tenet/performance plus one agent-chosen perf/* sub-type from the shared taxonomy in .agents/skills/issue-triage/references/labels.md (#4362), matching the memory-leak-fixer convention; the sub-type is decided by the win's dominant measured driver (a removed P/Invoke → perf/interop, removed allocations → perf/allocations, ...). ~~ Validated end to end ~~ The workflow self-tests on any PR touching the skill via a forced dry-run. Those runs correctly rejected a non-finding (SKFourByteTag's char[4] is already stack-allocated by .NET 9/10 escape analysis) and found real, fully-proven wins (SKShaper glyph-array extraction → perf/allocations; SKColor→SKColorF operator → perf/interop). A real dispatch run created a correctly-labelled issue (#4369) and draft PR (#4370), confirming the tenet/performance + perf/* labels land on both. No binding/runtime code changes: a new skill, a new workflow, benchmark docs/template, and two rows in AGENTS.md. Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-project-automation Bot added this to SkiaSharp Backlog Jul 3, 2026

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 16:00 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 16:20 — with GitHub Actions Inactive

github-actions Bot mentioned this pull request Jul 3, 2026

[aw] Memory Leak Fixer — SkiaSharp failed #4331

Closed

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 16:25 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 16:26 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 16:55 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 16:56 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 17:12 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 17:25 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 17:51 — with GitHub Actions Inactive

mattleibow had a problem deploying to gh-aw-agents July 3, 2026 17:51 — with GitHub Actions Error

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 18:08 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 18:30 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 18:31 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 19:04 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 19:05 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 19:55 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 21:28 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 21:29 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 21:49 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 23:17 — with GitHub Actions Inactive

mattleibow had a problem deploying to gh-aw-agents July 3, 2026 23:17 — with GitHub Actions Error

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 23:25 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 23:26 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 23:49 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 23:52 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 3, 2026 23:53 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 4, 2026 00:13 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 4, 2026 00:45 — with GitHub Actions Inactive

mattleibow temporarily deployed to gh-aw-agents July 4, 2026 00:55 — with GitHub Actions Inactive

mattleibow marked this pull request as ready for review July 6, 2026 23:02

mattleibow merged commit 63596a1 into main Jul 6, 2026
11 of 14 checks passed

mattleibow deleted the mattleibow-memory-leak-fixer-workflow branch July 6, 2026 23:02

github-project-automation Bot moved this to Done in SkiaSharp Backlog Jul 6, 2026

mattleibow added this to the 4.151.0-preview.2 milestone Jul 22, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add memory-leak-fixer skill + agentic workflow (self-testing dry-run PR trigger)#4330

Add memory-leak-fixer skill + agentic workflow (self-testing dry-run PR trigger)#4330
mattleibow merged 13 commits into
mainfrom
mattleibow-memory-leak-fixer-workflow

mattleibow commented Jul 3, 2026

Uh oh!

github-actions Bot commented Jul 3, 2026

Uh oh!

github-actions Bot commented Jul 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

mattleibow commented Jul 3, 2026

What

Self-testing dry-run loop

Notes

Uh oh!

github-actions Bot commented Jul 3, 2026

📦 Try the packages from this PR

Uh oh!

github-actions Bot commented Jul 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant