7025227: SSLSocketImpl does not close the TCP layer socket if a close notify...

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

src/share/classes/sun/security/ssl/SSLSocketImpl.java

@@ -1453,6 +1453,21 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl {
         }
     }
 
+    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 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl {
         }
 
         int state = getConnectionState();
+        boolean closeSocketCalled = false;
+        Throwable cachedThrowable = null;
         try {
             switch (state) {
             /*
@@ -1531,8 +1548,18 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl {
                         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 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl {
                     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 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl {
                 // 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 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl {
         }
     }
 
+    /**
+     * 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 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl {
         }
 
         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 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl {
          * 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;
     }