christophano / Effort.Extra

Apache License 2.0
6 stars 3 forks source link

KeyNotFoundException during CreateIfNotExists() call #15

Open AGBrown opened 6 years ago

AGBrown commented 6 years ago
Environment:

Nugets and versions

(expand for packages.config).

```xml ```

Test are run in parallel using `[assembly: Parallelizable(ParallelScope.All)]`. ##### Sample test This exception can be reproduced with a single test, duplicated several times within the fixture and run in parallel on a multi-core workstation either within visual studio or using the nunit console runner. The sample test code is:
(expand for full code).

```c# [Test] public void CanUseEffort() { var fixture = new Fixture(); // autofixture // --- Use Extra.Effort to pre-seed the database var data = new ObjectData(); var newModel = new Apple { Id = 1, Name = fixture.Create() }; data.Table().Add(newModel); var dataLoader = new ObjectDataLoader(data); using (var connection = DbConnectionFactory.CreateTransient(dataLoader)) using (var context = new TestDbContext(connection, false)) { // Initialise the transient database - this fails intermittently context.Database.CreateIfNotExists(); var dbSet = context.Apples; var actual = dbSet.ToList(); Assert.That(actual.Count, Is.EqualTo(1)); } } ```

##### Problem details When multiple tests are run in parallel, the call to `context.Database.CreateIfNotExists();` in the test intermittently fails with the following exception. This happens even if one test takes the form above, and the others don't do any Extra.Effort setup, just call `CreateIfNotExists` and then assert that the DbSet is empty. ``` Error : EffortPoc.EffortDemoFixture.CanPreLoadEntities System.Collections.Generic.KeyNotFoundException : The key '8f8c545d-5f35-4161-a84a-39ef98c2b003' was not found in the data collection. at Effort.Extra.ObjectDataLoader.CreateTableDataLoaderFactory() in C:\Dev\Effort.Extra\Effort.Extra\ObjectDataLoader.cs:line 61 at Effort.Internal.DbManagement.DbContainer.Initialize(DbSchema schema) at Effort.Provider.EffortProviderServices.<>c__DisplayClass4.b__3(DbConnection x) at Effort.Provider.EffortProviderServices.Wrap[T](DbConnection connection, Func`2 action) at System.Data.Entity.Internal.DatabaseOperations.Create(ObjectContext objectContext) at System.Data.Entity.Internal.DatabaseCreator.CreateDatabase(InternalContext internalContext, Func`3 createMigrator, ObjectContext objectContext) at System.Data.Entity.Database.CreateIfNotExists() at EffortPoc.EffortDemoFixture.CanPreLoadEntities() in EffortDemoFixture.cs:line 51 ``` The guid in the error changes each time the exception is encountered. ##### Other details If the tests are run sequentially, the first test selected to be run takes a few seconds. Subsequent tests take milliseconds. If the tests are run in parallel then all of the tests that run first (at the same time) take a few seconds to run and intermittently one of them will fail with this exception.
AGBrown commented 6 years ago

I initially created this at zzzprojects/EntityFramework-Effort#113, but then realised it more likely applies here.

christophano commented 6 years ago

Oh man, parallel issues are so much fun to track down! I eventually managed to recreate the issue and I seem to have proved my hunch that it was due to the ObjectDataCollection not being a thread safe collection. I've added simple locking in the AddOrUpdate and TryGetValue methods. Fortunately it's an internal class and those are the only methods I use, so I don't think I'll need to worry about the inherited methods.

Are you able to test from the latest commit before I build and push to NuGet, or will I need to create a release before you can test it?

AGBrown commented 6 years ago

Are you able to test from the latest commit before I build and push to NuGet

Sure thing. I'll give it a go.

AGBrown commented 6 years ago

TL;WR: I repro'd the error using a test; I packed a nuget at at both cb18ba471f544ef409870f969ec9043f9f3ceeb3 and 26c5c034f2632e9cfe4dc1b2a34a0f337529de18; the repro test passes on both those commits so from the test's point of view they are good to go.


I've put together an MVCE test using NUnit parallel tests that repeatedly fails on the current 1.4.0 nuget. Anecdotally it fails every 1 in 5 (ish) runs with just two parallel tests. The failures are various (not just the KeyNotFoundException, but also exceptions including "System.InvalidOperationException : Collection was modified; enumeration operation may not execute").

I've then packed a nuget at both cb18ba471f544ef409870f969ec9043f9f3ceeb3 and 26c5c034f2632e9cfe4dc1b2a34a0f337529de18.

The MVCE test does not fail on either of the two new commits with either 16 or 32 parallel threads (equal to the number of cores and number of hyper threads on the dev machine); I've then repeated that test run of n parallel tests 100x using nunit console runner and I wasn't able to get a test failure.

I've not been able to add the tests to your project (as we discussed) as I don't have an accessible VS2017 instance running to properly add the code ... sorry. I think you mentioned you've got tests that repro the issue anyway so I'm not sure you need mine. Here they are just in case:

ChildModel.cs

(expand for source code).

```c# using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; namespace EffortTests { [Table("ChildModel")] public class ChildModel { [Required] [Key] public int Id { get; set; } [Required] [StringLength(50)] public string Nk { get; set; } [Required] public int ParentModelId { get; set; } public ParentModel ParentModel { get; set; } } } ```

  ParentModel.cs
(expand for source code).

```c# using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; namespace EffortTests { [Table("ParentModel")] public class ParentModel { public ParentModel() { ChildModels = new List(); } [Required] [Key] public int Id { get; set; } [Required] [StringLength(50)] public string Nk { get; set; } public ICollection ChildModels { get; set; } } } ```

  TestDbContext.cs
(expand for source code).

```c# using System.ComponentModel.DataAnnotations.Schema; using System.Data.Common; using System.Data.Entity; namespace EffortTests { public class TestDbContext : DbContext { static TestDbContext() { Database.SetInitializer(null); } public TestDbContext(DbConnection connection, bool contextOwnsConnection) : base(connection, contextOwnsConnection) { } protected override void OnModelCreating(DbModelBuilder modelBuilder) { base.OnModelCreating(modelBuilder); ConfigureTestEf(modelBuilder); } private void ConfigureTestEf(DbModelBuilder modelBuilder) { modelBuilder.Entity() .ToTable("ParentModel") .Property(x => x.Id) .HasDatabaseGeneratedOption(DatabaseGeneratedOption.Identity); modelBuilder.Entity() .ToTable("ChildModel") .Property(x => x.Id) .HasDatabaseGeneratedOption(DatabaseGeneratedOption.Identity); modelBuilder.Entity() .HasRequired(x => x.ParentModel) .WithMany(x => x.ChildModels) .HasForeignKey(x => x.ParentModelId); } public DbSet ParentModels { get; set; } public DbSet ChildModels { get; set; } } } ```

  EffortDemoFixture.cs
(expand for source code).

```c# using System.Data.Common; using System.Data.Entity; using System.Linq; using Effort; using Effort.Extra; using NUnit.Framework; using Ploeh.AutoFixture; namespace EffortTests { [TestFixture] public class EffortDemoFixture { #region Fixture setup and DbContext factory methods ///

/// Creates a . The caller must dispose of it. /// private static TestDbContext CreateTestDbContext() { var connection = DbConnectionFactory.CreateTransient(); try { return CreateTestDbContext(connection); } catch (System.Exception) { connection.Dispose(); throw; } } /// /// Creates a . The caller must dispose of it. /// private static TestDbContext CreateTestDbContext(DbConnection connection, bool contextOwnsConnection = true) { return new TestDbContext(connection, contextOwnsConnection); } #endregion Fixture setup and DbContext factory methods [Test] public void CanUseEffort() { // ARRANGE ------------------------------------------------------- using (var context = CreateTestDbContext()) { context.Database.CreateIfNotExists(); // ACT ----------------------------------------------------------- var dbSet = context.Set(); var actual = dbSet.ToList(); // ASSERT -------------------------------------------------------- Assert.That(actual.Count, Is.EqualTo(0)); } } [Test] public void CanPreLoadEntities( // Uncomment to scale number of parallel tests by factors of 2 [Values(1, 2)] int i // 2 ,[Values(1, 2)] int j // 4 ,[Values(1, 2)] int k // 8 ,[Values(1, 2)] int l // 16 //,[Values(1, 2)] int m // 32 ) { // ARRANGE ------------------------------------------------------- var fixture = new Fixture(); // --- Use Extra.Effort to pre-seed the database using code-generated objects --- var expected = fixture.Create(); using (var conn = SetupTestEntitiesAndConnection(i, expected)) using (var context = CreateTestDbContext(conn)) { context.Database.CreateIfNotExists(); // ACT ----------------------------------------------------------- var dbSet = context.Set(); var actual = dbSet.ToList(); // ASSERT -------------------------------------------------------- Assert.That(actual.Count, Is.EqualTo(1)); Assert.That(actual.First().Nk, Is.EqualTo(expected)); } } private static DbConnection SetupTestEntitiesAndConnection(int i, string nk) { var data = new ObjectData(); var newModel = new ParentModel() {Id = i, Nk = nk }; data.Table().Add(newModel); var dataLoader = new ObjectDataLoader(data); var connection = DbConnectionFactory.CreateTransient(dataLoader); return connection; } } } ```