Skip to content

Commit a3ddafa

Browse files
committed
ai-triage: classify #3431
1 parent a3e531b commit a3ddafa

File tree

1 file changed

+105
-0
lines changed

1 file changed

+105
-0
lines changed
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
{
2+
"meta": {
3+
"schemaVersion": "1.0",
4+
"number": 3431,
5+
"repo": "mono/SkiaSharp",
6+
"analyzedAt": "2026-02-13T00:24:40Z",
7+
"currentLabels": ["community ✨"]
8+
},
9+
"summary": "PR fixes NullReferenceException in Uno Platform SKXamlCanvas by separating pixel buffer and bitmap disposal — fixes #3430",
10+
"classification": {
11+
"type": { "value": "type/bug", "confidence": 0.97 },
12+
"area": { "value": "area/SkiaSharp.Views.Uno", "confidence": 0.99 },
13+
"platforms": ["os/Windows-WinUI", "os/WASM", "os/Linux", "os/macOS"],
14+
"tenets": ["tenet/reliability"],
15+
"partner": "partner/unoplatform"
16+
},
17+
"evidence": {
18+
"bugSignals": {
19+
"severity": "high",
20+
"isRegression": true,
21+
"errorType": "crash",
22+
"errorMessage": "NullReferenceException when canvas resizes — bitmap field nulled by FreeBitmap() called from pixel reallocation path",
23+
"reproQuality": "complete",
24+
"targetFrameworks": ["net8.0"]
25+
},
26+
"reproEvidence": {
27+
"stepsToReproduce": [
28+
"Use SKXamlCanvas in an Uno Platform Skia-based app",
29+
"Resize the canvas so that bitmap dimensions change but pixel dimensions also change",
30+
"FreeBitmap() at line 106 nulls the bitmap that was just recreated at line 91",
31+
"bitmap.PixelBuffer access at line 72 throws NullReferenceException"
32+
],
33+
"environmentDetails": "Uno Platform Skia backend (Desktop/WASM), SkiaSharp 3.116.0+",
34+
"relatedIssues": [3430]
35+
},
36+
"versionAnalysis": {
37+
"mentionedVersions": ["3.116.0"],
38+
"workedIn": "2.88.9",
39+
"brokeIn": "3.116.0",
40+
"currentRelevance": "likely",
41+
"relevanceReason": "The bug still exists in main — CreateBitmap() still has the coupled FreeBitmap() calls that null both pixels and bitmap together."
42+
},
43+
"regression": {
44+
"isRegression": true,
45+
"confidence": 0.85,
46+
"reason": "Reporter states 2.88.9 was the last known good version. The Uno Skia canvas was rewritten for the v3 line with separate pixel/bitmap management but coupled disposal.",
47+
"workedInVersion": "2.88.9",
48+
"brokeInVersion": "3.116.0"
49+
}
50+
},
51+
"analysis": {
52+
"summary": "PR #3431 correctly fixes the root cause of #3430 by decoupling pixel buffer (GCHandle-pinned byte[]) and WriteableBitmap lifecycle management. The original FreeBitmap() method freed both resources together, but CreateBitmap() has two independent conditions that each called it — bitmap size mismatch and pixel size mismatch — causing cross-contamination on resize.",
53+
"rationale": "This is a community PR fixing a confirmed type/bug in area/SkiaSharp.Views.Uno. The Skia backend of Uno's SKXamlCanvas uses a separate GCHandle-pinned byte array (unlike WinUI which gets pixels directly from bitmap.GetPixels()), so pixel and bitmap lifetimes must be independent. The fix is minimal and correct: split FreeBitmap() into FreePixels() and FreeBitmap(), reorder allocation so pixels are allocated first, and update DoUnloaded() to free both. CI is currently failing.",
54+
"keySignals": [
55+
{ "text": "FreeBitmap() nulls both pixels and bitmap, but called from two independent conditions", "source": "SKXamlCanvas.Skia.cs lines 87 and 106", "interpretation": "Root cause: coupled disposal in a split-condition method causes cross-resource destruction on resize." },
56+
{ "text": "WinUI version uses bitmap.GetPixels() — single lifecycle, no separate pixel buffer", "source": "SKXamlCanvas.cs (WinUI) line 240", "interpretation": "Explains why WinUI doesn't have this bug — pixels are owned by the bitmap." },
57+
{ "text": "DoUnloaded() only called FreeBitmap() — leaked GCHandle-pinned pixel buffer", "source": "SKXamlCanvas.Skia.cs line 34-35", "interpretation": "Secondary bug: unloading leaked pinned memory. PR fixes this by calling both FreeBitmap() and FreePixels()." },
58+
{ "text": "CI status: failure — #3.119.2-pr.3431.1 failed", "source": "Azure Pipelines status check", "interpretation": "PR needs CI investigation before merge. May be flaky or unrelated failure." },
59+
{ "text": "Wasm variant has only pixels (no bitmap field) — not affected by this specific bug", "source": "SKXamlCanvas.Wasm.cs lines 26-29, 78-86", "interpretation": "Wasm canvas correctly manages only pixel buffers. The bug is specific to the Skia backend which has both bitmap and pixels." }
60+
],
61+
"codeInvestigation": [
62+
{ "file": "source/SkiaSharp.Views.Uno/SkiaSharp.Views.Uno.WinUI.Skia/SKXamlCanvas.Skia.cs", "lines": "83-127", "finding": "CreateBitmap() calls FreeBitmap() at two points: line 87 (bitmap size mismatch) and line 106 (pixel size mismatch). FreeBitmap() nulls BOTH pixels and bitmap. On resize, the second call destroys the bitmap that was just recreated between lines 89-101, causing NRE at line 72 (bitmap.PixelBuffer).", "relevance": "direct" },
63+
{ "file": "source/SkiaSharp.Views.Uno/SkiaSharp.Views.Uno.WinUI.Skia/SKXamlCanvas.Skia.cs", "lines": "34-35", "finding": "DoUnloaded() only calls FreeBitmap() which frees pixels via GCHandle.Free() and nulls bitmap, but if pixels is already null (e.g. never allocated), the GCHandle isn't freed. Also, the PR correctly adds FreePixels() call.", "relevance": "direct" },
64+
{ "file": "source/SkiaSharp.Views/SkiaSharp.Views.WinUI/SKXamlCanvas.cs", "lines": "230-277", "finding": "WinUI version gets pixels from bitmap.GetPixels() — single lifecycle. FreeBitmap() just nulls bitmap/brush/pixels pointer. No GCHandle, no separate allocation. This is why the bug doesn't exist on WinUI.", "relevance": "related" },
65+
{ "file": "source/SkiaSharp.Views.Uno/SkiaSharp.Views.Uno.WinUI.Wasm/SKXamlCanvas.Wasm.cs", "lines": "73-100", "finding": "Wasm version has no bitmap field — only manages pixels/pixelsHandle. FreeBitmap() frees the GCHandle and clears the HTML canvas. No coupled disposal issue.", "relevance": "related" }
66+
],
67+
"nextQuestions": [
68+
"Why is CI failing? Is it a test failure related to this change or a pre-existing/flaky issue?",
69+
"Should the PR include a unit test that verifies resize doesn't throw?",
70+
"Does SKSwapChainPanel.Skia have a similar coupled-disposal pattern?"
71+
],
72+
"resolution": {
73+
"hypothesis": "The Uno Skia SKXamlCanvas manages pixels (GCHandle-pinned byte[]) and bitmap (WriteableBitmap) independently, but FreeBitmap() freed both together. CreateBitmap() has two separate conditions calling FreeBitmap() — one for bitmap size mismatch and one for pixel size mismatch. On resize, the pixel-reallocation path destroys the just-recreated bitmap.",
74+
"proposals": [
75+
{ "title": "Merge PR #3431", "description": "The PR correctly splits FreeBitmap() into FreePixels() and FreeBitmap(), reorders allocation (pixels first, then bitmap), and updates DoUnloaded() to call both. Fix is minimal and surgical.", "confidence": 0.90, "effort": "trivial" },
76+
{ "title": "Add resize test", "description": "Add a unit test that creates an SKXamlCanvas, triggers a size change, and verifies no NRE. Currently no test coverage for Uno Skia canvas resize.", "confidence": 0.85, "effort": "small" }
77+
],
78+
"recommendedProposal": "Merge PR #3431",
79+
"recommendedReason": "The fix is correct, minimal, and matches the analysis from the #3430 triage. It also fixes a secondary GCHandle leak in DoUnloaded(). CI failure needs investigation first."
80+
}
81+
},
82+
"output": {
83+
"actionability": {
84+
"suggestedAction": "keep-open",
85+
"confidence": 0.92,
86+
"reason": "PR is a correct fix for #3430. CI failure needs investigation before merge. The code change is reviewed and looks correct."
87+
},
88+
"actions": [
89+
{
90+
"type": "update-labels",
91+
"description": "Apply bug, Uno views area, platform, reliability, and partner labels",
92+
"risk": "low",
93+
"confidence": 0.95,
94+
"labels": ["type/bug", "area/SkiaSharp.Views.Uno", "os/Windows-WinUI", "os/WASM", "os/Linux", "os/macOS", "tenet/reliability", "partner/unoplatform"]
95+
},
96+
{
97+
"type": "link-related",
98+
"description": "Cross-reference the bug this PR fixes",
99+
"risk": "low",
100+
"confidence": 0.99,
101+
"linkedIssue": 3430
102+
}
103+
]
104+
}
105+
}

0 commit comments

Comments
 (0)