Skip to content

[leak-fix] Fix ListView.RefreshCommand memory leak (Fixes #36539) - #36659

Closed
github-actions[bot] wants to merge 1 commit into
mainfrom
leak-fix/issue-36539-eb7d4c4e2a86f7fc
Closed

[leak-fix] Fix ListView.RefreshCommand memory leak (Fixes #36539)#36659
github-actions[bot] wants to merge 1 commit into
mainfrom
leak-fix/issue-36539-eb7d4c4e2a86f7fc

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Note

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

Note

🤖 AI-generated PR — produced automatically by the Memory Leak Fixer agentic workflow from issue #36539. The regression test below was observed to FAIL on unpatched main and PASS with this fix, on the runner. Please review carefully before merging.

Fixes #36539
Refs: #36539
Target branch: main
Attempt: 2/3

The leak

ListView.RefreshCommand subscribes to the command's CanExecuteChanged event (src/Controls/src/Core/ListView/ListView.cs, OnRefreshCommandChanged). The subscription was a plain newCommand.CanExecuteChanged += OnCommandCanExecuteChanged and was only removed when the command was reassigned — never on unload. When the command's CanExecuteChanged is a plain CLR event (as is common for custom/ViewModel ICommand implementations), the delegate target is the ListView, so a shared/long-lived command strongly roots the ListView (and its page). MAUI's own Command raises CanExecuteChanged through a WeakEventManager and would not leak, but arbitrary ICommand implementations do.

The fix

Replace the plain subscription with the existing idiomatic WeakCommandSubscription (the same helper Button/CommandElement already use). It links the ListView to the handler via a DependentHandle so the command holds the ListView only weakly, and disposes the previous subscription when the command is replaced. No behavior change to RefreshAllowed/CanExecute evaluation.

Regression test

ListViewMemoryLeakTests.ListViewDoesNotLeakWhenRefreshCommandIsLongLived in src/Controls/tests/Core.UnitTests/ListViewMemoryLeakTests.cs. It assigns a long-lived ICommand (with a plain CanExecuteChanged event) to RefreshCommand, drops the ListView, forces GC, and asserts the ListView is collected.

State Result
Without fix (unpatched main) ❌ test FAILS (control retained)
With fix ✅ test PASSES (control collected)
Without fix: Failed ListViewDoesNotLeakWhenRefreshCommandIsLongLived [814 ms]
  ListView should not be alive! It is being rooted by the RefreshCommand's CanExecuteChanged subscription.
With fix:    Passed ListViewDoesNotLeakWhenRefreshCommandIsLongLived [54 ms]

All 71 neighbouring ListViewTests still pass.

Scope

Managed cross-platform change (src/Controls/src/Core/ListView/ListView.cs) → all platforms. No public API change.

Generated by Memory Leak Fixer · 280.8 AIC · ⌖ 29.8 AIC · ⊞ 19.7K ·

Replace the plain ICommand.CanExecuteChanged subscription with a WeakCommandSubscription so a long-lived RefreshCommand (whose CanExecuteChanged is a plain CLR event) no longer roots the ListView and its page. Adds a Controls.Core.UnitTests regression test that fails without this fix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added agentic-workflows perf/memory-leak 💦 Memory usage grows / objects live forever (sub: perf) labels Jul 19, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

agentic-workflows perf/memory-leak 💦 Memory usage grows / objects live forever (sub: perf)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[leak-scan] ListView.RefreshCommand — non-weak ICommand.CanExecuteChanged retains the ListView

1 participant