Avoid clearing ResolvedServices #1049
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tratcher
approved these changes
Feb 1, 2019
src/DependencyInjection/DI/src/ServiceLookup/ServiceProviderEngineScope.cs
Outdated
Show resolved
Hide resolved
halter73
reviewed
Feb 1, 2019
| // Not clearing ResolvedServices here because there might be a compilation running in background | ||
| // trying to get a cached singleton service instance and if it won't find | ||
| // it it will try to create a new one tripping the Debug.Assert in CaptureDisposable | ||
| // and leaking a Disposable object in Release mode |
There was a problem hiding this comment.
What should happen in CaptureDisposable if the container is already disposed? Shouldn't we throw an ODE that can be observed by the top-level GetService() call?
There was a problem hiding this comment.
It shouldn't get called after dispose. We have ODE checks on all customer accessed entry points. This one is internal that that's why it was able to slip past those checks.
maryamariyan
pushed a commit
to maryamariyan/runtime
that referenced
this pull request
Feb 28, 2020
Commit migrated from dotnet/extensions@0947a8d
maryamariyan
pushed a commit
to maryamariyan/runtime
that referenced
this pull request
Mar 2, 2020
Commit migrated from dotnet/extensions@0947a8d
maryamariyan
pushed a commit
to maryamariyan/runtime
that referenced
this pull request
Mar 11, 2020
Commit migrated from dotnet/extensions@0947a8d
maryamariyan
pushed a commit
to maryamariyan/runtime
that referenced
this pull request
Mar 27, 2020
Commit migrated from dotnet/extensions@0947a8d
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The race that was happening is the following:
The solution is to keep service cache around and let GC clean it.
Fixes: #1043