dotnet / dotnet/wpf

Adding a child during a layout pass throws in Canvas, and the added child is never measured in any panel

Open
#11,846 1 comment 0 reactions 0 assignees View on GitHub
Bug Cost:M Priority:2
Dominant language
C#
Stars
7.7k
Forks
1.3k
Avg merge
1d 11h
Merged PRs (30d)
61

Description

### Description

Two related problems, both reproducible in ~60 lines.

**1. `Canvas` enumerates its children during layout.**

`Canvas.MeasureOverride` and `Canvas.ArrangeOverride` walk `InternalChildren` with `foreach`. If measuring or arranging a child causes a child to be added to that same `Canvas`, `VisualCollection.Enumerator.MoveNext` throws `InvalidOperationException: The enumerator is not valid because the collection changed`.

`UniformGrid.ArrangeOverride` has the same shape (its `MeasureOverride` indexes). `DockPanel`, `WrapPanel` and `Grid` index and do not throw.

**2. The added child is never measured - in *any* panel.**

This is the part that makes the first problem hard to fix correctly. Every mutating path in `UIElementCollection` calls `_visualParent.InvalidateMeasure()`, but `UIElement.InvalidateMeasure` does nothing while the parent is mid-measure:

```csharp
public void InvalidateMeasure()
{
if( !MeasureDirty
&& !MeasureInProgress ) // parent is measuring: whole block skipped
{
if(!NeverMeasured) // and a never-measured element is barred from the queue
{ ... MeasureQueue.Add(this) ... }
```

The invalidation is dropped, and no later pass picks the new child up. So a panel that indexes instead of enumerating does not throw, but silently leaves the child unmeasured - it renders as nothing.

Case B in the repro demonstrates this against a minimal indexing panel. Changing `Canvas` to a plain indexed loop would therefore trade a loud crash for an invisible element, which is worse to diagnose, not better.

### Reproduction Steps

`repro.csproj`:

```xml


Exe
net472;net10.0-windows
true
false

```

`Program.cs`:

```csharp
using System;
using System.Runtime.InteropServices;
using System.Windows;
using System.Windows.Controls;

internal sealed class Counter : FrameworkElement
{
public int MeasureCount;
protected override Size MeasureOverride(Size availableSize)
{
MeasureCount++;
return new Size(10, 10);
}
}

// Adds a sibling to its own parent from inside its measure pass, at index 0,
// i.e. behind the position the parent's walk has already passed.
internal sealed class Mutator : FrameworkElement
{
public Panel Target;
public UIElement Insert;
private bool _done;

protected override Size MeasureOverride(Size availableSize)
{
if (!_done)
{
_done = true;
Target.Children.Insert(0, Insert);
}
return new Size(10, 10);
}
}

// A panel that indexes instead of enumerating, with no repeat pass.
internal sealed class IndexingPanel : Panel
{
protected override Size MeasureOverride(Size constraint)
{
UIElementCollection children = InternalChildren;
for (int i = 0; i < children.Count; i++)
{
children[i]?.Measure(new Size(double.PositiveInfinity, double.PositiveInfinity));
}
return new Size();
}
}

internal static class Program
{
[STAThread]
private static void Main()
{
Console.WriteLine(RuntimeInformation.FrameworkDescription);
Console.WriteLine();

Counter addedA = new Counter();
try
{
Canvas canvas = new Canvas();
canvas.Children.Add(new Counter());
canvas.Children.Add(new Mutator { Target = canvas, Insert = addedA });
canvas.Measure(new Size(100, 100));
Console.WriteLine($"A Canvas : no exception, addedA.MeasureCount={addedA.MeasureCount}");
}
catch (Exception ex)
{
Console.WriteLine($"A Canvas : {ex.GetType().FullName}");
Console.WriteLine($" {ex.Message}");
Console.WriteLine($" addedA.MeasureCount={addedA.MeasureCount}");
}

Counter addedB = new Counter();
IndexingPanel panel = new IndexingPanel();
panel.Children.Add(new Counter());
panel.Children.Add(new Mutator { Target = panel, Insert = addedB });
panel.Measure(new Size(100, 100));
Console.WriteLine($"B IndexingPanel : no exception, addedB.MeasureCount={addedB.MeasureCount}");
}
}
```

Run:

```
dotnet run -f net10.0-windows
dotnet run -f net472
```

### Expected behavior

Preferred: an element added to a panel during a layout pass should still be measured - in the current pass or a subsequent one - rather than having its parent's `InvalidateMeasure` silently discarded because the parent happens to be mid-measure.

At minimum, the failure should be diagnosable. The exception should identify the panel and the child that mutated the collection, instead of surfacing as a bare `VisualCollection.Enumerator` failure whose stack contains no application frames.

Separately, `Canvas.MeasureOverride`/`ArrangeOverride` and `UniformGrid.ArrangeOverride` are inconsistent with `DockPanel`, `WrapPanel` and `Grid`, which index.

### Actual behavior

```
.NET 10.0.11

A Canvas : System.InvalidOperationException
The enumerator is not valid because the collection changed.
addedA.MeasureCount=0
B IndexingPanel : no exception, addedB.MeasureCount=0
```

On `.NET Framework 4.8.9337.0` the output is byte-identical apart from the version banner.

Case A throws. Case B does not throw, but the added child is still never measured.

### Regression?

No. Output is identical on .NET Framework 4.8.9337.0 and .NET 10.0.11.

`git log -S "foreach (UIElement child in InternalChildren)" -- .../Canvas.cs` returns exactly one commit - the May 2019 bulk source import that opened WPF. Both occurrences were present in that first blob and have never been modified; the only subsequent commits to the file are style cleanups. The behavior predates the public history.

### Known Workarounds

Subclass the panel, index instead of enumerating, and repeat while the count changes:

```csharp
protected override Size MeasureOverride(Size constraint)
{
UIElementCollection children = InternalChildren;
int count;
do
{
count = children.Count;
for (int i = 0; i < children.Count; ++i)
{
children[i]?.Measure(childConstraint);
}
}
while (children.Count != count);

return new Size();
}
```

The repeat is required, not defensive - without it, a child inserted behind the walk position is never measured (case B).

Limitations: it only applies if you own the panel, so it cannot be applied to a stock `Canvas` used directly; and it cannot be fixed from the calling side either, because there is no public API to ask whether the current code is running inside a layout pass.

### Impact

Intensity: case A is an unhandled exception on the UI thread, i.e. a process crash. Case B is a silently missing visual.

Reach: any application where a control lazily creates a sibling visual during layout - for example, an overlay or adornment layer created on first request from inside a child's measure.

Diagnosis cost is the main problem. The mutating caller has already returned by the time the enumerator throws, so the resulting stack contains only WPF frames and no application frames. We root-caused an instance of case A in a large commercial WPF application, and it required heap dump analysis to identify the code path.

### Configuration

- .NET 10.0.11 (Microsoft.WindowsDesktop.App 10.0.11) and .NET Framework 4.8.9337.0
- Windows, x64
- Not specific to either runtime or configuration - output is identical on both.

### Other information

All references verified against `main`:

- `PresentationFramework/System/Windows/Controls/Canvas.cs` - `foreach` at lines 259 (`MeasureOverride`) and 282 (`ArrangeOverride`).
- `PresentationFramework/System/Windows/Controls/Primitives/UniformGrid.cs` - `foreach` at line 208 (`ArrangeOverride`); `MeasureOverride` at 161 indexes.
- `PresentationCore/System/Windows/UIElement.cs` - `InvalidateMeasure` at line 249; the `!MeasureInProgress` guard at 252 and the `if(!NeverMeasured)` gate at 258 are what drop the invalidation.
- `PresentationFramework/System/Windows/Controls/UIElementCollection.cs` - eight call sites for `_visualParent.InvalidateMeasure()` (129, 149, 177, 206, 271, 296, 317, 368), all unconditional.
- `DockPanel`, `WrapPanel` and `Grid` index in both overrides. `VirtualizingStackPanel`'s only `foreach` over `InternalChildren` is inside a `[Conditional("DEBUG")]` verifier, not a layout path.

Contributor guide

Open the contributing guide

Research direction

Start by running the supplied repro in Program.cs for net10.0-windows and net472. Read Canvas.cs, UniformGrid.cs, UIElement.cs, and UIElementCollection.cs, then compare the layout loops with DockPanel, WrapPanel, and Grid. Done means a child added during layout is measured in the current or a subsequent pass, without an unhelpful collection-enumerator failure.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
desktop
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.