7025227: SSLSocketImpl does not close the TCP layer socket if a close notify cannot be sent to the peer
6932403: SSLSocketImpl state issue
Reviewed-by: xuelei
--- a/jdk/src/share/classes/sun/security/ssl/SSLSocketImpl.java Thu Apr 21 15:55:59 2011 -0700
+++ b/jdk/src/share/classes/sun/security/ssl/SSLSocketImpl.java Fri Apr 22 11:03:39 2011 +0100
@@ -1453,6 +1453,21 @@
}
}
+ private void closeSocket(boolean selfInitiated) throws IOException {
+ if ((debug != null) && Debug.isOn("ssl")) {
+ System.out.println(threadName() + ", called closeSocket(selfInitiated)");
+ }
+ if (self == this) {
+ super.close();
+ } else if (autoClose) {
+ self.close();
+ } else if (selfInitiated) {
+ // layered && non-autoclose
+ // read close_notify alert to clear input stream
+ waitForClose(false);
+ }
+ }
+
/*
* Closing the connection is tricky ... we can't officially close the
* connection until we know the other end is ready to go away too,
@@ -1491,6 +1506,8 @@
}
int state = getConnectionState();
+ boolean closeSocketCalled = false;
+ Throwable cachedThrowable = null;
try {
switch (state) {
/*
@@ -1531,8 +1548,18 @@
return; // connection was closed while we waited
}
if (state != cs_SENT_CLOSE) {
- warning(Alerts.alert_close_notify);
- connectionState = cs_SENT_CLOSE;
+ try {
+ warning(Alerts.alert_close_notify);
+ connectionState = cs_SENT_CLOSE;
+ } catch (Throwable th) {
+ // we need to ensure socket is closed out
+ // if we encounter any errors.
+ connectionState = cs_ERROR;
+ // cache this for later use
+ cachedThrowable = th;
+ closeSocketCalled = true;
+ closeSocket(selfInitiated);
+ }
}
}
// If state was cs_SENT_CLOSE before, we don't do the actual
@@ -1569,22 +1596,11 @@
return;
}
- if (self == this) {
- super.close();
- } else if (autoClose) {
- self.close();
- } else if (selfInitiated) {
- // layered && non-autoclose
- // read close_notify alert to clear input stream
- waitForClose(false);
+ if (!closeSocketCalled) {
+ closeSocketCalled = true;
+ closeSocket(selfInitiated);
}
- // See comment in changeReadCiphers()
- readCipher.dispose();
- writeCipher.dispose();
-
- // state will be set to cs_CLOSED in the finally block below
-
break;
}
} finally {
@@ -1595,6 +1611,20 @@
// notify any threads waiting for the closing to finish
this.notifyAll();
}
+ if (closeSocketCalled) {
+ // Dispose of ciphers since we've closed socket
+ disposeCiphers();
+ }
+ if (cachedThrowable != null) {
+ /*
+ * Rethrow the error to the calling method
+ * The Throwable caught can only be an Error or RuntimeException
+ */
+ if (cachedThrowable instanceof Error)
+ throw (Error) cachedThrowable;
+ if (cachedThrowable instanceof RuntimeException)
+ throw (RuntimeException) cachedThrowable;
+ }
}
}
@@ -1641,6 +1671,24 @@
}
}
+ /**
+ * Called by closeInternal() only. Be sure to consider the
+ * synchronization locks carefully before calling it elsewhere.
+ */
+ private void disposeCiphers() {
+ // See comment in changeReadCiphers()
+ synchronized (readLock) {
+ readCipher.dispose();
+ }
+ // See comment in changeReadCiphers()
+ writeLock.lock();
+ try {
+ writeCipher.dispose();
+ } finally {
+ writeLock.unlock();
+ }
+ }
+
//
// EXCEPTION AND ALERT HANDLING
//
@@ -1761,7 +1809,9 @@
}
int oldState = connectionState;
- connectionState = cs_ERROR;
+ if (connectionState < cs_ERROR) {
+ connectionState = cs_ERROR;
+ }
/*
* Has there been an error received yet? If not, remember it.
@@ -1792,13 +1842,17 @@
* Clean up our side.
*/
closeSocket();
+ // Another thread may have disposed the ciphers during closing
+ if (connectionState < cs_CLOSED) {
+ connectionState = (oldState == cs_APP_CLOSED) ? cs_APP_CLOSED
+ : cs_CLOSED;
- // See comment in changeReadCiphers()
- readCipher.dispose();
- writeCipher.dispose();
+ // We should lock readLock and writeLock if no deadlock risks.
+ // See comment in changeReadCiphers()
+ readCipher.dispose();
+ writeCipher.dispose();
+ }
- connectionState = (oldState == cs_APP_CLOSED) ? cs_APP_CLOSED
- : cs_CLOSED;
throw closeReason;
}