Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UIElement.Measure not threadsafe in .NET 7+ #8447

Closed
sra-michaelgold opened this issue Nov 21, 2023 · 4 comments
Closed

UIElement.Measure not threadsafe in .NET 7+ #8447

sra-michaelgold opened this issue Nov 21, 2023 · 4 comments
Labels
needs more information Not enough information has been provided. Please share more detail as requested

Comments

@sra-michaelgold
Copy link

sra-michaelgold commented Nov 21, 2023

Description

It seems a change was introduced as part of the .NET 7 release that makes UIElement.Measure no longer thread-safe.

Our application loads a number of XPS reports (our custom objects that extend ContentControl) in separate dispatcher threads that we manage. Prior to upgrading to .NET 7 we can call Measure on the reports in parallel with no issues. Once upgrading to .NET 7 (same also happens in .NET 8) we start to get the exception below.

I think the problem may have been introduced in this commit:
dotnet/runtime@826896f
where hashtables were changed to dictionaries. Though the hashtable is not necessarily threadsafe, it seems to effectively be acting "more threadsafe" than the dictionary. That said, it's hard to follow this code, so it could be a red-herring.

I can get around the issue by synchronizing the calls to .Measure(), but ideally the behavior in .NET7+ would work the same as .NET 6 (and prior).

 ---> System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.ComponentModel.ReflectPropertyDescriptor.AddValueChanged(Object component, EventHandler handler)
   at MS.Internal.Data.ValueChangedEventManager.AddListener(Object source, PropertyDescriptor pd, IWeakEventListener listener, EventHandler`1 handler)
   at MS.Internal.Data.ValueChangedEventManager.AddHandler(Object source, EventHandler`1 handler, PropertyDescriptor pd)
   at MS.Internal.Data.PropertyPathWorker.ReplaceItem(Int32 k, Object newO, Object parent)
   at MS.Internal.Data.PropertyPathWorker.UpdateSourceValueState(Int32 k, ICollectionView collectionView, Object newValue, Boolean isASubPropertyChange)
   at System.Windows.Data.BindingExpression.Activate(Object item)
   at System.Windows.Data.BindingExpression.AttachToContext(AttachAttempt attempt)
   at System.Windows.Data.BindingExpression.AttachOverride(DependencyObject target, DependencyProperty dp)
   at System.Windows.StyleHelper.GetInstanceValue(UncommonField`1 dataField, DependencyObject container, FrameworkElement feChild, FrameworkContentElement fceChild, Int32 childIndex, DependencyProperty dp, Int32 i, EffectiveValueEntry& entry)
   at System.Windows.StyleHelper.GetChildValueHelper(UncommonField`1 dataField, ItemStructList`1& valueLookupList, DependencyProperty dp, DependencyObject container, FrameworkObject child, Int32 childIndex, Boolean styleLookup, EffectiveValueEntry& entry, ValueLookupType& sourceType, FrameworkElementFactory templateRoot)
   at System.Windows.StyleHelper.GetChildValue(UncommonField`1 dataField, DependencyObject container, Int32 childIndex, FrameworkObject child, DependencyProperty dp, FrugalStructList`1& childRecordFromChildIndex, EffectiveValueEntry& entry, ValueLookupType& sourceType, FrameworkElementFactory templateRoot)
   at System.Windows.StyleHelper.GetValueFromTemplatedParent(DependencyObject container, Int32 childIndex, FrameworkObject child, DependencyProperty dp, FrugalStructList`1& childRecordFromChildIndex, FrameworkElementFactory templateRoot, EffectiveValueEntry& entry)
   at System.Windows.StyleHelper.InvalidatePropertiesOnTemplateNode(DependencyObject container, FrameworkObject child, Int32 childIndex, FrugalStructList`1& childRecordFromChildIndex, Boolean isDetach, FrameworkElementFactory templateRoot)
   at System.Xaml.XamlObjectWriter.Logic_CreateAndAssignToParentStart(ObjectWriterContext ctx)
   at System.Xaml.XamlObjectWriter.WriteStartMember(XamlMember property)
   at System.Windows.FrameworkTemplate.LoadTemplateXaml(XamlReader templateReader, XamlObjectWriter currentWriter)

Reproduction Steps

Create a number of STA dispatcher threads and load the same XAML based control in all of the threads and call Measure() on the controls

Expected behavior

No exceptions

Actual behavior

 System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.ComponentModel.ReflectPropertyDescriptor.AddValueChanged(Object component, EventHandler handler)
   at MS.Internal.Data.ValueChangedEventManager.AddListener(Object source, PropertyDescriptor pd, IWeakEventListener listener, EventHandler`1 handler)
   at MS.Internal.Data.ValueChangedEventManager.AddHandler(Object source, EventHandler`1 handler, PropertyDescriptor pd)
   at MS.Internal.Data.PropertyPathWorker.ReplaceItem(Int32 k, Object newO, Object parent)
   at MS.Internal.Data.PropertyPathWorker.UpdateSourceValueState(Int32 k, ICollectionView collectionView, Object newValue, Boolean isASubPropertyChange)
   at System.Windows.Data.BindingExpression.Activate(Object item)
   at System.Windows.Data.BindingExpression.AttachToContext(AttachAttempt attempt)
   at System.Windows.Data.BindingExpression.AttachOverride(DependencyObject target, DependencyProperty dp)
   at System.Windows.StyleHelper.GetInstanceValue(UncommonField`1 dataField, DependencyObject container, FrameworkElement feChild, FrameworkContentElement fceChild, Int32 childIndex, DependencyProperty dp, Int32 i, EffectiveValueEntry& entry)
   at System.Windows.StyleHelper.GetChildValueHelper(UncommonField`1 dataField, ItemStructList`1& valueLookupList, DependencyProperty dp, DependencyObject container, FrameworkObject child, Int32 childIndex, Boolean styleLookup, EffectiveValueEntry& entry, ValueLookupType& sourceType, FrameworkElementFactory templateRoot)
   at System.Windows.StyleHelper.GetChildValue(UncommonField`1 dataField, DependencyObject container, Int32 childIndex, FrameworkObject child, DependencyProperty dp, FrugalStructList`1& childRecordFromChildIndex, EffectiveValueEntry& entry, ValueLookupType& sourceType, FrameworkElementFactory templateRoot)
   at System.Windows.StyleHelper.GetValueFromTemplatedParent(DependencyObject container, Int32 childIndex, FrameworkObject child, DependencyProperty dp, FrugalStructList`1& childRecordFromChildIndex, FrameworkElementFactory templateRoot, EffectiveValueEntry& entry)
   at System.Windows.StyleHelper.InvalidatePropertiesOnTemplateNode(DependencyObject container, FrameworkObject child, Int32 childIndex, FrugalStructList`1& childRecordFromChildIndex, Boolean isDetach, FrameworkElementFactory templateRoot)
   at System.Xaml.XamlObjectWriter.Logic_CreateAndAssignToParentStart(ObjectWriterContext ctx)
   at System.Xaml.XamlObjectWriter.WriteStartMember(XamlMember property)
   at System.Windows.FrameworkTemplate.LoadTemplateXaml(XamlReader templateReader, XamlObjectWriter currentWriter)

Regression?

Yes, this worked fine in .NET6 and broke in .NET7+

Known Workarounds

Synchronize access to Measure()

Impact

Medium - there is a work-around, but it may have some performance impacts

Configuration

.NET7, .NET8
Windows 10 x64
I don't know if it is specific to this config.

Other information

No response

@pchaurasia14
Copy link
Member

@sra-michaelgold - Can you please share a sample repro if possible. (or a TTD) ?

@pchaurasia14 pchaurasia14 added the needs more information Not enough information has been provided. Please share more detail as requested label Nov 21, 2023
@Laniusexcubitor
Copy link

Looking at the callstack of your exception, I see that you bind to objects that do not implement INotifyPropertyChanged, creating a binding leak. Implementing INotifyPropertyChanged on those bound objects or wrapping them with a ViewModel might be a better workaround than synchronizing calls. Actually I would not even call it a workaround. It's just the better way for the bindings and should not have a thread problem.

@sra-michaelgold
Copy link
Author

@pchaurasia14 - I will try to get a repro up and running. @Laniusexcubitor - You raise a good point. Since these are essentially reports, we may be able to set our bindings to OneTime, which avoids this whole issue.

I will report back with either a repro or an update on changing the bindings.

@sra-michaelgold
Copy link
Author

As a late follow up to this, I took @Laniusexcubitor's suggestion and implemented INPC on all the relevant view models (even if their values never changed after construction). I also set Binding Mode to OneTime/OneWay wherever possible and it has resolved the issue in the sense that the ValueChangedEventManager.AddListener() is no longer called and the error is thus no longer thrown.

Thank you

@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs more information Not enough information has been provided. Please share more detail as requested
Projects
None yet
Development

No branches or pull requests

3 participants