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

Reset database object during the Dispose method #3553

Merged
2 commits merged into from
Sep 22, 2020

Conversation

Marusyk
Copy link
Member

@Marusyk Marusyk commented Sep 22, 2020

Reset database object during the Dispose method

Fixes #3531

Please review,
Thank you in advance

@Tratcher
Copy link
Member

This doesn't seem like an appropriate solution. In general nulling out fields in Dispose leads to race conditions and null reference exceptions.

The assumption in #3531 that you should be able to call Dispose to disconnect and then call other methods and have it reconnect does not match the proper use of the Dispose pattern.

Proposal: Dispose should set an internal _disposed bool field and any APIs called after should check that field and throw ObjectDisposedException.

@Marusyk
Copy link
Member Author

Marusyk commented Sep 22, 2020

What do you think about this dotnet/runtime#42183

@Tratcher
Copy link
Member

Unnecessary.

Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

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

Much better.

@Tratcher Tratcher added this to the 6.0.0-alpha1 milestone Sep 22, 2020
@Tratcher Tratcher self-assigned this Sep 22, 2020
@ghost
Copy link

ghost commented Sep 22, 2020

Hello @Tratcher!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 6683bac into dotnet:master Sep 22, 2020
@Marusyk Marusyk deleted the rmarusyk/3531 branch September 23, 2020 09:32
JunTaoLuo pushed a commit to dotnet/aspnetcore that referenced this pull request Nov 17, 2020
* Reset database object during the Dispose method

* Throw ObjectDisposedException when object has already disposed\n\nCommit migrated from dotnet/extensions@6683bac
@ghost ghost locked as resolved and limited conversation to collaborators May 24, 2023
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential issue while disposing ConnectionMultiplexer in RedisCache?
3 participants