Author: remm Date: Thu Feb 21 16:37:20 2019 New Revision: 1854066 URL: http://svn.apache.org/viewvc?rev=1854066&view=rev Log: Refactor to redo fix for 63182. The root cause is that the pending flag is released once processing start, and concurrent unsynced access from non container threads can cause awaitBytes to happen concurrently.
Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1854066&r1=1854065&r2=1854066&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java Thu Feb 21 16:37:20 2019 @@ -790,7 +790,7 @@ public abstract class AbstractProcessor } if (!isRequestBodyFullyRead()) { - registerReadInterest(); + registerReadInterest(true); } return false; @@ -800,7 +800,7 @@ public abstract class AbstractProcessor protected abstract boolean isRequestBodyFullyRead(); - protected abstract void registerReadInterest(); + protected abstract void registerReadInterest(boolean body); protected abstract boolean isReadyForWrite(); Modified: tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java?rev=1854066&r1=1854065&r2=1854066&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java (original) +++ tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java Thu Feb 21 16:37:20 2019 @@ -903,7 +903,7 @@ public abstract class AbstractProtocol<S // processor. Continue to poll for the next request. connections.remove(socket); release(processor); - wrapper.registerReadInterest(); + wrapper.registerReadInterest(true); } else if (state == SocketState.SENDFILE) { // Sendfile in progress. If it fails, the socket will be // closed. If it works, the socket either be added to the @@ -993,7 +993,7 @@ public abstract class AbstractProtocol<S // - this is an upgraded connection // - the request line/headers have not been completely // read - socket.registerReadInterest(); + socket.registerReadInterest(true); } } Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1854066&r1=1854065&r2=1854066&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Thu Feb 21 16:37:20 2019 @@ -1134,8 +1134,8 @@ public class AjpProcessor extends Abstra @Override - protected final void registerReadInterest() { - socketWrapper.registerReadInterest(); + protected final void registerReadInterest(boolean body) { + socketWrapper.registerReadInterest(!body); } Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1854066&r1=1854065&r2=1854066&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Thu Feb 21 16:37:20 2019 @@ -1215,8 +1215,8 @@ public class Http11Processor extends Abs @Override - protected final void registerReadInterest() { - socketWrapper.registerReadInterest(); + protected final void registerReadInterest(boolean body) { + socketWrapper.registerReadInterest(!body); } Modified: tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java?rev=1854066&r1=1854065&r2=1854066&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java Thu Feb 21 16:37:20 2019 @@ -107,7 +107,7 @@ public class UpgradeServletInputStream e if (ContainerThreadMarker.isContainerThread()) { processor.addDispatch(DispatchType.NON_BLOCKING_READ); } else { - socketWrapper.registerReadInterest(); + socketWrapper.registerReadInterest(true); } // Switching to non-blocking. Don't know if data is available. Modified: tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java?rev=1854066&r1=1854065&r2=1854066&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java Thu Feb 21 16:37:20 2019 @@ -263,7 +263,7 @@ class StreamProcessor extends AbstractPr @Override - protected final void registerReadInterest() { + protected final void registerReadInterest(boolean body) { // Should never be called for StreamProcessor as isReadyForRead() is // overridden throw new UnsupportedOperationException(); Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1854066&r1=1854065&r2=1854066&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Thu Feb 21 16:37:20 2019 @@ -2724,7 +2724,7 @@ public class AprEndpoint extends Abstrac @Override - public void registerReadInterest() { + public void registerReadInterest(boolean polling) { // Make sure an already closed socket is not added to the poller synchronized (closedLock) { if (closed) { Modified: tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java?rev=1854066&r1=1854065&r2=1854066&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Thu Feb 21 16:37:20 2019 @@ -590,7 +590,7 @@ public class Nio2Endpoint extends Abstra this.readCompletionHandler = new CompletionHandler<Integer, ByteBuffer>() { @Override public void completed(Integer nBytes, ByteBuffer attachment) { - boolean notify = false; + boolean readNotify = false; if (log.isDebugEnabled()) { log.debug("Socket: [" + Nio2SocketWrapper.this + "], Interest: [" + readInterest + "]"); } @@ -599,16 +599,16 @@ public class Nio2Endpoint extends Abstra failed(new EOFException(), attachment); } else { if (readInterest && !Nio2Endpoint.isInline()) { - readInterest = false; - notify = true; + readNotify = true; } else { // Release here since there will be no // notify/dispatch to do the release. readPending.release(); } + readInterest = false; } } - if (notify) { + if (readNotify) { getEndpoint().processSocket(Nio2SocketWrapper.this, SocketEvent.OPEN_READ, false); } } @@ -810,7 +810,7 @@ public class Nio2Endpoint extends Abstra socketBufferHandler.configureReadBufferForRead(); nRead = Math.min(nRead, len); socketBufferHandler.getReadBuffer().get(b, off, nRead); - } else if (nRead == 0 && !block && ContainerThreadMarker.isContainerThread()) { + } else if (nRead == 0 && !block) { readInterest = true; } if (log.isDebugEnabled()) { @@ -873,7 +873,7 @@ public class Nio2Endpoint extends Abstra // data that was just read if (nRead > 0) { nRead = populateReadBuffer(to); - } else if (nRead == 0 && !block && ContainerThreadMarker.isContainerThread()) { + } else if (nRead == 0 && !block) { readInterest = true; } } @@ -1475,11 +1475,11 @@ public class Nio2Endpoint extends Abstra @Override - public void registerReadInterest() { + public void registerReadInterest(boolean polling) { synchronized (readCompletionHandler) { if (readPending.availablePermits() == 0) { readInterest = true; - } else { + } else if (polling) { // If no read is pending, start waiting for data awaitBytes(); } Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1854066&r1=1854065&r2=1854066&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Thu Feb 21 16:37:20 2019 @@ -1243,7 +1243,7 @@ public class NioEndpoint extends Abstrac @Override - public void registerReadInterest() { + public void registerReadInterest(boolean polling) { getPoller().add(getSocket(), SelectionKey.OP_READ); } @@ -1420,7 +1420,7 @@ public class NioEndpoint extends Abstrac } else if (handshake == -1 ) { close(socket, key); } else if (handshake == SelectionKey.OP_READ){ - socketWrapper.registerReadInterest(); + socketWrapper.registerReadInterest(true); } else if (handshake == SelectionKey.OP_WRITE){ socketWrapper.registerWriteInterest(); } Modified: tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java?rev=1854066&r1=1854065&r2=1854066&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java Thu Feb 21 16:37:20 2019 @@ -763,7 +763,7 @@ public abstract class SocketWrapperBase< } - public abstract void registerReadInterest(); + public abstract void registerReadInterest(boolean polling); public abstract void registerWriteInterest(); Modified: tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java?rev=1854066&r1=1854065&r2=1854066&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java (original) +++ tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java Thu Feb 21 16:37:20 2019 @@ -124,6 +124,9 @@ public class TestNonBlockingAPI extends Assert.assertEquals(HttpServletResponse.SC_OK, rc); if (async) { Assert.assertEquals(2000000 * 8, servlet.listener.body.length()); + TestAsyncReadListener listener = (TestAsyncReadListener) servlet.listener; + Assert.assertEquals(listener.notReadyCount, listener.containerThreadCount); + Assert.assertEquals(listener.isReadyCount, listener.nonContainerThreadCount); } else { Assert.assertEquals(5 * 8, servlet.listener.body.length()); } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org