This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push: new 613babf BZ64202: Loop over read 613babf is described below commit 613babf191855c9bfed845b6926c012965840849 Author: remm <r...@apache.org> AuthorDate: Mon Mar 9 23:53:12 2020 +0100 BZ64202: Loop over read Zero application bytes read after a notify can happen with TLS so it needs a loop. Like write, there is no global operation timeout tracking at the moment [this may or may not be a good concept here]. Refactor and harmonize read and write. --- java/org/apache/tomcat/util/net/NioEndpoint.java | 120 +++++++++++------------ webapps/docs/changelog.xml | 8 ++ 2 files changed, 68 insertions(+), 60 deletions(-) diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java index bbcf4ca..5905bf3 100644 --- a/java/org/apache/tomcat/util/net/NioEndpoint.java +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java @@ -1136,100 +1136,100 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> } - private int fillReadBuffer(boolean block, ByteBuffer to) throws IOException { - int nRead; + private int fillReadBuffer(boolean block, ByteBuffer buffer) throws IOException { + int n = 0; NioChannel socket = getSocket(); if (socket instanceof ClosedNioChannel) { throw new ClosedChannelException(); } - nRead = socket.read(to); - if (nRead == -1) { - throw new EOFException(); - } - if (block && nRead == 0) { + if (block) { long timeout = getReadTimeout(); - try { - readBlocking = true; - registerReadInterest(); - synchronized (readLock) { - if (readBlocking) { - try { - if (timeout > 0) { - readLock.wait(timeout); - } else { - readLock.wait(); - } - } catch (InterruptedException e) { - // Continue ... - } + do { + n = socket.read(buffer); + if (n == -1) { + throw new EOFException(); + } + if (n == 0) { + readBlocking = true; + registerReadInterest(); + synchronized (readLock) { if (readBlocking) { - throw new SocketTimeoutException(); + try { + if (timeout > 0) { + readLock.wait(timeout); + } else { + readLock.wait(); + } + } catch (InterruptedException e) { + // Continue ... + } + if (readBlocking) { + readBlocking = false; + throw new SocketTimeoutException(); + } + readBlocking = false; } } } - nRead = socket.read(to); - if (nRead == -1) { - throw new EOFException(); - } - } finally { - readBlocking = false; + } while (n == 0); // TLS needs to loop as reading zero application bytes is possible + } else { + n = socket.read(buffer); + if (n == -1) { + throw new EOFException(); } } - return nRead; + return n; } @Override - protected void doWrite(boolean block, ByteBuffer from) throws IOException { + protected void doWrite(boolean block, ByteBuffer buffer) throws IOException { + int n = 0; NioChannel socket = getSocket(); if (socket instanceof ClosedNioChannel) { throw new ClosedChannelException(); } if (block) { long timeout = getWriteTimeout(); - try { - int n = 0; - do { - n = socket.write(from); - if (n == -1) { - throw new EOFException(); - } - if (n == 0) { - writeBlocking = true; - registerWriteInterest(); - synchronized (writeLock) { - if (writeBlocking) { - try { - if (timeout > 0) { - writeLock.wait(timeout); - } else { - writeLock.wait(); - } - } catch (InterruptedException e) { - // Continue ... - } - if (writeBlocking) { - throw new SocketTimeoutException(); + do { + n = socket.write(buffer); + if (n == -1) { + throw new EOFException(); + } + if (n == 0) { + writeBlocking = true; + registerWriteInterest(); + synchronized (writeLock) { + if (writeBlocking) { + try { + if (timeout > 0) { + writeLock.wait(timeout); + } else { + writeLock.wait(); } + } catch (InterruptedException e) { + // Continue ... } + if (writeBlocking) { + writeBlocking = false; + throw new SocketTimeoutException(); + } + writeBlocking = false; } } - } while (from.hasRemaining()); - } finally { - writeBlocking = false; - } + } + } while (buffer.hasRemaining()); // If there is data left in the buffer the socket will be registered for // write further up the stack. This is to ensure the socket is only // registered for write once as both container and user code can trigger // write registration. } else { - int n = 0; do { - n = socket.write(from); + n = socket.write(buffer); if (n == -1) { throw new EOFException(); } - } while (n > 0 && from.hasRemaining()); + } while (n > 0 && buffer.hasRemaining()); } updateLastWrite(); } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index eb453ac..fdfc254 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -45,6 +45,14 @@ issues do not "pop up" wrt. others). --> <section name="Tomcat 10.0.0-M3 (markt)" rtext="in development"> + <subsection name="Coyote"> + <changelog> + <fix> + <fix>64202</fix>: Use a loop on NIO blocking reads, as it is possible + zero bytes are produced by a network read. (markt/remm) + </fix> + </changelog> + </subsection> <subsection name="Other"> <changelog> <fix> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org