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.

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

Commit messages:
 - Add testcase for socket leak
 - Only duplexClose when socket isConnected()
 - 8256818: SSLSocket that is never bound or connected leaks socket resources

Changes: https://git.openjdk.java.net/jdk/pull/1363/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1363&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8256818
  Stats: 130 lines in 5 files changed: 98 ins; 15 del; 17 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1363.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1363/head:pull/1363

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

Reply via email to