Efcore: Including collection navigation after optional navigation throws NRE with async in 2.0.0

[caleblloyd]

We are trying to upgrade our MySQL provider to 2.0.0. Our Discriminator Test that tests TPH Inheritance is resulting in this NullReferenceException

Here is the Discriminator Test - read it for a good laugh, I'm put some time into making sure the Hierarchies were fun :smile:

Here is the Query that is causing the Exception:

                var teachers = await db.People.OfType<PersonTeacher>()
                    .Where(m => m.Id >= _fixture.LowestTeacherId && m.Id <= _fixture.HighestTeacherId)
                    .OrderBy(m => m.Id)
                    .Include(m => m.Family)
                    .ThenInclude(m => m.Members)
                    .Include(m => m.Students)
                    .ThenInclude(m => m.Family)
                    .ThenInclude(m => m.Members)
                    .ToListAsync();

Here is the Exception:

Failed   Pomelo.EntityFrameworkCore.MySql.PerfTests.Tests.Models.DiscriminatorTest.TestDiscriminator
Error Message:
 System.NullReferenceException : Object reference not set to an instance of an object.
Stack Trace:
   at Microsoft.EntityFrameworkCore.Query.Internal.IncludeCompiler.IncludeLoadTreeNodeBase.<_AwaitMany>d__8.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.EntityFrameworkCore.Query.Internal.IncludeCompiler.<_IncludeAsync>d__18`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.TaskLiftingExpressionVisitor.<_ExecuteAsync>d__8`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.AsyncSelectEnumerable`2.AsyncSelectEnumerator.<MoveNext>d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at System.Linq.AsyncEnumerable.SelectEnumerableAsyncIterator`2.<MoveNextCore>d__7.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at System.Linq.AsyncEnumerable.AsyncIterator`1.<MoveNext>d__10.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.EntityFrameworkCore.Query.Internal.AsyncLinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.<MoveNext>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at System.Linq.AsyncEnumerable.<Aggregate_>d__6`3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at Pomelo.EntityFrameworkCore.MySql.PerfTests.Tests.Models.DiscriminatorTest.<TestDiscriminator>d__2.MoveNext() in /home/caleb/Projects/caleb/Pomelo.EntityFrameworkCore.Mysql/test/EFCore.MySql.FunctionalTests/Tests/Models/DiscriminatorTest.cs:line 96
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

[caleblloyd]

The NullReferenceException appears to be triggered by some EF internal logic in the ThenInclude statements

[maumar]

Still repros in the current bits, and only in async. Full repro:

        [Fact]
        public virtual void Repro8693()
        {
            using (var ctx = new MyContext9038())
            {
                ctx.Database.EnsureDeleted();
                ctx.Database.EnsureCreated();

                var famalies = new List<PersonFamily>
                {
                    new PersonFamily
                    {
                        LastName = "Garrison",
                    },
                    new PersonFamily
                    {
                        LastName = "Cartman",
                    },
                    new PersonFamily
                    {
                        LastName = "McCormick",
                    },
                    new PersonFamily
                    {
                        LastName = "Broflovski",
                    },
                    new PersonFamily
                    {
                        LastName = "Marsh",
                    },
                };
                var teachers = new List<PersonTeacher>
                {
                    new PersonTeacher {Name = "Ms. Frizzle"},
                    new PersonTeacher {Name = "Mr. Garrison", Family = famalies[0]},
                    new PersonTeacher {Name = "Mr. Hat", Family = famalies[0]},
                };
                var students = new List<PersonKid>
                {
                    new PersonKid {Name = "Arnold", Grade = 2, Teacher = teachers[0]},
                    new PersonKid {Name = "Phoebe", Grade = 2, Teacher = teachers[0]},
                    new PersonKid {Name = "Wanda", Grade = 2, Teacher = teachers[0]},

                    new PersonKid {Name = "Eric", Grade = 4, Teacher = teachers[1], Family = famalies[1]},
                    new PersonKid {Name = "Kenny", Grade = 4, Teacher = teachers[1], Family = famalies[2]},
                    new PersonKid {Name = "Kyle", Grade = 4, Teacher = teachers[1], Family = famalies[3]},
                    new PersonKid {Name = "Stan", Grade = 4, Teacher = teachers[1], Family = famalies[4]},
                };

                ctx.People.AddRange(teachers);
                ctx.People.AddRange(students);
                ctx.SaveChanges();
            }

            using (var ctx = new MyContext9038())
            {
                var teachersTask = ctx.People.OfType<PersonTeacher>()
                    .Include(m => m.Students)
                    .ThenInclude(m => m.Family)
                    .ThenInclude(m => m.Members)
                    .ToListAsync();

                teachersTask.Wait();
            }
        }

        public abstract class Person
        {
            public int Id { get; set; }

            public string Name { get; set; }

            public int? TeacherId { get; set; }

            public PersonFamily Family { get; set; }
        }

        public class PersonKid : Person
        {
            public int Grade { get; set; }

            public PersonTeacher Teacher { get; set; }
        }

        public class PersonTeacher : Person
        {
            public ICollection<PersonKid> Students { get; set; }
        }

        public class PersonFamily
        {
            public int Id { get; set; }

            public string LastName { get; set; }

            public ICollection<Person> Members { get; set; }
        }

        public class MyContext9038 : DbContext
        {
            public DbSet<Person> People { get; set; }

            public DbSet<PersonFamily> Families { get; set; }


            protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
            {
                optionsBuilder.UseSqlServer(@"Server=.;Database=Repro9038;Trusted_Connection=True;MultipleActiveResultSets=True");
            }

            protected override void OnModelCreating(ModelBuilder modelBuilder)
            {
                modelBuilder.Entity<PersonTeacher>().HasBaseType<Person>();
                modelBuilder.Entity<PersonKid>().HasBaseType<Person>();
                modelBuilder.Entity<PersonFamily>();

                modelBuilder.Entity<PersonKid>(entity =>
                    {
                        entity.Property("Discriminator")
                            .HasMaxLength(63);
                        entity.HasIndex("Discriminator");

                        entity.HasOne(m => m.Teacher)
                            .WithMany(m => m.Students)
                            .HasForeignKey(m => m.TeacherId)
                            .HasPrincipalKey(m => m.Id)
                            .OnDelete(DeleteBehavior.Restrict);
                    });
            }
        }

[caleblloyd]

@ajcvickers this will create bugs for anyone using TPH Inheritance with ThenInclude and retrieving via Async methods when upgrading to 2.0.0. Should it be higher priority?

[ajcvickers]

@caleblloyd We will discuss again, but at this point we have essentially no time left to get anything into 2.0 unless it is a ship-stopper type bug.
@divega Thoughts?

[divega]

@ajcvickers although it doesn't seem a ship stopper, it seems pretty bad. Can we have someone from the query team try to understand the fix?

[ajcvickers]

@maumar @smitpatel Since @anpete is out, can one of you spend some time today doing some more investigation? In particular:

• What scenarios does it repro--is it whenever two collections are included, or is it narrower than that?
• Any ideas on what the fix would look like?

[smitpatel]

Assigning to @maumar as he has done some investigation on issue already.
Assign me back if you want me to fix this.

[ajcvickers]

@maumar Also, adding to my list above, what workarounds could people use if they hit this?

[maumar]

This happens for two-level navigation include, when the first level is empty optional navigation (i.e. the included value is null). We then try to access the null value to include the collection and hence the exception. We are probably missing a null safeguard somewhere - investigating further.

Workarounds:

• use sync:

ctx.People.OfType<PersonTeacher>()
    .Include(m => m.Students)
    .ThenInclude(m => m.Family)
    .ThenInclude(m => m.Members)
    .ToList()

break query into multiple includes:

var teachersTask = ctx.People.OfType<PersonTeacher>()
    .Include(m => m.Students)
    .ThenInclude(m => m.Family).ToListAsync();

var familiesTask = ctx.Families.Include(f => f.Members).ToArrayAsync();

teachersTask.Wait();
familiesTask.Wait();

var teachers = teachersTask.Result;

filter out elements that contain null navigation:

var kidsTask1 = ctx.People.OfType<PersonKid>()
    .Where(k => k.Family != null)
    .Include(m => m.Family)
    .ThenInclude(m => m.Members)
    .ToListAsync();

var kidsTask2 = ctx.People.OfType<PersonKid>()
    .Where(k => k.Family == null)
    .ToListAsync();

kidsTask1.Wait();
kidsTask2.Wait();

var kids = kidsTask1.Result.Concat(kidsTask2.Result);

[DSilence]

This is something that we've immediately noticed after updating. A lot of our queries broke, since we're using async everywhere. Kinda frustrating that this hasn't been fixed before stable release.

[ajcvickers]

@maumar Do you think this should be considered for patch?

[maumar]

I would say so, given that this used to work before 2.0 (which we didn't realize)

[marianosz]

Hi guys. Any ETA for this? Is blocking our company update to NET Core 2. Thanks!

[maumar]

@marianosz I'm working on this issue at the moment

[ajcvickers]

@maumar Can you write the patch Risk/Justification for this one?

[maumar]

Risk: Low - fix is very localized (one line change, constrained to blocks that return non-generic Task), fix is using well established pattern for noop async operations - returning Task.CompletedTask.
Impact: Medium/High - it's a regression introduced in 2.0 and was reported by multiple customers. There are workarounds, but they are not ideal - either switching to synchronous execution on rewrite the query somewhat.

[maciejjarzynski]

I am also blocked with this issue with updating to Core 2.0 in the company I am working at.
Is there a more or less estimated time of releasing EF Core 2.0.1?

[ajcvickers]

@maciejjarzynski We don't have a ship date yet and it's not something that the EF team have control over. Details are still being worked out internally, but we'll try to provide a timeframe when we know.

[TsengSR]

Any chance for a this (and other 2.0.1 fixes) as MyGet package until there's a final version?

[grokky1]

Any chance for a this (and other 2.0.1 fixes) as MyGet package until there's a final version?

+1 Would help enormously to get a prerelease.

[smitpatel]

The general pattern where this issue is hit,
If anywhere in your include path (specified using Include/ThenInclude), if you are including an optional navigation followed by including collection navigation then during async execution it throws NullReferenceException.
e.g.

db.Entity.Include("OptionalNavigation.CollectionNavigation).ToListAsync()
db.Entity.Include(e => e.OptionalNavigation).ThenInclude(e => e.CollectionNavigation).ToListAsync()

Optional navigation does not have to be at root level, it can be anywhere in the include path followed by collection nav.
e.g.

db.Entity.Include(e => SomeReferenceOrCollectionNav).ThenInclude(e => e.OptionalNav).ThenInclude(e => e.CollectionNav).ToListAsync()

Any other async query operator other than ToListAsync could cause the same issue. Work-Around is to use sync query,

[Eilon]

This patch bug is approved for the 2.0.x patch. Please send a PR to the feature/2.0.1 branch and get it reviewed and merged. When we have the rel/2.0.1 branches ready please port the commit to that branch.

[mbashov]

NullReferenceException still happens on the current dev branch if there are multiple includes for collections and one of them has an optional-then-collection path:

[Fact]
public async Task Repro9038_2()
{
    using (CreateDatabase9038())
    {
        using (var context = new MyContext9038(_options))
        {
            var result = await context.Set<PersonTeacher9038>()
                .Include(m => m.Family.Members)
                .Include(m => m.Students)
                .ToListAsync();

            Assert.Equal(2, result.Count);
            Assert.True(result.All(r => r.Students.Any()));
            Assert.Null(result.Single(t => t.Name == "Mr. Frizzle").Family);
            Assert.NotNull(result.Single(t => t.Name == "Mr. Garrison").Family);
        }
    }
}

[ajcvickers]

@smitpatel Can you look at the case posted by @mbashov?

[anpete]

@mbashov Can you post the stack trace?

[smitpatel]

@mbashov - Thanks for providing such a well-defined test. I am working on improving current fix.

[danielpmo1371]

Hello, guys. Any news about a fix for this?

[smitpatel]

@danielpmo1371 - This issue has been fixed in 2.0.1 patch.

[AndriySvyryd]

And we will ship 2.0.1 as soon as some technical issues are resolved.

[danielpmo1371]

Thank you for the immediate response. I've been using .Net Core for a few months now and haven't ever done a patch before.

Can you please refer some documentation about how to applying this patch, please?

[danielpmo1371]

I have made managed to use the workaround suggested by @maumar.
*Thank you @maumar *

@smitpatel or @AndriySvyryd I have just found:
_https://github.com/aspnet/EntityFrameworkCore/wiki/getting-and-building-the-code_
Is that the way to do the patch?

[AndriySvyryd]

@danielpmo1371 We'll ship 2.0.1 as a regular NuGet package, see https://docs.microsoft.com/en-us/nuget/consume-packages/reinstalling-and-updating-packages

[chmarti]

@AndriySvyryd is there any update on a timeline for the 2.0.1 NuGet package to be released? This is blocking my client from migrating to EF Core 2.0. I'm trying to decide if I need to go through and apply your workaround to all my queries, or if I can just wait for 2.0.1 to be released.

[ErikEJ]

@chmarti @grokky1 I am guessing November! https://dotnet.myget.org/gallery/aspnet-2-0-2-october2017-patch

[Eilon]

BTW that feed is not the correct feed, so please be careful using it because it's a fairly random set of packages on there (many of which are not meant for public consumption). We should have a publicly-consumable feed later this week. I'll update all "patch" bug issues when that feed is ready.

[ErikEJ]

@Eilon Looking forward to that - finally!

[Eilon]

Hi, we have a public test feed that you can use to try out the ASP.NET/EF Core 2.0.3 patch!

To try out the pre-release patch, please refer to the following guide:

• https://github.com/aspnet/Announcements/issues/274 (info)
• https://github.com/aspnet/Home/issues/2254 (discussions and questions)

We are looking for feedback on this patch. We'd like to know if you have any issues with this patch by updating your apps and libraries to the latest packages and seeing if it fixes the issues you've had, or if it introduces any new issues. If you have any issues or questions, please reply on this issue to let us know as soon as possible.

Thanks,
Eilon

[dazinator]

Just hit this. Going to try updating to the patch from the patch preview feed.. fingers crossed.

[dazinator]

Yep, patch version fixed it.

[Eilon]

@dazinator thanks for confirming!

[deepak-khopade-isdr]

Upgrading from .NET Core 1.x to 2.0.3 and getting this error on Ubuntu 16.04; however, as workarounds suggested, tried using Sync and its working fine, however, would still wait for async to work before we really count on .NET Core 2.x.

[GermanKuber-zz]

Hello, guys. Any news about a fix for this?

[YaredDejene]

This error is fixed in EF Core v2.0.1.

[p2atran]

I'm still seeing this issue with 2.0.3. Using sync or async still reproduces the exception each time. The only difference I have is that I have an annotation on the property that is null

      [ForeignKey("TeamId")]
      public Blah myProp { get; set; }

myProp is null even when I use .Include()

@ajcvickers Is there a known bug where using both annotations and fluent api in .NET Core 2 causes the same NullReferenceException?

[maumar]

@p2atran this issue was specific to async path, if you see errors for sync then it must be a different problem. Can you file a new bug with the full code listing that reproduces the issue? We will investigate accordingly.

[p2atran]

@maumar

It looks like this thread is the same issue:
https://github.com/aspnet/EntityFrameworkCore/issues/11341

but it looks like you guys are saying it wasn't supposed to work and accidentally worked in core 1