RFR: 8212136: Remove BaseSSLSocketImpl finalizer method [v2]

[Xue-Lei Andrew Fan]

On Thu, 7 Apr 2022 20:17:28 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> Please review the update to remove finalizer method in the SunJSSE provider implementation.  It is one of the efforts to clean up the use of finalizer method in JDK.
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - typo blank linke correction
>  - revise the update

> Thanks for the explanation: this is my first exposure to the `java.lang.ref.Cleaner` API, so am getting up to speed. Sorry if these are dumb comments/questions.
> 
> I see now what was being talked about in your other PR: #8136 and to not use a reference to `this` which would keep it from being GC'd. I also see how you're keeping a cleaner object that has outside ("static") references to the actual things that need to be released, but don't we need to do the similar cleaning for the underlying Socket somehow? What do Sockets do to make sure the underlying file descriptors/native memory are properly released?
> 

The Socket implementation will take care of the file description/native memory release, I think.

> That said, we still need to send the close_notify at the TLS layer, right? Simply removing the finalize() method is just going to silently shutdown the connection, and then the Socket is going to do whatever it does for finalization/Cleaning.

It is expected to send the close_notify at the TLS layer.  But the current code using finalizer, which is not reliable.  The underlying socket may have been closed when the SSLSocket finalizing action is triggered.  Generally, application should call close() method explicitly, otherwise the finalizer is not expect to work reliable.   I was wondering it may be safe to remove the finalizer.

I agree that adding a best effort cleanup may be better.  I was wondering to check if it is possible to clean the socket in the socket creation factories so that the object reference issues could be resolved.  But as socket is a kind of resource, application layer may make the clean up as well.

Socket s = ...
cleaner.register(this, s::close); 


I'm still looking for a solution to clean up resource by using a method of 'this'.  Please advice if anyone has experiences.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8065