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

Fix DiagnosticListener memory leak #16047

Merged
merged 1 commit into from
Jun 13, 2019

Conversation

roji
Copy link
Member

@roji roji commented Jun 12, 2019

When IsConfigured is called, it applies all services. This caused a DiagnosticListener to get instantiated on each DbContext instantiation, and since it wasn't disposed it caused a leak.

Fixes #15173

PR for master (3.0): #16046

(cherry picked from commit 904ad9c)

When IsConfigured is called, it applies all services. This caused a
DiagnosticListener to get instantiated on each DbContext instantiation,
and since it wasn't disposed it caused a leak.

Fixes #15173

(cherry picked from commit 904ad9c)
This was referenced Jun 12, 2019
@ajcvickers ajcvickers added this to the 2.2.x milestone Jun 12, 2019
@ajcvickers
Copy link
Member

ajcvickers commented Jun 12, 2019

Description

Applications that call IsConfigured from OnConfiguring will leak DiagnosticListener instances because an instance is being created and not used. This is a leak because the DiagnosticListener is disposable and the diagnostics system holds on to all constructed listeners until they are disposed, which never happens.

Customer Impact

Reported through support, and will likely also impact other customers, including those running on Azure.

Regression?

Yes. This leak did not exist in 2.1. (IsConfigured didn't work correctly in 2.1, but it didn't leak.)

Risk

Very low; avoid creating the instance when it is not needed.

@vivmishra vivmishra modified the milestones: 2.2.x, 2.2.6 Jun 13, 2019
@vivmishra
Copy link

Approved for 2.2.6. Checkin the fix now.

@ajcvickers
Copy link
Member

ajcvickers commented Jun 13, 2019

@roji Don't check in!

The branch is not ready yet--I'm working on it..

@ajcvickers ajcvickers merged commit 01da710 into release/2.2 Jun 13, 2019
@ghost ghost deleted the DiagnosticListenersEverywhere-2.2 branch June 13, 2019 21:01
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