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