RC2 Deadlock problem with CountAsync() that does not happen with Count()

[iberodev]

I have found an inconsistency when querying a query.CountAsync();. It does not throw any error nor displays any error on console, it simply gets into a deadlock state (the application is still up but the next line of code does not get to execute)

The problem does not happen when using the same query to count the items in a synchronous way: query.Count();

Scenario:

I have an entity that has 2 optional references to another entity

public class Shipment 
{
  public int Id { get; set; }
  public string CarrierName { get; set; }
  public int? CargoReportId { get; set; }
  public CargoReport CargoReport { get; set; }
  public int? AltCargoReportId { get; set; }
  public CargoReport AltCargoReport { get; set; }
}

public class Report
{
  public int Id { get; set; }
  public string Reference { get; set; }
}

Now I try to count the items in database with a simple query and the synchronous call works fine but the asynchronous call gets the code onto a deadlock state:

private IQueryable<Shipment> GetTestQuery(ShipmentDirection direction) {
    var shipmentsQuery = _context.Shipments
            .Include(s => s.CargoReport)
            .Include(s => s.AltCargoReport)
            .Where(s => s.CarrierName == "carrier"
                    || s.CargoReport.Reference.Contains("whatever") 
                    || s.AltCargoReport.Reference.Contains("whatever"));
    return shipmentsQuery;
}

public async Task<int> MyTestRepositoryCount()
{
  var count1 = await _context.Shipments.CountAsync(); //this works well
  var shipmentsQuery = GetTestQuery(direction);
  var count2 = shipmentsQuery.Count(); //this works well
  var count3= await shipmentsQuery.CountAsync(); //this DOES NOT work. No exception, just deadlock
  return count3; //this never executes
}

I know the problem has to do with CargoReport and AltCargoReport being a null reference because If I had something like this:

private IQueryable<Shipment> GetTestQuery(ShipmentDirection direction) {
   var shipmentsQuery = _context.Shipments
            .Include(s => s.CargoReport)
            .Include(s => s.AltCargoReport)
            .Where(s => //s.CarrierName == "carrier" || //Noticed this condition commented out
                       s.CargoReport.Reference.Contains("whatever") 
                    || s.AltCargoReport.Reference.Contains("whatever"));
    return shipmentsQuery;
}

I would get the following exception when executing the count2 line:

Microsoft.EntityFrameworkCore.Storage.Internal.SqlServerConnection:Debug: Closing connection to database 'SpeediCargo' on server '(localdb)\mssqllocaldb'.
Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler:Error: An exception occurred in the database while iterating the results of a query.
System.NullReferenceException: Object reference not set to an instance of an object.
at lambda_method(Closure , TransparentIdentifier2 ) at System.Linq.Enumerable.WhereSelectEnumerableIterator2.MoveNext()
at System.Linq.Enumerable.Count[TSource](IEnumerable1 source) at lambda_method(Closure , QueryContext ) at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass19_11.b__1(QueryContext qc)

System.NullReferenceException: Object reference not set to an instance of an object.
at lambda_method(Closure , TransparentIdentifier2 ) at System.Linq.Enumerable.WhereSelectEnumerableIterator2.MoveNext()
at System.Linq.Enumerable.Count[TSource](IEnumerable1 source) at lambda_method(Closure , QueryContext ) at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass19_11.b__1(QueryContext qc)
Exception thrown: 'System.NullReferenceException' in Microsoft.EntityFrameworkCore.dll
Exception thrown: 'System.NullReferenceException' in mscorlib.ni.dll

So a few things here:

1. How should I have the conditions on CargoReport and AltCargoReport to handle null? Shouldn't Entity Framework take care of that for me?
2. Why the .Count() works well but the .CountAsync() doesn't? shouldn't be both consistent?
3. In case it's fine that the behaviour is not consistent. Shouldn't the .CountAsync() call at least throw a null reference exception if that's the case for the deadlock?

Thanks guys.

[anpete]

@maumar for the nav prop null expansion, should this be null ref'ing?

[anpete]

@iberodev Does the deadlock occur if you remove the Includes. BTW, the Includes are not needed if you are just doing a Count.

[iberodev]

@anpete Yes, if I remove the Includes the deadlock still happens when using .CountAsync(). No difference.
PS: Thanks for the tip about the Includes not being necessary for the count. Didn't know that.

[anpete]

Ping @maumar

[anpete]

@iberodev No luck repro'ing at my end yet. Are you able to create a self-contained repro for this?

[maumar]

@anpete it looks like a bug in the way we protect from null refs during optional navigation rewrites. Both CargoReport and AltCargoReport are optional, so only the first one will be translated to sql. The second one is done on the client. We do apply our null compensation logic, but we produce something like this:

(t3.Outer.Outer.Inner != null ? t3.Outer.Outer.Inner.Reference : null).Contains(
            value: whatever
        )

[maumar]

filed #5613 to track this

[iberodev]

@anpete I have created a sample repository with a project showing the deadlock issue.
Instructions in the README. I hope it helps.

https://github.com/iberodev/EntityFramework5481

PS: The mapping for the navigation property does not seem quite right but this is how it was in our project. Tomorrow I will have a look at it but I don't think it has anything to do with the deadlock issue.

[anpete]

@iberodev Thanks!

[anpete]

Looks like the deadlock is in IX-Async|GroupJoin. Fix is to write our own version. Note for triage, to hit this the query needs to hit two optional nav props.

[divega]

Clearing up milestone again, so we can discuss with @anpete in triage.

[divega]

Triage: we will attempt to fix this for RTM.