Skip to content

Commit ddfc89c

Browse files
mattleibowmattleibow
committed
ai-triage: classify #3427 
1 parent 6488b96 commit ddfc89c

File tree

1 file changed

+130
-0
lines changed

1 file changed

+130
-0
lines changed

‎repos/mono-SkiaSharp/ai-triage/3427.json‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
{
2+
"meta": {
3+
"schemaVersion": "1.0",
4+
"number": 3427,
5+
"repo": "mono/SkiaSharp",
6+
"analyzedAt": "2026-02-13T00:25:00Z",
7+
"currentLabels": ["community ✨"]
8+
},
9+
"summary": "Community PR that fixes a Blazor disposal race condition (#3322). When SKCanvasView is disposed in Blazor, DOM elements may already have been removed before Dispose() runs, causing ResizeObserver.unobserve() to throw TypeError ('parameter 1 is not of type Element'). The fix adds null-guard early returns in SizeWatcher.observe(), SizeWatcher.unobserve(), and removes a misleading console.error in SKHtmlCanvas.init() — all in JavaScript. This is a well-known Blazor lifecycle issue (dotnet/aspnetcore#45777).",
10+
"classification": {
11+
"type": { "value": "type/bug", "confidence": 0.95 },
12+
"area": { "value": "area/SkiaSharp.Views.Blazor", "confidence": 0.98 },
13+
"platforms": ["os/WASM"],
14+
"tenets": ["tenet/reliability"]
15+
},
16+
"evidence": {
17+
"bugSignals": {
18+
"severity": "medium",
19+
"isRegression": false,
20+
"errorType": "JSException (TypeError)",
21+
"errorMessage": "TypeError: Failed to execute 'unobserve' on 'ResizeObserver': parameter 1 is not of type 'Element'.",
22+
"stackTrace": "Microsoft.JSInterop.JSException: TypeError: Failed to execute 'unobserve' on 'ResizeObserver': parameter 1 is not of type 'Element'.\n at Microsoft.JSInterop.WebAssembly.WebAssemblyJSRuntime.InvokeJS(...)\n at SkiaSharp.Views.Blazor.Internal.JSModuleInterop.Invoke(...)\n at SkiaSharp.Views.Blazor.Internal.SizeWatcherInterop.Stop()\n at SkiaSharp.Views.Blazor.Internal.SizeWatcherInterop.OnDisposingModule()\n at SkiaSharp.Views.Blazor.Internal.JSModuleInterop.Dispose()\n at SkiaSharp.Views.Blazor.SKCanvasView.Dispose()",
23+
"reproQuality": "partial",
24+
"targetFrameworks": ["net8.0-browser"]
25+
},
26+
"reproEvidence": {
27+
"stepsToReproduce": [
28+
"Create a Blazor WASM application with multiple SKCanvasView components",
29+
"Rapidly show and hide (add/remove from DOM) multiple SKCanvasView instances via conditional rendering",
30+
"Observe unhandled exception from Dispose() when the component is removed"
31+
],
32+
"screenshots": [
33+
{ "url": "https://github.com/user-attachments/assets/59b9940f-dd7e-4763-bd78-a45bf2b73f99", "description": "Console error message during disposal" }
34+
],
35+
"environmentDetails": "SkiaSharp 3.116.0, .NET 8.0 Blazor WASM, Visual Studio 2022 on Windows",
36+
"relatedIssues": [3322, 2441]
37+
},
38+
"versionAnalysis": {
39+
"mentionedVersions": ["3.116.0", "2.88.3"],
40+
"currentRelevance": "likely",
41+
"relevanceReason": "The SizeWatcher.ts unobserve() method still lacks null guards in the current main branch. Issue #2441 reported the same class of bug in 2.88.3, and #3322 reports it in 3.116.0 — the root cause has never been fixed."
42+
},
43+
"fixStatus": {
44+
"likelyFixed": false,
45+
"confidence": 0.95,
46+
"reason": "The current SizeWatcher.ts and SizeWatcher.js on main still lack null guards in both observe() and unobserve(). This PR provides the fix but has not been merged.",
47+
"relatedPRs": [3427]
48+
}
49+
},
50+
"analysis": {
51+
"summary": "Blazor components can have their DOM elements removed before Dispose() is invoked — a well-documented Blazor lifecycle limitation (dotnet/aspnetcore#45777). When SKCanvasView.Dispose() is called, it triggers SizeWatcherInterop.Stop() which calls SizeWatcher.unobserve() in JavaScript. If the element has already been removed from the DOM, the elements Map returns undefined, and ResizeObserver.unobserve(undefined) throws TypeError. Similarly, SizeWatcher.observe() can fail if the element was removed between the async ImportAsync and the Start() call. The PR correctly adds null guards at the JS level and removes a misleading console.error in SKHtmlCanvas.init() that logged 'No canvas element was provided' for this normal disposal scenario.",
52+
"rationale": "This is clearly a bug — an unhandled exception thrown during normal component disposal. It's in the Blazor views package (area/SkiaSharp.Views.Blazor) and only affects WASM. Severity is medium: the exception is unhandled and logged as critical by Blazor's renderer, but it doesn't crash the application or lose data. It's not a regression in the traditional sense — the code never handled this Blazor lifecycle edge case. The PR's approach (JS-side null guards) is the correct fix pattern for this class of Blazor disposal issue.",
53+
"keySignals": [
54+
{ "text": "TypeError: Failed to execute 'unobserve' on 'ResizeObserver': parameter 1 is not of type 'Element'", "source": "issue #3322 body", "interpretation": "ResizeObserver.unobserve() was called with undefined — the element was already removed from the DOM before disposal." },
55+
{ "text": "In Blazor, elements can be removed from the DOM before a component's Dispose() method is invoked", "source": "PR #3427 body", "interpretation": "This is a known Blazor platform limitation documented at dotnet/aspnetcore#45777. The fix must be defensive null-checking in JS." },
56+
{ "text": "#2441 — same Blazor disposal exception from v2.88.3", "source": "similar-issues-ai bot on #3322", "interpretation": "This is a long-standing bug reported at least twice. The root cause was never addressed." },
57+
{ "text": "SizeWatcher.unobserve() calls observer.unobserve(element) without null-checking the Map.get() result", "source": "code search: SizeWatcher.ts:44-45", "interpretation": "Direct cause — Map.get() returns undefined for removed elements, causing the TypeError." }
58+
],
59+
"codeInvestigation": [
60+
{ "file": "source/SkiaSharp.Views/SkiaSharp.Views.Blazor/wwwroot/SizeWatcher.ts", "lines": "37-45", "finding": "SizeWatcher.unobserve() calls this.elements.get(elementId) and passes result directly to ResizeObserver.unobserve() without null check. If element was removed from DOM and the map entry is missing/undefined, this throws TypeError.", "relevance": "direct" },
61+
{ "file": "source/SkiaSharp.Views/SkiaSharp.Views.Blazor/wwwroot/SizeWatcher.ts", "lines": "16-34", "finding": "SizeWatcher.observe() resolves element via querySelector but doesn't null-check the result before casting to SizeWatcherElement and setting properties on it. Would throw if element is already gone.", "relevance": "direct" },
62+
{ "file": "source/SkiaSharp.Views/SkiaSharp.Views.Blazor/wwwroot/SKHtmlCanvas.ts", "lines": "47-53", "finding": "SKHtmlCanvas.init() logs console.error('No canvas element was provided.') when element is null and returns null. The error log is misleading during normal Blazor disposal — the element was provided but already removed from DOM.", "relevance": "direct" },
63+
{ "file": "source/SkiaSharp.Views/SkiaSharp.Views.Blazor/Internal/SizeWatcherInterop.cs", "lines": "47-48", "finding": "OnDisposingModule() calls Stop(), which calls the JS unobserve. The C# side correctly uses ?. null-conditional on callbackReference but the JS side lacks equivalent guards.", "relevance": "related" },
64+
{ "file": "source/SkiaSharp.Views/SkiaSharp.Views.Blazor/Internal/JSModuleInterop.cs", "lines": "37-41", "finding": "Dispose() calls OnDisposingModule() then Module.Dispose(). If OnDisposingModule throws (due to JS TypeError), Module.Dispose() is never called — potential resource leak.", "relevance": "related" },
65+
{ "file": "source/SkiaSharp.Views/SkiaSharp.Views.Blazor/SKCanvasView.razor.cs", "lines": "171-178", "finding": "SKCanvasView.Dispose() uses ?. null-conditional operators for sizeWatcher/interop/dpiWatcher, which handles the case where OnAfterRenderAsync never completed. This is correct defensive coding on the C# side.", "relevance": "context" },
66+
{ "file": "source/SkiaSharp.Views/SkiaSharp.Views.Blazor/SKGLView.razor.cs", "lines": "190-195", "finding": "SKGLView.Dispose() does NOT use ?. null-conditional — it would throw NullReferenceException if OnAfterRenderAsync never completed. This is a separate but related bug.", "relevance": "related" }
67+
],
68+
"errorFingerprint": "JSException-TypeError-ResizeObserver-unobserve-Blazor",
69+
"workarounds": [
70+
"Wrap SKCanvasView usage in a try-catch in the parent component's Dispose method to suppress the exception",
71+
"Avoid rapidly showing/hiding multiple SKCanvasView instances — use CSS visibility instead of conditional rendering to keep elements in the DOM"
72+
],
73+
"nextQuestions": [
74+
"Should SKGLView.Dispose() also use ?. null-conditional operators like SKCanvasView does (potential NullReferenceException)?",
75+
"Should JSModuleInterop.Dispose() wrap OnDisposingModule() in try-catch to ensure Module.Dispose() always runs?",
76+
"Are there similar null-guard issues in DpiWatcher.js?"
77+
],
78+
"resolution": {
79+
"hypothesis": "Blazor's component lifecycle allows DOM elements to be removed before Dispose() is called. The SizeWatcher JS code assumes elements are still in the DOM during unobserve(), causing TypeError when they're not. The PR's null-guard approach is the standard fix for this Blazor lifecycle issue.",
80+
"proposals": [
81+
{ "title": "Merge PR #3427 with review", "description": "The PR adds correct null guards in SizeWatcher.observe(), SizeWatcher.unobserve(), and removes the misleading console.error in SKHtmlCanvas.init(). Changes are minimal, JS-only, and follow the standard pattern for handling Blazor disposal races. The PR modifies both .ts (source) and .js (compiled) files consistently. Needs rebase on main and review of the unchecked PR checklist items.", "confidence": 0.85, "effort": "trivial" },
82+
{ "title": "Additional hardening in JSModuleInterop.Dispose()", "description": "Wrap OnDisposingModule() call in try-catch within JSModuleInterop.Dispose() to ensure Module.Dispose() always runs even if the JS interop call fails. This provides defense-in-depth for any future JS disposal errors.", "codeSnippet": "public void Dispose()\n{\n try { OnDisposingModule(); } catch { }\n Module.Dispose();\n}", "confidence": 0.70, "effort": "trivial" },
83+
{ "title": "Fix SKGLView.Dispose() null-conditional operators", "description": "SKGLView.Dispose() calls dpiWatcher.Unsubscribe(), sizeWatcher.Dispose(), interop.Dispose() without ?. null-conditional operators, unlike SKCanvasView.Dispose(). This would throw NullReferenceException if OnAfterRenderAsync never completed. Should be fixed as part of this PR or a follow-up.", "codeSnippet": "public void Dispose()\n{\n dpiWatcher?.Unsubscribe(OnDpiChanged);\n sizeWatcher?.Dispose();\n interop?.Dispose();\n}", "confidence": 0.80, "effort": "trivial" }
84+
],
85+
"recommendedProposal": "Merge PR #3427 with review",
86+
"recommendedReason": "The PR directly fixes the reported bug with minimal, correct changes. The additional hardening proposals can be follow-up items or requested as part of the PR review."
87+
}
88+
},
89+
"output": {
90+
"actionability": {
91+
"suggestedAction": "needs-investigation",
92+
"confidence": 0.85,
93+
"reason": "Community PR with correct approach. Needs code review, rebase on main, and consideration of additional hardening (SKGLView null-safety, JSModuleInterop try-catch). The fix is sound but the PR checklist shows 'Rebased on top of main' is unchecked."
94+
},
95+
"missingInfo": [
96+
"PR needs rebase on top of current main branch",
97+
"PR checklist item 'Updated documentation' is unchecked — confirm if docs changes are needed"
98+
],
99+
"actions": [
100+
{
101+
"type": "update-labels",
102+
"description": "Apply bug, Blazor views, WASM, and reliability labels",
103+
"risk": "low",
104+
"confidence": 0.95,
105+
"labels": ["type/bug", "area/SkiaSharp.Views.Blazor", "os/WASM", "tenet/reliability"]
106+
},
107+
{
108+
"type": "link-related",
109+
"description": "Cross-reference with original bug report #3322",
110+
"risk": "low",
111+
"confidence": 0.95,
112+
"linkedIssue": 3322
113+
},
114+
{
115+
"type": "link-related",
116+
"description": "Cross-reference with older duplicate #2441 (same class of Blazor disposal bug)",
117+
"risk": "low",
118+
"confidence": 0.80,
119+
"linkedIssue": 2441
120+
},
121+
{
122+
"type": "add-comment",
123+
"description": "Post review feedback acknowledging the fix and suggesting additional hardening",
124+
"risk": "high",
125+
"confidence": 0.80,
126+
"comment": "Thanks for the fix — this is a well-known Blazor lifecycle issue and your approach of adding null guards in the JS layer is the right pattern.\n\nThe changes in `SizeWatcher.ts`/`SizeWatcher.js` and `SKHtmlCanvas.ts`/`SKHtmlCanvas.js` look correct. A couple of observations for consideration:\n\n1. `SKGLView.razor.cs` has the same disposal vulnerability — it calls `dpiWatcher.Unsubscribe()`, `sizeWatcher.Dispose()`, and `interop.Dispose()` without `?.` null-conditional operators (unlike `SKCanvasView.razor.cs` which already uses them). Would you be able to add that fix to this PR as well?\n\n2. `JSModuleInterop.Dispose()` could benefit from a try-catch around `OnDisposingModule()` to ensure `Module.Dispose()` always runs even if the JS interop call throws.\n\nWould you be able to rebase on top of the current `main` branch?"
127+
}
128+
]
129+
}
130+
}

0 commit comments

Comments
 (0)