[leak-fix] Fix BackButtonBehavior.Command memory leak (Fixes #36538)

[github-actions]

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!

Important

This pull request was generated automatically by the Memory Leak Fixer agentic workflow in dotnet/maui. It contains a regression test empirically proven to fail without the fix and pass with it. Please review carefully — it is AI-generated.

Fixes #36538
Refs: #36538
Target branch: main
Attempt: 1/3

The leak

BackButtonBehavior subscribed to ICommand.CanExecuteChanged with a strong delegate:

newCommand.CanExecuteChanged += CanExecuteChanged;

The matching unsubscribe only ran when the Command property changed. Because an ICommand is commonly stored on a long-lived view model (or otherwise shared/retained), that strong subscription keeps the BackButtonBehavior — and everything it transitively roots — alive for as long as the command lives.

Retention path (from issue #36538):

ICommand (long-lived) -> CanExecuteChanged event -> BackButtonBehavior.CanExecuteChanged delegate -> BackButtonBehavior

The fix

Implement the existing internal ICommandElement contract and route command lifecycle callbacks through CommandElement, matching Button, ImageButton, RefreshView, and other command controls. CommandElement owns the WeakCommandSubscription, disposes it before command replacement, and uses a DependentHandle so the command references the behavior only weakly.

BackButtonBehavior.CommandProperty now declares its dependency on CommandParameterProperty, and CanExecuteChanged passes CommandProperty to CommandElement.GetCanExecute. If both properties are bound and the command resolves first during reparenting, the pending parameter binding is applied before the first CanExecute evaluation instead of evaluating with a null or stale parameter.

public class BackButtonBehavior : BindableObject, ICommandElement
{
    public static readonly BindableProperty CommandProperty =
        BindableProperty.Create(
            nameof(Command),
            typeof(ICommand),
            typeof(BackButtonBehavior),
            null,
            BindingMode.OneTime,
            propertyChanging: CommandElement.OnCommandChanging,
            propertyChanged: CommandElement.OnCommandChanged);

    static BackButtonBehavior()
    {
        CommandProperty.DependsOn(CommandParameterProperty);
    }

    void ICommandElement.CanExecuteChanged(object sender, EventArgs e)
    {
        IsEnabledCore = CommandElement.GetCanExecute(this, CommandProperty);
    }

    WeakCommandSubscription ICommandElement.CleanupTracker { get; set; }
}

No public API change (ICommandElement is internal and its members are implemented explicitly). User-defined IsEnabled state remains combined with command-derived enabled state.

Regression tests

The existing CommandsSubscribedToCanExecuteCollect theory includes BackButtonBehavior rows for both weak-event and strong-event command variants. The strong-event variant (useWeakEventHandler: false) reproduces the leak. Because BackButtonBehavior now implements ICommandElement, the shared test also verifies collection of its WeakCommandSubscription cleanup tracker and proxy.

BackButtonBehaviorAppliesBoundCommandParameterBeforeCanExecute binds both Command and CommandParameter, applies the binding context with the command binding registered first, and verifies every CanExecute invocation receives the resolved parameter.

BackButtonBehaviorPreservesUserDefinedDisabledStateForExecutableCommand verifies an explicit IsEnabled = false survives both assignment of an always-executable command and a later CanExecuteChanged notification.

Verified in this run (net TFM, Controls.Core.UnitTests):

State	BackButtonBehavior, false	Result
Without fix (red)	Assert.False() Failure — Expected: False, Actual: True (behavior still alive)	❌ FAIL
With fix (green)	behavior collected	✅ PASS

Red run (unpatched source): Total tests: 16, Passed: 15, Failed: 1 — only the BackButtonBehavior, false case failed.
Green run (with fix): Total tests: 16, Passed: 16.
The full CommandTests class passes 47 tests. The complete Controls.Core.UnitTests project passes 5,669 tests with 30 existing tests skipped.

Scope

• Managed, cross-platform only: src/Controls/src/Core/Shell/BackButtonBehavior.cs and src/Controls/tests/Core.UnitTests/CommandTests.cs.
• No test was muted, skipped, or weakened; no [ActiveIssue] / Skip= / timeout changes.
• Reuses the same ICommandElement/CommandElement weak-subscription and command-parameter dependency lifecycle as sibling command controls.

---

🤖 Generated by the Memory Leak Fixer agentic workflow (dotnet/maui). This PR is AI-generated — please review before merging.

Generated by Memory Leak Fixer · 542.6 AIC · ⌖ 29.2 AIC · ⊞ 19.6K · ◷

[azure-pipelines]

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

[kubaflo]

🔍 AI-generated review follow-up (GitHub Copilot CLI, on behalf of @kubaflo)

@MauiBot Review 4696114698 analyzed current head 48cf3f004872, passed the regression gate, reported 0 errors, 0 warnings, and 0 suggestions, and ranked the submitted WeakCommandSubscription fix as the best candidate with the narrowest blast radius. No implementation change or no-op commit is warranted.

The low-confidence NEEDS_DISCUSSION verdict is CI-only: the report says required-check lookup was unavailable/pending, while the interrupted deep Shell UI run produced no results and was itself classified as a likely infrastructure timeout/hang. The [leak-fix] title prefix remains intentionally for campaign tracking.

[kubaflo]

🔍 AI-generated review follow-up (GitHub Copilot CLI, on behalf of @kubaflo)

@MauiBot Review 4697130188 analyzed current head 48cf3f004872, passed the regression gate, reported 0 errors, 0 warnings, and 0 suggestions, and selected the submitted direct WeakCommandSubscription implementation as the least-risk winner. Public head check runs were also reported successful. No implementation change or no-op commit is warranted; the [leak-fix] title marker remains intentional for campaign tracking.

[kubaflo]

🔍 AI-generated review follow-up (GitHub Copilot CLI, on behalf of @kubaflo)

@MauiBot Review 4698451917 evaluated current head 48cf3f00487 and found 0 errors, 0 warnings, and 0 suggestions. The gate reproduced the BackButtonBehavior retention without the fix and passed 45/45 with it; deep UI coverage passed 309/309, and the targeted command-collection run passed 16/16.

The review selected the submitted WeakCommandSubscription implementation as the smallest targeted, idiomatic fix. No code change or no-op commit is warranted. The branch is ready for re-review.

[kubaflo]

🔍 AI-generated review follow-up (GitHub Copilot CLI, on behalf of @kubaflo)

@MauiBot Review 4699604580 evaluated the current head 48cf3f00487, passed the gate, reported 0 errors, 0 warnings, and 0 suggestions, found no actionable code issue, and selected the submitted WeakCommandSubscription implementation over all alternatives. The NEEDS_DISCUSSION verdict is solely due to the review runner lacking authenticated required-check visibility, not a code concern. No code change is needed.

[MauiBot]

Expert Review — 1 findings

See inline comments for details.

[kubaflo]

🔍 AI-generated update (GitHub Copilot CLI, on behalf of @kubaflo)

@MauiBot The regression-prevention suggestion from review 4702290187 is addressed in fa8ab0f83cc. The new test verifies an explicit BackButtonBehavior.IsEnabled = false remains false after assigning an always-executable command and after a subsequent CanExecuteChanged notification.

The full CommandTests class and complete Controls core suite pass. Ready for re-review.

[kubaflo]

🔍 AI-generated review response (GitHub Copilot CLI, on behalf of @kubaflo)

@MauiBot Review 4703498653 reports zero errors, warnings, suggestions, or inline findings against head fa8ab0f83cc. The regression gate passed. The only incomplete result is the interrupted deep Shell UI run, which the review classifies as an infrastructure/timeout condition suitable for rerun.

No code change or no-op commit is needed. Releasing the already queued improved-reviewer rerun to retry that UI leg.

[MauiBot]

AI Review Summary

@github-actions[bot] — new AI review results are available based on this last commit: fa8ab0f. 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: 0395a53b

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
🧪 CommandTests CommandTests	✅ FAIL — 153s	✅ PASS — 118s

🔴 Without fix — 🧪 CommandTests: FAIL ✅ · 153s

Error-relevant lines (filtered from the build log):

     at Microsoft.Maui.Controls.Core.UnitTests.CommandTests.CommandsSubscribedToCanExecuteCollect(Type controlType, Boolean useWeakEventHandler) in /_/src/Controls/tests/Core.UnitTests/CommandTests.cs:line 399
     at Microsoft.Maui.Controls.Core.UnitTests.CommandTests.BackButtonBehaviorAppliesBoundCommandParameterBeforeCanExecute() in /_/src/Controls/tests/Core.UnitTests/CommandTests.cs:line 284
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

🟢 With fix — 🧪 CommandTests: PASS ✅ · 118s

(no coded error found; showing last 1200 chars)

alse) [< 1 ms]
  Passed ThrowsWithNullConstructor [1 ms]
  Passed ExecuteDoesNotRunIfParameterIsWrongValueType [< 1 ms]
  Passed GenericThrowsWithNullExecuteAndCanExecuteValid [< 1 ms]
  Passed ThrowsWithNullExecuteValidCanExecute [< 1 ms]
  Passed CanExecuteUsesParameterIfReferenceTypeAndSetToNull [< 1 ms]
  Passed ChangeCanExecute [< 1 ms]
  Passed GenericExecute [< 1 ms]
  Passed ThrowsWithNullCanExecute [< 1 ms]
  Passed ThrowsWithNullParameterizedConstructor [< 1 ms]
  Passed CanExecuteUsesParameterIfNullableAndSetToNull [1 ms]
  Passed ExecuteDoesNotRunIfValueTypeAndSetToNull [< 1 ms]
  Passed Constructor [< 1 ms]
  Passed ThrowsWithNullParameterizedCanExecute [< 1 ms]
  Passed BackButtonBehaviorPreservesUserDefinedDisabledStateForExecutableCommand [< 1 ms]
  Passed ExecuteRunsIfNullableAndSetToNull [< 1 ms]
  Passed ExecuteParameterized [3 ms]
  Passed GenericThrowsWithValidExecuteAndCanExecuteNull [< 1 ms]
[xUnit.net 00:00:04.37]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed ExecuteWithCanExecute [< 1 ms]
  Passed CanExecuteReturnsFalseIfParameterIsWrongReferenceType [< 1 ms]

Test Run Successful.
Total tests: 47
     Passed: 47
 Total time: 5.0707 Seconds

📁 Fix files reverted (1 files)

• src/Controls/src/Core/Shell/BackButtonBehavior.cs

---

📱 UI Tests — Shell

Detected UI test categories: Shell

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

---

📋 Pre-Flight — Context & Validation

Issue: #36538 - [leak-scan] BackButtonBehavior.Command — non-weak ICommand.CanExecuteChanged retains the Shell BackButtonBehavior
PR: #36566 - [leak-fix] Fix BackButtonBehavior.Command memory leak (Fixes #36538)
Platforms Affected: all platforms (managed Controls/Shell code); testing platform requested: android
Files Changed: 1 implementation, 1 test

Key Findings

• The issue reports that BackButtonBehavior used a strong ICommand.CanExecuteChanged subscription, so long-lived commands could retain transient Shell back-button behavior instances and their page graph.
• The PR replaces the BackButton-specific subscription code with the shared internal ICommandElement/CommandElement weak subscription path used by sibling command controls.
• Prior MauiBot findings about command-parameter binding order and user-disabled IsEnabled preservation are fixed in the current local diff.
• GitHub CLI is unauthenticated in this environment; context was gathered from the checked-out pr-review-36566 branch, local diff against origin/main, and public GitHub API calls.

Code Review Summary

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

Key code review findings:

• No actionable implementation findings remain in the current PR diff.
• Prior major finding fixed: BackButtonBehavior.CommandProperty now depends on CommandParameterProperty, and CanExecuteChanged calls CommandElement.GetCanExecute(this, CommandProperty).
• Prior moderate finding fixed: BackButtonBehaviorPreservesUserDefinedDisabledStateForExecutableCommand covers explicit IsEnabled = false preservation.
• CI confidence is capped because authenticated required-check lookup was unavailable; public fallback comments/check evidence indicated green or infrastructure-only interruptions.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #36566	Implement ICommandElement and route BackButtonBehavior command lifecycle through shared CommandElement/WeakCommandSubscription; add command-parameter dependency and regression tests.	✅ PASSED (Gate)	BackButtonBehavior.cs, CommandTests.cs	Original PR; gate already completed externally.

---

🔬 Code Review — Deep Analysis

Code Review — PR #36566

Independent Assessment

What this changes: BackButtonBehavior now uses the shared ICommandElement/CommandElement weak command-subscription infrastructure instead of directly subscribing to ICommand.CanExecuteChanged. It also wires CommandProperty.DependsOn(CommandParameterProperty) and adds unit coverage for binding order, disabled-state preservation, and weak-subscription collection.

Inferred motivation: Avoid retaining BackButtonBehavior/Shell pages through long-lived commands while matching the command-parameter timing behavior used by other command controls.

Reconciliation with PR Narrative

Author claims: Fixes memory leak #36538 by replacing a strong CanExecuteChanged subscription with WeakCommandSubscription, with regression tests and no public API change.

Agreement/disagreement: Agrees. The current code matches existing ICommandElement patterns and addresses the issue’s retention path.

Prior Review Reconciliation

Prior ❌ Error Finding	Source	Status	Evidence
BackButtonBehavior did not wire CommandProperty.DependsOn(CommandParameterProperty) / GetCanExecute(this, CommandProperty)	MauiBot inline [major] / review	✅ Fixed	Current BackButtonBehavior.cs:24-27 registers the dependency and BackButtonBehavior.cs:118-120 calls CommandElement.GetCanExecute(this, CommandProperty).
Missing regression coverage for user-disabled IsEnabled preservation	MauiBot inline [moderate]	✅ Fixed	CommandTests.cs:287-302 adds BackButtonBehaviorPreservesUserDefinedDisabledStateForExecutableCommand.

Blast Radius Assessment

• Runs for all instances: No — only BackButtonBehavior instances with command usage.
• Startup impact: Minimal; static constructor only registers a bindable-property dependency.
• Static/shared state: No mutable shared state; subscription tracker is per instance.

CI Status

• Required-check result: gh pr checks --required unavailable because GitHub CLI is unauthenticated.
• Public fallback: PR discussion reports green command/unit coverage and later zero-findings reviews; current authenticated required-check surface remains unavailable here.
• Classification: Required-check surface undetermined in this environment.
• Action taken: Capped confidence low; no LGTM per skill Rule #6.

Findings

No actionable ❌ Error, ⚠️ Warning, or 💡 Suggestion findings.

Failure-Mode Probing

• Command outlives behavior/page: weak subscription avoids command event retaining the behavior.
• Command set/replaced/null: OnCommandChanging disposes prior tracker; null command recalculates enabled state to true.
• Bound CommandParameter resolves after Command: dependency forcing is active via GetCanExecute(this, CommandProperty).
• User explicitly disables back button: _userDefinedIsEnabled && _commandEnabled preserves disabled state; covered by test.

Verdict: NEEDS_DISCUSSION

Confidence: low
Summary: Code review found no unresolved implementation issues, and public CI evidence is green. However, authenticated required-check status could not be collected, so the skill rules prohibit LGTM and cap confidence.

---

🛠️ Fix — Analysis & Comparison

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix	BackButtonBehavior-local WeakCommandSubscription, no ICommandElement.	❌ FAIL	1 file	Left CommandParameterProperty on CommandElement.OnCommandParameterChanged, causing an invalid cast after removing ICommandElement.
2	try-fix	Fully local weak-subscription implementation with local command and parameter callbacks.	✅ PASS	1 file	Functionally passes, but expert review says it is not better than the PR because it duplicates shared command logic and weakens shared test observability.
3	try-fix	Nested weak CanExecuteChangedProxy retained by the command instead of WeakCommandSubscription.	✅ PASS	1 file	Fixes behavior retention but can leak stale proxy subscriptions until the command raises again; weaker lifecycle than PR.
PR	PR #36566	Shared ICommandElement/CommandElement weak subscription with command-parameter dependency.	✅ PASSED (Gate)	2 files	Original PR; selected as most idiomatic and lifecycle-complete.

Cross-Pollination

Model	Round	New Ideas?	Details
gpt-5.5 expert loop	1	Yes	Candidate 2 should keep the local weak-subscription approach but own the CommandParameter callback as well.
maui-expert-reviewer	1	No better passing idea	Candidate 2 is probably correct but not demonstrably better than the PR's shared ICommandElement implementation.
gpt-5.5 expert loop	2	Yes	Candidate 3 can avoid WeakCommandSubscription entirely with a nested weak proxy.
maui-expert-reviewer	2	No	Candidate 3 is weaker than the PR because stale proxy subscriptions can remain rooted by long-lived commands that never raise again.

Exhausted: Yes
Selected Fix: PR #36566 — it passes the gate, matches established MAUI command-element conventions, centralizes future command lifecycle fixes, preserves command-parameter dependency behavior, and avoids the stale-proxy/subscription observability gaps found in the alternatives.

---

📝 Recommended PR Title & Description

Assessment: ✏️ Recommend updating — the description is accurate and high quality, but the current title uses a workflow-style [leak-fix] prefix instead of the repository's searchable component-focused title format.

Recommended title

[All] Shell: Fix BackButtonBehavior.Command memory leak (Fixes #36538)

Recommended description

> [!IMPORTANT]
> This pull request was generated automatically by the **Memory Leak Fixer** agentic workflow in dotnet/maui. It contains a regression test empirically proven to fail without the fix and pass with it. Please review carefully — it is AI-generated.

Fixes #36538
Refs: dotnet/maui#36538
Target branch: main
Attempt: 1/3

## The leak

`BackButtonBehavior` subscribed to `ICommand.CanExecuteChanged` with a **strong** delegate:

```csharp
newCommand.CanExecuteChanged += CanExecuteChanged;
```

The matching unsubscribe only ran when the `Command` property changed. Because an `ICommand` is commonly stored on a long-lived view model (or otherwise shared/retained), that strong subscription keeps the `BackButtonBehavior` — and everything it transitively roots — alive for as long as the command lives.

Retention path from issue #36538:

`ICommand (long-lived) -> CanExecuteChanged event -> BackButtonBehavior.CanExecuteChanged delegate -> BackButtonBehavior`

## The fix

Implement the existing internal `ICommandElement` contract and route command lifecycle callbacks through `CommandElement`, matching `Button`, `ImageButton`, `RefreshView`, and other command controls. `CommandElement` owns the `WeakCommandSubscription`, disposes it before command replacement, and uses a `DependentHandle` so the command references the behavior only weakly.

`BackButtonBehavior.CommandProperty` now declares its dependency on `CommandParameterProperty`, and `CanExecuteChanged` passes `CommandProperty` to `CommandElement.GetCanExecute`. If both properties are bound and the command resolves first during reparenting, the pending parameter binding is applied before the first `CanExecute` evaluation instead of evaluating with a null or stale parameter.

```csharp
public class BackButtonBehavior : BindableObject, ICommandElement
{
    public static readonly BindableProperty CommandProperty =
        BindableProperty.Create(
            nameof(Command),
            typeof(ICommand),
            typeof(BackButtonBehavior),
            null,
            BindingMode.OneTime,
            propertyChanging: CommandElement.OnCommandChanging,
            propertyChanged: CommandElement.OnCommandChanged);

    static BackButtonBehavior()
    {
        CommandProperty.DependsOn(CommandParameterProperty);
    }

    void ICommandElement.CanExecuteChanged(object sender, EventArgs e)
    {
        IsEnabledCore = CommandElement.GetCanExecute(this, CommandProperty);
    }

    WeakCommandSubscription ICommandElement.CleanupTracker { get; set; }
}
```

No public API change (`ICommandElement` is internal and its members are implemented explicitly). User-defined `IsEnabled` state remains combined with command-derived enabled state.

## Regression tests

The existing `CommandsSubscribedToCanExecuteCollect` theory includes `BackButtonBehavior` rows for both weak-event and strong-event command variants. The strong-event variant (`useWeakEventHandler: false`) reproduces the leak. Because `BackButtonBehavior` now implements `ICommandElement`, the shared test also verifies collection of its `WeakCommandSubscription` cleanup tracker and proxy.

`BackButtonBehaviorAppliesBoundCommandParameterBeforeCanExecute` binds both `Command` and `CommandParameter`, applies the binding context with the command binding registered first, and verifies every `CanExecute` invocation receives the resolved parameter.

`BackButtonBehaviorPreservesUserDefinedDisabledStateForExecutableCommand` verifies an explicit `IsEnabled = false` survives both assignment of an always-executable command and a later `CanExecuteChanged` notification.

---

🏁 Report — Final Recommendation

Comparative Fix Report — PR #36566

Candidates compared

Rank	Candidate	Regression result	Assessment
1	pr	✅ Passed	Best candidate. Uses the existing shared ICommandElement / CommandElement / WeakCommandSubscription pattern already used by command controls, fixes the leak, preserves command-parameter ordering, preserves explicit disabled state, and keeps shared leak-test observability.
1	pr-plus-reviewer	✅ Passed	Equivalent to pr. The expert reviewer found no actionable issues, so no reviewer-driven changes were applied. It ranks alongside the PR but is not a distinct improvement.
3	try-fix-2	✅ Passed	Functionally plausible local weak-subscription implementation, but weaker than the PR because it duplicates shared command lifecycle logic and loses the generic ICommandElement.CleanupTracker / proxy observability in the shared leak test path.
4	try-fix-3	✅ Passed	Fixes retention of BackButtonBehavior, but can leave stale proxy subscriptions rooted by a long-lived command until a later CanExecuteChanged event occurs. This turns the original leak into a smaller lifecycle leak and is weaker than the PR.
5	try-fix-1	❌ Failed	Must rank below all passing candidates. It removed ICommandElement while leaving CommandParameterProperty on CommandElement.OnCommandParameterChanged, causing an InvalidCastException.

Winning candidate

Winner: pr

The raw PR fix is the strongest solution because it reuses the established shared command infrastructure instead of creating BackButton-specific lifecycle code. It addresses the root leak, handles command replacement deterministically, preserves binding and enabled-state semantics, and is covered by targeted regression tests that already passed the gate. Since the expert reviewer recommended no code changes, pr-plus-reviewer is equivalent and does not supersede the submitted PR.

Candidate details

pr

BackButtonBehavior now implements the internal ICommandElement contract. CommandProperty uses CommandElement.OnCommandChanging and CommandElement.OnCommandChanged, CommandParameterProperty uses CommandElement.OnCommandParameterChanged, and CanExecuteChanged delegates to CommandElement.GetCanExecute(this, CommandProperty).

This is the most maintainable fix because it aligns BackButtonBehavior with existing command controls such as Button, ImageButton, TextCell, MenuItem, RefreshView, and SearchBar.

pr-plus-reviewer

The expert reviewer produced no actionable findings and wrote an empty inline findings file. No sandbox patch was applied, so this candidate is identical to pr.

try-fix-1

This candidate attempted to keep a local WeakCommandSubscription while removing ICommandElement. It failed because CommandParameterProperty still used CommandElement.OnCommandParameterChanged, which casts the bindable object to ICommandElement.

try-fix-2

This candidate fixed the callback ownership issue from try-fix-1 by implementing all command and command-parameter lifecycle handling locally. It passed targeted tests, but it duplicates shared logic and reduces coverage through the shared ICommandElement cleanup-tracker path. There is no demonstrated benefit over the PR.

try-fix-3

This candidate used a nested weak proxy retained by the command. It passed targeted tests and avoids retaining BackButtonBehavior, but stale proxy subscriptions can remain indefinitely if the long-lived command never raises CanExecuteChanged again. That lifecycle behavior is inferior to the PR's WeakCommandSubscription approach.

Final recommendation

Proceed with the PR fix as submitted. Do not replace it with any STEP 5a try-fix candidate.

---

🧭 Next Steps — review latest findings

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

[kubaflo]

🔍 AI-generated follow-up (GitHub Copilot CLI, on behalf of @kubaflo)

@MauiBot Review #4704615522 found no actionable implementation issues on fa8ab0f83cc5b10ee39a5b7ca91d75cd67cfb590. The gate passed with 47/47 focused command tests, and the Shell deep UI run passed 309/309 tests.

The suggested title rewrite was not applied because the [leak-fix] prefix is intentionally retained for this monitored PR campaign. No code or title change was needed, so no redundant review rerun was requested.

[kubaflo]

/azp run

[azure-pipelines]

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

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).

[Copilot]

Pull request overview

This PR fixes a memory leak in Shell’s BackButtonBehavior.Command handling by switching from a strong CanExecuteChanged subscription to MAUI’s existing weak-command subscription infrastructure (ICommandElement + CommandElement). It also adds/extends unit tests to prevent regressions, including coverage for binding-order timing and preservation of user-defined disabled state.

Changes:

• Update BackButtonBehavior to implement internal ICommandElement and route command lifecycle through CommandElement (weak subscription + cleanup).
• Register CommandProperty dependency on CommandParameterProperty so CanExecute evaluation can force-apply a pending parameter binding when needed.
• Add regression tests for leak collection, correct command-parameter propagation timing, and preserving user-disabled IsEnabled behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/Controls/src/Core/Shell/BackButtonBehavior.cs	Switches command handling to CommandElement/ICommandElement weak subscription and adds DependsOn for parameter binding timing.
src/Controls/tests/Core.UnitTests/CommandTests.cs	Adds new BackButtonBehavior-specific regression tests and extends the shared command GC/leak theory to include BackButtonBehavior.

[kubaflo]

🔍 AI-generated merge-readiness analysis (GitHub Copilot CLI, on behalf of @kubaflo): good to merge.

All three current-head pipelines completed for fa8ab0f8; every failure is outside the BackButtonBehavior.cs / CommandTests.cs change path:

• maui-pr 1510832: only CreateTimerNonRepeatingDoesNotRepeat failed with the recurring dispatcher timer-race NRE tracked by [ci-scan] Recurring: CreateTimerNonRepeatingDoesNotRepeat fails with NullReferenceException on macOS ARM64 (maui-pr) #35910/[ci-scan-net11] DispatcherTests.CreateTimerNonRepeatingDoesNotRepeat fails with NullReferenceException on Helix (net11.0) #36192.
• maui-pr-devicetests 1510834:
  • Android: four GC-sensitive GestureDoesNotLeak cases retained a Label.
  • MacCatalyst: CarouselViewDoesNotLeakWithDefaultItemsLayout, tracked by [ci-scan] Recurring: CarouselView Does Not Leak With Default ItemsLayout fails on MacCatalyst device tests #36116.
  • Windows: SwitchingWindowsPostsToTheNewWindow, tracked by [ci-scan-net11] Essentials.DeviceTests: SwitchingWindowsPostsToTheNewWindow fails on Windows — Assert.Contains collection empty (net11.0) #36043.
    All three failure groups pass on the exact base device build 1503431 and again in comparable successful build 1510860.
• maui-pr-uitests 1510833: unrelated SwipeView, HybridWebView, RefreshView, CollectionView, and DisplayAlert tests failed. The evidence is strongly flaky/environmental: Issue36154 passed 159/184 times over seven days (four passes in this build); HybridWebView passed 2420/2458 (21 passes in this build); Issue30690 passed 3117/3122 (35 passes in this build); CollectionView scrolling passed 3185/3189 in the last day (296 passes in this build); and Issue33287 passed 420/434 (four passes in this build) and is tracked by [ci-scan-net11] Issue33287.DisplayAlertAsyncShouldNotCrashWhenPageUnloaded consistently times out on MacCatalyst (net11.0) #36554/[ci-scan] Recurring: DisplayAlertAsyncShouldNotCrashWhenPageUnloaded times out on macOS in maui-pr-uitests #36260. Each class passed again in a later concurrent build.

No failure correlates with Shell back-button command subscription behavior, so no PR code change is warranted.

[github-actions]

Tests Failure Analysis

@kubaflo — test-failure review results are available based on commit fa8ab0f.
To request a fresh review after new comments, commits, or CI runs, comment /review tests.

Test Failure Review: Not ready - click to expand

Overall verdict: Not ready. Three legs/tests are deterministic regressions vs main (red on the PR, green on all 5 recent base builds sampled per definition, red on none): two Windows device-test build breaks and the Issue36154SwipeViewShouldRevealItems UI test — these forbid a green verdict. The remaining ~29 failures could not be attributed either way (device-test work items that did not confirm clean, unexplained/aborted build legs, and HybridWebView/gesture-leak flakiness with indeterminate attribution); none of the 32 distinct failures also appear on the base branch, so nothing is dismissible as pre-existing.

• ✗ PR-related — deterministic regressions vs base (~3 items): two Windows device-test build breaks (DeviceTestsWindows (Windows) - build error, PowerShell exit 1) plus Issue36154SwipeViewShouldRevealItems, all red on the PR and green on every sampled base build; treat as likely PR-caused pending human confirmation.
• i Uncertain — unexplained/aborted build legs & unverified device tests (~24 items): 17 failed build legs with no extractable failure, 2 cancelled macOS UITests legs (CollectionView, Layout), 5 device-test checks green-but-unconfirmed, and crashed work items such as device-test work item incomplete (com.microsoft.maui.controls.devicetests-Signed).
• i Uncertain — flaky-on-base UI/leak tests (~15 items): HybridWebView timeouts (VerifyHybridWebView_DefaultValues, [ci-scan] Recurring: Android API 30 emulator fails to boot in maui-pr-uitests — all Controls (API 30) legs blocked #36489), gesture/CarouselView leak asserts, and Android SDK provisioning noise ([ci-scan] Recurring: Windows Helix unit tests fail — Controls.Build.Tasks DLLs locked by .NET Host during Run Helix Tests #36141), all indeterminate.

Coverage: 162 checks · 146 passing · 16 failing · 0 pending · 0 inaccessible · 1 unmapped · 17 unexplained build legs · 0 unaccounted failing checks · 2 aborted failing checks · 0 canceled-build checks · 5 device-test unverified · 29 unattributed · 3 regressed-vs-base. Deterministic ceiling: Not ready — regressed-vs-base > 0, unexplained legs, aborted checks, and unverified device tests all present.

Builds (this PR): maui-pr 1510832, maui-pr-devicetests 1510834, maui-pr-uitests 1510833. Base sampling (main, 5 recent builds per definition): maui-pr 1503430, maui-pr-devicetests 1503431, maui-pr-uitests 1503618.

Recommended action

Have a human inspect the three regressed-vs-base items — especially the two Windows device-test build breaks and Issue36154SwipeViewShouldRevealItems — and confirm the 5 unverified device-test checks before merging.

[kubaflo]

🔍 AI-generated follow-up analysis (GitHub Copilot CLI, on behalf of @kubaflo)

The /review tests three-regression verdict double-counts wrapper failures; no BackButtonBehavior regression is established.

• The two Windows “build errors” are duplicate wrappers around one Helix failure. The Windows build itself succeeded and 10/11 work items passed. The sole failure was SwitchingWindowsPostsToTheNewWindow, with the exact ActiveWindowTracker_Tests.cs:112 empty-collection assertion tracked by [ci-scan-net11] Essentials.DeviceTests: SwitchingWindowsPostsToTheNewWindow fails on Windows — Assert.Contains collection empty (net11.0) #36043.
• Issue36154SwipeViewShouldRevealItems is an intermittent coordinate-drag UI failure. It passed on exact-base build 1503618, earlier main build 1503332, same-base concurrent builds 1510823 and 1510859, and four other platform attempts in the current build.
• The timer NRE remains covered by [ci-scan] Recurring: CreateTimerNonRepeatingDoesNotRepeat fails with NullReferenceException on macOS ARM64 (maui-pr) #35910/[ci-scan-net11] DispatcherTests.CreateTimerNonRepeatingDoesNotRepeat fails with NullReferenceException on Helix (net11.0) #36192 and the CarouselView leak by [ci-scan] Recurring: CarouselView Does Not Leak With Default ItemsLayout fails on MacCatalyst device tests #36116.

The PR changes only BackButtonBehavior.cs and CommandTests.cs; the affected Core unit-test assembly passed. No PR code change is required, and the previous merge-readiness verdict remains valid.

[kubaflo]

🔍 AI-generated (GitHub Copilot CLI, on behalf of @kubaflo)

Closing as superseded. Issue #36538 is a duplicate of #36345, whose fix — #36370 "[leak-fix] Fix BackButtonBehavior.Command memory leak" — is already merged into inflight/current. Rebasing this branch onto the current base auto-drops the leak-fix commits with git's "patch contents already upstream", confirming the fix is byte-identical to what already shipped. The only residual delta was an unrelated command-parameter-dependency tweak (out of scope for this leak issue).

The BackButtonBehavior.Command leak is fixed and covered on base. Reopen if the residual behavioral change is wanted as its own focused PR.