On Sat, 21 Nov 2020 08:32:17 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. This pull request has now been integrated. Changeset: 93b6ab56 Author: Christoph Langer <clan...@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/93b6ab56 Stats: 129 lines in 5 files changed: 97 ins; 15 del; 17 mod 8256818: SSLSocket that is never bound or connected leaks socket resources Reviewed-by: xuelei ------------- PR: https://git.openjdk.java.net/jdk/pull/1363