dotnet / dotnet/wpf

System.ExecutionEngineException Within TextContainer from incorrectly checking UndoManager Scope

Open
#11,129 2 comments 1 reaction 0 assignees View on GitHub
:beetle: bug Investigate
Dominant language
C#
Stars
7.7k
Forks
1.3k
Avg merge
1d 11h
Merged PRs (30d)
61

Description

### Description

If you have a context menu on a `TextBox` that has a tooltip that contains has read-only text component that uses a `TextContainer` internally, like a `FlowDocumentScrollViewer` that has its document bound to a value; when that binding updates the `TextContainer` will not check itself to see if it has an undoscope, but rather check via the inherited attached property `UndoManager.UndoManagerInstanceProperty` which will resolve the undo manager from the outer scope of the `TextBox` that has the context menu.
Because it then checks to see if there's an UndoManager assigned to this `TextContainer` it then throws a `System.ExecutionEngineException` if it thinks it's in this invalid state (why the extreme hard crash?)

### Reproduction Steps

Try this sample app, and hover over the menu item in the context menu to repro

.xaml
```









```

.cs
```
using System.ComponentModel;
using System.Diagnostics;
using System.Reflection;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Documents;
using System.Windows.Threading;

namespace ToolTips;

///
/// Interaction logic for MainWindow.xaml
///
public partial class MainWindow : Window
{
public MainWindow()
{
InitializeComponent();
DataContext = new ToolTipData();
}

private void FlowDocumentScrollViewer_Loaded(object sender, RoutedEventArgs e)
{
Dispatcher.BeginInvoke(() =>
{
var type = typeof(TextBox).Assembly.GetType("MS.Internal.Documents.UndoManager");
var method = type?.GetMethod("GetUndoManager", BindingFlags.NonPublic | BindingFlags.Static);

var result = method.Invoke(null, new object[] { sender });
if (result is not null)
{
// if we're in here, the bug is about to happen, we've collected an undo scope from the outer text box
FieldInfo field = result.GetType().GetField("_scope", BindingFlags.NonPublic | BindingFlags.Instance);
var fieldValue = field?.GetValue(result) as TextBox;
// you'll notice that the scope is the same instance
Debug.Assert(fieldValue == _textBox);
}

var toolTip = DataContext as ToolTipData;
toolTip!.FlowDocument = new FlowDocument();
var paragraph = new Paragraph(new Run("HelloToolTip2"));
toolTip.FlowDocument.Blocks.Add(paragraph);

toolTip.RaiseChanged(); // this will cause the crash as the flow document found the outer undoscope during the binding refresh
});
}
}

internal class ToolTipData : INotifyPropertyChanged
{
public ToolTipData()
{
FlowDocument = new FlowDocument();
var paragraph = new Paragraph(new Run("HelloToolTip"));
FlowDocument.Blocks.Add(paragraph);
PropertyChanged?.Invoke(this, new(nameof(FlowDocument)));
}

public FlowDocument FlowDocument { get; set; }

public event PropertyChangedEventHandler? PropertyChanged;

public void RaiseChanged()
{
PropertyChanged?.Invoke(this, new(nameof(FlowDocument)));
}
}
```

### Expected behavior

Application should not crash, binding value of the document should update.

### Actual behavior

Application hard crashes when the binding for the document goes to update.
Crash is in `TextContainer.DisableUndo(FrameworkElement uiScope)`
`Invariant.Assert(_undoManager != null, "UndoManager not created.");`

### Regression?

_No response_

### Known Workarounds

Depending on how your app is structured, you can use InheritanceBehavior on a control to prevent the attached property returning any value, yet alone the incorrect one for this scenario. This will also affect resources and relative sources, so not ideal in all scenarios.

### Impact

Hard crash for our application.

### Configuration

_No response_

### Other information

_No response_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.