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 bug to ensure correct provider disposal #272

Merged
merged 1 commit into from
Feb 2, 2021

Conversation

mjcheetham
Copy link
Collaborator

We are not correctly disposing of the host providers registered with the host provider registry. Fix this by actually calling Dispose() on the correct objects(!)

We are not correctly disposing of the host providers registered with the
host provider registry. Fix this by actually calling `Dispose()` on the
correct objects(!)
@mjcheetham mjcheetham added the bug A bug in Git Credential Manager label Feb 2, 2021
Copy link
Contributor

@derrickstolee derrickstolee left a comment

Choose a reason for hiding this comment

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

I see... foreach loops don't work correctly on ValueCollections, so you need to convert it to an enumerator?

The documentation example says this loop should work.

@mjcheetham
Copy link
Collaborator Author

@derrickstolee ..

I see... foreach loops don't work correctly on ValueCollections, so you need to convert it to an enumerator?

The documentation example says this loop should work.

The problem is that _hostProviders is of type IDictionary<HostProviderPriority, ICollection<IHostProvider>>, which means _hostProviders.Values is actually of type ICollection<ICollection<IHostProvider>>.

I need to unpack those outer collections.

Copy link
Contributor

@derrickstolee derrickstolee left a comment

Choose a reason for hiding this comment

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

...of course. .SelectMany() is different than .Select(). I haven't had to think about LINQ for a long time.

@mjcheetham mjcheetham merged commit 605912d into git-ecosystem:master Feb 2, 2021
@mjcheetham mjcheetham deleted the dispose-fix branch February 2, 2021 15:13
@mjcheetham mjcheetham mentioned this pull request Mar 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug in Git Credential Manager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants