dotnet / dotnet/maui

[leak-scan] VisualState.StateTriggers — removal skips trigger detachment

Open
#38,243 0 comments 0 reactions 0 assignees View on GitHub
agentic-workflows partner/syncfusion perf/memory-leak 💦
Dominant language
C#
Stars
23.3k
Forks
2k
Avg merge
1d 15h
Merged PRs (30d)
290

Description

> [!IMPORTANT]
> **AI-generated by the Daily Memory Leak Hunter — dotnet/maui workflow.** This finding was empirically confirmed against the shipped `Microsoft.Maui.Controls` 10.0.0 package on plain `net10.0`.

## Description

Clearing an attached `VisualState.StateTriggers` collection removes the trigger from the collection without calling `StateTriggerBase.SendDetached()`. A trigger that subscribed to a shared managed publisher in `OnAttached()` therefore remains subscribed after it is removed, retaining the trigger and anything it owns.

The same removal-notification gap exists for `Remove`, `RemoveAt`, and index replacement in the underlying list. The repro uses `Clear()` as the canonical case.

## Retention path

`shared publisher -> event delegate -> removed StateTriggerBase instance -> page-local payload`

- `src/Controls/src/Core/StateTriggerBase.cs:52-75` defines the paired `OnAttached`/`OnDetached` lifecycle and invokes teardown only through `SendDetached()`.
- `src/Controls/src/Core/VisualStateManager.cs:763-770` processes additions to `VisualState.StateTriggers`, but has no corresponding removal callback.
- `src/Controls/src/Core/VisualStateManager.cs:858-929` implements `WatchAddList`; `Clear`, `Remove`, `RemoveAt`, and the indexer setter mutate the list without notifying the owner, so removed triggers are not detached.

## Standalone repro

The reflection call only simulates the framework's internal trigger-attachment lifecycle without creating a platform window; collection mutation and retention use the public managed APIs.

`leakprobe.csproj`:

```xml


net10.0
enable
false






```

`LeakTest.cs`:

```csharp
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Microsoft.Maui.Controls;
using Xunit;

public sealed class LeakTest
{
const int N = 30;

enum Scenario
{
Control,
Leaky,
Mitigation
}

[Fact]
public void VisualState_StateTriggers_Clear_Leaks()
{
var control = CreateVictims(Scenario.Control);
var leaky = CreateVictims(Scenario.Leaky);
var mitigation = CreateVictims(Scenario.Mitigation);

for (var i = 0; i < 7; i++)
{
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
}

Assert.Equal(0, CountAlive(control));
Assert.Equal(N, CountAlive(leaky));
Assert.Equal(0, CountAlive(mitigation));
}

[System.Runtime.CompilerServices.MethodImpl(
System.Runtime.CompilerServices.MethodImplOptions.NoInlining)]
static List> CreateVictims(Scenario scenario)
{
var references = new List>(N);
var sendAttached = typeof(StateTriggerBase).GetMethod(
"SendAttached", BindingFlags.Instance | BindingFlags.NonPublic)!;
var sendDetached = typeof(StateTriggerBase).GetMethod(
"SendDetached", BindingFlags.Instance | BindingFlags.NonPublic)!;

for (var i = 0; i < N; i++)
{
var payload = new byte[1024 * 1024];
references.Add(new WeakReference(payload));

if (scenario == Scenario.Control)
continue;

var trigger = new SharedPublisherTrigger(payload);
var state = new VisualState { Name = "Probe" };
state.StateTriggers.Add(trigger);
var group = new VisualStateGroup();
group.States.Add(state);
VisualStateManager.SetVisualStateGroups(
new Label(), new VisualStateGroupList { group });

sendAttached.Invoke(trigger, null);
if (scenario == Scenario.Mitigation)
sendDetached.Invoke(trigger, null);

state.StateTriggers.Clear();
}

return references;
}

static int CountAlive(IEnumerable> references)
=> references.Count(reference => reference.TryGetTarget(out _));

sealed class SharedPublisherTrigger : StateTriggerBase
{
readonly byte[] _payload;

public SharedPublisherTrigger(byte[] payload) => _payload = payload;

protected override void OnAttached() => SharedPublisher.Changed += OnChanged;

protected override void OnDetached() => SharedPublisher.Changed -= OnChanged;

void OnChanged(object? sender, EventArgs e) => GC.KeepAlive(_payload);
}

static class SharedPublisher
{
public static event EventHandler? Changed;
}
}
```

Run:

```bash
dotnet test --logger "console;verbosity=normal"
```

## Observed results

| Scenario | Alive after full GC | Retained payload |
|---|---:|---:|
| Control (trigger never attached) | 0/30 | 0 MB |
| Mitigation (`SendDetached` before `Clear`) | 0/30 | 0 MB |
| Leaky (`Clear` while attached) | 30/30 | 30 MB |

## Scope

- **Affected platforms:** All; the collection and trigger lifecycle are cross-platform managed code.
- **Condition:** A trigger has subscribed to a long-lived publisher from `OnAttached()`, then is removed by mutating `VisualState.StateTriggers` without first detaching it.
- **Assessment:** This is a framework lifecycle bug rather than merely a usage footgun: collection removal should pair the framework-issued attachment with detachment.

## Suggested fix

Make `VisualState.StateTriggers` removal operations notify `VisualState` of removed triggers and call `SendDetached()` before clearing their `VisualState` association. Cover `Clear`, `Remove`, `RemoveAt`, and index replacement so every collection mutation preserves the attach/detach pairing.

> Generated by [Daily Memory Leak Hunter](https://github.com/dotnet/maui/actions/runs/34003449717) · gpt56 · 707.4 AIC · ⌖ 10.7 AIC · ⊞ 32.5K · [◷](https://github.com/search?q=repo%3Adotnet%2Fmaui+is%3Aissue+%22gh-aw-workflow-call-id%3A+dotnet%2Fmaui%2Fdaily-leak-hunter%22&type=issues)

Contributor guide

Open the contributing guide

Research direction

Read StateTriggerBase.cs:52-75 and VisualStateManager.cs:763-770, then inspect WatchAddList at lines 858-929. Run the supplied LeakTest.cs with dotnet test to reproduce the retention, and verify that Clear, Remove, RemoveAt, and index replacement detach removed triggers and eliminate the retained payloads.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
mobile-dev
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.