Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/7.0] Reset LocalView when returning context to the pool #29181

Merged
merged 2 commits into from
Sep 22, 2022

Conversation

ajcvickers
Copy link
Member

@ajcvickers ajcvickers commented Sep 22, 2022

Fixes #29164

Description

Fix memory leak when using context pooling by not continually registering a listener every time a context is obtained from the pool and a DbSet.Local is used.

Customer impact

Fixes memory leak when using context pooling, which is becoming more common.

How found

Customer reported.

Regression

No.

Testing

New testing added.

Risk

Low; small change.


This change treats the LocalView as one of the resettable services and resets it rather than severing when the context is returned to the pool. This fixes a memory leak because the view was previously recreated and re-registered with the same StateManager every time the context was re-used, these registrations were never cleared. But since the StateManager is reused, the local views can also be reused, meaning that the registration is retained rather than repeated across uses of the context instance.

Fixes #29164

This change treats the LocalView as one of the resettable services and resets it rather than severing when the context is returned to the pool. This fixes a memory leak because the view was previously recreated and re-registered with _the same StateManager_ every time the context was re-used, these registrations were never cleared. But since the StateManager is reused, the local views can also be reused, meaning that the registration is retained rather than repeated across uses of the context instance.
@ajcvickers ajcvickers requested a review from a team September 22, 2022 12:31
@ajcvickers ajcvickers added this to the 7.0.0 milestone Sep 22, 2022
{
await ((IResettableService)_localView).ResetStateAsync(cancellationToken).ConfigureAwait(false);
}
((IResettableService)this).ResetState();
Copy link
Member

Choose a reason for hiding this comment

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

Can also be _localView?.Reset()

@rbhanda rbhanda modified the milestones: 7.0.0, 7.0.0-rc2 Sep 22, 2022
@ajcvickers ajcvickers merged commit 5bece0a into release/7.0 Sep 22, 2022
@ajcvickers ajcvickers deleted the ItsBytesAndMegaChipsForTea0922 branch September 22, 2022 17:36
@ajcvickers ajcvickers removed this from the 7.0.0-rc2 milestone Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants