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.

Sign up for GitHub

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

Jump to bottom

Dispose the ContentFileProvider when the host is disposed #50181

Merged
merged 2 commits into from
Mar 25, 2021
Merged

Dispose the ContentFileProvider when the host is disposed #50181

merged 2 commits into from
Mar 25, 2021

Conversation

davidfowl
Copy link
Member

@davidfowl davidfowl commented Mar 24, 2021 •
edited
Loading

  • Today we don't and it results in a leak when hosts are created and destroyed.

Contributes to dotnet/aspnetcore#31125

Fixed #36550

@davidfowl
Dispose the ContentFileProvider when the host is disposed
9785baa 
- Today we don't and it results in a leak when hosts are created and destroyed.
@ghost
Copy link

ghost commented Mar 24, 2021

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Today we don't and it results in a leak when hosts are created and destroyed.

Contributes to dotnet/aspnetcore#31125

Author: davidfowl
Assignees: -
Labels:

area-Extensions-Hosting
Milestone: -

@eerhardt
Copy link
Member

Is there a test that can be added for this?

@davidfowl
Copy link
Member Author

Is there a test that can be added for this?

It's good you asked, I plan to but I can't get test working locally 😄. I'll write one then use the CI

@eerhardt
Copy link
Member

but I can't get test working locally

😮 , Do either of the following work for you:

  • from the repo root: .\.dotnet\dotnet.exe test .\src\libraries\Microsoft.Extensions.Hosting\tests\UnitTests\
  • from the repo root: .\build.cmd -vs .\src\libraries\Microsoft.Extensions.Hosting\Microsoft.Extensions.Hosting.sln and then use Test Explorer to run

@davidfowl
Copy link
Member Author

davidfowl commented Mar 25, 2021 •
edited
Loading

Rebuilt everything, trying command line as test explorer is failing me rn

eerhardt
eerhardt approved these changes Mar 25, 2021
View reviewed changes
Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM

@davidfowl
Copy link
Member Author

I'm like 99% sure these failures are unrelated 😄

@eerhardt eerhardt merged commit 7648e99 into main Mar 25, 2021
@jkotas jkotas deleted the davidfowl/fix-hosting-leak branch April 1, 2021 03:46
@davidfowl davidfowl added this to the 6.0.0 milestone Apr 2, 2021
@davidfowl davidfowl mentioned this pull request Apr 19, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators May 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@eerhardt eerhardt eerhardt approved these changes

@maryamariyan maryamariyan maryamariyan approved these changes

Assignees
No one assigned
Labels
area-Extensions-Hosting
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.

Properly dispose the PhysicalFileProvider when Host is disposed
3 participants
@davidfowl @eerhardt @maryamariyan