Shapes: Fix TransformGroup child subscription leaks

[HarishwaranVijayakumar]

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

• TransformGroup.Children replacement and Clear() can leak child transform PropertyChanged subscriptions.

Root Cause of the issue

TransformGroup subscribes to each child transform's PropertyChanged event when the child is added to the Children collection through the collection change handler.

There are two leak scenarios:

1. Collection Replacement (Children = new TransformCollection())

   The previous Children change callback only:

   • Unsubscribed from the old collection's CollectionChanged event
   • Subscribed to the new collection's CollectionChanged event

   It did not:

   • Iterate old collection items to unsubscribe their PropertyChanged handlers
   • Iterate new collection items to subscribe their PropertyChanged handlers

2. Collection Clear (Children.Clear())

   ObservableCollection raises CollectionChanged with Action = Reset and OldItems = null. The previous handler only checked OldItems and NewItems, so it skipped child PropertyChanged unsubscription on Clear().

In both cases, old child transforms can keep a delegate reference to TransformGroup.OnTransformPropertyChanged. If the child transform is long-lived, shared, cached, or static, it can retain the TransformGroup, associated Path, and the Path binding context.

Description of Change

• Added reference-counted child transform subscriptions with Dictionary<INotifyPropertyChanged, int> so a transform that appears multiple times in the same collection has one event subscription that remains attached until the last occurrence is removed.
• Added AttachCollection and DetachCollection helpers so Children replacement unsubscribes child transforms from the old collection and subscribes child transforms from the new collection.
• Added explicit NotifyCollectionChangedAction.Reset handling in OnChildrenCollectionChanged so Children.Clear() clears tracked subscriptions and rebuilt collections keep current child subscriptions.

Testing Enhancements

Added TransformGroupTests covering:

• Replacing Children unsubscribes old child transform PropertyChanged handlers
• Replacing Children subscribes child transforms already present in the new collection
• Shared transforms do not retain multiple detached TransformGroup instances
• Clearing Children unsubscribes all tracked child transforms

Issues Fixed

Fixes #36033

Tested the behaviour in the following platforms

• Windows
• Android
• iOS
• Mac

Before	After

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

Or

• Run remotely in PowerShell:

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

[dotnet-policy-service]

Hey there @@HarishwaranVijayakumar! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[MauiBot]

AI Review Summary

@HarishwaranVijayakumar — new AI review results are available based on this last commit: 47c1cf7. 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: 2a323a50

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
🧪 TransformGroupTests TransformGroupTests	✅ FAIL — 131s	✅ PASS — 69s

🔴 Without fix — 🧪 TransformGroupTests: FAIL ✅ · 131s

  Determining projects to restore...
  Restored /home/vsts/work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 855 ms).
  Restored /home/vsts/work/1/s/src/TestUtils/src/TestUtils/TestUtils.csproj (in 2.07 sec).
  Restored /home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj (in 3.29 sec).
  Restored /home/vsts/work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 6.41 sec).
  Restored /home/vsts/work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 8.63 sec).
  Restored /home/vsts/work/1/s/src/Essentials/src/Essentials.csproj (in 32 ms).
  Restored /home/vsts/work/1/s/src/Core/src/Core.csproj (in 102 ms).
  Restored /home/vsts/work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 865 ms).
  Restored /home/vsts/work/1/s/src/Core/maps/src/Maps.csproj (in 641 ms).
  1 of 10 projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508500
  Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508500
  Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508500
  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.14508500
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508500
  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.90-ci+azdo.14508500
  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.14508500
  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.15]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.53]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.54]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:02.48]     SharedTransformDoesNotRetainMultipleGroups [FAIL]
[xUnit.net 00:00:02.49]       TransformGroup #0 should be collected. Shared transform is retaining it.
[xUnit.net 00:00:02.49]       Stack Trace:
[xUnit.net 00:00:02.57]         /_/src/Controls/tests/Core.UnitTests/Shapes/TransformGroupTests.cs(64,0): at Microsoft.Maui.Controls.Core.UnitTests.Shapes.TransformGroupTests.SharedTransformDoesNotRetainMultipleGroups()
[xUnit.net 00:00:02.58]         --- End of stack trace from previous location ---
[xUnit.net 00:00:03.19]     ClearingChildrenUnsubscribesAllTransforms [FAIL]
[xUnit.net 00:00:03.19]       TransformGroup should be collected after Children.Clear(). Shared child transform is keeping it alive via stale PropertyChanged subscription.
[xUnit.net 00:00:03.19]       Stack Trace:
[xUnit.net 00:00:03.19]         /_/src/Controls/tests/Core.UnitTests/Shapes/TransformGroupTests.cs(82,0): at Microsoft.Maui.Controls.Core.UnitTests.Shapes.TransformGroupTests.ClearingChildrenUnsubscribesAllTransforms()
[xUnit.net 00:00:03.19]         --- End of stack trace from previous location ---
  Failed SharedTransformDoesNotRetainMultipleGroups [873 ms]
  Error Message:
   TransformGroup #0 should be collected. Shared transform is retaining it.
  Stack Trace:
     at Microsoft.Maui.Controls.Core.UnitTests.Shapes.TransformGroupTests.SharedTransformDoesNotRetainMultipleGroups() in /_/src/Controls/tests/Core.UnitTests/Shapes/TransformGroupTests.cs:line 64
--- End of stack trace from previous location ---
[xUnit.net 00:00:03.27]       Assert.NotEqual() Failure: Values are equal
[xUnit.net 00:00:03.27]       Expected: Not Microsoft.Maui.Controls.Shapes.Matrix
[xUnit.net 00:00:03.27]       Actual:       Microsoft.Maui.Controls.Shapes.Matrix
[xUnit.net 00:00:03.27]       Stack Trace:
[xUnit.net 00:00:03.27]         /_/src/Controls/tests/Core.UnitTests/Shapes/TransformGroupTests.cs(43,0): at Microsoft.Maui.Controls.Core.UnitTests.Shapes.TransformGroupTests.ReplacingChildrenSubscribesToNewChildPropertyChanged()
[xUnit.net 00:00:03.27]            at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
[xUnit.net 00:00:03.27]            at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
[xUnit.net 00:00:03.27]     ReplacingChildrenSubscribesToNewChildPropertyChanged [FAIL]
[xUnit.net 00:00:03.96]     ReplacingChildrenUnsubscribesFromOldChildPropertyChanged [FAIL]
[xUnit.net 00:00:03.96]       TransformGroup should be collected after Children replacement. Shared child transform is keeping it alive via stale PropertyChanged subscription.
[xUnit.net 00:00:03.96]       Stack Trace:
[xUnit.net 00:00:03.96]         /_/src/Controls/tests/Core.UnitTests/Shapes/TransformGroupTests.cs(24,0): at Microsoft.Maui.Controls.Core.UnitTests.Shapes.TransformGroupTests.ReplacingChildrenUnsubscribesFromOldChildPropertyChanged()
[xUnit.net 00:00:03.96]         --- End of stack trace from previous location ---
[xUnit.net 00:00:03.97]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Failed ClearingChildrenUnsubscribesAllTransforms [712 ms]
  Error Message:
   TransformGroup should be collected after Children.Clear(). Shared child transform is keeping it alive via stale PropertyChanged subscription.
  Stack Trace:
     at Microsoft.Maui.Controls.Core.UnitTests.Shapes.TransformGroupTests.ClearingChildrenUnsubscribesAllTransforms() in /_/src/Controls/tests/Core.UnitTests/Shapes/TransformGroupTests.cs:line 82
--- End of stack trace from previous location ---
  Failed ReplacingChildrenSubscribesToNewChildPropertyChanged [8 ms]
  Error Message:
   Assert.NotEqual() Failure: Values are equal
Expected: Not Microsoft.Maui.Controls.Shapes.Matrix
Actual:       Microsoft.Maui.Controls.Shapes.Matrix
  Stack Trace:
     at Microsoft.Maui.Controls.Core.UnitTests.Shapes.TransformGroupTests.ReplacingChildrenSubscribesToNewChildPropertyChanged() in /_/src/Controls/tests/Core.UnitTests/Shapes/TransformGroupTests.cs:line 43
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
  Failed ReplacingChildrenUnsubscribesFromOldChildPropertyChanged [757 ms]
  Error Message:
   TransformGroup should be collected after Children replacement. Shared child transform is keeping it alive via stale PropertyChanged subscription.
  Stack Trace:
     at Microsoft.Maui.Controls.Core.UnitTests.Shapes.TransformGroupTests.ReplacingChildrenUnsubscribesFromOldChildPropertyChanged() in /_/src/Controls/tests/Core.UnitTests/Shapes/TransformGroupTests.cs:line 24
--- End of stack trace from previous location ---

Test Run Failed.
Total tests: 4
     Failed: 4
 Total time: 4.4963 Seconds

🟢 With fix — 🧪 TransformGroupTests: PASS ✅ · 69s

  Determining projects to restore...
  All projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508500
  Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508500
  Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508500
  Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  ##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508500
  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.14508500
  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.14508500
  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.14508500
  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.47]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:03.00]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:03.01]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:03.45]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed SharedTransformDoesNotRetainMultipleGroups [146 ms]
  Passed ClearingChildrenUnsubscribesAllTransforms [96 ms]
  Passed ReplacingChildrenSubscribesToNewChildPropertyChanged [7 ms]
  Passed ReplacingChildrenUnsubscribesFromOldChildPropertyChanged [28 ms]

Test Run Successful.
Total tests: 4
     Passed: 4
 Total time: 4.7233 Seconds

📁 Fix files reverted (1 files)

• src/Controls/src/Core/Shapes/TransformGroup.cs

---

📱 UI Tests — Shape

Detected UI test categories: Shape

⚠️ Deep UI tests — 1 category (35 tests) could not run: OneTimeSetUp/fixture setup failure on the platform-pool agent — infrastructure, not a PR test failure (replaces in-process counts above).

🧪 UI Test Execution Results (deep, platform pool)

Category	Tests	Snapshot diffs
Shape	0/35 (setup failed; 35 marked failed)	—

⚠️ Shape — fixture setup failed for 35 tests

NUnit reported a OneTimeSetUp/fixture setup failure before test bodies ran; the TRX marked each affected test failed.

Multiple setup failure signatures were present; showing the first one. See the TRX artifact for all details.

OneTimeSetUp: System.TimeoutException : Timed out waiting for Go To Test button to appear
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2757
   at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2784
   at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 793
   at Microsoft.Maui.TestCases.Tests.UtilExtensions.NavigateToGallery(IApp app, String page) in /_/src/Controls/tests/TestCases.Shared.Tests/UtilExtensions.cs:line 37
   at Microsoft.Maui.TestCases.Tests._GalleryUITest.FixtureSetup() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/_GalleryUITest.cs:line 57
   at UITest.Appium.NUnit.UITestBase.OneTimeSetup() in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 221
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)

---

📋 Pre-Flight — Context & Validation

Issue: #36033 - TransformGroup.Children replacement leaks child transform PropertyChanged subscriptions
PR: #36150 - [WIP] Fix TransformGroup.Children replacement leaks child transform PropertyChanged subscriptions
Platforms Affected: Android, iOS, macOS/Mac Catalyst (issue verified on Android/iOS/macOS; change is platform-neutral Controls Shapes code)
Files Changed: 1 implementation, 1 test

Key Findings

• TransformGroup previously subscribed to child transform PropertyChanged events on collection additions, but replacing Children only detached the old collection's CollectionChanged handler and did not detach child property subscriptions.
• TransformCollection.Clear() raises NotifyCollectionChangedAction.Reset with no OldItems, so the previous add/remove-only handler could not unsubscribe cleared child transforms.
• PR #36150 fixes the leak by tracking child subscriptions independently, detaching old collection children on replacement, reconciling reset/clear, and preserving updates from new pre-populated collections.
• Changed files: src/Controls/src/Core/Shapes/TransformGroup.cs; src/Controls/tests/Core.UnitTests/Shapes/TransformGroupTests.cs.
• GitHub CLI was unavailable because authentication was not configured, so public GitHub API data and the local pr-review-36150 branch were used for context.

Code Review Summary

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

Key code review findings:

• ℹ src/Controls/src/Core/Shapes/TransformGroup.cs:120 implements duplicate-transform reference counting; an explicit test for add-same-transform-twice/remove-once/remove-final would better cover that key path.
• CI required-check status could not be verified through gh; public data showed maui-pr succeeded and an agent check failed, so confidence is capped.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #36150	Reference-count child PropertyChanged subscriptions with Dictionary<INotifyPropertyChanged, int>, attach/detach on Children replacement, and clear/rebuild on Reset.	✅ PASSED (Gate)	TransformGroup.cs, TransformGroupTests.cs	Original PR fix; gate result supplied by caller.

---

🔬 Code Review — Deep Analysis

Code Review — PR #36150

Independent Assessment

What this changes: TransformGroup now tracks child PropertyChanged subscriptions, unsubscribes when Children is replaced/cleared, and handles duplicate child transforms via reference counts.
Inferred motivation: Prevent stale child-transform event handlers from retaining abandoned TransformGroup instances.

Reconciliation with PR Narrative

Author claims: Fixes Children replacement/Clear leaks and adds tests, including duplicate-transform coverage.
Agreement/disagreement: Code matches the leak fix. Duplicate-transform behavior is implemented, but I did not find a focused duplicate-transform test.

Prior Review Reconciliation

No prior ❌ Error findings found.

CI Status

• Required-check result: unavailable via gh (gh auth login required)
• Public check-runs observed: maui-pr succeeded; agent failed
• Classification: undetermined required-gate status
• Action taken: capped confidence; cannot LGTM per skill rules

Findings

💡 Suggestion — Add explicit duplicate-transform reference-count coverage

src/Controls/src/Core/Shapes/TransformGroup.cs:120 implements the key count + 1 path for duplicate transforms. Please add a test where the same ScaleTransform is added twice, removed once, still updates the group, then removed again and no longer retains/subscribes.

Failure-Mode Probing

• Replacing Children with a long-lived shared child: old collection and children are detached; leak path is addressed.
• Children.Clear(): Reset clears tracked subscriptions, covering OldItems == null.
• Duplicate transform entries: reference count keeps one subscription until final removal.

Verdict: NEEDS_DISCUSSION

Confidence: low
Summary: The code approach looks sound, with one test-coverage suggestion. Verdict is not LGTM only because required CI status could not be verified and one public agent check failed.

---

🛠️ Fix — Analysis & Comparison

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix	Reconcile a unique subscription set from the current Children collection after every collection change.	✅ PASS	1 file	Simpler reset/replace path, but more allocation/scanning than PR fix; not demonstrably better.
2	try-fix	Track one subscription ledger entry per collection occurrence with List<INotifyPropertyChanged>.	✅ PASS	1 file	Minimal delta change, but duplicate transforms can still produce duplicate callbacks; not better than PR fix.
PR	PR #36150	Reference-count child PropertyChanged subscriptions with Dictionary<INotifyPropertyChanged, int>, attach/detach on Children replacement, and clear/rebuild on Reset.	✅ PASSED (Gate)	2 files	Original PR; gate result supplied by caller.

Cross-Pollination

Model	Round	New Ideas?	Details
gpt-5.5 / maui-expert-reviewer	1	Yes	Reconcile subscriptions from current collection state with a HashSet; implemented and tested as candidate #1.
gpt-5.5 / maui-expert-reviewer	2	Yes	Per-occurrence subscription ledger with a List; implemented and tested as candidate #2.
gpt-5.5 / maui-expert-reviewer	3	No	No meaningful new candidate: collection-owned cleanup, weak events, ownership/lifecycle cleanup, and clear-as-remove notifications are riskier or only move the same bookkeeping elsewhere.
claude-opus-4.6 / maui-expert-reviewer	3	No	PR ref-count dictionary dominates tested attempts: no per-change scan/allocation and no duplicate callbacks; weak/proxy/pull approaches add infrastructure or change semantics.
claude-opus-4.7 / maui-expert-reviewer	3	No	Dictionary reference-counting is canonical for O(1) delta subscribe/unsubscribe with one subscription per unique object and clean reset/replacement teardown.
gpt-5.3-codex / maui-expert-reviewer	3	No	PR approach is best O(delta) design; alternatives are slower, riskier, or larger architectural churn without clear gain.

Exhausted: Yes
Selected Fix: PR #36150 — both implemented alternatives passed the focused regression tests, but neither was demonstrably better. Candidate #1 is simpler but scans/allocates on every collection change. Candidate #2 is smaller but preserves duplicate callback behavior. Expert cross-pollination found no additional meaningfully different approach likely to beat the PR's reference-counted subscription design.

---

📝 Recommended PR Title & Description

Assessment: ✏️ Recommend updating — the current title still has a [WIP] prefix and the description duplicates the Testing Enhancements section.

Recommended title

[Controls] Shapes: Fix TransformGroup child subscription leaks

Recommended description

### Issue Details

`TransformGroup.Children` replacement and `Clear()` can leak child transform `PropertyChanged` subscriptions.

### Root Cause of the issue

`TransformGroup` subscribes to each child transform's `PropertyChanged` event when the child is added to the `Children` collection through the collection change handler.

There are two leak scenarios:

1. **Collection Replacement** (`Children = new TransformCollection()`)

   The previous `Children` change callback only:
   - Unsubscribed from the old collection's `CollectionChanged` event
   - Subscribed to the new collection's `CollectionChanged` event

   It did not:
   - Iterate old collection items to unsubscribe their `PropertyChanged` handlers
   - Iterate new collection items to subscribe their `PropertyChanged` handlers

2. **Collection Clear** (`Children.Clear()`)

   `ObservableCollection` raises `CollectionChanged` with `Action = Reset` and `OldItems = null`. The previous handler only checked `OldItems` and `NewItems`, so it skipped child `PropertyChanged` unsubscription on `Clear()`.

In both cases, old child transforms can keep a delegate reference to `TransformGroup.OnTransformPropertyChanged`. If the child transform is long-lived, shared, cached, or static, it can retain the `TransformGroup`, associated `Path`, and the `Path` binding context.

### Description of Change

- Added reference-counted child transform subscriptions with `Dictionary<INotifyPropertyChanged, int>` so a transform that appears multiple times in the same collection has one event subscription that remains attached until the last occurrence is removed.
- Added `AttachCollection` and `DetachCollection` helpers so `Children` replacement unsubscribes child transforms from the old collection and subscribes child transforms from the new collection.
- Added explicit `NotifyCollectionChangedAction.Reset` handling in `OnChildrenCollectionChanged` so `Children.Clear()` clears tracked subscriptions and rebuilt collections keep current child subscriptions.

### Testing Enhancements

Added `TransformGroupTests` covering:

- Replacing `Children` unsubscribes old child transform `PropertyChanged` handlers
- Replacing `Children` subscribes child transforms already present in the new collection
- Shared transforms do not retain multiple detached `TransformGroup` instances
- Clearing `Children` unsubscribes all tracked child transforms

### Issues Fixed

Fixes #36033

### Tested the behaviour in the following platforms

- [x] Windows
- [x] Android
- [x] iOS
- [x] Mac

---

🏁 Report — Final Recommendation

Comparative Analysis — PR #36150

Candidates

Candidate	Approach	Regression result	Ranking
pr	Reference-counted Dictionary<INotifyPropertyChanged, int> tracks one event subscription per unique child transform, preserves duplicate occurrences, detaches all tracked children on collection replacement, and rebuilds subscriptions on Reset.	✅ Passed gate	1
pr-plus-reviewer	Same as pr; expert reviewer found no actionable changes to apply.	✅ Passed gate	1
try-fix-1	Reconciles a HashSet<INotifyPropertyChanged> from the current collection after every change, treating the current collection as source of truth.	✅ Passed focused tests	3
try-fix-2	Tracks one subscription ledger entry per collection occurrence using List<INotifyPropertyChanged>.	✅ Passed focused tests	4

Comparison

pr and pr-plus-reviewer are functionally identical because the expert review produced no actionable findings. They are the strongest candidates: the implementation is incremental for normal add/remove changes, handles Reset and replacement explicitly, subscribes pre-populated replacement collections, and avoids duplicate callbacks while preserving correct lifetime semantics for duplicate transform entries.

try-fix-1 is viable and simpler conceptually because it reconciles from the current collection state for all changes. However, it scans the whole collection and allocates reconciliation sets/lists for every collection change, including simple add/remove operations. It is not better than the PR's O(delta) path for normal changes.

try-fix-2 is also viable, but it intentionally keeps one event subscription per duplicate collection occurrence. That preserves historical duplicate callback behavior and can recompute the matrix multiple times for a single child property change. The PR's reference-counting is preferable because it keeps duplicate lifetime semantics without duplicate callbacks.

No candidate failed regression tests. If any had failed, it would rank below the passing candidates regardless of design preference.

Winning candidate

Winner: pr

The raw PR fix wins because it is the same as pr-plus-reviewer after expert review, passes the supplied gate, and has the best tradeoff among passing candidates: precise subscription lifetime management, explicit Reset/replacement handling, and no avoidable duplicate callbacks or per-change full reconciliation cost.

---

🧭 Next Steps — review latest findings

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

[Copilot]

Pull request overview

Fixes a memory-leak scenario in TransformGroup where child PropertyChanged subscriptions could remain attached after Children collection replacement or Clear(), preventing old TransformGroup instances (and potentially their owning Path/BindingContext) from being collected.

Changes:

• Add reference-counted tracking for child PropertyChanged subscriptions and ensure attach/detach is handled on Children replacement.
• Handle NotifyCollectionChangedAction.Reset to correctly unsubscribe when Children.Clear() occurs.
• Add unit tests validating collection replacement and clear do not retain old TransformGroup instances (including shared-transform scenarios).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/Controls/tests/Core.UnitTests/Shapes/TransformGroupTests.cs	Adds regression tests to verify TransformGroup instances are collectible after Children replacement/clear and with shared transforms.
src/Controls/src/Core/Shapes/TransformGroup.cs	Fixes event subscription lifecycle by attaching/detaching child subscriptions (including Reset/Clear handling) and reference-counting repeated transforms.