[leak-fix] Fix ListView.RefreshCommand memory leak (Fixes #36344) - #36369
Conversation
ListView subscribed to RefreshCommand.CanExecuteChanged with a plain instance handler, so a long-lived ICommand whose CanExecuteChanged is a strong .NET event rooted the entire ListView and it was never released on unload. Replace the direct subscription with a WeakCommandSubscription (the same hardening already used by CommandElement, Button and RefreshView) so the command no longer keeps the ListView alive. Adds a Controls.Core.UnitTests regression test that fails without this fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@github-actions[bot] — new AI review results are available based on this last commit:
fb484f8. To request a fresh review after new comments or commits, comment/review rerun.
🔍 Custom Prompt Analysis — maintainer-requested
● AI-generated — supplementary analysis from a maintainer-supplied custom prompt (GitHub Copilot CLI).
Requested: Does this ListView.RefreshCommand leak fix fully cover command re-assignment/re-use, and are the memory tests adequate? Be concise.
ListView.RefreshCommand Leak Fix
Conclusion: The implementation looks sound for normal command reassignment and shared-command reuse, but the new memory test only proves the single-command leak scenario.
Command Reassignment / Reuse
- Reassignment is handled by disposing the previous
_refreshCommandSubscriptionbefore subscribing to the new command. - Clearing
RefreshCommandalso disposes the subscription and restoresRefreshAllowed = true. - Reusing the same long-lived command across multiple
ListViewinstances should be safe: eachWeakCommandSubscriptionweakly ties the callback to its ownListView, so a command event should not strongly root disposed/collected views. - Minor robustness note: disposal is guarded by
oldCommand != null; this matches normal bindable-property transitions, though the nearbySearchHandlerpattern disposes unconditionally.
Test Adequacy
- The new test is valuable: it uses a strong
CanExecuteChangedevent command, keeps the command alive, and verifies theListViewcan be collected. - It does not directly cover reassignment from command A to command B.
- It does not cover shared command reuse across multiple
ListViewinstances where one view is collected and another remains active. - Existing clear-subscription coverage uses
Microsoft.Maui.Controls.Command, whose weak event path would not reproduce the strong-event leak.
Recommended Follow-up
Add one focused test with a strong-event command for reassignment/clear, and optionally one shared-command test to prove one collected ListView does not affect another still using the same command.
🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix
Gate Result: ✅ PASSED
Platform: ANDROID · Base: main · Merge base: dab8abc4
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🧪 ListViewTests ListViewTests |
✅ FAIL — 106s | ✅ PASS — 81s |
🔴 Without fix — 🧪 ListViewTests: FAIL ✅ · 106s
Determining projects to restore...
Restored /home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj (in 5.59 sec).
Restored /home/vsts/work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 2.54 sec).
Restored /home/vsts/work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 8.97 sec).
Restored /home/vsts/work/1/s/src/TestUtils/src/TestUtils/TestUtils.csproj (in 5 ms).
Restored /home/vsts/work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 44 ms).
Restored /home/vsts/work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 10 ms).
Restored /home/vsts/work/1/s/src/Essentials/src/Essentials.csproj (in 31 ms).
Restored /home/vsts/work/1/s/src/Core/src/Core.csproj (in 64 ms).
Restored /home/vsts/work/1/s/src/Core/maps/src/Maps.csproj (in 29 ms).
1 of 10 projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14587797
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14587797
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14587797
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.14587797
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0/Microsoft.Maui.Maps.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14587797
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.14587797
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.14587797
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.21] Discovered: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.23] Starting: Microsoft.Maui.Controls.Core.UnitTests
Passed HeaderTemplatedSetToNull [35 ms]
Passed FooterViewButTemplated [7 ms]
Passed StopsListeningToCommandAfterCleared [2 ms]
Passed Tapping a different item (row) that is equal to the current item selection should still raise ItemSelected [12 ms]
Passed ScrollToDelayed [5 ms]
Passed HeaderTemplated [2 ms]
Passed SendRefreshingCanBind [2 ms]
Passed BeginRefreshDoesNothingWhenCannotExecute [< 1 ms]
Passed GetSizeRequest [6 ms]
Passed HeaderNoTemplate [< 1 ms]
Passed DefaultCacheStrategy [2 ms]
Passed RefreshCommandCanExecuteChanges(initial: False) [3 ms]
Passed RefreshCommandCanExecuteChanges(initial: True) [< 1 ms]
Passed ItemTemplateIsNullObjectExecutesToString [3 ms]
Passed FooterTemplateChanged [1 ms]
Passed HeaderTemplatedChangedToView [< 1 ms]
Passed HeaderTemplateChanged [< 1 ms]
Passed ForceUpdateSizeCalledOnViewCellDoesntCrash [28 ms]
Passed FooterTemplatedSetToNull [< 1 ms]
Passed DoesNotThrowWhenMovingInSource [3 ms]
Passed FooterViewChanged [< 1 ms]
Passed RefreshCommand [< 1 ms]
Passed GroupedCollectionChangedMultipleFires [20 ms]
Passed HeaderViewChanged [< 1 ms]
Passed EnforcesCachingStrategy(platform: "Android", expected: RecycleElement) [< 1 ms]
Passed EnforcesCachingStrategy(platform: "UWP", expected: RetainElement) [< 1 ms]
Passed EnforcesCachingStrategy(platform: "Other", expected: RetainElement) [< 1 ms]
[xUnit.net 00:00:02.73] RefreshCommandDoesNotLeakListView [FAIL]
[xUnit.net 00:00:02.73] ListView should not be alive!
[xUnit.net 00:00:02.74] Stack Trace:
[xUnit.net 00:00:02.74] /_/src/Controls/tests/Core.UnitTests/ListViewTests.cs(59,0): at Microsoft.Maui.Controls.Core.UnitTests.ListViewTests.RefreshCommandDoesNotLeakListView()
[xUnit.net 00:00:02.74] --- End of stack trace from previous location ---
Passed EnforcesCachingStrategy(platform: "iOS", expected: RecycleElement) [< 1 ms]
Passed TemplateNullObject [< 1 ms]
Passed We should be able to set selected item when using ReadOnlyList [1 ms]
Passed ChildElementsParentIsNulledWhenRemoved [< 1 ms]
Passed DoesNotRetainInRecycleMode [< 1 ms]
Passed DoesNotCrashWhenAddingToSource [< 1 ms]
Failed RefreshCommandDoesNotLeakListView [1 s]
Error Message:
ListView should not be alive!
Stack Trace:
at Microsoft.Maui.Controls.Core.UnitTests.ListViewTests.RefreshCommandDoesNotLeakListView() in /_/src/Controls/tests/Core.UnitTests/ListViewTests.cs:line 59
--- End of stack trace from previous location ---
Passed HeaderViewButTemplated [< 1 ms]
Passed EndRefresh [< 1 ms]
Passed CanRefreshAfterCantExecuteCommand [< 1 ms]
Passed FooterTemplated [< 1 ms]
Passed ScrollToInvalid [1 ms]
Passed ScrollTo [< 1 ms]
Passed HeaderAsView [< 1 ms]
Passed FooterTemplateChangedNoObject [< 1 ms]
Passed FooterObjectTemplatedChanged [1 ms]
Passed A cell being tapped from the UI should raise both tapped events, but not change ItemSelected [1 ms]
Passed You should be able to set ItemsSource without having set the other properties first without issue [< 1 ms]
Passed FooterTemplatedChangedToView [< 1 ms]
Passed CollectionChangedMultipleFires [< 1 ms]
Passed HeaderTemplateChangedNoObject [< 1 ms]
Passed Setting GroupDisplayBinding or GroupHeaderTemplate when the other is set should set the other one to null. [< 1 ms]
Passed UncollectableHeaderReferences [137 ms]
Passed Setting Header and Footer should pass BindingContext. [1 ms]
Passed TestTemplating [< 1 ms]
Passed FooterAsView [< 1 ms]
Passed ChildElementsParentIsNulledWhenReset [< 1 ms]
Passed RefreshCommandCanExecute(initial: False) [< 1 ms]
Passed RefreshCommandCanExecute(initial: True) [< 1 ms]
Passed DefaultGroupHeaderTemplates [< 1 ms]
Passed TestConstructor [3 ms]
Passed FooterChangedNoTemplate [< 1 ms]
Passed BeginRefresh [< 1 ms]
Passed GetSizeRequestUneven [< 1 ms]
Passed HeaderObjectTemplatedChanged [< 1 ms]
Passed ScrollToGroup [< 1 ms]
[xUnit.net 00:00:02.90] Finished: Microsoft.Maui.Controls.Core.UnitTests
Passed HeaderChangedNoTemplate [< 1 ms]
Passed Setting BindingContext should trickle down to Header and Footer. [< 1 ms]
Passed FooterNoTemplate [< 1 ms]
Passed SendRefreshingDoesNothingWhenCannotExecute [< 1 ms]
Passed SettingSelectItemManuallyBetweenTwoTapsFiresPropertyChanged [< 1 ms]
Passed SendRefreshing [< 1 ms]
Passed SettingIsRefreshingDoesntFireEvent [< 1 ms]
Passed Setting GroupDisplayBinding or GroupHeaderTemplate when the other is set should set the other one to null. [< 1 ms]
Passed ChildElementsParentIsNulledWhenCleared [< 1 ms]
Test Run Failed.
Total tests: 72
Passed: 71
Failed: 1
Total time: 3.3974 Seconds
🟢 With fix — 🧪 ListViewTests: PASS ✅ · 81s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14587797
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14587797
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14587797
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.14587797
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14587797
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0/Microsoft.Maui.Maps.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14587797
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.14587797
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.22] Discovered: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:01.25] Starting: Microsoft.Maui.Controls.Core.UnitTests
Passed HeaderTemplatedSetToNull [34 ms]
Passed FooterViewButTemplated [6 ms]
Passed StopsListeningToCommandAfterCleared [2 ms]
Passed Tapping a different item (row) that is equal to the current item selection should still raise ItemSelected [11 ms]
Passed ScrollToDelayed [4 ms]
Passed HeaderTemplated [2 ms]
Passed SendRefreshingCanBind [2 ms]
Passed BeginRefreshDoesNothingWhenCannotExecute [< 1 ms]
Passed GetSizeRequest [6 ms]
Passed HeaderNoTemplate [< 1 ms]
Passed DefaultCacheStrategy [2 ms]
Passed RefreshCommandCanExecuteChanges(initial: False) [3 ms]
Passed RefreshCommandCanExecuteChanges(initial: True) [< 1 ms]
Passed ItemTemplateIsNullObjectExecutesToString [3 ms]
Passed FooterTemplateChanged [1 ms]
Passed HeaderTemplatedChangedToView [1 ms]
Passed HeaderTemplateChanged [< 1 ms]
Passed ForceUpdateSizeCalledOnViewCellDoesntCrash [25 ms]
Passed FooterTemplatedSetToNull [< 1 ms]
Passed DoesNotThrowWhenMovingInSource [3 ms]
Passed FooterViewChanged [< 1 ms]
Passed RefreshCommand [< 1 ms]
Passed GroupedCollectionChangedMultipleFires [18 ms]
Passed HeaderViewChanged [< 1 ms]
Passed EnforcesCachingStrategy(platform: "Android", expected: RecycleElement) [< 1 ms]
Passed EnforcesCachingStrategy(platform: "UWP", expected: RetainElement) [< 1 ms]
Passed EnforcesCachingStrategy(platform: "Other", expected: RetainElement) [< 1 ms]
Passed EnforcesCachingStrategy(platform: "iOS", expected: RecycleElement) [< 1 ms]
Passed TemplateNullObject [< 1 ms]
Passed We should be able to set selected item when using ReadOnlyList [2 ms]
Passed ChildElementsParentIsNulledWhenRemoved [< 1 ms]
Passed DoesNotRetainInRecycleMode [< 1 ms]
Passed DoesNotCrashWhenAddingToSource [< 1 ms]
Passed RefreshCommandDoesNotLeakListView [34 ms]
Passed HeaderViewButTemplated [< 1 ms]
Passed EndRefresh [< 1 ms]
Passed CanRefreshAfterCantExecuteCommand [< 1 ms]
Passed FooterTemplated [< 1 ms]
Passed ScrollToInvalid [1 ms]
Passed ScrollTo [< 1 ms]
Passed HeaderAsView [< 1 ms]
Passed FooterTemplateChangedNoObject [< 1 ms]
Passed FooterObjectTemplatedChanged [< 1 ms]
Passed A cell being tapped from the UI should raise both tapped events, but not change ItemSelected [1 ms]
Passed You should be able to set ItemsSource without having set the other properties first without issue [< 1 ms]
Passed FooterTemplatedChangedToView [< 1 ms]
Passed CollectionChangedMultipleFires [< 1 ms]
Passed HeaderTemplateChangedNoObject [< 1 ms]
Passed Setting GroupDisplayBinding or GroupHeaderTemplate when the other is set should set the other one to null. [< 1 ms]
Passed UncollectableHeaderReferences [108 ms]
Passed Setting Header and Footer should pass BindingContext. [1 ms]
Passed TestTemplating [1 ms]
Passed FooterAsView [< 1 ms]
Passed ChildElementsParentIsNulledWhenReset [< 1 ms]
Passed RefreshCommandCanExecute(initial: False) [< 1 ms]
Passed RefreshCommandCanExecute(initial: True) [< 1 ms]
Passed DefaultGroupHeaderTemplates [< 1 ms]
Passed TestConstructor [3 ms]
Passed FooterChangedNoTemplate [< 1 ms]
Passed BeginRefresh [< 1 ms]
Passed GetSizeRequestUneven [< 1 ms]
Passed HeaderObjectTemplatedChanged [< 1 ms]
Passed ScrollToGroup [< 1 ms]
[xUnit.net 00:00:01.63] Finished: Microsoft.Maui.Controls.Core.UnitTests
Passed HeaderChangedNoTemplate [< 1 ms]
Passed Setting BindingContext should trickle down to Header and Footer. [< 1 ms]
Passed FooterNoTemplate [< 1 ms]
Passed SendRefreshingDoesNothingWhenCannotExecute [< 1 ms]
Passed SettingSelectItemManuallyBetweenTwoTapsFiresPropertyChanged [< 1 ms]
Passed SendRefreshing [< 1 ms]
Passed SettingIsRefreshingDoesntFireEvent [< 1 ms]
Passed Setting GroupDisplayBinding or GroupHeaderTemplate when the other is set should set the other one to null. [< 1 ms]
Passed ChildElementsParentIsNulledWhenCleared [< 1 ms]
Test Run Successful.
Total tests: 72
Passed: 72
Total time: 2.1568 Seconds
📁 Fix files reverted (1 files)
src/Controls/src/Core/ListView/ListView.cs
📱 UI Tests — ListView
Detected UI test categories: ListView
[!WARNING]
No UI test results were produced for the detected categories. The platform-pool run
returned no results — most often because the PR build failed (see the Gate section) or
the deep UI test stage was skipped. Fix the build/gate issues and comment/review rerun
to get UI test results.
📋 Pre-Flight — Context & Validation
Issue: #36344 - [leak-scan] ListView.RefreshCommand leaks the control via strong ICommand.CanExecuteChanged (no weak proxy, no unload teardown)
PR: #36369 - [leak-fix] Fix ListView.RefreshCommand memory leak (Fixes #36344)
Platforms Affected: android requested for testing; product change is managed Controls code and affects all platforms using ListView.RefreshCommand.
Files Changed: 1 implementation, 1 test
Key Findings
- Issue #36344 reports that
ListView.RefreshCommandsubscribed directly toICommand.CanExecuteChanged, so a long-lived command with a strong .NET event could retain theListView, cells, templates, and binding context after navigation unlessRefreshCommandwas reassigned. - PR #36369 replaces the direct strong event subscription with MAUI's existing
WeakCommandSubscriptionhelper and disposes the subscription whenRefreshCommandchanges or is cleared. - The PR adds
RefreshCommandDoesNotLeakListView, a Controls Core unit regression using a custom strong-eventICommand; the caller-provided gate result already proved the test fails without the fix and passes with the PR fix. - Public PR/issue discussion available without
ghauth contained only a/reviewrequest and no technical objections. Inline comments were empty. Prior MauiBot review summary reported no actionable findings. - The impacted UI test category is
ListView; existing local gate artifacts anduitests/content.mdwere left untouched.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 0
Key code review findings:
- No actionable implementation findings. The PR's localized weak-command subscription matches existing MAUI command patterns used by sibling controls.
- Required-check verification through
gh pr checks --requiredwas unavailable becauseghis unauthenticated in this environment, so code-review confidence is capped low and the verdict cannot be LGTM. - Failure-mode probes: long-lived commands no longer root
ListView; command replacement/clear disposes the prior subscription;CanExecuteChangedbehavior remains covered by existing unit tests; no startup/static state impact.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #36369 | Store a per-ListView WeakCommandSubscription for RefreshCommand; dispose it on command replacement/clear. |
PASSED (Gate) | src/Controls/src/Core/ListView/ListView.cs, src/Controls/tests/Core.UnitTests/ListViewTests.cs |
Original PR; gate already completed and was not re-run. |
🔬 Code Review — Deep Analysis
Code Review - PR #36369
Independent Assessment
What this changes: ListView.RefreshCommand now uses WeakCommandSubscription instead of directly subscribing OnCommandCanExecuteChanged to ICommand.CanExecuteChanged. A regression test verifies a long-lived strong-event command does not keep ListView alive.
Inferred motivation: Prevent RefreshCommand.CanExecuteChanged from strongly rooting disposed/unreferenced ListView instances while preserving RefreshAllowed updates.
Reconciliation with PR Narrative
Author claims: Fixes a ListView.RefreshCommand memory leak with strong-event ICommand implementations; uses existing MAUI weak command infrastructure; adds red/green leak coverage.
Agreement/disagreement: Agrees. The implementation at src/Controls/src/Core/ListView/ListView.cs:100 and :768-775 matches the stated fix. The test at src/Controls/tests/Core.UnitTests/ListViewTests.cs:39-61 targets the described leak.
Prior Review Reconciliation
No prior error findings found. Prior MauiBot review reported zero errors/warnings/suggestions; inline review comments were empty; issue comments contained only /review.
Blast Radius Assessment
- Runs for all instances: Only
ListViewinstances withRefreshCommandset. - Startup impact: No.
- Static/shared state: No.
- Scope: Managed Controls-level command subscription; no platform handler changes.
CI Status
- Required-check result: unavailable -
gh pr checks 36369 --repo dotnet/maui --requiredreturned exit 4 becauseghis unauthenticated. - Classification: undetermined.
- Action taken: capped confidence low; no LGTM per skill rule for undetermined CI.
Findings
No actionable Error, Warning, or Suggestion findings.
Failure-Mode Probing
- Command outlives
ListView:WeakCommandSubscriptionusesDependentHandle, so the command event roots the proxy, not theListView. - Command changes/clears: old subscription is disposed when
oldCommand != null, matching the property-change lifecycle. CanExecuteChangedafter collection: proxy disposes itself when the primary bindable object is gone.- Existing refresh behavior:
RefreshAllowed = newCommand.CanExecute(null)andOnCommandCanExecuteChangedbehavior are preserved.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: Code review found no implementation issues, and the approach is consistent with existing MAUI weak-command patterns. However, required CI status could not be verified in this environment, so the skill rules prohibit LGTM.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Static per-command weak subscriber registry keyed by ICommand; forwards to weak ListView references. |
PASS - 72/72 | 1 file | Passes, but worse than PR due to global state, locking, and multi-subscriber cleanup complexity. |
| 2 | try-fix | Self-removing weak event delegate closure per command assignment. | PASS - 72/72 | 1 file | Passes, but worse than PR because stale delegates remain on old commands until they raise again. |
| 3 | try-fix | Strong subscription scoped to handler lifecycle; subscribe only while handler-connected. | FAIL - 70/72 | 1 file | Fails RefreshCommandCanExecuteChanges because headless/no-handler CanExecuteChanged no longer updates RefreshAllowed. |
| PR | PR #36369 | Per-ListView WeakCommandSubscription field using existing MAUI helper; dispose on command replacement/clear. |
PASSED (Gate) | 2 files | Original PR; simplest passing candidate and uses existing pattern. |
Cross-Pollination
| Model/Reviewer | Round | New Ideas? | Details |
|---|---|---|---|
| maui-expert-reviewer | 1 | Yes | Proposed candidate 1: static per-command weak subscriber registry. |
| maui-expert-reviewer | 2 | Yes | After candidate 1 passed but was not better, proposed candidate 2: self-removing weak delegate closure. |
| maui-expert-reviewer | 3 | Yes | After candidate 2 passed but was not better, proposed candidate 3: handler-lifecycle-scoped strong subscription. |
| maui-expert-reviewer | Exhaustion | No | "NO NEW IDEAS": remaining realistic variants collapse into per-instance weak subscription with explicit disposal, or lose correctness/delay cleanup/add global mutable state/depend on handler lifecycle. |
Exhausted: Yes
Selected Fix: PR #36369 - The PR's WeakCommandSubscription approach is the best candidate. It passes the gate, preserves headless CanExecuteChanged behavior, disposes promptly on command replacement/clear, avoids global shared state, and reuses an existing MAUI helper rather than introducing custom weak-event machinery.
Test Command Used for Candidates
dotnet test src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj --no-restore --filter "FullyQualifiedName~ListViewTests" --logger "console;verbosity=normal"Candidate Artifacts
try-fix-1/content.md,try-fix-1/fix.diff,try-fix-1/test-output.logtry-fix-2/content.md,try-fix-2/fix.diff,try-fix-2/test-output.logtry-fix-3/content.md,try-fix-3/fix.diff,try-fix-3/test-output.log
🏁 Report — Final Recommendation
Comparative Fix Report — PR #36369
Candidates
| Rank | Candidate | Test Result | Assessment |
|---|---|---|---|
| 1 | pr |
PASSED (Gate) | Best candidate. Uses MAUI's existing WeakCommandSubscription, keeps ownership local to the ListView, disposes promptly on command replacement/clear, and preserves headless/control-level CanExecuteChanged behavior. |
| 2 | pr-plus-reviewer |
PASSED (same as pr) |
Expert reviewer found no actionable changes, so this is identical to pr. It ties with pr technically but does not improve on the submitted fix. |
| 3 | try-fix-2 |
PASS - 72/72 | Uses a self-removing weak delegate closure. It passes tests, but replaced/cleared commands retain stale delegates until they raise CanExecuteChanged again, so cleanup is delayed and repeated replacements can accumulate closure objects. |
| 4 | try-fix-1 |
PASS - 72/72 | Uses a static per-command weak subscriber registry. It passes tests, but adds global mutable state, locking, weak-reference cleanup, and multi-subscriber bookkeeping for a problem the existing per-instance helper already solves. |
| 5 | try-fix-3 |
FAIL - 70/72 | Handler-lifecycle-scoped strong subscription fixes the leak case but regresses existing RefreshCommandCanExecuteChanges behavior for no-handler/headless controls. Failed regression candidates must rank below passing candidates. |
Winning candidate
Winner: pr
The raw PR fix is the best solution. It is the smallest passing implementation, reuses the existing WeakCommandSubscription pattern used elsewhere in Controls, avoids new global/shared state, unsubscribes promptly when the command changes, and preserves current RefreshAllowed update semantics. The expert reviewer had no actionable feedback, so pr-plus-reviewer is functionally identical and does not supersede the original PR.
Notes
- Gate result supplied by the orchestrator: tests fail without the fix and pass with the PR fix.
- Required PR checks could not be independently queried because
ghis unauthenticated in this environment; this limits review confidence but does not change the candidate ranking because the comparative gate/candidate test results are already available.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
…6369) > [!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! > 🤖 **AI-generated.** This pull request was produced automatically by the **Memory Leak Fixer** agentic workflow (`leak-fixer`) for dotnet/maui. It contains a regression test empirically proven to fail without the fix and pass with it (red → green on this run's `main` source). Fixes #36344 Refs: #36344 Target branch: main Attempt: 1/3 ## The leak `ListView.OnRefreshCommandChanged` subscribed to `RefreshCommand.CanExecuteChanged` with a plain instance handler: ```csharp newCommand.CanExecuteChanged += OnCommandCanExecuteChanged; ``` Because `OnCommandCanExecuteChanged` is an instance method, the delegate's target is the `ListView`. When the assigned `ICommand` is long-lived (a singleton/DI/static command, or a reused view-model command) **and** exposes `CanExecuteChanged` as a strong .NET event, that event's invocation list roots the entire `ListView` — including its cells, templates and binding context. The matching unsubscribe (`-=`) only runs when `RefreshCommand` is reassigned, never when the `ListView` is simply unloaded, so the control is never released. > `Microsoft.Maui.Controls.Command` routes `CanExecuteChanged` through a `WeakEventManager`, so it does **not** reproduce the leak. The leak surfaces with any custom `ICommand` (e.g. CommunityToolkit's `RelayCommand`) that raises the event directly — which is exactly what the issue reports. ## The fix Replace the direct subscription with a `WeakCommandSubscription` — the same weak-subscription helper already used by `CommandElement`, `Button` and `RefreshView`: ```csharp _refreshCommandSubscription = new WeakCommandSubscription(this, newCommand, OnCommandCanExecuteChanged); ``` On reassignment the previous subscription is disposed. `WeakCommandSubscription` holds only a weak reference to the `ListView` (via `DependentHandle`), so the command can no longer keep it alive, while `CanExecuteChanged` is still delivered for as long as the `ListView` is reachable. No public API changes. ## Regression test `RefreshCommandDoesNotLeakListView` (in `Controls.Core.UnitTests/ListViewTests.cs`) assigns a custom strong-event `ICommand` to `ListView.RefreshCommand`, drops the only strong reference to the `ListView`, forces GC and asserts the `ListView` was collected while the command stays alive. Empirical red → green on this run's source: - **Without the fix (red):** `Failed RefreshCommandDoesNotLeakListView [895 ms]` — `ListView should not be alive!` (Failed: 1) - **With the fix (green):** `Passed RefreshCommandDoesNotLeakListView [47 ms]` (Passed: 1, Test Run Successful) - **No regressions:** full `ListViewTests` class — Total tests: 71, Passed: 71 (including `RefreshCommandCanExecute` and `SendRefreshingDoesNothingWhenCannotExecute`, which exercise the `CanExecute` path) ## Scope Managed, cross-platform change only — one product file (`src/Controls/src/Core/ListView/ListView.cs`) plus the test. No existing test was muted, skipped or weakened; no behavioural change to `RefreshCommand`/`CanExecute` handling. > Generated by [Memory Leak Fixer](https://github.com/dotnet/maui/actions/runs/28693580490) · 2.4K AIC · ⌖ 101 AIC · ⊞ 60.6K · [◷](https://github.com/search?q=repo%3Adotnet%2Fmaui+%22gh-aw-workflow-id%3A+leak-fixer%22&type=pullrequests) <!-- gh-aw-agentic-workflow: Memory Leak Fixer, engine: copilot, version: 1.0.60, model: claude-opus-4.8, id: 28693580490, workflow_id: leak-fixer, run: https://github.com/dotnet/maui/actions/runs/28693580490 --> <!-- gh-aw-workflow-id: leak-fixer --> <!-- gh-aw-workflow-call-id: dotnet/maui/leak-fixer --> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…6369) > [!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! > 🤖 **AI-generated.** This pull request was produced automatically by the **Memory Leak Fixer** agentic workflow (`leak-fixer`) for dotnet/maui. It contains a regression test empirically proven to fail without the fix and pass with it (red → green on this run's `main` source). Fixes #36344 Refs: #36344 Target branch: main Attempt: 1/3 ## The leak `ListView.OnRefreshCommandChanged` subscribed to `RefreshCommand.CanExecuteChanged` with a plain instance handler: ```csharp newCommand.CanExecuteChanged += OnCommandCanExecuteChanged; ``` Because `OnCommandCanExecuteChanged` is an instance method, the delegate's target is the `ListView`. When the assigned `ICommand` is long-lived (a singleton/DI/static command, or a reused view-model command) **and** exposes `CanExecuteChanged` as a strong .NET event, that event's invocation list roots the entire `ListView` — including its cells, templates and binding context. The matching unsubscribe (`-=`) only runs when `RefreshCommand` is reassigned, never when the `ListView` is simply unloaded, so the control is never released. > `Microsoft.Maui.Controls.Command` routes `CanExecuteChanged` through a `WeakEventManager`, so it does **not** reproduce the leak. The leak surfaces with any custom `ICommand` (e.g. CommunityToolkit's `RelayCommand`) that raises the event directly — which is exactly what the issue reports. ## The fix Replace the direct subscription with a `WeakCommandSubscription` — the same weak-subscription helper already used by `CommandElement`, `Button` and `RefreshView`: ```csharp _refreshCommandSubscription = new WeakCommandSubscription(this, newCommand, OnCommandCanExecuteChanged); ``` On reassignment the previous subscription is disposed. `WeakCommandSubscription` holds only a weak reference to the `ListView` (via `DependentHandle`), so the command can no longer keep it alive, while `CanExecuteChanged` is still delivered for as long as the `ListView` is reachable. No public API changes. ## Regression test `RefreshCommandDoesNotLeakListView` (in `Controls.Core.UnitTests/ListViewTests.cs`) assigns a custom strong-event `ICommand` to `ListView.RefreshCommand`, drops the only strong reference to the `ListView`, forces GC and asserts the `ListView` was collected while the command stays alive. Empirical red → green on this run's source: - **Without the fix (red):** `Failed RefreshCommandDoesNotLeakListView [895 ms]` — `ListView should not be alive!` (Failed: 1) - **With the fix (green):** `Passed RefreshCommandDoesNotLeakListView [47 ms]` (Passed: 1, Test Run Successful) - **No regressions:** full `ListViewTests` class — Total tests: 71, Passed: 71 (including `RefreshCommandCanExecute` and `SendRefreshingDoesNothingWhenCannotExecute`, which exercise the `CanExecute` path) ## Scope Managed, cross-platform change only — one product file (`src/Controls/src/Core/ListView/ListView.cs`) plus the test. No existing test was muted, skipped or weakened; no behavioural change to `RefreshCommand`/`CanExecute` handling. > Generated by [Memory Leak Fixer](https://github.com/dotnet/maui/actions/runs/28693580490) · 2.4K AIC · ⌖ 101 AIC · ⊞ 60.6K · [◷](https://github.com/search?q=repo%3Adotnet%2Fmaui+%22gh-aw-workflow-id%3A+leak-fixer%22&type=pullrequests) <!-- gh-aw-agentic-workflow: Memory Leak Fixer, engine: copilot, version: 1.0.60, model: claude-opus-4.8, id: 28693580490, workflow_id: leak-fixer, run: https://github.com/dotnet/maui/actions/runs/28693580490 --> <!-- gh-aw-workflow-id: leak-fixer --> <!-- gh-aw-workflow-call-id: dotnet/maui/leak-fixer --> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…6369) > [!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! > 🤖 **AI-generated.** This pull request was produced automatically by the **Memory Leak Fixer** agentic workflow (`leak-fixer`) for dotnet/maui. It contains a regression test empirically proven to fail without the fix and pass with it (red → green on this run's `main` source). Fixes #36344 Refs: #36344 Target branch: main Attempt: 1/3 ## The leak `ListView.OnRefreshCommandChanged` subscribed to `RefreshCommand.CanExecuteChanged` with a plain instance handler: ```csharp newCommand.CanExecuteChanged += OnCommandCanExecuteChanged; ``` Because `OnCommandCanExecuteChanged` is an instance method, the delegate's target is the `ListView`. When the assigned `ICommand` is long-lived (a singleton/DI/static command, or a reused view-model command) **and** exposes `CanExecuteChanged` as a strong .NET event, that event's invocation list roots the entire `ListView` — including its cells, templates and binding context. The matching unsubscribe (`-=`) only runs when `RefreshCommand` is reassigned, never when the `ListView` is simply unloaded, so the control is never released. > `Microsoft.Maui.Controls.Command` routes `CanExecuteChanged` through a `WeakEventManager`, so it does **not** reproduce the leak. The leak surfaces with any custom `ICommand` (e.g. CommunityToolkit's `RelayCommand`) that raises the event directly — which is exactly what the issue reports. ## The fix Replace the direct subscription with a `WeakCommandSubscription` — the same weak-subscription helper already used by `CommandElement`, `Button` and `RefreshView`: ```csharp _refreshCommandSubscription = new WeakCommandSubscription(this, newCommand, OnCommandCanExecuteChanged); ``` On reassignment the previous subscription is disposed. `WeakCommandSubscription` holds only a weak reference to the `ListView` (via `DependentHandle`), so the command can no longer keep it alive, while `CanExecuteChanged` is still delivered for as long as the `ListView` is reachable. No public API changes. ## Regression test `RefreshCommandDoesNotLeakListView` (in `Controls.Core.UnitTests/ListViewTests.cs`) assigns a custom strong-event `ICommand` to `ListView.RefreshCommand`, drops the only strong reference to the `ListView`, forces GC and asserts the `ListView` was collected while the command stays alive. Empirical red → green on this run's source: - **Without the fix (red):** `Failed RefreshCommandDoesNotLeakListView [895 ms]` — `ListView should not be alive!` (Failed: 1) - **With the fix (green):** `Passed RefreshCommandDoesNotLeakListView [47 ms]` (Passed: 1, Test Run Successful) - **No regressions:** full `ListViewTests` class — Total tests: 71, Passed: 71 (including `RefreshCommandCanExecute` and `SendRefreshingDoesNothingWhenCannotExecute`, which exercise the `CanExecute` path) ## Scope Managed, cross-platform change only — one product file (`src/Controls/src/Core/ListView/ListView.cs`) plus the test. No existing test was muted, skipped or weakened; no behavioural change to `RefreshCommand`/`CanExecute` handling. > Generated by [Memory Leak Fixer](https://github.com/dotnet/maui/actions/runs/28693580490) · 2.4K AIC · ⌖ 101 AIC · ⊞ 60.6K · [◷](https://github.com/search?q=repo%3Adotnet%2Fmaui+%22gh-aw-workflow-id%3A+leak-fixer%22&type=pullrequests) <!-- gh-aw-agentic-workflow: Memory Leak Fixer, engine: copilot, version: 1.0.60, model: claude-opus-4.8, id: 28693580490, workflow_id: leak-fixer, run: https://github.com/dotnet/maui/actions/runs/28693580490 --> <!-- gh-aw-workflow-id: leak-fixer --> <!-- gh-aw-workflow-call-id: dotnet/maui/leak-fixer --> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…6369) > [!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! > 🤖 **AI-generated.** This pull request was produced automatically by the **Memory Leak Fixer** agentic workflow (`leak-fixer`) for dotnet/maui. It contains a regression test empirically proven to fail without the fix and pass with it (red → green on this run's `main` source). Fixes #36344 Refs: #36344 Target branch: main Attempt: 1/3 ## The leak `ListView.OnRefreshCommandChanged` subscribed to `RefreshCommand.CanExecuteChanged` with a plain instance handler: ```csharp newCommand.CanExecuteChanged += OnCommandCanExecuteChanged; ``` Because `OnCommandCanExecuteChanged` is an instance method, the delegate's target is the `ListView`. When the assigned `ICommand` is long-lived (a singleton/DI/static command, or a reused view-model command) **and** exposes `CanExecuteChanged` as a strong .NET event, that event's invocation list roots the entire `ListView` — including its cells, templates and binding context. The matching unsubscribe (`-=`) only runs when `RefreshCommand` is reassigned, never when the `ListView` is simply unloaded, so the control is never released. > `Microsoft.Maui.Controls.Command` routes `CanExecuteChanged` through a `WeakEventManager`, so it does **not** reproduce the leak. The leak surfaces with any custom `ICommand` (e.g. CommunityToolkit's `RelayCommand`) that raises the event directly — which is exactly what the issue reports. ## The fix Replace the direct subscription with a `WeakCommandSubscription` — the same weak-subscription helper already used by `CommandElement`, `Button` and `RefreshView`: ```csharp _refreshCommandSubscription = new WeakCommandSubscription(this, newCommand, OnCommandCanExecuteChanged); ``` On reassignment the previous subscription is disposed. `WeakCommandSubscription` holds only a weak reference to the `ListView` (via `DependentHandle`), so the command can no longer keep it alive, while `CanExecuteChanged` is still delivered for as long as the `ListView` is reachable. No public API changes. ## Regression test `RefreshCommandDoesNotLeakListView` (in `Controls.Core.UnitTests/ListViewTests.cs`) assigns a custom strong-event `ICommand` to `ListView.RefreshCommand`, drops the only strong reference to the `ListView`, forces GC and asserts the `ListView` was collected while the command stays alive. Empirical red → green on this run's source: - **Without the fix (red):** `Failed RefreshCommandDoesNotLeakListView [895 ms]` — `ListView should not be alive!` (Failed: 1) - **With the fix (green):** `Passed RefreshCommandDoesNotLeakListView [47 ms]` (Passed: 1, Test Run Successful) - **No regressions:** full `ListViewTests` class — Total tests: 71, Passed: 71 (including `RefreshCommandCanExecute` and `SendRefreshingDoesNothingWhenCannotExecute`, which exercise the `CanExecute` path) ## Scope Managed, cross-platform change only — one product file (`src/Controls/src/Core/ListView/ListView.cs`) plus the test. No existing test was muted, skipped or weakened; no behavioural change to `RefreshCommand`/`CanExecute` handling. > Generated by [Memory Leak Fixer](https://github.com/dotnet/maui/actions/runs/28693580490) · 2.4K AIC · ⌖ 101 AIC · ⊞ 60.6K · [◷](https://github.com/search?q=repo%3Adotnet%2Fmaui+%22gh-aw-workflow-id%3A+leak-fixer%22&type=pullrequests) <!-- gh-aw-agentic-workflow: Memory Leak Fixer, engine: copilot, version: 1.0.60, model: claude-opus-4.8, id: 28693580490, workflow_id: leak-fixer, run: https://github.com/dotnet/maui/actions/runs/28693580490 --> <!-- gh-aw-workflow-id: leak-fixer --> <!-- gh-aw-workflow-call-id: dotnet/maui/leak-fixer --> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…6369) > [!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! > 🤖 **AI-generated.** This pull request was produced automatically by the **Memory Leak Fixer** agentic workflow (`leak-fixer`) for dotnet/maui. It contains a regression test empirically proven to fail without the fix and pass with it (red → green on this run's `main` source). Fixes #36344 Refs: #36344 Target branch: main Attempt: 1/3 ## The leak `ListView.OnRefreshCommandChanged` subscribed to `RefreshCommand.CanExecuteChanged` with a plain instance handler: ```csharp newCommand.CanExecuteChanged += OnCommandCanExecuteChanged; ``` Because `OnCommandCanExecuteChanged` is an instance method, the delegate's target is the `ListView`. When the assigned `ICommand` is long-lived (a singleton/DI/static command, or a reused view-model command) **and** exposes `CanExecuteChanged` as a strong .NET event, that event's invocation list roots the entire `ListView` — including its cells, templates and binding context. The matching unsubscribe (`-=`) only runs when `RefreshCommand` is reassigned, never when the `ListView` is simply unloaded, so the control is never released. > `Microsoft.Maui.Controls.Command` routes `CanExecuteChanged` through a `WeakEventManager`, so it does **not** reproduce the leak. The leak surfaces with any custom `ICommand` (e.g. CommunityToolkit's `RelayCommand`) that raises the event directly — which is exactly what the issue reports. ## The fix Replace the direct subscription with a `WeakCommandSubscription` — the same weak-subscription helper already used by `CommandElement`, `Button` and `RefreshView`: ```csharp _refreshCommandSubscription = new WeakCommandSubscription(this, newCommand, OnCommandCanExecuteChanged); ``` On reassignment the previous subscription is disposed. `WeakCommandSubscription` holds only a weak reference to the `ListView` (via `DependentHandle`), so the command can no longer keep it alive, while `CanExecuteChanged` is still delivered for as long as the `ListView` is reachable. No public API changes. ## Regression test `RefreshCommandDoesNotLeakListView` (in `Controls.Core.UnitTests/ListViewTests.cs`) assigns a custom strong-event `ICommand` to `ListView.RefreshCommand`, drops the only strong reference to the `ListView`, forces GC and asserts the `ListView` was collected while the command stays alive. Empirical red → green on this run's source: - **Without the fix (red):** `Failed RefreshCommandDoesNotLeakListView [895 ms]` — `ListView should not be alive!` (Failed: 1) - **With the fix (green):** `Passed RefreshCommandDoesNotLeakListView [47 ms]` (Passed: 1, Test Run Successful) - **No regressions:** full `ListViewTests` class — Total tests: 71, Passed: 71 (including `RefreshCommandCanExecute` and `SendRefreshingDoesNothingWhenCannotExecute`, which exercise the `CanExecute` path) ## Scope Managed, cross-platform change only — one product file (`src/Controls/src/Core/ListView/ListView.cs`) plus the test. No existing test was muted, skipped or weakened; no behavioural change to `RefreshCommand`/`CanExecute` handling. > Generated by [Memory Leak Fixer](https://github.com/dotnet/maui/actions/runs/28693580490) · 2.4K AIC · ⌖ 101 AIC · ⊞ 60.6K · [◷](https://github.com/search?q=repo%3Adotnet%2Fmaui+%22gh-aw-workflow-id%3A+leak-fixer%22&type=pullrequests) <!-- gh-aw-agentic-workflow: Memory Leak Fixer, engine: copilot, version: 1.0.60, model: claude-opus-4.8, id: 28693580490, workflow_id: leak-fixer, run: https://github.com/dotnet/maui/actions/runs/28693580490 --> <!-- gh-aw-workflow-id: leak-fixer --> <!-- gh-aw-workflow-call-id: dotnet/maui/leak-fixer --> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…6369) > [!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! > 🤖 **AI-generated.** This pull request was produced automatically by the **Memory Leak Fixer** agentic workflow (`leak-fixer`) for dotnet/maui. It contains a regression test empirically proven to fail without the fix and pass with it (red → green on this run's `main` source). Fixes #36344 Refs: #36344 Target branch: main Attempt: 1/3 ## The leak `ListView.OnRefreshCommandChanged` subscribed to `RefreshCommand.CanExecuteChanged` with a plain instance handler: ```csharp newCommand.CanExecuteChanged += OnCommandCanExecuteChanged; ``` Because `OnCommandCanExecuteChanged` is an instance method, the delegate's target is the `ListView`. When the assigned `ICommand` is long-lived (a singleton/DI/static command, or a reused view-model command) **and** exposes `CanExecuteChanged` as a strong .NET event, that event's invocation list roots the entire `ListView` — including its cells, templates and binding context. The matching unsubscribe (`-=`) only runs when `RefreshCommand` is reassigned, never when the `ListView` is simply unloaded, so the control is never released. > `Microsoft.Maui.Controls.Command` routes `CanExecuteChanged` through a `WeakEventManager`, so it does **not** reproduce the leak. The leak surfaces with any custom `ICommand` (e.g. CommunityToolkit's `RelayCommand`) that raises the event directly — which is exactly what the issue reports. ## The fix Replace the direct subscription with a `WeakCommandSubscription` — the same weak-subscription helper already used by `CommandElement`, `Button` and `RefreshView`: ```csharp _refreshCommandSubscription = new WeakCommandSubscription(this, newCommand, OnCommandCanExecuteChanged); ``` On reassignment the previous subscription is disposed. `WeakCommandSubscription` holds only a weak reference to the `ListView` (via `DependentHandle`), so the command can no longer keep it alive, while `CanExecuteChanged` is still delivered for as long as the `ListView` is reachable. No public API changes. ## Regression test `RefreshCommandDoesNotLeakListView` (in `Controls.Core.UnitTests/ListViewTests.cs`) assigns a custom strong-event `ICommand` to `ListView.RefreshCommand`, drops the only strong reference to the `ListView`, forces GC and asserts the `ListView` was collected while the command stays alive. Empirical red → green on this run's source: - **Without the fix (red):** `Failed RefreshCommandDoesNotLeakListView [895 ms]` — `ListView should not be alive!` (Failed: 1) - **With the fix (green):** `Passed RefreshCommandDoesNotLeakListView [47 ms]` (Passed: 1, Test Run Successful) - **No regressions:** full `ListViewTests` class — Total tests: 71, Passed: 71 (including `RefreshCommandCanExecute` and `SendRefreshingDoesNothingWhenCannotExecute`, which exercise the `CanExecute` path) ## Scope Managed, cross-platform change only — one product file (`src/Controls/src/Core/ListView/ListView.cs`) plus the test. No existing test was muted, skipped or weakened; no behavioural change to `RefreshCommand`/`CanExecute` handling. > Generated by [Memory Leak Fixer](https://github.com/dotnet/maui/actions/runs/28693580490) · 2.4K AIC · ⌖ 101 AIC · ⊞ 60.6K · [◷](https://github.com/search?q=repo%3Adotnet%2Fmaui+%22gh-aw-workflow-id%3A+leak-fixer%22&type=pullrequests) <!-- gh-aw-agentic-workflow: Memory Leak Fixer, engine: copilot, version: 1.0.60, model: claude-opus-4.8, id: 28693580490, workflow_id: leak-fixer, run: https://github.com/dotnet/maui/actions/runs/28693580490 --> <!-- gh-aw-workflow-id: leak-fixer --> <!-- gh-aw-workflow-call-id: dotnet/maui/leak-fixer --> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
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!
Fixes #36344
Refs: #36344
Target branch: main
Attempt: 1/3
The leak
ListView.OnRefreshCommandChangedsubscribed toRefreshCommand.CanExecuteChangedwith a plain instance handler:Because
OnCommandCanExecuteChangedis an instance method, the delegate's target is theListView. When the assignedICommandis long-lived (a singleton/DI/static command, or a reused view-model command) and exposesCanExecuteChangedas a strong .NET event, that event's invocation list roots the entireListView— including its cells, templates and binding context. The matching unsubscribe (-=) only runs whenRefreshCommandis reassigned, never when theListViewis simply unloaded, so the control is never released.The fix
Replace the direct subscription with a
WeakCommandSubscription— the same weak-subscription helper already used byCommandElement,ButtonandRefreshView:On reassignment the previous subscription is disposed.
WeakCommandSubscriptionholds only a weak reference to theListView(viaDependentHandle), so the command can no longer keep it alive, whileCanExecuteChangedis still delivered for as long as theListViewis reachable. No public API changes.Regression test
RefreshCommandDoesNotLeakListView(inControls.Core.UnitTests/ListViewTests.cs) assigns a custom strong-eventICommandtoListView.RefreshCommand, drops the only strong reference to theListView, forces GC and asserts theListViewwas collected while the command stays alive.Empirical red → green on this run's source:
Failed RefreshCommandDoesNotLeakListView [895 ms]—ListView should not be alive!(Failed: 1)Passed RefreshCommandDoesNotLeakListView [47 ms](Passed: 1, Test Run Successful)ListViewTestsclass — Total tests: 71, Passed: 71 (includingRefreshCommandCanExecuteandSendRefreshingDoesNothingWhenCannotExecute, which exercise theCanExecutepath)Scope
Managed, cross-platform change only — one product file (
src/Controls/src/Core/ListView/ListView.cs) plus the test. No existing test was muted, skipped or weakened; no behavioural change toRefreshCommand/CanExecutehandling.