Fix for PathGeometry.Figures.Clear() leaks when clearing shared PathFigure instances by BagavathiPerumal · Pull Request #36057 · dotnet/maui

BagavathiPerumal · 2026-06-22T11:38:05Z

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!

Root cause of the issue

The issue occurs because PathGeometry subscribes to the PropertyChanged and InvalidatePathSegmentRequested events of each PathFigure in its Figures collection. These event subscriptions are removed only when the collection change notification provides an OldItems list, such as during Remove or RemoveAt operations.

However, when Figures.Clear() is called, the underlying ObservableCollection raises a Reset collection change notification, where OldItems is null. As a result, the existing unsubscription logic is skipped, leaving previously added PathFigureinstances subscribed to the PathGeometry.

This creates a memory leak scenario where shared or retained PathFigure objects continue to hold references to the PathGeometry. Consequently, the associated Path control, page, and its BindingContext cannot be garbage collected even after navigation away from the page.

Description of Change

The fix introduces a private _subscribedFigures collection within PathGeometry to explicitly track all PathFigure instances whose events are currently subscribed. To centralize subscription management, three helper methods were added:

SubscribeFigure – subscribes to a figure's events and records it in the tracking collection.
UnsubscribeFigure – removes event subscriptions for a specific figure and removes it from the tracking collection.
UnsubscribeAllFigures – unsubscribes all tracked figures and clears the tracking collection.

The CollectionChanged handler has been updated to handle the Reset action raised by Figures.Clear(). When a reset occurs, UnsubscribeAllFigures() is invoked to ensure that all previously subscribed figures are properly detached, regardless of whether OldItems is available. The else if condition guarding the Reset branch has been changed to an independent if statement, ensuring the Reset handler always executes even if OldItems happens to be non-null, preventing silent subscription divergence in custom collection scenarios.

Additionally, the CollectionChanged handler now skips the unsubscribe and resubscribe cycle when the action is Move. Since OldItems and NewItems contain the same figure instance on a Move, both blocks are guarded with e.Action != NotifyCollectionChangedAction.Move to eliminate the unnecessary round-trip while keeping the figure correctly subscribed and Invalidate() still firing as expected.

These changes guarantee that event subscriptions are always cleaned up correctly, preventing retained references and eliminating the memory leak without modifying the public API or altering existing collection behavior.

Issues Fixed

Fixes #35809

Tested the behavior in the following platforms.

Android
Windows
iOS
Mac

Before fix	After fix
Android 35809-BeforeFix.mov	Android 35809-AfterFix.mov

github-actions · 2026-06-22T11:38:16Z

🚀 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 -- 36057

Or

Run remotely in PowerShell:

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

vishnumenon2684 · 2026-06-24T05:14:15Z

/azp run maui-pr-uitests , maui-pr-devicetests

azure-pipelines · 2026-06-24T05:14:33Z

Azure Pipelines successfully started running 2 pipeline(s).

MauiBot

Expert Review — 2 findings

See inline comments for details.

MauiBot · 2026-07-05T20:51:38Z

+		// Tracks figures whose PropertyChanged and InvalidatePathSegmentRequested events are
+		// subscribed so we can unsubscribe them even when the collection is cleared (Reset
+		// action does not populate OldItems).
+		readonly List<PathFigure> _subscribedFigures = new List<PathFigure>();


🔍 AI-Generated Review (multi-model)

[major] Regression Prevention / Architectural Layer — This same Clear()-retention leak pattern (Reset action fires with OldItems == null, so the old per-item -= unsubscribe loop never runs) still exists unfixed in two sibling types that use the identical CollectionChanged wiring style:

PathFigure.UpdatePathSegmentCollection / OnPathSegmentCollectionChanged (src/Controls/src/Core/Shapes/PathFigure.cs) — figure.Segments.Clear() will still leave oldPathSegment.PropertyChanged -= OnPathSegmentPropertyChanged never called, retaining the PathFigure alive via any surviving PathSegment reference.

GeometryGroup.UpdateChildren / OnChildrenCollectionChanged (src/Controls/src/Core/Shapes/GeometryGroup.cs) — group.Children.Clear() has the same gap for Geometry children.

Concrete failing scenario: var seg = new LineSegment(); figure.Segments.Add(seg); figure.Segments.Clear(); — figure remains reachable through seg.PropertyChanged and a WeakReference<PathFigure> test analogous to FiguresClear_AllowsPathGeometryToBeGarbageCollected would fail here today. Since this PR introduces the exact fix pattern (a subscribed-items tracking list unsubscribed on Reset) for PathGeometry, please confirm whether PathFigure/GeometryGroup should get the same fix in this PR or a tracked follow-up issue — otherwise the underlying bug class remains only partially fixed.

MauiBot · 2026-07-05T20:51:39Z

+		// its PropertyChanged delegate chain, so TryGetTarget would return true.
+		Assert.False(weakRef.TryGetTarget(out _),
+			"PathGeometry was retained by the cleared PathFigure (event-handler leak in Figures.Clear()).");
+	}


🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention — adjacent scenario coverage — The new tests only cover a single PathFigure added once and then Clear(). Two adjacent scenarios exercised by the new production logic in PathGeometry.cs have no regression coverage:

Duplicate figures: the same PathFigure instance added twice to Figures (Figures.Add(x); Figures.Add(x);) then a partial Remove(x) followed by Clear() — this exercises _subscribedFigures.Remove(figure) (PathGeometry.cs:217) picking the first matching reference vs. the actual remaining subscription count staying balanced.

Move action: Figures.Move(0, 1) on a 2+ item collection — this exercises the new e.Action != NotifyCollectionChangedAction.Move guards (PathGeometry.cs:251 and :268), added specifically to skip resubscription churn on move; there's currently no test proving a moved figure keeps invalidating correctly and isn't double-subscribed/leaked afterward.
Both were reasoned about only informally; a regression test for each would lock in the intended behavior against future refactors of this same code area.

MauiBot

AI Review Summary

@BagavathiPerumal — new AI review results are available based on this last commit: bb2199f. 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: 89a4e02d

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
🧪 PathGeometryTests `PathGeometryTests`	✅ FAIL — 102s	✅ PASS — 67s

🔴 Without fix — 🧪 PathGeometryTests: FAIL ✅ · 102s

  Determining projects to restore...
  Restored /home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj (in 4.66 sec).
  Restored /home/vsts/work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 8.77 sec).
  Restored /home/vsts/work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 2.06 sec).
  Restored /home/vsts/work/1/s/src/TestUtils/src/TestUtils/TestUtils.csproj (in 5 ms).
  Restored /home/vsts/work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 9 ms).
  Restored /home/vsts/work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 51 ms).
  Restored /home/vsts/work/1/s/src/Essentials/src/Essentials.csproj (in 30 ms).
  Restored /home/vsts/work/1/s/src/Core/maps/src/Maps.csproj (in 30 ms).
  Restored /home/vsts/work/1/s/src/Core/src/Core.csproj (in 58 ms).
  1 of 10 projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14572631
  Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14572631
  Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14572631
  Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14572631
  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.90-ci+azdo.14572631
  Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14572631
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14572631
  Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0/Microsoft.Maui.Controls.Maps.dll
  Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0/Microsoft.Maui.Controls.Xaml.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.16]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.36]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.37]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.48]     FiguresClear_AllowsPathGeometryToBeGarbageCollected [FAIL]
[xUnit.net 00:00:01.48]       PathGeometry was retained by the cleared PathFigure (event-handler leak in Figures.Clear()).
[xUnit.net 00:00:01.48]       Stack Trace:
[xUnit.net 00:00:01.48]         /_/src/Controls/tests/Core.UnitTests/Shapes/PathGeometryTests.cs(81,0): at Microsoft.Maui.Controls.Core.UnitTests.Shapes.PathGeometryTests.FiguresClear_AllowsPathGeometryToBeGarbageCollected()
[xUnit.net 00:00:01.48]            at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
[xUnit.net 00:00:01.48]            at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
[xUnit.net 00:00:01.51]     FiguresClear_UnsubscribesFigurePropertyChangedHandler [FAIL]
[xUnit.net 00:00:01.51]     FiguresClear_UnsubscribesFigureSegmentInvalidateHandler [FAIL]
[xUnit.net 00:00:01.51]       Assert.Equal() Failure: Values differ
[xUnit.net 00:00:01.51]       Expected: 2
[xUnit.net 00:00:01.51]       Actual:   3
[xUnit.net 00:00:01.51]       Stack Trace:
[xUnit.net 00:00:01.51]         /_/src/Controls/tests/Core.UnitTests/Shapes/PathGeometryTests.cs(35,0): at Microsoft.Maui.Controls.Core.UnitTests.Shapes.PathGeometryTests.FiguresClear_UnsubscribesFigurePropertyChangedHandler()
[xUnit.net 00:00:01.51]            at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
[xUnit.net 00:00:01.51]            at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
[xUnit.net 00:00:01.51]       Assert.Equal() Failure: Values differ
[xUnit.net 00:00:01.51]       Expected: 2
[xUnit.net 00:00:01.51]       Actual:   3
[xUnit.net 00:00:01.51]       Stack Trace:
[xUnit.net 00:00:01.51]         /_/src/Controls/tests/Core.UnitTests/Shapes/PathGeometryTests.cs(62,0): at Microsoft.Maui.Controls.Core.UnitTests.Shapes.PathGeometryTests.FiguresClear_UnsubscribesFigureSegmentInvalidateHandler()
[xUnit.net 00:00:01.51]            at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
[xUnit.net 00:00:01.51]            at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
[xUnit.net 00:00:01.51]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Failed FiguresClear_AllowsPathGeometryToBeGarbageCollected [44 ms]
  Error Message:
   PathGeometry was retained by the cleared PathFigure (event-handler leak in Figures.Clear()).
  Stack Trace:
     at Microsoft.Maui.Controls.Core.UnitTests.Shapes.PathGeometryTests.FiguresClear_AllowsPathGeometryToBeGarbageCollected() in /_/src/Controls/tests/Core.UnitTests/Shapes/PathGeometryTests.cs:line 81
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
  Failed FiguresClear_UnsubscribesFigurePropertyChangedHandler [10 ms]
  Error Message:
   Assert.Equal() Failure: Values differ
Expected: 2
Actual:   3
  Stack Trace:
     at Microsoft.Maui.Controls.Core.UnitTests.Shapes.PathGeometryTests.FiguresClear_UnsubscribesFigurePropertyChangedHandler() in /_/src/Controls/tests/Core.UnitTests/Shapes/PathGeometryTests.cs:line 35
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
  Failed FiguresClear_UnsubscribesFigureSegmentInvalidateHandler [1 ms]
  Error Message:
   Assert.Equal() Failure: Values differ
Expected: 2
Actual:   3
  Stack Trace:
     at Microsoft.Maui.Controls.Core.UnitTests.Shapes.PathGeometryTests.FiguresClear_UnsubscribesFigureSegmentInvalidateHandler() in /_/src/Controls/tests/Core.UnitTests/Shapes/PathGeometryTests.cs:line 62
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Test Run Failed.
Total tests: 3
     Failed: 3
 Total time: 2.0969 Seconds

🟢 With fix — 🧪 PathGeometryTests: PASS ✅ · 67s

  Determining projects to restore...
  All projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14572631
  Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14572631
  Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14572631
  Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14572631
  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.90-ci+azdo.14572631
  Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14572631
  Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0/Microsoft.Maui.Controls.Xaml.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14572631
  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.30]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.89]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.91]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:03.05]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed FiguresClear_AllowsPathGeometryToBeGarbageCollected [49 ms]
  Passed FiguresClear_UnsubscribesFigurePropertyChangedHandler [8 ms]
  Passed FiguresClear_UnsubscribesFigureSegmentInvalidateHandler [3 ms]

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

📁 Fix files reverted (1 files)

src/Controls/src/Core/Shapes/PathGeometry.cs

📱 UI Tests — Shape

Detected UI test categories: Shape

✅ Deep UI tests — 35 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
`Shape`	35/35 ✓	—
📎 Download `drop-deep-uitests` artifact (TRX + snapshot diffs)

📋 Pre-Flight — Context & Validation

Issue: Unknown - GitHub CLI authentication unavailable; inferred from PR tests and code review as a PathGeometry.Figures.Clear() event-handler retention leak.
PR: #36057 - Fix PathGeometry.Figures.Clear() retaining cleared PathFigure handlers
Platforms Affected: android requested for testing; implementation is shared Controls shape code and can affect all platforms using PathGeometry.
Files Changed: 1 implementation, 1 test

Key Findings

The PR changes src/Controls/src/Core/Shapes/PathGeometry.cs and adds src/Controls/tests/Core.UnitTests/Shapes/PathGeometryTests.cs.
The root failure is that ObservableCollection<T>.Clear() raises Reset with OldItems = null, so the previous implementation could not unsubscribe handlers from cleared PathFigure instances.
Gate result was provided as passed: tests fail without the fix and pass with the PR fix. The gate output was not rerun or overwritten.
GitHub CLI authentication is unavailable in this environment, so PR body, issue body, discussion, inline comments, and required checks could not be fetched directly by this orchestrator.

Code Review Summary

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

Key code review findings:

No ❌ Error findings were identified in the PR implementation.
Blast radius: all PathGeometry instances allocate/use a subscribed-figure tracker, but behavior changes are localized to collection mutation.
Failure probe: Figures.Clear() with shared figures is addressed by unsubscribing tracked figures on Reset.
Failure probe: duplicate same PathFigure entries remain correctly balanced because subscriptions are tracked per add and one occurrence is removed per remove.
CI status is undetermined/red from available public signals and unauthenticated required-check query, so code review cannot be LGTM under the skill rules.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #36057	Track subscribed `PathFigure` instances in `PathGeometry`; unsubscribe tracked figures on `Reset`/collection replacement and skip `Move` resubscription churn.	✅ PASSED (Gate)	`PathGeometry.cs`, `PathGeometryTests.cs`	Original PR

🔬 Code Review — Deep Analysis

Code Review — PR #36057

Independent Assessment

What this changes: PathGeometry now tracks subscribed PathFigure instances so Figures.Clear() / Reset can unsubscribe figure PropertyChanged and segment invalidation handlers even when NotifyCollectionChangedEventArgs.OldItems is null. It also avoids unsubscribe/resubscribe churn on Move.

Inferred motivation: Prevent a shared/rooted PathFigure from retaining a cleared PathGeometry, which can then retain the associated Path/page/BindingContext through event delegate chains.

Reconciliation with PR Narrative

Author claims: The PR fixes PathGeometry.Figures.Clear() leaks for shared PathFigure instances, adds centralized subscribe/unsubscribe helpers, handles Reset, skips Move, and adds unit tests.

Agreement/disagreement: The code matches the narrative. The linked issue's leak path (shared PathFigure -> PropertyChanged delegate -> PathGeometry -> Path -> BindingContext) is addressed by UnsubscribeAllFigures() on Reset. I found no mismatch between claim and implementation.

Prior Review Reconciliation

No prior ❌ Error findings found.

Blast Radius Assessment

Runs for all instances: Yes, all PathGeometry instances now allocate/use _subscribedFigures, but behavior changes only around Figures collection mutation.
Startup impact: No.
Static/shared state: No.

CI Status

Required-check result: gh pr checks --required could not run because gh is unauthenticated.
Public check-run result: fail/red. Failing checks include maui-pr, maui-pr-uitests, maui-pr-devicetests, plus Build Analysis.
Classification: undetermined. Public Build Analysis shows known infra UITest timeouts/cancellations and unrelated UI/device failures; maui-pr annotation shows missing .buildtasks/Microsoft.Maui.Core.Before.targets, not in touched files. Azure logs were not fully accessible anonymously.
Action taken: invoked azdo-build-investigator; ci-analysis skill was unavailable. Confidence capped low; no LGTM.

Findings

No ❌ Error, ⚠️ Warning, or 💡 Suggestion findings identified in the reviewed code.

Failure-Mode Probing

Figures.Clear() with shared figures: tracked figures are unsubscribed on Reset, preventing stale delegate retention.
Duplicate same PathFigure added multiple times: duplicate subscriptions are tracked; removing one occurrence removes one subscription, leaving remaining occurrences subscribed.
Move action: skips unsubscribe/resubscribe and still invalidates once, preserving subscription state.
Custom Reset with OldItems: old items are unsubscribed, then remaining tracked items are cleared defensively.
Replacing Figures: old collection handler and all figure handlers are removed before subscribing the new collection.

Verdict: NEEDS_DISCUSSION

Confidence: low, due to red/undetermined CI and unauthenticated required-check query.

Summary: The code change itself appears correct, localized, and well-covered by unit tests for both invalidation and GC eligibility. I would not request code changes based on the diff, but CI is red and could not be fully classified, so this cannot be LGTM under the skill rules.

🛠️ Fix — Analysis & Comparison

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix	Add internal `PathFigureCollection` pre-clear notification; `PathGeometry` unsubscribes figures before `ClearItems()` raises `Reset`.	✅ PASS	2 production files	Passed focused tests after nullable event correction; not clearly better than PR due to hidden lifecycle coupling.
2	try-fix	Keep a last-known `PathFigure[]` snapshot in `PathGeometry`; unsubscribe snapshot entries on `Reset`.	✅ PASS	1 production file	Avoids collection hook but adds snapshot invariant and per-mutation allocation.
3	try-fix	Use weak `PathFigure` invalidation events plus `PathGeometry` membership guard for stale notifications.	✅ PASS	2 production files	Fixes retention but broadens notification semantics and adds weak-event overhead.
PR	PR #36057	Track subscribed `PathFigure` instances in `PathGeometry`; unsubscribe all tracked figures on `Reset`/collection replacement and skip `Move` churn.	✅ PASSED (Gate)	1 production file, 1 test file	Original PR.

Cross-Pollination

Model	Round	New Ideas?	Details
maui-expert-reviewer	1	Yes	Candidate #1: pre-clear notification in `PathFigureCollection`.
maui-expert-reviewer	2	Yes	Candidate #2: last-known `PathFigure[]` snapshot in `PathGeometry`.
maui-expert-reviewer	3	Yes	Candidate #3: weak invalidation event plus membership guard.
maui-expert-reviewer	4	No	No robust category remains beyond tracking, pre-clear old-item exposure, snapshots, weak events, or ownership/subscription relocation; remaining ideas are trivial variations or add more coupling.

Exhausted: Yes
Selected Fix: PR #36057 — all three alternatives passed the focused regression tests, but none is demonstrably better. The PR's fix is the most localized and explicit: one production file, no collection lifecycle hook, no snapshot allocation invariant, and no broader weak-event semantic change.

📝 Recommended PR Title & Description

Assessment: ✏️ Recommend updating — the current title/description accurately describe the raw PR, but the winning pr-plus-reviewer fix also covers sibling PathFigure.Segments.Clear() and GeometryGroup.Children.Clear() leaks plus additional edge-case tests.

Recommended title

[Controls] Shapes: Fix Clear() event-handler leaks in geometry collections

Recommended description

### Root cause of the issue

The issue occurs because `PathGeometry` subscribes to the `PropertyChanged` and `InvalidatePathSegmentRequested` events of each `PathFigure` in its `Figures` collection. These event subscriptions were removed only when the collection change notification provided an `OldItems` list, such as during `Remove` or `RemoveAt` operations.

However, when `Figures.Clear()` is called, the underlying `ObservableCollection<T>` raises a `Reset` collection change notification where `OldItems` is null. As a result, the previous unsubscription logic was skipped, leaving previously added `PathFigure` instances subscribed to the `PathGeometry`.

This creates a memory leak scenario where shared or retained `PathFigure` objects continue to hold references to the `PathGeometry`. Consequently, the associated `Path` control, page, and its `BindingContext` cannot be garbage collected even after navigation away from the page.

The same Clear/Reset event-subscription pattern also exists in sibling Shapes collection owners:

- `PathFigure.Segments.Clear()` can leave cleared `PathSegment` instances subscribed to the `PathFigure`.
- `GeometryGroup.Children.Clear()` can leave cleared child `Geometry` instances subscribed to the `GeometryGroup`.

### Description of Change

The fix introduces explicit tracking collections for subscribed child items so event handlers can be removed even when a collection reset does not provide `OldItems`:

- `PathGeometry` tracks subscribed `PathFigure` instances in its `Figures` collection.
- `PathFigure` tracks subscribed `PathSegment` instances in its `Segments` collection.
- `GeometryGroup` tracks subscribed child `Geometry` instances in its `Children` collection.

Each owner now centralizes subscription management through helper methods that subscribe, unsubscribe a single item, and unsubscribe all tracked items. The `CollectionChanged` handlers now handle `NotifyCollectionChangedAction.Reset` by unsubscribing all tracked children, ensuring `Clear()` detaches event handlers regardless of whether `OldItems` is available.

The collection handlers also skip the unsubscribe/resubscribe cycle for `Move` actions. Since `OldItems` and `NewItems` contain the same instance on a move, the existing subscription remains valid and `Invalidate()` still fires as expected without adding duplicate handlers.

Additional unit coverage verifies:

- `PathGeometry.Figures.Clear()` detaches both `PathFigure.PropertyChanged` and `PathFigure.InvalidatePathSegmentRequested`.
- A cleared shared `PathFigure` no longer retains its `PathGeometry`.
- Duplicate `PathFigure` entries keep subscription counts balanced across partial `Remove()` and `Clear()`.
- `Figures.Move()` preserves correct single-subscription invalidation behavior.
- `PathFigure.Segments.Clear()` no longer retains `PathFigure` through cleared segment subscriptions.
- `GeometryGroup.Children.Clear()` no longer retains `GeometryGroup` through cleared child geometry subscriptions.

These changes prevent retained references without modifying public API or altering intended collection behavior.

### Issues Fixed

Fixes https://github.com/dotnet/maui/issues/35809

**Tested the behavior in the following platforms.**
- [x] Android
- [x] Windows
- [x] iOS
- [x] Mac

| Before fix | After fix |
|---------|--------|
| **Android**<br> <video src="https://github.com/user-attachments/assets/323b81a5-fb4a-4814-9263-eb6d6fb73a06" width="300" height="600"> | **Android**<br> <video src="https://github.com/user-attachments/assets/6892cfc1-a1f1-42b8-a711-0470e0e34808" width="300" height="600"> |

🏁 Report — Final Recommendation

Comparative Report — PR #36057

Candidates

Rank	Candidate	Regression result	Assessment
1	`pr-plus-reviewer`	✅ PASS — 7 focused `PathGeometryTests` in sandbox	Best overall. Keeps the raw PR's localized `PathGeometry.Figures.Clear()` tracking fix, addresses the expert reviewer's major sibling-leak finding in `PathFigure.Segments` and `GeometryGroup.Children`, and adds coverage for duplicate figures and `Move`.
2	`pr`	✅ PASS — gate passed	Sound for the reported issue and lowest production-file scope, but leaves the same Clear/Reset retention pattern unfixed in adjacent shape collection owners and lacks tests for duplicate entries and `Move`.
3	`try-fix-2`	✅ PASS	Viable one-file alternative using a last-known `PathFigure[]` snapshot, but less direct than the PR's subscription tracking and adds a snapshot invariant plus per-mutation allocation.
4	`try-fix-1`	✅ PASS	Fixes Clear via a `PathFigureCollection` pre-clear hook, but adds hidden lifecycle coupling and a new internal collection event in an additional production type.
5	`try-fix-3`	✅ PASS	Fixes retention with weak events and membership guarding, but broadens `PathFigure` notification semantics and adds weak-event overhead/reflection risk for a localized leak.

Key comparison

All candidates passed their focused regression tests, so no passing candidate is ranked below a failing one. The raw PR is the best narrow fix for issue #35809, but the expert reviewer found the same bug class in sibling collection owners that share the same ObservableCollection<T>.Clear() Reset behavior. Applying the same tracking pattern to those owners is coherent with the PR's chosen design and prevents future near-duplicate leak reports.

Winner

Winner: pr-plus-reviewer

pr-plus-reviewer wins because it preserves the raw PR fix, incorporates the expert reviewer's actionable feedback, and validates the expanded behavior with focused tests. The extra production-file scope is justified because the sibling leaks are the same event-subscription lifecycle bug class and the applied pattern remains localized to Shapes collection owners.

🧭 Next Steps — review latest findings

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

…igure instances (#36057)  > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you!  ### Root cause of the issue The issue occurs because PathGeometry subscribes to the PropertyChanged and InvalidatePathSegmentRequested events of each PathFigure in its Figures collection. These event subscriptions are removed only when the collection change notification provides an OldItems list, such as during Remove or RemoveAt operations. However, when Figures.Clear() is called, the underlying ObservableCollection<T> raises a Reset collection change notification, where OldItems is null. As a result, the existing unsubscription logic is skipped, leaving previously added PathFigureinstances subscribed to the PathGeometry. This creates a memory leak scenario where shared or retained PathFigure objects continue to hold references to the PathGeometry. Consequently, the associated Path control, page, and its BindingContext cannot be garbage collected even after navigation away from the page. ### Description of Change The fix introduces a private _subscribedFigures collection within PathGeometry to explicitly track all PathFigure instances whose events are currently subscribed. To centralize subscription management, three helper methods were added: - SubscribeFigure – subscribes to a figure's events and records it in the tracking collection. - UnsubscribeFigure – removes event subscriptions for a specific figure and removes it from the tracking collection. - UnsubscribeAllFigures – unsubscribes all tracked figures and clears the tracking collection. The CollectionChanged handler has been updated to handle the Reset action raised by Figures.Clear(). When a reset occurs, UnsubscribeAllFigures() is invoked to ensure that all previously subscribed figures are properly detached, regardless of whether OldItems is available. The else if condition guarding the Reset branch has been changed to an independent if statement, ensuring the Reset handler always executes even if OldItems happens to be non-null, preventing silent subscription divergence in custom collection scenarios. Additionally, the CollectionChanged handler now skips the unsubscribe and resubscribe cycle when the action is Move. Since OldItems and NewItems contain the same figure instance on a Move, both blocks are guarded with e.Action != NotifyCollectionChangedAction.Move to eliminate the unnecessary round-trip while keeping the figure correctly subscribed and Invalidate() still firing as expected. These changes guarantee that event subscriptions are always cleaned up correctly, preventing retained references and eliminating the memory leak without modifying the public API or altering existing collection behavior. ### Issues Fixed  Fixes #35809  **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before fix | After fix | |---------|--------| | **Android**<br> <video src="https://github.com/user-attachments/assets/323b81a5-fb4a-4814-9263-eb6d6fb73a06" width="300" height="600"> | **Android**<br> <video src="https://github.com/user-attachments/assets/6892cfc1-a1f1-42b8-a711-0470e0e34808" width="300" height="600"> |

Rebased onto inflight/current, which already contains #36057's shared-PathFigure Clear() fix using strong CollectionChanged/PropertyChanged subscriptions — the exact mechanism #36366 reports (a shared/long-lived PathFigureCollection roots the PathGeometry, and through it the owning Path/page/BindingContext). Converts those subscriptions to weak proxies, preserving #36057's teardown behavior guarded by Shapes/PathGeometryTests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e27685d0-fe80-460a-aa05-83d2ab9bf032

…igure instances (#36057)  > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you!  ### Root cause of the issue The issue occurs because PathGeometry subscribes to the PropertyChanged and InvalidatePathSegmentRequested events of each PathFigure in its Figures collection. These event subscriptions are removed only when the collection change notification provides an OldItems list, such as during Remove or RemoveAt operations. However, when Figures.Clear() is called, the underlying ObservableCollection<T> raises a Reset collection change notification, where OldItems is null. As a result, the existing unsubscription logic is skipped, leaving previously added PathFigureinstances subscribed to the PathGeometry. This creates a memory leak scenario where shared or retained PathFigure objects continue to hold references to the PathGeometry. Consequently, the associated Path control, page, and its BindingContext cannot be garbage collected even after navigation away from the page. ### Description of Change The fix introduces a private _subscribedFigures collection within PathGeometry to explicitly track all PathFigure instances whose events are currently subscribed. To centralize subscription management, three helper methods were added: - SubscribeFigure – subscribes to a figure's events and records it in the tracking collection. - UnsubscribeFigure – removes event subscriptions for a specific figure and removes it from the tracking collection. - UnsubscribeAllFigures – unsubscribes all tracked figures and clears the tracking collection. The CollectionChanged handler has been updated to handle the Reset action raised by Figures.Clear(). When a reset occurs, UnsubscribeAllFigures() is invoked to ensure that all previously subscribed figures are properly detached, regardless of whether OldItems is available. The else if condition guarding the Reset branch has been changed to an independent if statement, ensuring the Reset handler always executes even if OldItems happens to be non-null, preventing silent subscription divergence in custom collection scenarios. Additionally, the CollectionChanged handler now skips the unsubscribe and resubscribe cycle when the action is Move. Since OldItems and NewItems contain the same figure instance on a Move, both blocks are guarded with e.Action != NotifyCollectionChangedAction.Move to eliminate the unnecessary round-trip while keeping the figure correctly subscribed and Invalidate() still firing as expected. These changes guarantee that event subscriptions are always cleaned up correctly, preventing retained references and eliminating the memory leak without modifying the public API or altering existing collection behavior. ### Issues Fixed  Fixes #35809  **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before fix | After fix | |---------|--------| | **Android**<br> <video src="https://github.com/user-attachments/assets/323b81a5-fb4a-4814-9263-eb6d6fb73a06" width="300" height="600"> | **Android**<br> <video src="https://github.com/user-attachments/assets/6892cfc1-a1f1-42b8-a711-0470e0e34808" width="300" height="600"> |

BagavathiPerumal added 2 commits June 22, 2026 16:34

fix-35809-Made code changes to resolve the memory leak in PathGeometry.

da5dbe6

fix-35809-Changes updated.

bb2199f

BagavathiPerumal added the community ✨ Community Contribution label Jun 22, 2026

dotnet-policy-service Bot added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Jun 22, 2026

BagavathiPerumal temporarily deployed to gh-aw-agents June 22, 2026 11:38 — with GitHub Actions Inactive

BagavathiPerumal temporarily deployed to gh-aw-agents June 22, 2026 11:40 — with GitHub Actions Inactive

BagavathiPerumal temporarily deployed to gh-aw-agents June 22, 2026 11:42 — with GitHub Actions Inactive

github-actions Bot added the area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing label Jun 22, 2026

BagavathiPerumal temporarily deployed to gh-aw-agents June 22, 2026 11:42 — with GitHub Actions Inactive

sheiksyedm marked this pull request as ready for review June 23, 2026 14:21

This was referenced Jun 24, 2026

slow macOS - "##[error]The job running on agent Azure Pipelines 9 ran longer than the maximum time of 60 minutes." dotnet/dnceng#1883

Open

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

This was referenced Jun 24, 2026

[PR Review Queue] 2026-06-24 #36098

Closed

[PR Review Queue] 2026-06-25 #36126

Closed

[PR Review Queue] 2026-06-26 #36151

Closed

[PR Review Queue] 2026-06-29 #36203

Closed

[PR Review Queue] 2026-06-30 #36229

Closed

This comment has been minimized.

Sign in to view

github-actions Bot added the s/agent-review-in-progress AI review is currently running for this PR label Jul 5, 2026

MauiBot reviewed Jul 5, 2026

View reviewed changes

MauiBot added the s/agent-gate-passed AI verified tests catch the bug (fail without fix, pass with fix) label Jul 5, 2026

MauiBot added s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) labels Jul 5, 2026

MauiBot reviewed Jul 5, 2026

View reviewed changes

MauiBot removed the s/agent-review-in-progress AI review is currently running for this PR label Jul 5, 2026

kubaflo approved these changes Jul 5, 2026

View reviewed changes

kubaflo changed the base branch from main to inflight/current July 5, 2026 22:47

kubaflo merged commit 4c111a3 into dotnet:inflight/current Jul 5, 2026
151 of 169 checks passed

github-actions Bot added this to the .NET 10 SR9 milestone Jul 5, 2026

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

[repo-status] Daily Status Report — July 6, 2026 #36407

Closed

kubaflo mentioned this pull request Jul 6, 2026

July 13th, Candidate #36411

Merged

This was referenced Jul 13, 2026

[leak-fix] Fix PathGeometry.Figures memory leak (Fixes #36366) #36535

Closed

[leak-fix] Fix PathFigure.Segments memory leak #36547

Closed

github-actions Bot locked and limited conversation to collaborators Aug 5, 2026

Navigation Menu

Uh oh!

Fix for PathGeometry.Figures.Clear() leaks when clearing shared PathFigure instances - #36057

Fix for PathGeometry.Figures.Clear() leaks when clearing shared PathFigure instances#36057
kubaflo merged 2 commits into
dotnet:inflight/currentfrom
BagavathiPerumal:fix-35809

BagavathiPerumal commented Jun 22, 2026

Uh oh!

github-actions Bot commented Jun 22, 2026

Uh oh!

vishnumenon2684 commented Jun 24, 2026

Uh oh!

azure-pipelines Bot commented Jun 24, 2026

Uh oh!

This comment has been minimized.

MauiBot left a comment

Uh oh!

MauiBot Jul 5, 2026

Uh oh!

MauiBot Jul 5, 2026

Uh oh!

MauiBot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

BagavathiPerumal commented Jun 22, 2026

Root cause of the issue

Description of Change

Issues Fixed

Uh oh!

github-actions Bot commented Jun 22, 2026

Uh oh!

vishnumenon2684 commented Jun 24, 2026

Uh oh!

azure-pipelines Bot commented Jun 24, 2026

Uh oh!

This comment has been minimized.

MauiBot left a comment

Choose a reason for hiding this comment

Expert Review — 2 findings

Uh oh!

MauiBot Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

MauiBot Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

MauiBot left a comment

Choose a reason for hiding this comment

AI Review Summary

Gate Result: ✅ PASSED

🧪 UI Test Execution Results (deep, platform pool)

Key Findings

Code Review Summary

Fix Candidates

Code Review — PR #36057

Independent Assessment

Reconciliation with PR Narrative

Prior Review Reconciliation

Blast Radius Assessment

CI Status

Findings

Failure-Mode Probing

Verdict: NEEDS_DISCUSSION

Fix Candidates

Cross-Pollination

Comparative Report — PR #36057

Candidates

Key comparison

Winner

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants