BrokenRulesCollection: InvalidOperationException: Collection was modified;

[StefanOssendorf]

Describe the bug
When querying the BrokenRulesCollection with any kind of operation (Select, Any, ...) it's possible to provoke an InvalidOperationException: Collection was modified; enumeration operation may not execute.. This can happen when at the same time an async rule finishes and modifies the BrokenRulesCollection.

In our case it happend during myBO.BrokenRulesCollection.Any(br => br.Severity == severity).

Possible solution
I think we should somehow ensure we return a snapshot of the list to avoid these kind of error.

Version and Platform
CSLA version: 9.0.0
OS: Windows
Platform: WPF, ASP.NET Core

Stack Trace or Exception Detail
InvalidOperationException: Collection was modified; enumeration operation may not execute.

[hurcane]

In my opinion, this is a .NET behavior, and not a bug in Csla. You cannot modify a collection while an IEnumerable is active on the collection.

A snapshot is the fix, but you can do that yourself by adding.ToList() or .ToArray() on the end of your Linq query. If you change the behavior of the Csla method to always call ToList internally, you are adding overhead that is unnecessary when we only need the IEnumerable to iterate the broken rules.

Given how frequently the method is used, I think this overhead has a high chance of measurably inflating memory usage and increased CPU usage.

[StefanOssendorf]

I agree that we have to take memory and processing time into account. But s ToList() does not solve the problem. It's just as any other method iterating over the collection. Which suffers from the same racing condition.

[Bowman74]

I think @StefanOssendorf is correct. Sure it is normal .Net behavior when multithreaded operations are not accounted for. There are several thread safe collection types that could prevent this. However, the BrokenRulesCollection currently (and for many good reasons) ultimately inherits from ObservableCollection which is not by default thread safe.

ToList and ToArray are also not thread safe, they just reduce the chance of it occurring but doing one quick and immediate function to get a copy of the list. But it still could cause the issue.

The "correct" way to deal with this (IMOHO) is to either inherit from a thread safe collection type (very difficult due to all the other functionality the collection type it currently inherits from has) or modify the BrokenRulesColleciton so it can only be accessed through something like a SemaphoreSlim that ensures that the list is not being changed while also being iterated through.