On Thu, 7 Apr 2022 20:17:28 GMT, Xue-Lei Andrew Fan <xue...@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

Reply via email to