canton7 / Stylet

A very lightweight but powerful ViewModel-First MVVM framework for WPF for .NET Framework and .NET Core, inspired by Caliburn.Micro.
MIT License
981 stars 143 forks source link

BindableCollection races against any call which adds/removes elements before it #318

Open canton7 opened 2 years ago

canton7 commented 2 years ago

Discussed in https://github.com/canton7/Stylet/discussions/306

Originally posted by **Yoooi0** November 21, 2021 There seems to be an issue with thread safety in BindableCollection. When two threads modify the collection at the same time, one calling [ClearItems](https://github.com/canton7/Stylet/blob/5e97f9e1ea8c45e9d7033e6e7c7ea8ce41c3420a/Stylet/BindableCollection.cs#L216), the other calling [RemoveItem](https://github.com/canton7/Stylet/blob/5e97f9e1ea8c45e9d7033e6e7c7ea8ce41c3420a/Stylet/BindableCollection.cs#L203), it can cause `OnCollectionChanging` to throw an exception. ``` System.Reflection.TargetInvocationException: "An error occurred while dispatching a call to the UI Thread" Inner Exception ArgumentOutOfRangeException: "Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')" at Stylet.Execute.OnUIThreadSync(Action action) at Stylet.BindableCollection`1.RemoveItem(Int32 index) at System.Collections.ObjectModel.Collection`1.Remove(T item) ``` Example timeline: * Create BindableCollection with 1 item * Thread 1 calls `collection.ClearItems()` * Thread 1 enters `Execute.OnUIThreadSync` * Thread 2 calls `collection.Remove(firstItem)`, this calls `RemoveItem` with `index` argument set to `0` * Thread 2 waits for thread 1 * Thread 1 clears the collection by calling `base.ClearItems()` * Thread 2 enters `Execute.OnUIThreadSync` * Thread 2 throws exception when calling `OnCollectionChanging` because the item at index 0 does not exist anymore