Skip to content

Commit

Permalink
Improve exception message for memory leak warning
Browse files Browse the repository at this point in the history
Resolves #17623
  • Loading branch information
smitpatel committed Sep 26, 2019
1 parent 4497db6 commit adeab1b
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 12 deletions.
24 changes: 24 additions & 0 deletions src/EFCore/Properties/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1222,4 +1222,13 @@
<data name="ClashingNonOwnedDerivedEntityType" xml:space="preserve">
<value>The type '{entityType}' cannot be marked as owned because the derived entity type - '{derivedType}' has been configured as non-owned.</value>
</data>
<data name="ClientProjectionCapturingConstantInMethodArgument" xml:space="preserve">
<value>Client projection contains reference to constant expression of '{constantType}' which is being passed as argument to method '{methodName}'. 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.</value>
</data>
<data name="ClientProjectionCapturingConstantInMethodInstance" xml:space="preserve">
<value>Client projection contains reference to constant expression of '{constantType}' through instance method '{methodName}'. This could potentially cause memory leak. Consider making the method static so that it does not capture constant in the instance. See https://go.microsoft.com/fwlink/?linkid=2103067 for more information.</value>
</data>
<data name="ClientProjectionCapturingConstantInTree" xml:space="preserve">
<value>Client projection contains reference to constant expression of '{constantType}'. 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.</value>
</data>
</root>
54 changes: 48 additions & 6 deletions src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -172,17 +172,47 @@ public ConstantVerifyingExpressionVisitor(ITypeMappingSource typeMappingSource)
_typeMappingSource = typeMappingSource;
}

private bool ValidConstant(ConstantExpression constantExpression)
{
return constantExpression.Value == null
|| _typeMappingSource.FindMapping(constantExpression.Type) != null;
}

protected override Expression VisitConstant(ConstantExpression constantExpression)
{
if (constantExpression.Value == null
|| _typeMappingSource.FindMapping(constantExpression.Type) != null)
if (!ValidConstant(constantExpression))
{
throw new InvalidOperationException(
CoreStrings.ClientProjectionCapturingConstantInTree(constantExpression.Type.DisplayName()));
}

return constantExpression;
}

protected override Expression VisitMethodCall(MethodCallExpression methodCallExpression)
{
if (RemoveConvert(methodCallExpression.Object) is ConstantExpression constantInstance
&& !ValidConstant(constantInstance))
{
throw new InvalidOperationException(
CoreStrings.ClientProjectionCapturingConstantInMethodInstance(
constantInstance.Type.DisplayName(),
methodCallExpression.Method.Name));
}

foreach (var argument in methodCallExpression.Arguments)
{
return constantExpression;
if (RemoveConvert(argument) is ConstantExpression constantArgument
&& !ValidConstant(constantArgument))
{
throw new InvalidOperationException(
CoreStrings.ClientProjectionCapturingConstantInMethodArgument(
constantArgument.Type.DisplayName(),
methodCallExpression.Method.Name));
}
}

throw new InvalidOperationException(
$"Client projection contains reference to constant expression of type: {constantExpression.Type.DisplayName()}. " +
"This could potentially cause memory leak.");
return base.VisitMethodCall(methodCallExpression);
}

protected override Expression VisitExtension(Expression extensionExpression)
Expand All @@ -192,6 +222,18 @@ protected override Expression VisitExtension(Expression extensionExpression)
? extensionExpression
: base.VisitExtension(extensionExpression);
}

private static Expression RemoveConvert(Expression expression)
{
while (expression != null
&& (expression.NodeType == ExpressionType.Convert
|| expression.NodeType == ExpressionType.ConvertChecked))
{
expression = RemoveConvert(((UnaryExpression)expression).Operand);
}

return expression;
}
}

private class EntityMaterializerInjectingExpressionVisitor : ExpressionVisitor
Expand Down
24 changes: 18 additions & 6 deletions test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.TestModels.Northwind;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;
Expand Down Expand Up @@ -5617,8 +5618,12 @@ public virtual void Client_code_using_instance_method_throws()
{
using (var context = CreateContext())
{
Assert.Throws<InvalidOperationException>(
() => context.Customers.Select(c => InstanceMethod(c)).ToList());
Assert.Equal(
CoreStrings.ClientProjectionCapturingConstantInMethodInstance(
GetType().DisplayName(),
nameof(InstanceMethod)),
Assert.Throws<InvalidOperationException>(
() => context.Customers.Select(c => InstanceMethod(c)).ToList()).Message);
}
}

Expand All @@ -5629,8 +5634,12 @@ public virtual void Client_code_using_instance_in_static_method()
{
using (var context = CreateContext())
{
Assert.Throws<InvalidOperationException>(
() => context.Customers.Select(c => StaticMethod(this, c)).ToList());
Assert.Equal(
CoreStrings.ClientProjectionCapturingConstantInMethodArgument(
GetType().DisplayName(),
nameof(StaticMethod)),
Assert.Throws<InvalidOperationException>(
() => context.Customers.Select(c => StaticMethod(this, c)).ToList()).Message);
}
}

Expand All @@ -5641,8 +5650,11 @@ public virtual void Client_code_using_instance_in_anonymous_type()
{
using (var context = CreateContext())
{
Assert.Throws<InvalidOperationException>(
() => context.Customers.Select(c => new { A = this }).ToList());
Assert.Equal(
CoreStrings.ClientProjectionCapturingConstantInTree(
GetType().DisplayName()),
Assert.Throws<InvalidOperationException>(
() => context.Customers.Select(c => new { A = this }).ToList()).Message);
}
}

Expand Down

0 comments on commit adeab1b

Please sign in to comment.