SSH.NET icon indicating copy to clipboard operation
SSH.NET copied to clipboard

Remove unnecessary finalizers

Open Rob-Hague opened this issue 1 year ago • 5 comments

All of the finalizers in the library are no-ops, but their existence means that when Dispose (and thus GC.SuppressFinalize) is not called, the objects' lifetimes are extended unnecessarily while they are waiting to be finalized.

Rob-Hague avatar Oct 14 '23 15:10 Rob-Hague

I could be wrong, but I think that if we call GC.SuppressFinalize(this) inside the Dispose() method it tells GC that it does not need to run the finalizer at all. If so, nothing will change because of this PR. @Rob-Hague What do you think about it?

https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose

WojciechNagorski avatar Oct 17 '23 19:10 WojciechNagorski

You are correct. The PR is just small clean-up in the case that Dispose is not called.

Rob-Hague avatar Oct 17 '23 20:10 Rob-Hague

@drieseng What do you think about this PR.

WojciechNagorski avatar Oct 18 '23 06:10 WojciechNagorski

@drieseng What do you think about this PR.

I'm ok with removing the finalizer when there's no added value, but it made sense to keep the finalizer for the few cases I reviewed (BaseClient, SftpFileReader and PasswordConnectionInfo).

drieseng avatar Oct 30 '23 18:10 drieseng

@drieseng What do you think about this PR.

I'm ok with removing the finalizer when there's no added value, but it made sense to keep the finalizer for the few cases I reviewed (BaseClient, SftpFileReader and PasswordConnectionInfo).

None of them make use of any unmanaged resources, so it doesn't make sense for them to have finalizers.

Rob-Hague avatar Oct 30 '23 20:10 Rob-Hague