[leak-fix] Fix GradientBrush.GradientStops memory leak (Fixes #36363)

[github-actions]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Note

🔍 AI-generated PR. This fix and its regression tests were produced by the Memory Leak Fixer workflow. Please review carefully before merging.

Fixes #36363

The leak

GradientBrush.UpdateGradientStops subscribed the brush directly to GradientStopCollection.CollectionChanged and to PropertyChanged on every contained GradientStop. A shared or long-lived non-empty collection could therefore retain transient brushes through both the collection-level and per-stop event handlers.

The fix

Route the collection subscription through WeakNotifyCollectionChangedProxy and each stop subscription through WeakNotifyPropertyChangedProxy. A private GradientStopSubscriptions helper owns the collection proxy and per-stop proxies while the brush is alive. It removes subscriptions when collections or stops change, handles reset/reentrant replacement safely, and unsubscribes remaining proxies from its finalizer.

The handler delegates remain strongly cached by the live brush so weak forwarding continues after GC. The public GradientBrush type remains finalizer-free and there are no public API additions.

Regression tests

GradientBrushMemoryTests covers:

• GradientBrushDoesNotLeakWhenSharingGradientStops with a shared non-empty collection
• GradientStopChangesStillInvalidateAfterGc while the brush remains alive
• ClearingGradientStopsAllowsReentrantReplacement during reset cleanup
• ClearingAndReusingGradientStopsKeepsNewStopsSubscribed after clearing and reusing the same collection

The review gate verified the focused tests red without the fix and green with the fix, and the Brush UI category completed successfully.

Scope

Managed cross-platform change in src/Controls/src/Core/GradientBrush.cs. No existing test was muted, skipped, or weakened.

Generated by Memory Leak Fixer

[MauiBot]

Expert Review — 1 findings

See inline comments for details.

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36521

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36521"

[kubaflo]

🔍 AI-generated update (GitHub Copilot CLI, on behalf of @kubaflo)

@MauiBot Addressed the non-empty GradientStops leak finding in 16141ad53ff: collection and per-stop subscriptions are weak, cleanup is privately owned, and regression coverage now includes post-GC notifications and reset reentrancy. Ready for re-review.

[kubaflo]

🔍 AI-generated update (GitHub Copilot CLI, on behalf of @kubaflo)

@MauiBot Updated the PR description to match 16141ad53ff: it now documents both weak subscription paths, the private helper finalizer, all three regression tests, and the unchanged public API surface. The [leak-fix] title is retained intentionally for workflow grouping and already names GradientBrush.GradientStops explicitly. Ready for re-review.

[kubaflo]

🔍 AI-generated response (GitHub Copilot CLI, on behalf of @kubaflo)

@MauiBot The latest review found no current code issues. The live description already documents both weak subscription paths, the private GradientStopSubscriptions finalizer, all three regression tests, and no public API change at 16141ad53ff. No no-op commit or duplicate trigger was posted.

[MauiBot]

AI Review Summary

@github-actions[bot] — new AI review results are available based on this last commit: 16141ad. To request a fresh review after new comments or commits, comment /review rerun.

---

🗂️ Review Sessions — click to expand

🚦 Gate — Test Before & After Fix

Gate Result: ✅ PASSED

Platform: ANDROID · Base: main · Merge base: 0395a53b

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
🧪 GradientBrushMemoryTests GradientBrushMemoryTests	✅ FAIL — 159s	✅ PASS — 128s

🔴 Without fix — 🧪 GradientBrushMemoryTests: FAIL ✅ · 159s

  Determining projects to restore...
  Restored /home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj (in 9.42 sec).
  Restored /home/vsts/work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 10.73 sec).
  Restored /home/vsts/work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 756 ms).
  Restored /home/vsts/work/1/s/src/TestUtils/src/TestUtils/TestUtils.csproj (in 10 ms).
  Restored /home/vsts/work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 30 ms).
  Restored /home/vsts/work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 101 ms).
  Restored /home/vsts/work/1/s/src/Essentials/src/Essentials.csproj (in 38 ms).
  Restored /home/vsts/work/1/s/src/Core/maps/src/Maps.csproj (in 41 ms).
  Restored /home/vsts/work/1/s/src/Core/src/Core.csproj (in 131 ms).
  1 of 10 projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0/Microsoft.Maui.Maps.dll
  Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0/Microsoft.Maui.Controls.Xaml.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0/Microsoft.Maui.Controls.Maps.dll
  TestUtils -> /home/vsts/work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
  Controls.Core.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.34]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.51]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.53]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:04.43]       LinearGradientBrush should not be alive!
[xUnit.net 00:00:04.43]     GradientBrushDoesNotLeakWhenSharingGradientStops [FAIL]
[xUnit.net 00:00:04.43]       Stack Trace:
[xUnit.net 00:00:04.43]         /_/src/Controls/tests/Core.UnitTests/GradientBrushMemoryTests.cs(25,0): at Microsoft.Maui.Controls.Core.UnitTests.GradientBrushMemoryTests.GradientBrushDoesNotLeakWhenSharingGradientStops()
[xUnit.net 00:00:04.43]         --- End of stack trace from previous location ---
  Failed GradientBrushDoesNotLeakWhenSharingGradientStops [1 s]
  Error Message:
   LinearGradientBrush should not be alive!
  Stack Trace:
     at Microsoft.Maui.Controls.Core.UnitTests.GradientBrushMemoryTests.GradientBrushDoesNotLeakWhenSharingGradientStops() in /_/src/Controls/tests/Core.UnitTests/GradientBrushMemoryTests.cs:line 25
--- End of stack trace from previous location ---
[xUnit.net 00:00:04.53]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed GradientStopChangesStillInvalidateAfterGc [52 ms]
  Passed ClearingGradientStopsAllowsReentrantReplacement [10 ms]

Test Run Failed.
Total tests: 3
     Passed: 2
     Failed: 1
 Total time: 5.2414 Seconds

🟢 With fix — 🧪 GradientBrushMemoryTests: PASS ✅ · 128s

  Determining projects to restore...
  All projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0/Microsoft.Maui.Maps.dll
  Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0/Microsoft.Maui.Controls.Xaml.dll
  ##vso[build.updatebuildnumber]10.0.100-ci+azdo.14634768
  Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0/Microsoft.Maui.Controls.Maps.dll
  TestUtils -> /home/vsts/work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
  Controls.Core.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.25]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.41]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.44]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.74]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed GradientBrushDoesNotLeakWhenSharingGradientStops [156 ms]
  Passed GradientStopChangesStillInvalidateAfterGc [34 ms]
  Passed ClearingGradientStopsAllowsReentrantReplacement [5 ms]

Test Run Successful.
Total tests: 3
     Passed: 3
 Total time: 3.5586 Seconds

📁 Fix files reverted (1 files)

• src/Controls/src/Core/GradientBrush.cs

---

📱 UI Tests — Brush

Detected UI test categories: Brush

✅ Deep UI tests — 42 passed, 0 failed across 1 category on platform-pool agent (replaces in-process counts above).

🧪 UI Test Execution Results (deep, platform pool)

Category	Tests	Snapshot diffs
Brush	42/42 ✓	—
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)

---

📋 Pre-Flight — Context & Validation

Issue: #36363 - [leak-scan] GradientBrush.GradientStops shared GradientStopCollection strongly roots the brush via CollectionChanged
PR: #36521 - [leak-fix] Fix GradientBrush.GradientStops memory leak (Fixes #36363)
Platforms Affected: All managed Controls platforms; testing platform requested: android
Files Changed: 1 implementation, 1 test

Key Findings

• Issue #36363 reports a managed memory leak where a shared/long-lived GradientStopCollection retains transient GradientBrush instances through strong CollectionChanged and per-stop PropertyChanged subscriptions.
• PR #36521 changes src/Controls/src/Core/GradientBrush.cs to use weak collection and per-stop property-change proxies owned by a private helper, plus regression coverage in GradientBrushMemoryTests.
• Gate artifact already shows focused tests fail without the fix and pass with the fix; per instruction, gate verification was not re-run.
• GitHub CLI in this shell is unauthenticated, so PR metadata/comments were gathered from public API where possible and required-check status remains undetermined.
• Impacted UI test category: Brush (gradient brush invalidation/rendering behavior); written to uitests/ai-categories.md.

Code Review Summary

Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 1

Key code review findings:

• Suggestion: Add explicit same-collection Clear() followed by Add() coverage to protect the ResetStops() then later NewItems path (src/Controls/tests/Core.UnitTests/GradientBrushMemoryTests.cs:48-68).
• Prior major finding that non-empty shared stops still leak through per-stop subscriptions is fixed by current per-stop WeakNotifyPropertyChangedProxy usage (src/Controls/src/Core/GradientBrush.cs:132-134).
• Failure mode: shared non-empty stops outlive brush; current PR avoids strong rooting because sources strongly hold proxy objects, while proxies weakly reference brush delegates.
• Failure mode: live brush after GC still invalidates because handler delegates are cached on the brush (src/Controls/src/Core/GradientBrush.cs:20-21).
• Blast radius: cross-platform Controls core behavior for all GradientBrush instances; no static/shared state or startup path.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #36521	Use weak collection and per-stop property-change proxies owned by GradientStopSubscriptions; clean up on replacement/reset and via helper finalizer.	PASSED (Gate)	src/Controls/src/Core/GradientBrush.cs, src/Controls/tests/Core.UnitTests/GradientBrushMemoryTests.cs	Original PR

---

🔬 Code Review — Deep Analysis

Code Review — PR #36521

Independent Assessment

What this changes: Replaces strong GradientBrush subscriptions to GradientStopCollection.CollectionChanged and each GradientStop.PropertyChanged with weak proxy subscriptions owned by a private GradientStopSubscriptions helper (src/Controls/src/Core/GradientBrush.cs:71-178). Adds memory/regression tests (src/Controls/tests/Core.UnitTests/GradientBrushMemoryTests.cs:9-68).

Inferred motivation: Prevent shared/long-lived GradientStopCollection instances from retaining transient brushes through event delegates.

Reconciliation with PR Narrative

Author claims: Fixes #36363 by weakly subscribing both collection and per-stop events, keeping delegates alive while the brush is alive, cleaning up on reset/replacement/finalization, and adding regression tests.

Agreement/disagreement: Agree. The current code weakens both leak paths. The prior collection-only approach would still leak through per-stop PropertyChanged, but current WeakNotifyPropertyChangedProxy usage at GradientBrush.cs:132-134 addresses that.

Prior Review Reconciliation

Prior Error Finding	Source	Status	Evidence
Non-empty shared GradientStopCollection still leaks the brush through per-stop PropertyChanged subscriptions	MauiBot inline [major] on old commit 11dbd6b	Fixed	Current code creates per-stop WeakNotifyPropertyChangedProxy instances at GradientBrush.cs:132-134; regression test uses a non-empty shared collection at GradientBrushMemoryTests.cs:13-25.
Latest MauiBot review reported no errors	MauiBot top-level review on 16141ad	Current	Review summary shows Errors: 0; no current inline blocking findings found.

Blast Radius Assessment

• Runs for all instances: Yes, all GradientBrush instances now allocate/use proxy helper state when GradientStops is assigned.
• Startup impact: Low; no static initialization or platform startup path.
• Static/shared state: None; helper/proxies are per-brush.
• Scope: Cross-platform Controls core behavior; affects gradient invalidation and memory lifetime.

CI Status

• Required-check result: Undetermined via gh pr checks 36521 --repo dotnet/maui --required in this shell because gh is unauthenticated.
• Supplemental evidence from PR narrative/gate artifact: focused gate passed, and the PR states Brush UI category completed.
• Classification: Undetermined for required gates.
• Action taken: Capped confidence low and did not return LGTM.

Findings

Suggestion — Add same-collection Clear/Add coverage

src/Controls/tests/Core.UnitTests/GradientBrushMemoryTests.cs:48-68

Existing tests cover shared non-empty leak, GC-pressure invalidation, and reentrant replacement during Clear(). A useful additional regression would cover clearing and then adding to the same GradientStopCollection, then verifying the new stop still invalidates the brush. Current implementation appears correct via ResetStops() followed by later e.NewItems handling on Add, but explicit coverage would protect this path.

No blocking code correctness findings found.

Failure-Mode Probing

• Shared non-empty stops outlive brush: Brush is no longer strongly rooted because collection and stop event sources only strongly hold proxy objects; proxies weakly reference the brush delegates.
• Brush remains alive under GC pressure: Cached delegate fields at GradientBrush.cs:20-21 keep weak-forwarding targets alive while the brush is alive.
• Reset/reentrant replacement: ResetStops() copies/clears proxies before unsubscribing (GradientBrush.cs:162-176), so reentrant GradientStops replacement during Invalidate() does not mutate the enumerated proxy list.
• Null/new collection replacement: Old parents are cleared and subscriptions removed before early return or subscribing new stops (GradientBrush.cs:55-82).

Verdict: NEEDS_DISCUSSION

Confidence: low
Summary: Code review found the current leak fix sound, and the prior blocking finding appears fixed. However, required CI could not be verified with gh pr checks --required due missing authentication, so per the skill rules this cannot be LGTM; supplemental public/gate evidence is healthy.

---

🛠️ Fix — Analysis & Comparison

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix-1	Move weak boundary to event sources: add internal WeakEventManager-backed notifications on GradientStopCollection and GradientStop; GradientBrush subscribes to those internal weak events and tracks active stops.	PASS	4 files	Empirically valid, but broader blast radius than PR because it changes gradient event-source behavior and bindable property callbacks.
2	try-fix-2	Static brush-side weak fan-out: source events point at static handlers; ConditionalWeakTable maps collections/stops to weak brush lists; brush tracks deterministic stop cleanup.	PASS	1 file	Empirically valid and source-side API-neutral, but introduces custom static subscription infrastructure and leaves cleanup dependent on deterministic unsubscribe/event fan-out.
3	try-fix-3	Listener-scoped direct subscriptions: subscribe to collection/stop events only while InvalidateGradientBrushRequested has listeners.	PASS	1 file	Focused test passes, but self-review found a moderate behavior risk: collection mutations before any listener may miss immediate Parent/binding-context maintenance.
PR	PR #36521	Use existing WeakNotifyCollectionChangedProxy/WeakNotifyPropertyChangedProxy objects owned by private GradientStopSubscriptions; clean up on replacement/reset and via helper finalizer.	PASSED (Gate)	1 implementation file + tests	Best balance: uses existing weak-event infrastructure, avoids source-side behavior changes/static registries, and covers reset/reentrant replacement.

Candidate Details

try-fix-1 — Source-side weak gradient events

• Artifact: CustomAgentLogsTmp/PRState/36521/PRAgent/try-fix-1/content.md
• Result: PASS; focused GradientBrushMemoryTests passed.
• Diff summary: GradientBrush.cs, GradientStop.cs, GradientStopCollection.cs, and GradientBrushMemoryTests.cs.
• Analysis: This is a valid architecture but not better than the PR because it modifies event-source types and bindable property callbacks for all gradient stop consumers. The PR keeps the weak subscription boundary local to GradientBrush.

try-fix-2 — Static weak fan-out registry

• Artifact: CustomAgentLogsTmp/PRState/36521/PRAgent/try-fix-2/content.md
• Result: PASS; focused GradientBrushMemoryTests passed.
• Diff summary: GradientBrush.cs only.
• Analysis: This avoids source-side changes, but creates bespoke static subscription tables for collections and stops. Although it prevents brush retention, it is more complex and higher maintenance risk than using the repository's existing weak event proxy primitives.

try-fix-3 — Listener-scoped direct subscriptions

• Artifact: CustomAgentLogsTmp/PRState/36521/PRAgent/try-fix-3/content.md
• Result: PASS; focused GradientBrushMemoryTests passed.
• Diff summary: GradientBrush.cs only.
• Failure/risk analysis: Self-review recorded one moderate finding: collection changes made before any invalidation subscriber is attached may not immediately set Parent or inherited binding context on added stops. This satisfies the leak test by avoiding subscriptions for unobserved brushes, but changes observable lifecycle behavior.

Cross-Pollination

Model/Reviewer Loop	Round	New Ideas?	Details
Expert loop	1	Yes	Source-side weak events; passed but broader blast radius.
Expert loop	2	Yes	Static brush-side weak fan-out; passed but custom static registry is more complex than PR.
Expert loop	3	Yes	Listener-scoped direct subscriptions; passed but has moderate behavior risk.
Expert loop	4	No	Remaining variations collapse into the already-tested categories: weak proxy owner (the PR), source-side weak events, static weak registry, or lifecycle-scoped direct subscriptions. Further changes would be trivial variations or higher-risk refactors.

Exhausted: Yes
Selected Fix: PR #36521 — It is not beaten by the alternatives. The PR uses existing MAUI weak-event proxy infrastructure, keeps changes localized to GradientBrush, avoids global/static registries and source-type behavior changes, and preserves immediate collection/stop observation semantics while passing the gate.

---

🧭 Next Steps — review latest findings

No alternative fix was selected for this run. Review the session findings and CI results before merging.

[kubaflo]

🔍 AI-generated review follow-up (GitHub Copilot CLI, on behalf of @kubaflo)

@MauiBot Added the suggested same-collection Clear/Add coverage in 10a8974372. ClearingAndReusingGradientStopsKeepsNewStopsSubscribed clears the collection, adds a new stop, forces GC, and verifies that changing the new stop still invalidates the brush. The PR description is current and the PR is ready for re-review.