Fix memory leak when clearing shared PathSegments from PathFigure

[devanathan-vaithiyanathan]

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!

Issue Details

Memory leak in PathFigure.Segments.Clear() when PathSegment objects are shared. The clear operation raises a reset change, and old segment handlers were not fully unsubscribed, so shared segments could keep old PathFigure instances alive and retain the related Path visual tree and page/viewmodel objects.

Description of Change

Added segment subscription tracking using a _subscribedSegments list to prevent memory leaks. The code now properly unsubscribes from segments when .Clear() is called by handling the Reset collection action. Subscription logic was extracted into reusable methods: SubscribeToPathSegmentPropertyChanged(), UnsubscribeFromPathSegmentPropertyChanged(), and UnsubscribeFromAllPathSegmentPropertyChanged(). This prevents cleared segments from triggering invalidation after removal.

Issues Fixed

Fixes #35796

Tested the behavior in the following platforms.

• Android
• Windows
• iOS
• Mac

Before	After
iOS Before.mov	iOS After.mov

[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 -- 35873

Or

• Run remotely in PowerShell:

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

[kubaflo]

/review -b feature/enhanced-reviewer

[MauiBot]

Expert Review — 1 findings

See inline comments for details.

[MauiBot]

[major] Logic and Correctness — This makes subscriptions unique by segment instance, but PathSegmentCollection is an ObservableCollection<PathSegment> and allows the same segment object to appear multiple times. Scenario: add the same LineSegment twice, then remove one occurrence; UnsubscribeFromPathSegmentPropertyChanged detaches the only handler, so changing the remaining segment no longer invalidates the figure. Please track per-occurrence subscriptions or keep a ref count and only detach when the last occurrence is removed.

[MauiBot]

Expert Review — 1 findings

See inline comments for details.

[MauiBot]

[major] Logic and Correctness - _subscribedSegments.Contains(pathSegment) treats segment subscriptions as a unique set, but PathSegmentCollection inherits ObservableCollection<PathSegment> and allows the same segment instance to appear multiple times. If the same LineSegment is added twice and one occurrence is removed or replaced, UnsubscribeFromPathSegmentPropertyChanged detaches the only handler while another occurrence remains in Segments; subsequent property changes no longer invalidate the path, leaving rendered geometry stale. Track reference counts per PathSegment instead, and add a duplicate-segment regression test.

[MauiBot]

AI Review Summary

@devanathan-vaithiyanathan — new AI review results are available based on this last commit: 01d6358. 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: dc08af3b

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
🧪 PathSegmentTests PathSegmentTests	✅ FAIL — 102s	✅ PASS — 63s

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

  Determining projects to restore...
  Restored /home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj (in 5.03 sec).
  Restored /home/vsts/work/1/s/src/TestUtils/src/TestUtils/TestUtils.csproj (in 167 ms).
  Restored /home/vsts/work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 230 ms).
  Restored /home/vsts/work/1/s/src/Essentials/src/Essentials.csproj (in 968 ms).
  Restored /home/vsts/work/1/s/src/Core/src/Core.csproj (in 108 ms).
  Restored /home/vsts/work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 8.27 sec).
  Restored /home/vsts/work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 22 ms).
  Restored /home/vsts/work/1/s/src/Core/maps/src/Maps.csproj (in 657 ms).
  Restored /home/vsts/work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 585 ms).
  1 of 10 projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14420410
  Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14420410
  Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14420410
  Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14420410
  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.14420410
  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.14420410
  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.14420410
  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.14]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.22]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.23]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
  Passed TestBezierSegmentConstructor [8 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144", count: 2) [11 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96", count: 7) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "", count: 0) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48", count: 1) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 9) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 8) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0", count: 4) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96", count: 6) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 10) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150", count: 3) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0", count: 5) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0", count: 4) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0", count: 5) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144", count: 2) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96", count: 7) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96", count: 6) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150", count: 3) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "", count: 0) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 9) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 10) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 8) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48", count: 1) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 10) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0", count: 4) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96", count: 6) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 8) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96", count: 7) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 9) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150", count: 3) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "", count: 0) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144", count: 2) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48", count: 1) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0", count: 5) [< 1 ms]
  Passed TestLineSegmentConstructor [< 1 ms]
  Passed TestArcSegmentConstructor [1 ms]
[xUnit.net 00:00:01.34]       Assert.Equal() Failure: Values differ
[xUnit.net 00:00:01.34]       Expected: 2
[xUnit.net 00:00:01.34]       Actual:   3
[xUnit.net 00:00:01.34]       Stack Trace:
[xUnit.net 00:00:01.35]         /_/src/Controls/tests/Core.UnitTests/PathSegmentTests.cs(147,0): at Microsoft.Maui.Controls.Core.UnitTests.PathSegmentTests.ClearingSegmentsDetachesPropertyChangedFromRemovedSegments()
[xUnit.net 00:00:01.35]            at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
[xUnit.net 00:00:01.35]            at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
[xUnit.net 00:00:01.35]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.34]     ClearingSegmentsDetachesPropertyChangedFromRemovedSegments [FAIL]
  Failed ClearingSegmentsDetachesPropertyChangedFromRemovedSegments [4 ms]
  Error Message:
   Assert.Equal() Failure: Values differ
Expected: 2
Actual:   3
  Stack Trace:
     at Microsoft.Maui.Controls.Core.UnitTests.PathSegmentTests.ClearingSegmentsDetachesPropertyChangedFromRemovedSegments() in /_/src/Controls/tests/Core.UnitTests/PathSegmentTests.cs:line 147
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
  Passed TestQuadraticBezierSegmentConstructor [< 1 ms]

Test Run Failed.
Total tests: 38
     Passed: 37
     Failed: 1
 Total time: 1.8274 Seconds

🟢 With fix — 🧪 PathSegmentTests: PASS ✅ · 63s

  Determining projects to restore...
  All projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14420410
  Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14420410
  Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14420410
  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.90-ci+azdo.14420410
  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.14420410
  Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0/Microsoft.Maui.Maps.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14420410
  Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0/Microsoft.Maui.Controls.Maps.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14420410
  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.24]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.46]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.47]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
  Passed TestBezierSegmentConstructor [23 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144", count: 2) [23 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96", count: 7) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "", count: 0) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48", count: 1) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 9) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 8) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0", count: 4) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96", count: 6) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 10) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150", count: 3) [< 1 ms]
  Passed TestPolyBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0", count: 5) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0", count: 4) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0", count: 5) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144", count: 2) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96", count: 7) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96", count: 6) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150", count: 3) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "", count: 0) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 9) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 10) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 8) [< 1 ms]
  Passed TestPolyQuadraticBezierSegmentConstructor(points: "0 48", count: 1) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 10) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0", count: 4) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96", count: 6) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 8) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96", count: 7) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, "···, count: 9) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150", count: 3) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "", count: 0) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144", count: 2) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48", count: 1) [< 1 ms]
  Passed TestPolyLineSegmentConstructor(points: "0 48, 0 144, 96 150, 100 0, 192 0", count: 5) [< 1 ms]
  Passed TestLineSegmentConstructor [1 ms]
  Passed TestArcSegmentConstructor [4 ms]
[xUnit.net 00:00:02.64]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed ClearingSegmentsDetachesPropertyChangedFromRemovedSegments [6 ms]
  Passed TestQuadraticBezierSegmentConstructor [5 ms]

Test Run Successful.
Total tests: 38
     Passed: 38
 Total time: 3.7743 Seconds

📁 Fix files reverted (1 files)

• src/Controls/src/Core/Shapes/PathFigure.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: #35796 - PathFigure.Segments.Clear() leaks Path visual trees when removed segments are shared
PR: #35873 - Fix memory leak when clearing shared PathSegments from PathFigure
Platforms Affected: Android, iOS (platform-neutral Shapes core code)
Files Changed: 1 implementation, 1 test

Key Findings

• PathSegmentCollection.Clear() raises Reset without OldItems, so the old implementation leaves removed segment PropertyChanged subscriptions attached.
• PR fix handles Reset/Clear but tracks subscriptions as unique segment instances, which breaks duplicate entries of the same PathSegment in one collection.
• Existing inline review from MauiBot reports the duplicate-entry regression; it remains present in the current PR diff.

Code Review Summary

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

Key code review findings:

• ❌ src/Controls/src/Core/Shapes/PathFigure.cs:163-180 treats duplicate PathSegment instances as a single subscription; removing one duplicate detaches the handler while another occurrence remains.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #35873	Track unique subscribed segments in a list and handle Reset by detaching all tracked items then rebuilding from the sender collection.	✅ PASSED (Gate)	PathFigure.cs, PathSegmentTests.cs	Original PR; gate passed, but duplicate segment regression remains.

---

Code Review — Deep Analysis

Code Review — PR #35873

Independent Assessment

What this changes: PathFigure now tracks subscribed PathSegment instances, handles NotifyCollectionChangedAction.Reset, and detaches segment PropertyChanged handlers when Segments.Clear() removes items.
Inferred motivation: Avoid stale event subscriptions retaining removed/shared segments and causing invalidation after removal.

Reconciliation with PR Narrative

Author claims: Fixes a memory leak from PathFigure.Segments.Clear() when shared PathSegment objects remain alive.
Agreement/disagreement: The reset/clear leak scenario is addressed, but the new unique-instance subscription tracking regresses duplicate-segment semantics.

Prior Review Reconciliation

Prior ❌ Error Finding	Source	Status	Evidence
Duplicate segment instances can lose invalidation because _subscribedSegments is treated as unique.	MauiBot inline review	❌ Unresolved	Current PathFigure.cs:163-180 still uses Contains, subscribes once, and unsubscribes on first removed occurrence.

Blast Radius Assessment

• Runs for all instances: Yes — every PathFigure now uses the subscription list.
• Startup impact: No.
• Static/shared state: No static state, but per-instance event subscription state affects rendering invalidation.

CI Status

• Required-check result: gh pr checks --required unavailable due missing GitHub auth.
• API fallback: check status could not be fully verified in this environment.
• Classification: undetermined.
• Action taken: confidence capped low.

Findings

❌ Error — Duplicate segment instances can lose invalidation

PathSegmentCollection is an unrestricted ObservableCollection<PathSegment>, so the same segment object can appear more than once. The new logic treats _subscribedSegments as a unique set at src/Controls/src/Core/Shapes/PathFigure.cs:163; removing one duplicate then detaches the only handler at lines 179-180 even if another occurrence remains in Segments. Subsequent mutations of the remaining segment no longer invalidate the figure. Use reference counting, or only unsubscribe when the segment no longer exists in the collection.

Failure-Mode Probing

• Duplicate segment added twice, one occurrence removed: invalidation is lost incorrectly.
• Segments.Clear(): removed segments are detached correctly.
• Segments replaced/null: previous collection and tracked segments are detached.
• Shared segment across different figures: per-figure handlers still work unless duplicate references exist within one figure.

Verdict: NEEDS_CHANGES

Confidence: low (code issue confidence is high, but CI status is unverifiable)
Summary: The intended leak fix is directionally sound, but the implementation introduces a concrete correctness regression for duplicate segment references. This should be fixed before merge.

---

Fix — Analysis & Comparison

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix-1	Reference-count PathSegment subscriptions with Dictionary<PathSegment, int>; detach only when the final occurrence is removed, and rebuild counts on Reset/Clear.	✅ PASS	PathFigure.cs; candidate-only duplicate tests in PathSegmentTests.cs	Passes gate-scoped PathSegment tests and duplicate probes. Demonstrably better than PR because PR fails RemovingOneDuplicateSegmentKeepsRemainingSegmentSubscribed.
PR	PR #35873	Track unique subscribed segments in a list and handle Reset by detaching all tracked items then rebuilding from sender collection.	✅ PASSED (Gate); ❌ fails duplicate probe	PathFigure.cs, PathSegmentTests.cs	Original PR fixes Clear/Reset leak but mishandles duplicate segment occurrences.

Cross-Pollination

Model	Round	New Ideas?	Details
maui-expert-reviewer / gpt-5.5	1	Yes	Proposed reference-counted subscription tracking to preserve duplicate collection multiplicity.

Exhausted: No — stopped per user criterion because candidate 1 passed all scoped tests and is demonstrably better than the PR fix.
Selected Fix: Candidate #1 — It preserves the PR's Clear/Reset leak fix and also preserves duplicate PathSegment semantics that the PR regresses.

---

Report — Final Recommendation

Comparative Analysis - PR #35873

Candidate ranking

Rank	Candidate	Regression result	Assessment
1	pr-plus-reviewer	Passed by construction from the reviewed reference-count fix; equivalent to try-fix-1	Best path: keeps the PR's Clear/Reset leak fix, applies the expert reviewer's reference-counted subscription feedback, and adds duplicate-segment coverage.
2	try-fix-1	PASS	Technically sound and empirically validated with the scoped PathSegmentTests plus duplicate probes. It is effectively the same implementation as pr-plus-reviewer, but as an external try-fix candidate rather than the PR fix with reviewer feedback applied.
3	pr	Gate PASS; duplicate probe FAIL	Fixes the original Clear/Reset leak, but failed the duplicate-occurrence regression probe and has an unresolved major expert-review finding. Per the ranking rule, this failed-regression candidate ranks below the passing candidates.

Candidate details

pr

The raw PR tracks subscribed segments in a unique List<PathSegment>. That handles Clear()/Reset where OldItems is unavailable, but it treats duplicate occurrences of the same segment object as one subscription. Removing one duplicate occurrence detaches the only handler while another occurrence remains, so later mutations no longer invalidate the path.

pr-plus-reviewer

The expert reviewer feedback changes subscription tracking to reference counts by PathSegment. The handler is attached once for the first occurrence, duplicate additions increment the count, removals decrement the count, and the handler is detached only after the final occurrence is removed. This preserves the PR's leak fix and covers the duplicate semantics gap identified by review.

try-fix-1

This candidate independently implemented the same reference-counted approach with Dictionary<PathSegment, int>. It passed the scoped PathSegment tests and duplicate probes, and its comparison run showed the raw PR fails RemovingOneDuplicateSegmentKeepsRemainingSegmentSubscribed.

Winner

Winner: pr-plus-reviewer. It is the best candidate because it keeps the PR-owned fix path while incorporating the expert reviewer's actionable correction. try-fix-1 is equally strong technically, but pr-plus-reviewer is the reviewed PR fix plus the same improvement, so it is the preferred final candidate.

---

Future Action — review latest findings

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