Skip to content

Blazor supports DisplayName for models#14

Open
hila-f-qodo wants to merge 17 commits intodevin_pr_code_review_bench_100_devin2_base_blazor_supports_displayname_for_models_pr84from
devin_pr_code_review_bench_100_devin2_head_blazor_supports_displayname_for_models_pr84
Open

Blazor supports DisplayName for models#14
hila-f-qodo wants to merge 17 commits intodevin_pr_code_review_bench_100_devin2_base_blazor_supports_displayname_for_models_pr84from
devin_pr_code_review_bench_100_devin2_head_blazor_supports_displayname_for_models_pr84

Conversation

@hila-f-qodo
Copy link
Copy Markdown

@hila-f-qodo hila-f-qodo commented Jan 26, 2026

Benchmark PR from qodo-benchmark#84


Open with Devin

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View issue and 4 additional flags in Devin Review.

Open in Devin Review


internal static class ExpressionMemberAccessor
{
private static readonly ConcurrentDictionary<Expression, MemberInfo> _memberInfoCache = new();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Memory leak due to ineffective Expression-keyed cache using reference equality

The _memberInfoCache in ExpressionMemberAccessor uses Expression as a dictionary key, but Expression objects use reference equality by default. Since lambda expressions like () => model.Property create new Expression objects each time a component renders, the cache will never hit and will grow unbounded.

Click to expand

How the bug is triggered

Every time a DisplayName component re-renders (or a parent component re-renders), a new lambda expression object is created for the For parameter. For example:

<DisplayName For="@(() => _product.Name)" />

This creates a new Expression<Func<TValue>> instance on each render. In ExpressionMemberAccessor.cs:30:

return _memberInfoCache.GetOrAdd(accessor, static expr => { ... });

Since ConcurrentDictionary uses reference equality for Expression keys, the cache lookup never matches existing entries.

Actual vs Expected Behavior

  • Actual: Cache grows unbounded as new expression objects are added but never retrieved. Each render adds a new entry.
  • Expected: Cache should use structural equality (like LambdaExpressionComparer used in src/Mvc/Mvc.ViewFeatures/src/ModelExpressionProvider.cs:27) to match equivalent expressions.

Impact

Memory leak in long-running Blazor applications using the DisplayName component, as the static cache will accumulate entries indefinitely.

Recommendation: Either use a custom IEqualityComparer<Expression> similar to LambdaExpressionComparer from MVC, or cache by MemberInfo directly (which does support equality properly) instead of by Expression. Alternatively, remove the ineffective cache entirely.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants