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

Add test that demonstrates the current behavior of a recovered channe… #1450

Merged
merged 1 commit into from
Dec 22, 2023

Conversation

lukebakken
Copy link
Contributor

…l that is disposed.

Fixes #1086

@lukebakken lukebakken self-assigned this Dec 22, 2023
@lukebakken lukebakken added this to the 7.0.0 milestone Dec 22, 2023
@michaelklishin
Copy link
Member

I don't know how common it is in the .NET ecosystem to forcefully close a connection (channel, etc) in Dispose. I assume that close it in a destructor (or similar) would be even less evident?

@paulomorgado
Copy link
Contributor

@michaelklishin, Close-like invocation in Dispose is very common and recommended.

Finalizers (destructors) should be used as a last resource and handle only unmanaged resources, as it is not guaranteed that all managed objects the class was referencing are still "alive" at the time the finalizer is invoked.

Also, using a finalizer requires one more step from the GC and will leave them in memory longer. Not to mention badly implemented finalizers that can deadlock or take down a process.

The common practice is for the Dispose method to suppress the execution of the finalizer when invoked.

For more information, refer to Cleaning up unmanaged resources and sub-sections.

@lukebakken lukebakken marked this pull request as ready for review December 22, 2023 13:07
@lukebakken
Copy link
Contributor Author

Thanks for the input @michaelklishin and @paulomorgado

@lukebakken lukebakken merged commit fb94c1d into main Dec 22, 2023
11 checks passed
@lukebakken lukebakken deleted the rabbitmq-dotnet-client-1086 branch December 22, 2023 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IModel.IsClosed set to false after dispose
3 participants