You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2022/02/28 16:34:32 UTC
[tomcat] branch 9.0.x updated: Fix regression of bugfix 65757
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 7d3d359 Fix regression of bugfix 65757
7d3d359 is described below
commit 7d3d359aaa8b7fe9463a73915340a347ad5e4614
Author: remm <re...@apache.org>
AuthorDate: Mon Feb 28 17:25:37 2022 +0100
Fix regression of bugfix 65757
With sequential use, the id needs to be reset to be accurate.
Test case code submitted by Istvan Szekely.
---
.../apache/catalina/connector/CoyoteAdapter.java | 2 +
java/org/apache/coyote/Request.java | 4 +
.../catalina/nonblocking/TestNonBlockingAPI.java | 164 ++++++++++++++++++++-
webapps/docs/changelog.xml | 12 +-
4 files changed, 176 insertions(+), 6 deletions(-)
diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java b/java/org/apache/catalina/connector/CoyoteAdapter.java
index 2e8a4d5..aed20f3 100644
--- a/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -300,6 +300,7 @@ public class CoyoteAdapter implements Adapter {
}
req.getRequestProcessor().setWorkerThreadName(null);
+ req.clearRequestThread();
// Recycle the wrapper request and response
if (!success || !request.isAsync()) {
updateWrapperErrorCount(request, response);
@@ -432,6 +433,7 @@ public class CoyoteAdapter implements Adapter {
}
req.getRequestProcessor().setWorkerThreadName(null);
+ req.clearRequestThread();
// Recycle the wrapper request and response
if (!async) {
diff --git a/java/org/apache/coyote/Request.java b/java/org/apache/coyote/Request.java
index a07d485..535c51e 100644
--- a/java/org/apache/coyote/Request.java
+++ b/java/org/apache/coyote/Request.java
@@ -692,6 +692,10 @@ public final class Request {
return threadId;
}
+ public void clearRequestThread() {
+ threadId = 0;
+ }
+
public void setRequestThread() {
threadId = Thread.currentThread().getId();
}
diff --git a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
index 99fedcd..782dce7 100644
--- a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
+++ b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
@@ -979,6 +979,34 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
}
+ @Test
+ public void testDelayedNBReadWrite() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+
+ Context ctx = tomcat.addContext("", null);
+ CountDownLatch latch1 = new CountDownLatch(2);
+ DelayedNBReadWriteServlet servlet = new DelayedNBReadWriteServlet(latch1);
+ String servletName = DelayedNBReadWriteServlet.class.getName();
+ Tomcat.addServlet(ctx, servletName, servlet);
+ ctx.addServletMappingDecoded("/", servletName);
+
+ tomcat.start();
+
+ CountDownLatch latch2 = new CountDownLatch(1);
+ List<Throwable> exceptions = new ArrayList<>();
+
+ Thread t = new Thread(
+ new RequestPostExecutor("http://localhost:" + getPort() + "/", latch2, exceptions));
+ t.start();
+
+ latch1.await(3000, TimeUnit.MILLISECONDS);
+ latch2.await(3000, TimeUnit.MILLISECONDS);
+
+ if (exceptions.size() > 0) {
+ Assert.fail();
+ }
+ }
+
private static final class RequestExecutor implements Runnable {
private final String url;
private final CountDownLatch latch;
@@ -1007,6 +1035,34 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
+ private static final class RequestPostExecutor implements Runnable {
+ private final String url;
+ private final CountDownLatch latch;
+ private final List<Throwable> exceptions;
+
+ public RequestPostExecutor(String url, CountDownLatch latch, List<Throwable> exceptions) {
+ this.url = url;
+ this.latch = latch;
+ this.exceptions = exceptions;
+ }
+
+ @Override
+ public void run() {
+ try {
+ ByteChunk result = new ByteChunk();
+ int rc = postUrl("body".getBytes("utf-8"), url, result, null);
+ Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+ Assert.assertTrue(result.toString().contains("OK"));
+ } catch (Throwable e) {
+ e.printStackTrace();
+ exceptions.add(e);
+ } finally {
+ latch.countDown();
+ }
+ }
+
+ }
+
@WebServlet(asyncSupported = true)
private static final class DelayedNBWriteServlet extends TesterServlet {
private static final long serialVersionUID = 1L;
@@ -1038,6 +1094,105 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
+ @WebServlet(asyncSupported = true)
+ private static final class DelayedNBReadWriteServlet extends TesterServlet {
+ private static final long serialVersionUID = 1L;
+ private final transient CountDownLatch latch;
+
+ public DelayedNBReadWriteServlet(CountDownLatch latch) {
+ this.latch = latch;
+ }
+
+ @Override
+ protected void doPost(HttpServletRequest request, HttpServletResponse response)
+ throws ServletException, IOException {
+ final AsyncContext ctx = request.startAsync();
+ ctx.setTimeout(1000);
+
+ Thread readWriteListener = new Thread(new ReadWriteListener(latch, ctx));
+ readWriteListener.start();
+ }
+ }
+
+ private static final class ReadWriteListener implements Runnable {
+ private final transient CountDownLatch latch;
+ private final transient AsyncContext ctx;
+
+ public ReadWriteListener(CountDownLatch latch, AsyncContext ctx){
+ this.latch = latch;
+ this.ctx = ctx;
+ }
+
+ @Override
+ public void run() {
+ try {
+ setListeners();
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ }
+
+ private void setListeners() throws IOException {
+ final ServletInputStream is = ctx.getRequest().getInputStream();
+ final ServletOutputStream os = ctx.getResponse().getOutputStream();
+
+ is.setReadListener(new ReadListener() {
+ @Override
+ public void onDataAvailable() {
+
+ try {
+ byte buffer[] = new byte[1 * 4];
+ while (is.isReady() && !is.isFinished()) {
+ is.read(buffer);
+ }
+ String body = new String(buffer, StandardCharsets.UTF_8);
+ Assert.assertTrue(body.equals("body"));
+
+ } catch (IOException ex) {
+ ex.printStackTrace();
+ }
+ }
+
+ @Override
+ public void onAllDataRead() {
+ latch.countDown();
+ }
+
+ @Override
+ public void onError(Throwable t) {
+ }
+ });
+
+ os.setWriteListener(new WriteListener() {
+ private boolean written = false;
+
+ @Override
+ public void onWritePossible() throws IOException {
+ ServletOutputStream out = ctx.getResponse().getOutputStream();
+ if (out.isReady() && !written) {
+ out.println("OK");
+ written = true;
+ }
+ if (out.isReady() && written) {
+ out.flush();
+ if (out.isReady()) {
+ ctx.complete();
+ latch.countDown();
+ }
+ }
+ }
+
+ @Override
+ public void onError(Throwable t) {
+ t.printStackTrace();
+ }
+
+ });
+ }
+
+ }
+
+
private static final class Emitter implements Serializable {
private static final long serialVersionUID = 1L;
@@ -1111,7 +1266,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
protected void doPost(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
- CountDownLatch latch = new CountDownLatch(1);
+ final CountDownLatch latch = new CountDownLatch(1);
// Dispatch to "/error" will end up here
if (req.getDispatcherType().equals(DispatcherType.ASYNC)) {
@@ -1120,8 +1275,8 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
return;
}
- AsyncContext asyncCtx = req.startAsync();
- ServletInputStream is = req.getInputStream();
+ final AsyncContext asyncCtx = req.startAsync();
+ final ServletInputStream is = req.getInputStream();
is.setReadListener(new ReadListener() {
@Override
@@ -1156,7 +1311,6 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
asyncCtx.dispatch("/error");
}).start();
-
}
}
@@ -1535,4 +1689,4 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
// NO-OP
}
}
-}
+}
\ No newline at end of file
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index d62502e..a4e90f3 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -105,6 +105,16 @@
issues do not "pop up" wrt. others).
-->
<section name="Tomcat 9.0.60 (remm)" rtext="in development">
+ <subsection name="Coyote">
+ <changelog>
+ <fix>
+ Fix regression introduced with <bug>65757</bug> bugfix which better
+ identified non request threads but which introduced a similar problem
+ when user code was doing sequential operations in a single thread.
+ Test case code submitted by Istvan Szekely. (remm)
+ </fix>
+ </changelog>
+ </subsection>
<subsection name="Jasper">
<changelog>
<fix>
@@ -320,7 +330,7 @@
</fix>
<fix>
<bug>65757</bug>: Missing initial IO listener notification on Servlet
- container dispatch to another container thread. (remm)
+ container dispatch to another container thread. (remm)
</fix>
<fix>
Expand the fix for <bug>65757</bug> so that rather than just checking if
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org