efcore: Document that modifying entity states while iterating over entries can result in "Collection was modified" exception

System.InvalidOperationException : Collection was modified; enumeration operation may not execute.
   at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary`2.ValueCollection.Enumerator.MoveNext()
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityReferenceMap.<GetEntriesForState>d__12.MoveNext() in src\EFCore\ChangeTracking\Internal\EntityReferenceMap.cs:line 363
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at Microsoft.EntityFrameworkCore.GraphUpdatesTestBase`1.<Detaching_dependent_entity_will_remove_references_to_it>b__107_0(DbContext context) in test\EFCore.Specification.Tests\GraphUpdatesTestBase.cs:line 919
   at Microsoft.EntityFrameworkCore.TestUtilities.TestHelpers.<>c__DisplayClass39_0`1.<ExecuteWithStrategyInTransaction>b__0(TContext context) in test\EFCore.Specification.Tests\TestUtilities\TestHelpers.cs:line 429
   at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.<>c__2`1.<Execute>b__2_0(<>f__AnonymousType0`2 s) in src\EFCore\Storage\ExecutionStrategyExtensions.cs:line 73
   at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.<>c__DisplayClass12_0`2.<Execute>b__0(DbContext c, TState s) in src\EFCore\Storage\ExecutionStrategyExtensions.cs:line 338
   at Microsoft.EntityFrameworkCore.Storage.ExecutionStrategy.ExecuteImplementation[TState,TResult](Func`3 operation, Func`3 verifySucceeded, TState state) in src\EFCore\Storage\ExecutionStrategy.cs:line 195
   at Microsoft.EntityFrameworkCore.Storage.ExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded) in src\EFCore\Storage\ExecutionStrategy.cs:line 159
   at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.Execute[TState,TResult](IExecutionStrategy strategy, Func`2 operation, Func`2 verifySucceeded, TState state) in src\EFCore\Storage\ExecutionStrategyExtensions.cs:line 336
   at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.Execute[TState,TResult](IExecutionStrategy strategy, TState state, Func`2 operation) in src\EFCore\Storage\ExecutionStrategyExtensions.cs:line 286
   at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.Execute[TState](IExecutionStrategy strategy, TState state, Action`1 operation) in src\EFCore\Storage\ExecutionStrategyExtensions.cs:line 70
   at Microsoft.EntityFrameworkCore.TestUtilities.TestHelpers.ExecuteWithStrategyInTransaction[TContext](Func`1 createContext, Action`2 useTransaction, Action`1 testOperation, Action`1 nestedTestOperation1, Action`1 nestedTestOperation2, Action`1 nestedTestOperation3) in test\EFCore.Specification.Tests\TestUtilities\TestHelpers.cs:line 421
   at Microsoft.EntityFrameworkCore.GraphUpdatesTestBase`1.ExecuteWithStrategyInTransaction(Action`1 testOperation, Action`1 nestedTestOperation1, Action`1 nestedTestOperation2, Action`1 nestedTestOperation3) in test\EFCore.Specification.Tests\GraphUpdatesFixtureBase.cs:line 2782
   at Microsoft.EntityFrameworkCore.GraphUpdatesTestBase`1.Detaching_dependent_entity_will_remove_references_to_it()

Use feature/net472 branch to investigate

About this issue

Original URL
State: closed
Created 5 years ago
Comments: 16 (12 by maintainers)

Commits related to this issue

Update API docs * Document connection obtained from DatabaseFacade.GetDbConnection() should normally not be disposed Fixes #11415 * Add XML docs referencing how to determine the default CommandTimeou... — committed to dotnet/efcore by ajcvickers 4 years ago
Update API docs * Document connection obtained from DatabaseFacade.GetDbConnection() should normally not be disposed Fixes #11415 * Add XML docs referencing how to determine the default CommandTimeou... — committed to dotnet/efcore by ajcvickers 4 years ago
Update API docs (#22626) * Update API docs * Document connection obtained from DatabaseFacade.GetDbConnection() should normally not be disposed Fixes #11415 * Add XML docs referencing how to dete... — committed to dotnet/efcore by ajcvickers 4 years ago

Most upvoted comments

@roji I agree that the perf impact of doing a snapshot every time is likely too much–this is an area that can be sensitive to perf. Let’s discuss in triage.

ajcvickers on Oct 25, 2019

Thanks! Worked without problems.

jamesport079 on Oct 24, 2019

This happens because of the behavior introduced in 7e65b82821ce2540d12d35c6449712f07c56a527, which the failing test exercises (removing references to an entity when it is detached. Detaching_dependent_entity_will_remove_references_to_it iterates over the change tracker entries. It then sets all entries to state detached, which internally removes them from the entity reference map for their old state (_unchangedReferenceMap in this case). So we have EntityReferenceMap.GetEntriesForState iterating over _unchangedReferenceMap on the one hand, and EntityReferenceMap.Remove modifying the same dictionary.

The somewhat mysterious thing is that .NET Framework fails on this while .NET Core doesn’t - but the dictionary implementation has likely changed so much that it’s not implausible. I’ve tried to put together a quick minimal repro of remove-while-iterating on Dictionary which works on Core and fails on Framework, but it’s more subtle than a 2 minute effort. In any case, the exception is the expected behavior here, and it could happen in .NET Core as well.

We could fix this simply by adding ToList in the test. However, the possibly problematic thing is that it’s not obvious that setting an entry state change modifies the change tracker in a way that would trigger this exception (i.e. I’d expect people to fall into this pit a lot). We could switch to ConcurrentDictionary but that’d probably be bad for perf, any thoughts @ajcvickers?

@jamesport079 you should be able to at least work around your ReloadAsync issue by putting ToList after Entries<LabWork>, allowing you to continue being in 3.0.

roji on Oct 24, 2019