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

I parsed the finalize() code one more time.  The sending close_notify may be not an expected part of the finalize() implementation.

The finalize() calls the close() method.  If the socket is layered over a preexisting socket, the preexisting socket is closed by calling it close() method:
`    self.close();
`
Otherwise, the SSLSocket.close() method will be called:
`    super.close();
`

See the BaseSSLSocketImpl.close() method:

    @Override
    public void close() throws IOException {
        if (self == this) {
            super.close();
        } else {
            self.close();
        }
    }


For layered over socket case, if the preexisting socket is not an SSLSocket object(which is the common case), no close_notify will be sent off course.  If the preexisting socket is an SSLSocket object (which may be not common), the SSLSocketImpl.close() will be called and the close_notify could be sent.

For non-layered over sockets, as super.close() is called, there is no close_notify delivered during the finalizing.

Based on the cases above, the close_notify delivery may be not an expected behavior during finalization in practice.  I would like to remove the finalize() method without adding a cleaner, as shown in the current PR.   But if you read the code and behavior differently , it's acceptable to me to clean up the preexisting socket, by adding a cleaner like:


    BaseSSLSocketImpl(Socket socket) {
        super();
        this.self = socket;
        this.consumedInput = null;

+       CleanerFactory.cleaner().register(this, self::close);
    }

Please let me know your preference.

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

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