Author: remm
Date: Tue Feb 19 14:58:00 2019
New Revision: 1853886

URL: http://svn.apache.org/viewvc?rev=1853886&view=rev
Log:
63182: Avoid extra notifications when using non container threads on read 
causing thread safety problems. Tentative fix, but the pattern is clearly 
causing a thread safety problem there. Also move ContainerMarkerThread, but 
leave the old location around for compatibility.

Added:
    tomcat/trunk/java/org/apache/tomcat/util/net/ContainerThreadMarker.java
      - copied, changed from r1853866, 
tomcat/trunk/java/org/apache/coyote/ContainerThreadMarker.java
Modified:
    tomcat/trunk/java/org/apache/coyote/ContainerThreadMarker.java
    tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
    tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/coyote/ContainerThreadMarker.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ContainerThreadMarker.java?rev=1853886&r1=1853885&r2=1853886&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ContainerThreadMarker.java (original)
+++ tomcat/trunk/java/org/apache/coyote/ContainerThreadMarker.java Tue Feb 19 
14:58:00 2019
@@ -24,22 +24,15 @@ package org.apache.coyote;
  */
 public class ContainerThreadMarker {
 
-    private static final ThreadLocal<Boolean> marker = new ThreadLocal<>();
-
     public static boolean isContainerThread() {
-        Boolean flag = marker.get();
-        if (flag == null) {
-            return false;
-        } else {
-            return flag.booleanValue();
-        }
+        return 
org.apache.tomcat.util.net.ContainerThreadMarker.isContainerThread();
     }
 
     public static void set() {
-        marker.set(Boolean.TRUE);
+        org.apache.tomcat.util.net.ContainerThreadMarker.set();
     }
 
     public static void clear() {
-        marker.set(Boolean.FALSE);
+        org.apache.tomcat.util.net.ContainerThreadMarker.clear();
     }
 }

Copied: tomcat/trunk/java/org/apache/tomcat/util/net/ContainerThreadMarker.java 
(from r1853866, tomcat/trunk/java/org/apache/coyote/ContainerThreadMarker.java)
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/ContainerThreadMarker.java?p2=tomcat/trunk/java/org/apache/tomcat/util/net/ContainerThreadMarker.java&p1=tomcat/trunk/java/org/apache/coyote/ContainerThreadMarker.java&r1=1853866&r2=1853886&rev=1853886&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ContainerThreadMarker.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/ContainerThreadMarker.java Tue 
Feb 19 14:58:00 2019
@@ -14,7 +14,7 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License.
  */
-package org.apache.coyote;
+package org.apache.tomcat.util.net;
 
 /**
  * Used to mark threads that have been allocated by the container to process

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=1853886&r1=1853885&r2=1853886&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Tue Feb 19 
14:58:00 2019
@@ -811,7 +811,7 @@ public class Nio2Endpoint extends Abstra
                     socketBufferHandler.configureReadBufferForRead();
                     nRead = Math.min(nRead, len);
                     socketBufferHandler.getReadBuffer().get(b, off, nRead);
-                } else if (nRead == 0 && !block) {
+                } else if (nRead == 0 && !block && 
ContainerThreadMarker.isContainerThread()) {
                     readInterest = true;
                 }
 
@@ -876,7 +876,7 @@ public class Nio2Endpoint extends Abstra
                     // data that was just read
                     if (nRead > 0) {
                         nRead = populateReadBuffer(to);
-                    } else if (nRead == 0 && !block) {
+                    } else if (nRead == 0 && !block && 
ContainerThreadMarker.isContainerThread()) {
                         readInterest = true;
                     }
                 }

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=1853886&r1=1853885&r2=1853886&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java 
(original)
+++ tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java 
Tue Feb 19 14:58:00 2019
@@ -48,12 +48,10 @@ import javax.servlet.http.HttpServletReq
 import javax.servlet.http.HttpServletResponse;
 
 import org.junit.Assert;
-import org.junit.Assume;
 import org.junit.Ignore;
 import org.junit.Test;
 
 import org.apache.catalina.Context;
-import org.apache.catalina.connector.Connector;
 import org.apache.catalina.startup.BytesStreamer;
 import org.apache.catalina.startup.TesterServlet;
 import org.apache.catalina.startup.Tomcat;
@@ -95,11 +93,6 @@ public class TestNonBlockingAPI extends
 
     @Test
     public void testNonBlockingReadAsync() throws Exception {
-        Tomcat tomcat = getTomcatInstance();
-        Connector connector = tomcat.getConnector();
-        // Skip for NIO2
-        Assume.assumeFalse("This test may fail for NIO2",
-                connector.getProtocolHandlerClassName().contains("Nio2"));
         doTestNonBlockingRead(false, true);
     }
 
@@ -124,7 +117,7 @@ public class TestNonBlockingAPI extends
         tomcat.start();
 
         Map<String, List<String>> resHeaders = new HashMap<>();
-        int rc = postUrl(true, new DataWriter(async ? 0 : 500, async ? 1000000 
: 5),
+        int rc = postUrl(true, new DataWriter(async ? 0 : 500, async ? 2000000 
: 5),
                 "http://localhost:"; + getPort() + "/", new ByteChunk(), 
resHeaders, null);
 
         Assert.assertEquals(HttpServletResponse.SC_OK, rc);

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1853886&r1=1853885&r2=1853886&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue Feb 19 14:58:00 2019
@@ -69,6 +69,10 @@
         <code>ServletRequest.getServerName()</code> and
         <code>ServletRequest.getServerPort()</code>. (markt)
       </fix>
+      <fix>
+        <bug>63182</bug>: Avoid extra notifications when using non container
+        threads on read causing thread safety problems. (remm)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="WebSocket">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to