On Sun, 22 Nov 2020 18:27:56 GMT, Christoph Langer <clan...@openjdk.org> wrote:
>> There is a flaw in sun.security.ssl.SSLSocketImpl::close() which leads to >> leaking socket resources after JDK-8224829. >> >> The close method calls duplexCloseOutput() and duplexCloseInput(). In case >> of an exception in any of these methods, the call to closeSocket() is >> bypassed, and the underlying Socket may not be closed. >> >> This manifests in a real life leak after JDK-8224829 has introduced a call >> to getSoLinger() on the path of duplexCloseOutput -> closeNotify. If socket >> impl / OS socket hadn't been created yet it is done at that place. But then >> after duplexCloseOutput eventually fails with a SocketException since the >> socket wasn't connected, closing fails to call Socket::close(). >> >> This problem can be reproduced by this code: >> SSLSocket sslSocket = >> (SSLSocket)SSLSocketFactory.getDefault().createSocket(); >> sslSocket.getSSLParameters(); >> sslSocket.close(); >> >> This is what happens when SSLContext.getDefault().getDefaultSSLParameters() >> is called, with close() being eventually called by the finalizer. >> >> I'll open this PR as draft for now to start discussion. I'll create a >> testcase to reproduce the issue and add it soon. >> >> I propose to modify the close method such that duplexClose is only done on a >> connected/bound socket. Maybe it even suffices to only do it when connected. >> >> Secondly, I'm proposing to improve exception handling a bit. So in case >> there's an IOException on the path of duplexClose, it is caught and logged. >> But the real close moves to the finally block since it should be done >> unconditionally. > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Small test improvement Marked as reviewed by xuelei (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/1363
