[leak-scan] VisualState.StateTriggers — removal skips trigger detachment
- 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
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