[Bug]: Cache dynamic Filter throws InvalidOperationException during ReFilter

[Mankov-A]

Describe the bug 🐞

After upgrading DynamicData to 9.4.31, changing a dynamic cache filter predicate started throwing InvalidOperationException.

The same code worked with DynamicData 9.1.2.

The operator shape is:

sourceCache
    .Connect()
    .Filter(predicateObservable)

where predicateObservable is an IObservable<Func<TObject, bool>>.

Exception:

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
   at System.Collections.Generic.Dictionary`2.KeyCollection.Enumerator.MoveNext()
   at DynamicData.Cache.Internal.Filter.Dynamic`3.Subscription.ReFilter(TState predicateState)
   at DynamicData.Cache.Internal.Filter.Dynamic`3.Subscription.OnPredicateStateNext(TState predicateState)

I suspect the issue is in Cache/Internal/Filter.Dynamic.cs, in ReFilter.

The code appears to enumerate dictionary keys and update the same dictionary inside the loop:

foreach (var key in _itemStatesByKey.Keys)
{
    var itemState = _itemStatesByKey[key];
    var isIncluded = _predicate.Invoke(predicateState, itemState.Item);

    _itemStatesByKey[key] = new()
    {
        IsIncluded = isIncluded,
        Item = itemState.Item
    };
}

On .NET Framework, assigning an existing Dictionary<TKey, TValue> value invalidates enumeration of Dictionary.Keys, so MoveNext() throws InvalidOperationException.

I have not verified whether the same repro fails on .NET 6/8/9.

Step to reproduce

1. Create a SourceCache<TObject, TKey>.
2. Connect to it with .Connect().
3. Apply dynamic cache filtering with .Filter(predicateObservable), where predicateObservable is an IObservable<Func<TObject, bool>>.
4. Add at least one item to the cache.
5. Push a new predicate into predicateObservable.
6. Observe InvalidOperationException from Filter.Dynamic.Subscription.ReFilter.

Reproduction repository

Expected behavior

Changing the dynamic predicate should re-filter the cache without throwing.

Screenshots 🖼️

No response

IDE

No response

Operating system

No response

Version

No response

Device

No response

DynamicData Version

9.4.31

Additional information ℹ️

Real stack trace from the app:

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
   at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary`2.KeyCollection.Enumerator.MoveNext()
   at DynamicData.Cache.Internal.Filter.Dynamic`3.Subscription.ReFilter(TState predicateState)
   at DynamicData.Cache.Internal.Filter.Dynamic`3.Subscription.OnPredicateStateNext(TState predicateState)
   at System.Reactive.AnonymousObserver`1.OnNextCore(T value)
   at System.Reactive.ObserverBase`1.OnNext(T value)
   at System.Reactive.Sink`1.ForwardOnNext(TTarget value)
   at System.Reactive.Linq.ObservableImpl.Select`2.Selector._.OnNext(TSource value)
   at System.Reactive.Sink`1.ForwardOnNext(TTarget value)
   at System.Reactive.Linq.ObservableImpl.Throttle`1._.Propagate()

Possible fix would be to avoid mutating the dictionary while enumerating Keys, for example by iterating over a snapshot of keys or by buffering state updates and applying them after enumeration.

[JakenVeina]

So, I haven't been able to reproduce what you describe on current .NET runtimes. And I'm having a HELL of a lot of trouble setting up a Windows VM to try it on .NET Framework. So, standby.

For reference, this was my attempt at a reproduction.

public static class EntryPoint
{
    public static void Main()
    {
	    using var itemsSource = new SourceCache<Item, int>(static item => item.Id);
	    using var predicateSource = new BehaviorSubject<Func<Item, bool>>(static item => (item.Id % 2) == 0);

	    using var subscription = itemsSource
		    .Connect()
		    .Filter(predicateSource)
		    .Subscribe();
            
	    itemsSource.AddOrUpdate(new Item()
	    {
		    Id = 1,
		    Name = "Item #1"
	    });
            
	    predicateSource.OnNext(static item => (item.Id % 2) != 0);
    }
}

public class Item
{
	public int Id { get; set; }
            
	public string Name { get; set; }
}

[JakenVeina]

Yep. Confirmed this is only an issue for targets prior to .NET Core 3.0. Per docs, they managed to rework .Remove() and .Clear() in .NET Core 3.0 to eliminate the need for them to invalidate enumerations. Which I guess is actually not true, cause .ReFilter() is doing neither, it's doing a replace.