# HG changeset patch # User xuelei # Date 1557158059 25200 # Node ID 7611b664f5222874fbdc92f0a4322d01f1e083ad # Parent 3c8618453098c65e1dd2c823b45ae8de737fe5ee 8219991: New fix of the deadlock in sun.security.ssl.SSLSocketImpl Reviewed-by: alanb, dfuchs diff -r 3c8618453098 -r 7611b664f522 src/share/classes/sun/security/ssl/SSLSocketImpl.java --- a/src/share/classes/sun/security/ssl/SSLSocketImpl.java Sun Mar 15 22:00:43 2020 +0100 +++ b/src/share/classes/sun/security/ssl/SSLSocketImpl.java Mon May 06 08:54:19 2019 -0700 @@ -824,6 +824,10 @@ // reading lock private final ReentrantLock readLock = new ReentrantLock(); + // closing status + private volatile boolean isClosing; + private volatile boolean hasDepleted; + AppInputStream() { this.appDataIsAvailable = false; this.buffer = ByteBuffer.allocate(4096); @@ -869,8 +873,7 @@ * and returning "-1" on non-fault EOF status. */ @Override - public int read(byte[] b, int off, int len) - throws IOException { + public int read(byte[] b, int off, int len) throws IOException { if (b == null) { throw new NullPointerException("the target buffer is null"); } else if (off < 0 || len < 0 || len > b.length - off) { @@ -898,12 +901,40 @@ throw new SocketException("Connection or inbound has closed"); } + // Check if the input stream has been depleted. + // + // Note that the "hasDepleted" rather than the isClosing + // filed is checked here, in case the closing process is + // still in progress. + if (hasDepleted) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.fine("The input stream has been depleted"); + } + + return -1; + } + // Read the available bytes at first. // // Note that the receiving and processing of post-handshake message // are also synchronized with the read lock. readLock.lock(); try { + // Double check if the Socket is invalid (error or closed). + if (conContext.isBroken || conContext.isInboundClosed()) { + throw new SocketException( + "Connection or inbound has closed"); + } + + // Double check if the input stream has been depleted. + if (hasDepleted) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.fine("The input stream is closing"); + } + + return -1; + } + int remains = available(); if (remains > 0) { int howmany = Math.min(remains, len); @@ -936,7 +967,17 @@ return -1; } } finally { - readLock.unlock(); + // Check if the input stream is closing. + // + // If the deplete() did not hold the lock, clean up the + // input stream here. + try { + if (isClosing) { + readLockedDeplete(); + } + } finally { + readLock.unlock(); + } } } @@ -1014,34 +1055,48 @@ * socket gracefully, without impact the performance too much. */ private void deplete() { - if (conContext.isInboundClosed()) { + if (conContext.isInboundClosed() || isClosing) { return; } - readLock.lock(); - try { - // double check - if (conContext.isInboundClosed()) { - return; + isClosing = true; + if (readLock.tryLock()) { + try { + readLockedDeplete(); + } finally { + readLock.unlock(); } - - if (!(conContext.inputRecord instanceof SSLSocketInputRecord)) { - return; - } + } + } - SSLSocketInputRecord socketInputRecord = - (SSLSocketInputRecord)conContext.inputRecord; - try { - socketInputRecord.deplete( - conContext.isNegotiated && (getSoTimeout() > 0)); - } catch (IOException ioe) { - if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { - SSLLogger.warning( - "input stream close depletion failed", ioe); - } + /** + * Try to use up the input records. + * + * Please don't call this method unless the readLock is held by + * the current thread. + */ + private void readLockedDeplete() { + // double check + if (hasDepleted || conContext.isInboundClosed()) { + return; + } + + if (!(conContext.inputRecord instanceof SSLSocketInputRecord)) { + return; + } + + SSLSocketInputRecord socketInputRecord = + (SSLSocketInputRecord)conContext.inputRecord; + try { + socketInputRecord.deplete( + conContext.isNegotiated && (getSoTimeout() > 0)); + } catch (Exception ex) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning( + "input stream close depletion failed", ex); } } finally { - readLock.unlock(); + hasDepleted = true; } } }