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

Reply via email to