InvalidOperationException when using constructor parameter in LINQ projection

[davidkvc]

When creating a projection using IQueryable.Select
I would expect that I can pass arbitrary variable from the context as a parameter to constructed object in the Select projection.
Currently, this throws an exception if I try to do this with the returned object although EF is OK if I do that with some of the inner objects (properties).

Is there any limitation why this does not work that I am missing or is this a bug ?

Steps to reproduce

class TestContext : DbContext
{
    public DbSet<FromModel> Froms { get; set; }
}

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

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

    public WithParam WithParam { get; set; }

    public ToModel() {}
    public ToModel(object value) {}
}

class WithParam
{
    public string Name { get; }

    public WithParam(object us, string name)
    {
        Name = name;
        //do something with `us`        
    }
}

private static Expression<Func<FromModel, ToModel>> CreateMapperWorking(object value) => 
    //if we passed `value` to ToModel ctor EF Core would throw the exception
    x => new ToModel
    {
        Skey = x.Skey,
        //EF Core has no problem with this constructor parameter being a reference to arbitrary object
        WithParam = new WithParam(value)
    };

private static Expression<Func<FromModel, ToModel>> CreateMapperFailing(object value) => 
    //we have passed the `value` to ToModel ctor so we will get the exception
    x => new ToModel(value)
    {
        Skey = x.Skey,
        //EF Core has no problem with this constructor parameter being a reference to arbitrary object
        WithParam = new WithParam(value)
    };

[Fact]
public async Task ExamplePassingTest()
{
    using var ctx = new TestContext();

    var v = new object();
    _ = await ctx.Froms
        .AsNoTracking()
        .Select(CreateMapperWorking(v))
        .FirstOrDefaultAsync();

    Assert.True(true);
}

[Fact]
public async Task ExampleFailingTest()
{
    using var ctx = new TestContext();

    var v = new object();
    _ = await ctx.Froms
        .AsNoTracking()
        .Select(CreateMapperFailing(v))
        .FirstOrDefaultAsync();

    //above throws System.InvalidOperationException

    Assert.True(true);
}

From the failing test I get

System.InvalidOperationException : Client projection contains reference to constant expression of 'AutoMapperBug.Proof+<>c__DisplayClass10_0'. This could potentially cause memory leak. Consider assigning this constant to local variable and using the variable in the query instead. See https://go.microsoft.com/fwlink/?linkid=2103067 for more information.
System.InvalidOperationException : Client projection contains reference to constant expression of 'AutoMapperBug.Proof+<>c__DisplayClass10_0'. This could potentially cause memory leak. Consider assigning this constant to local variable and using the variable in the query instead. See https://go.microsoft.com/fwlink/?linkid=2103067 for more information.
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.ConstantVerifyingExpressionVisitor.VisitConstant(ConstantExpression constantExpression)
   at System.Linq.Expressions.ConstantExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitMember(MemberExpression node)
   at System.Linq.Expressions.MemberExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitArguments(ExpressionVisitor visitor, IArgumentProvider nodes)
   at System.Linq.Expressions.ExpressionVisitor.VisitNew(NewExpression node)
   at System.Linq.Expressions.NewExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitAndConvert[T](T node, String callerName)
   at System.Linq.Expressions.ExpressionVisitor.VisitMemberInit(MemberInitExpression node)
   at System.Linq.Expressions.MemberInitExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitBlockExpressions(ExpressionVisitor visitor, BlockExpression block)
   at System.Linq.Expressions.ExpressionVisitor.VisitBlock(BlockExpression node)
   at System.Linq.Expressions.BlockExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitLambda[T](Expression`1 node)
   at System.Linq.Expressions.Expression`1.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.InjectEntityMaterializers(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.VisitShapedQueryExpression(ShapedQueryExpression shapedQueryExpression)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at System.Linq.Expressions.Expression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.QueryCompilationContext.CreateQueryExecutor[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](Expression query, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass12_0`1.<ExecuteAsync>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.ExecuteAsync[TResult](Expression query, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.ExecuteAsync[TResult](Expression expression, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ExecuteAsync[TSource,TResult](MethodInfo operatorMethodInfo, IQueryable`1 source, Expression expression, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ExecuteAsync[TSource,TResult](MethodInfo operatorMethodInfo, IQueryable`1 source, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.FirstOrDefaultAsync[TSource](IQueryable`1 source, CancellationToken cancellationToken)
   at AutoMapperBug.Proof.NoAutoMapper2() in C:\Users\d61230\source\repos\Playground\AutoMapperBug\Proof.cs:line 141
--- End of stack trace from previous location where exception was thrown ---

Further technical details

EF Core version: 3.1.1
Database provider: Devart.Data.Oracle.EFCore 9.11.951
Target framework: .NET Core 3.1
Operating system: Windows 10
IDE: Visual Studio 2019 16.5

[bart-degreed]

I've found a workaround for EF Core 3.1x: Use an intermediate let clause.

IServiceProvider serviceProvider = GetServiceProvider();

var query =
        from passport in _dbContext.Passports
        let tempContext = ActivatorUtilities.CreateInstance<AppDbContext>(serviceProvider)
        select new Passport(tempContext)
        {
            FirstName = passport.FirstName
        };

[bart-degreed]

I'm having a hard time working around this bug, as I'm composing queries at runtime for https://github.com/json-api-dotnet/JsonApiDotNetCore.

@smitpatel @ajcvickers I'd be happy to submit a PR here that backports the fix to the release/3.1 branch. Would you be able to ship that as part of EF Core 3.1.4?

[ajcvickers]

@bart-degreed We will discuss patching 3.1.

[bart-degreed]

Anything I can do to help move this forward?

[ajcvickers]

@bart-degreed Sorry for the delay--I didn't mark the issue correctly for re-triage. Smit has fixed this and we'll look at it next week.

[bart-degreed]

Any updates on this?

[ajcvickers]

@bart-degreed Sorry again for the delay. On reading through this again it's not clear to me exactly why it is hard for you to use either the workaround you posted or to not use the parameterized constructor for these cases. (The directors are unlikely to approve a patch if the workaround is reasonable and the issue hasn't been reported by many customers.)

[bart-degreed]

Sure, let me try to make my case.

I'm one of the maintainers of JsonApiDotNetCore (JADNC), which is a library that implements the json:api spec for ASP.NET Core API projects. Users decorate their entities with attributes to indicate what to expose and which operations to allow. JADNC uses these, along with incoming query string parameters for filtering, sorting, paging, sparse fieldset selection and auto-inclusion of related entities to build an expression tree that is consumed by EF Core. The query results are then transformed into a json:api compliant response.

Queries we generate an expression tree for roughly resemble the following:

_appDbContext.Blogs
    .Where(blog => blog.Articles.Any())
    .OrderBy(blog => blog.Articles.Count)
    .Skip(0).Take(3)
    .Select(blog => new Blog
    {
        Id = blog.Id,
        Name = blog.Name,
        Articles = blog.Articles
            .Where(article => article.Author.FirstName != null && article.Revisions.Any())
            .OrderBy(article => article.Author.LastName)
            .Skip(0).Take(5)
            .Select(article => new Article
            {
                Id = article.Id,
                Author = article.Author,
                Revisions = article.Revisions
                    .Where(revision => revision.PublishTime > new DateTime(2000, 1, 1) && revision.Author.Age > 21)
                    .OrderByDescending(revision => revision.PublishTime)
                    .ThenBy(revision => revision.Author.LastName)
                    .Skip(12).Take(6)
                    .Select(revision => new Revision
                    {
                        Id = revision.Id,
                        PublishTime = revision.PublishTime,
                        Author = new Author
                        {
                            LastName = revision.Author.LastName
                        }
                    })
                    .ToList()
            })
            .ToList()
    });

I'm also an employee at Degreed, where we are in the process of adopting JADNC. Because we have additional requirements, I'm changing the expression tree building in JADNC and ran into this bug.

JADNC allows constructor injection similar to EF Core. This is used, for example, to expose a calculated field IsAccountBlocked that depends on ISystemClock. The entity gets AppDbContext injected, while AppDbContext gets ISystemClock injected.

To apply the workaround, we'd need to generate transparent identifiers at the top of the query and transcend those captures down the tree. I've spent a few hours trying to implement that, but it turned out to be quite complex. Spending more time on it may help, but I'm also concerned to complicate the expression tree too much for this bug workaround, making debugging harder and potentially causing failures in non-SQL Server query providers.

For example, to represent the next query:

var query =
    from passport in _appDbContext.Passports
    let tempContext = ActivatorUtilities.CreateInstance<AppDbContext>(_serviceProvider)
    select new Passport(tempContext)
    {
        FirstName = passport.FirstName
    };

we'd need to generate the following expression tree:

#EntityQueryable<Passport>.Select(
// --- Quoted - begin
    (Passport passport) => new {
        passport = passport,
        tempContext = ActivatorUtilities.CreateInstance(#Experiment._serviceProvider, new object[] {  })
    }
// --- Quoted - end)
.Select(
// --- Quoted - begin
    ({ Passport passport, AppDbContext tempContext } <>h__TransparentIdentifier0) => new Passport(<>h__TransparentIdentifier0.tempContext) {
        FirstName = <>h__TransparentIdentifier0.passport.FirstName
    }
// --- Quoted - end)

Looking at the fix in EF Core 5 this seemed like a low-risk change that would help us a lot, but I may be wrong here.

Instead of backporting the fix, an option to disable leak tracking would be good enough for us too:

services.AddDbContext<AppDbContext>(options =>
{
    options.UseSqlServer(SqlServerConnectionString);

    options.EnableLeakTracking = false; // <--
});

Please let me know if there is anything I can do to help.

[smitpatel]

Computation of tempContext for each row of the result is at best unnecessary and at worst bad for perf when value does not even change. You should compute tempContext outside of the query and use the value directly in your constructor.

[bart-degreed]

Yes that has always been my goal, but the bug being discussed here prevents me from doing that in EF Core 3.

[ajcvickers]

@bart-degreed We discussed this again, but we're still a bit puzzled by the pattern. Can you explain a bit more about why you need to dynamically create DbContext instances inside the query? It doesn't appear to act as a query root, which likely wouldn't work anyway unless it was the same instance as the main query root, in which case there would be no point in having it anyway. If you just need a constant value from it, then why does that have to be inline in the query?

[bart-degreed]

Thanks for looking into this. Let me try to explain better by going back to the original problem with a simpler example.

This assumes the next entity definition:

public class Blog
{
    public long Id {get; set; }
    public string Name { get; set; }

    public Blog(AppDbContext appDbContext) { }
}

The next query works:

var query =
    from blog in _appDbContext.Blogs
    select blog;

var result = query.ToArray();

Next, I want to select a subset of the fields, so I change my code to:

var appDbContext = _appDbContext;

var query =
    from blog in _appDbContext.Blogs
    select new Blog(appDbContext)
    {
        Id = blog.Id,
        Name = blog.Name
    };

var result = query.ToArray();

Which fails with:

System.InvalidOperationException: Client projection contains reference to constant expression of 'EntityFrameworkWorkerService.Experiment+<>c__DisplayClass4_0'. This could potentially cause memory leak. Consider assigning this constant to local variable and using the variable in the query instead. See https://go.microsoft.com/fwlink/?linkid=2103067 for more information.

To workaround that, I need to add a let clause:

var appDbContext = _appDbContext;

var query =
    from blog in _appDbContext.Blogs
    let temp = appDbContext
    select new Blog(temp)
    {
        Id = blog.Id,
        Name = blog.Name
    };

var result = query.ToArray();

So the only reason this is inside the query (using a let clause) is to workaround this bug. We do not need to create DbContext instances, we just need 'an' instance (where the service provider decides whether that be a singleton, a scoped instance that is reused or a new instance each time).

Our visitor that builds an Expression for Select extension method calls tries to first find the 'best' constructor on the entity and then, one-by-one, resolve its arguments using ActivatorUtilities, in a similar way that EF Core does. We could probably call into that, but I'd rather not take a dependency on EF Core internals that are subject to change in future versions. But the use case is basically the same, though: create an entity instance with injected constructor parameters (which would be DbContext most of the time, but a few others are allowed too).

The problem with our visitor is the same as the code snippets above demonstrate: the Expression for the query will reference a constant expression (the constructor parameter that was resolved while composing the query), but this bug incorrectly detects that as a memory leak.

Hope this helps!

[ajcvickers]

@bart-degreed Thanks for the additional info. We discussed again with this additional understanding and considered again with regard to the release planning process for patches. We don't believe that this meets the bar to patch for 3.1, since the scenario doesn't seem common, we don't have evidence that this is impacting many customers, and workarounds are available.

[bart-degreed]

Oh well, it was worth a try. Thanks for considering and providing your motivation.