You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/10/15 23:16:47 UTC
[tomcat] branch 7.0.x updated (fe65d4c -> 87dbf83)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from fe65d4c Remove sync that triggers deadlock with BZ 63816 test case
new 5e3b0aa Expand test for https://bz.apache.org/bugzilla/show_bug.cgi?id=63816
new 8e17930 Refactor
new f353d63 Refactor
new d2d46f0 Remove an illegal state transition
new 4eec200 Hack to fix failing test
new 79af7e2 Fix back-port to Java 6 errors
new 87dbf83 Refactor the unit test to avoid race conditions
The 7 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
java/org/apache/coyote/AbstractProcessor.java | 5 +-
java/org/apache/coyote/AsyncStateMachine.java | 29 +-
.../apache/catalina/core/TestAsyncContextImpl.java | 7 +-
.../core/TestAsyncContextStateChanges.java | 370 +++++++++++++++++++++
webapps/docs/changelog.xml | 6 +-
5 files changed, 391 insertions(+), 26 deletions(-)
create mode 100644 test/org/apache/catalina/core/TestAsyncContextStateChanges.java
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 05/07: Hack to fix failing test
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 4eec200b72e8059c4cc7b501f83d45cb9cdfc05c
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Oct 15 21:36:52 2019 +0100
Hack to fix failing test
---
.../org/apache/catalina/core/TestAsyncContextStateChanges.java | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/test/org/apache/catalina/core/TestAsyncContextStateChanges.java b/test/org/apache/catalina/core/TestAsyncContextStateChanges.java
index 5df1740..dc1cde9 100644
--- a/test/org/apache/catalina/core/TestAsyncContextStateChanges.java
+++ b/test/org/apache/catalina/core/TestAsyncContextStateChanges.java
@@ -196,6 +196,16 @@ public class TestAsyncContextStateChanges extends TomcatBaseTest {
if (endTiming == EndTiming.THREAD_AFTER_EXIT) {
try {
threadLatch.await();
+ /*
+ * As much as I dislike it, I don't see any easy way around
+ * this hack. The latch above is released as the Servlet
+ * exits but we need to wait for the post processing to
+ * complete for the test to work as intended. In real-world
+ * applications this does mean that there is a real chance
+ * of an ISE. We may need to increase this delay for some CI
+ * systems.
+ */
+ Thread.sleep(1000);
} catch (InterruptedException e) {
// Ignore
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 07/07: Refactor the unit test to avoid race conditions
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 87dbf83dd51616cbca6b5ee7406684ebcade1c4e
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Oct 15 23:11:59 2019 +0100
Refactor the unit test to avoid race conditions
If an I/O error occurs on a non-container thread that will trigger a
container thread to process the error - primarily to fire on onError()
event. If the app tries to handle the error on the non-container thread
a race condition is very likely.
---
.../core/TestAsyncContextStateChanges.java | 100 +++++++++++++++------
1 file changed, 72 insertions(+), 28 deletions(-)
diff --git a/test/org/apache/catalina/core/TestAsyncContextStateChanges.java b/test/org/apache/catalina/core/TestAsyncContextStateChanges.java
index 5613c07..5c2001f 100644
--- a/test/org/apache/catalina/core/TestAsyncContextStateChanges.java
+++ b/test/org/apache/catalina/core/TestAsyncContextStateChanges.java
@@ -160,7 +160,11 @@ public class TestAsyncContextStateChanges extends TomcatBaseTest {
servletRequest = req;
asyncContext = req.startAsync();
asyncContext.addListener(new Listener());
- asyncContext.setTimeout(1000);
+ if (!asyncEnd.isError()) {
+ // Use a short timeout so the test does not pause for too long
+ // waiting for the timeout to be triggered.
+ asyncContext.setTimeout(1000);
+ }
Thread t = new NonContainerThread();
switch (endTiming) {
@@ -231,35 +235,35 @@ public class TestAsyncContextStateChanges extends TomcatBaseTest {
}
}
- try {
- switch (asyncEnd) {
- case COMPLETE:
- case ERROR_COMPLETE: {
- asyncContext.complete();
- break;
+ if (endTiming != EndTiming.THREAD_AFTER_EXIT) {
+ try {
+ switch (asyncEnd) {
+ case COMPLETE:
+ case ERROR_COMPLETE: {
+ asyncContext.complete();
+ break;
+ }
+ case DISPATCH:
+ case ERROR_DISPATCH: {
+ dispatch = true;
+ asyncContext.dispatch();
+ break;
+ }
+ case NONE:
+ case ERROR_NONE: {
+ break;
+ }
}
- case DISPATCH:
- case ERROR_DISPATCH: {
- dispatch = true;
- asyncContext.dispatch();
- break;
+
+ // The request must stay in async mode until doGet() exists
+ if (servletRequest.isAsyncStarted()) {
+ failed.set(false);
}
- case NONE:
- case ERROR_NONE: {
- break;
+ } finally {
+ if (endTiming == EndTiming.THREAD_BEFORE_EXIT) {
+ threadLatch.countDown();
}
}
-
- // The request must stay in async mode until doGet() exists
- if (servletRequest.isAsyncStarted() != (endTiming == EndTiming.THREAD_AFTER_EXIT && !asyncEnd.isNone())) {
- failed.set(false);
- }
- } finally {
- if (endTiming == EndTiming.THREAD_BEFORE_EXIT) {
- threadLatch.countDown();
- } else if (endTiming == EndTiming.THREAD_AFTER_EXIT) {
- endLatch.countDown();
- }
}
}
}
@@ -276,12 +280,52 @@ public class TestAsyncContextStateChanges extends TomcatBaseTest {
@Override
public void onTimeout(AsyncEvent event) throws IOException {
- // NO-OP
+ // Need to handle timeouts for THREAD_AFTER_EXIT in the listener to
+ // avoid concurrency issues.
+ if (endTiming == EndTiming.THREAD_AFTER_EXIT) {
+ switch (asyncEnd) {
+ case COMPLETE: {
+ asyncContext.complete();
+ break;
+ }
+ case DISPATCH: {
+ dispatch = true;
+ asyncContext.dispatch();
+ break;
+ }
+ default:
+ // NO-OP
+ }
+ }
+ if (servletRequest.isAsyncStarted() == asyncEnd.isNone()) {
+ failed.set(false);
+ }
+ endLatch.countDown();
}
@Override
public void onError(AsyncEvent event) throws IOException {
- // NO-OP
+ // Need to handle errors for THREAD_AFTER_EXIT in the listener to
+ // avoid concurrency issues.
+ if (endTiming == EndTiming.THREAD_AFTER_EXIT) {
+ switch (asyncEnd) {
+ case ERROR_COMPLETE: {
+ asyncContext.complete();
+ break;
+ }
+ case ERROR_DISPATCH: {
+ dispatch = true;
+ asyncContext.dispatch();
+ break;
+ }
+ default:
+ // NO-OP
+ }
+ if (servletRequest.isAsyncStarted() == asyncEnd.isNone()) {
+ failed.set(false);
+ }
+ endLatch.countDown();
+ }
}
@Override
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 06/07: Fix back-port to Java 6 errors
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 79af7e2c5dd57127e8d695e5bd3b3490defb75a0
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Oct 15 21:48:21 2019 +0100
Fix back-port to Java 6 errors
---
test/org/apache/catalina/core/TestAsyncContextStateChanges.java | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/test/org/apache/catalina/core/TestAsyncContextStateChanges.java b/test/org/apache/catalina/core/TestAsyncContextStateChanges.java
index dc1cde9..5613c07 100644
--- a/test/org/apache/catalina/core/TestAsyncContextStateChanges.java
+++ b/test/org/apache/catalina/core/TestAsyncContextStateChanges.java
@@ -18,7 +18,6 @@ package org.apache.catalina.core;
import java.io.IOException;
import java.io.OutputStream;
-import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -58,7 +57,7 @@ public class TestAsyncContextStateChanges extends TomcatBaseTest {
@Parameterized.Parameters(name = "{index}: end [{0}], timing [{1}]")
public static Collection<Object[]> parameters() {
- List<Object[]> parameterSets = new ArrayList<>();
+ List<Object[]> parameterSets = new ArrayList<Object[]>();
for (AsyncEnd asyncEnd : AsyncEnd.values()) {
for (EndTiming endTiming : EndTiming.values()) {
parameterSets.add(new Object[] { asyncEnd, endTiming });
@@ -102,7 +101,7 @@ public class TestAsyncContextStateChanges extends TomcatBaseTest {
AsyncServlet bug63816Servlet = new AsyncServlet();
Wrapper wrapper = Tomcat.addServlet(ctx, "bug63816Servlet", bug63816Servlet);
wrapper.setAsyncSupported(true);
- ctx.addServletMappingDecoded("/*", "bug63816Servlet");
+ ctx.addServletMapping("/*", "bug63816Servlet");
tomcat.start();
@@ -225,7 +224,7 @@ public class TestAsyncContextStateChanges extends TomcatBaseTest {
OutputStream os = resp.getOutputStream();
resp.setContentType("text/plain");
for (int i = 0; i < 16; i++) {
- os.write(TestCoyoteAdapter.TEXT_8K.getBytes(StandardCharsets.UTF_8));
+ os.write(TestCoyoteAdapter.TEXT_8K.getBytes("UTF-8"));
}
} catch (IOException e) {
// Expected
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 04/07: Remove an illegal state transition
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d2d46f0a220066329c258e5fd6d3be355c5ea232
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Oct 15 21:27:10 2019 +0100
Remove an illegal state transition
---
java/org/apache/coyote/AsyncStateMachine.java | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/coyote/AsyncStateMachine.java b/java/org/apache/coyote/AsyncStateMachine.java
index 8363afa..3200fbe 100644
--- a/java/org/apache/coyote/AsyncStateMachine.java
+++ b/java/org/apache/coyote/AsyncStateMachine.java
@@ -273,7 +273,7 @@ public class AsyncStateMachine<S> {
// Processing is on a container thread so no need to transfer
// processing to a new container thread
state = AsyncState.MUST_COMPLETE;
- } else if (state == AsyncState.STARTED || state == AsyncState.COMPLETE_PENDING) {
+ } else if (state == AsyncState.STARTED) {
state = AsyncState.COMPLETING;
// A dispatch to a container thread is always required.
// If on a non-container thread, need to get back onto a container
@@ -329,7 +329,7 @@ public class AsyncStateMachine<S> {
// Processing is on a container thread so no need to transfer
// processing to a new container thread
state = AsyncState.MUST_DISPATCH;
- } else if (state == AsyncState.STARTED || state == AsyncState.DISPATCH_PENDING) {
+ } else if (state == AsyncState.STARTED) {
state = AsyncState.DISPATCHING;
// A dispatch to a container thread is always required.
// If on a non-container thread, need to get back onto a container
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/07: Expand test for
https://bz.apache.org/bugzilla/show_bug.cgi?id=63816
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 5e3b0aa030e726c3f01c6ee3517f757900784399
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Oct 15 14:18:26 2019 +0100
Expand test for https://bz.apache.org/bugzilla/show_bug.cgi?id=63816
Expanded to cover https://bz.apache.org/bugzilla/show_bug.cgi?id=63817
and additional, similar scenarios. Correct bugs in existing tests.
---
java/org/apache/coyote/AbstractProcessor.java | 5 +-
java/org/apache/coyote/AsyncStateMachine.java | 11 +-
.../apache/catalina/core/TestAsyncContextImpl.java | 7 +-
.../core/TestAsyncContextStateChanges.java | 317 +++++++++++++++++++++
webapps/docs/changelog.xml | 6 +-
5 files changed, 334 insertions(+), 12 deletions(-)
diff --git a/java/org/apache/coyote/AbstractProcessor.java b/java/org/apache/coyote/AbstractProcessor.java
index fdd90a6..5405fc4 100644
--- a/java/org/apache/coyote/AbstractProcessor.java
+++ b/java/org/apache/coyote/AbstractProcessor.java
@@ -86,6 +86,9 @@ public abstract class AbstractProcessor<S> implements ActionHook, Processor<S> {
* @param t The error which occurred
*/
protected void setErrorState(ErrorState errorState, Throwable t) {
+ // Use the return value to avoid processing more than one async error
+ // in a single async cycle.
+ boolean setError = response.setError();
boolean blockIo = this.errorState.isIoAllowed() && !errorState.isIoAllowed();
this.errorState = this.errorState.getMostSevere(errorState);
// Don't change the status code for IOException since that is almost
@@ -97,7 +100,7 @@ public abstract class AbstractProcessor<S> implements ActionHook, Processor<S> {
if (t != null) {
request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, t);
}
- if (blockIo && isAsync()) {
+ if (blockIo && isAsync() && setError) {
if (asyncStateMachine.asyncError()) {
getEndpoint().processSocketAsync(socketWrapper, SocketStatus.ERROR);
}
diff --git a/java/org/apache/coyote/AsyncStateMachine.java b/java/org/apache/coyote/AsyncStateMachine.java
index 178c1ad..6aa7a50 100644
--- a/java/org/apache/coyote/AsyncStateMachine.java
+++ b/java/org/apache/coyote/AsyncStateMachine.java
@@ -145,6 +145,7 @@ public class AsyncStateMachine<S> {
MUST_DISPATCH (true, true, false, true),
DISPATCH_PENDING(true, true, false, false),
DISPATCHING (true, false, false, true),
+ MUST_ERROR (true, true, false, false),
ERROR (true, true, false, false);
private final boolean isAsync;
@@ -273,7 +274,7 @@ public class AsyncStateMachine<S> {
private synchronized boolean doComplete() {
boolean triggerDispatch = false;
- if (state == AsyncState.STARTING) {
+ if (state == AsyncState.STARTING || state == AsyncState.MUST_ERROR) {
// Processing is on a container thread so no need to transfer
// processing to a new container thread
state = AsyncState.MUST_COMPLETE;
@@ -334,7 +335,7 @@ public class AsyncStateMachine<S> {
private synchronized boolean doDispatch() {
boolean triggerDispatch = false;
- if (state == AsyncState.STARTING) {
+ if (state == AsyncState.STARTING || state == AsyncState.MUST_ERROR) {
// Processing is on a container thread so no need to transfer
// processing to a new container thread
state = AsyncState.MUST_DISPATCH;
@@ -378,7 +379,11 @@ public class AsyncStateMachine<S> {
public synchronized boolean asyncError() {
- state = AsyncState.ERROR;
+ if (state == AsyncState.STARTING) {
+ state = AsyncState.MUST_ERROR;
+ } else {
+ state = AsyncState.ERROR;
+ }
return !ContainerThreadMarker.isContainerThread();
}
diff --git a/test/org/apache/catalina/core/TestAsyncContextImpl.java b/test/org/apache/catalina/core/TestAsyncContextImpl.java
index 2e9dbcf..93f48c7 100644
--- a/test/org/apache/catalina/core/TestAsyncContextImpl.java
+++ b/test/org/apache/catalina/core/TestAsyncContextImpl.java
@@ -848,6 +848,8 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
private final boolean completeOnError;
private final boolean completeOnTimeout;
private final String dispatchUrl;
+ // Assumes listener is fired after container thread that initiated async
+ // has exited.
private boolean asyncStartedCorrect = true;
public TrackingListener(boolean completeOnError,
@@ -1896,7 +1898,6 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
}
Assert.assertEquals(expectedTrack, getTrack());
- Assert.assertTrue(bug59219Servlet.isAsyncStartedCorrect());
}
@@ -1931,10 +1932,6 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
} else
throw new ServletException();
}
-
- public boolean isAsyncStartedCorrect() {
- return trackingListener.isAsyncStartedCorrect();
- }
}
@Test
diff --git a/test/org/apache/catalina/core/TestAsyncContextStateChanges.java b/test/org/apache/catalina/core/TestAsyncContextStateChanges.java
new file mode 100644
index 0000000..5df1740
--- /dev/null
+++ b/test/org/apache/catalina/core/TestAsyncContextStateChanges.java
@@ -0,0 +1,317 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.core;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import javax.servlet.AsyncContext;
+import javax.servlet.AsyncEvent;
+import javax.servlet.AsyncListener;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.Wrapper;
+import org.apache.catalina.connector.TestCoyoteAdapter;
+import org.apache.catalina.startup.SimpleHttpClient;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+
+/*
+ * Derived from a test for https://bz.apache.org/bugzilla/show_bug.cgi?id=63816
+ * Expanded to cover https://bz.apache.org/bugzilla/show_bug.cgi?id=63817 and
+ * additional scenarios.
+ */
+@RunWith(Parameterized.class)
+public class TestAsyncContextStateChanges extends TomcatBaseTest {
+
+ @Parameterized.Parameters(name = "{index}: end [{0}], timing [{1}]")
+ public static Collection<Object[]> parameters() {
+ List<Object[]> parameterSets = new ArrayList<>();
+ for (AsyncEnd asyncEnd : AsyncEnd.values()) {
+ for (EndTiming endTiming : EndTiming.values()) {
+ parameterSets.add(new Object[] { asyncEnd, endTiming });
+ }
+ }
+ return parameterSets;
+ }
+
+ @Parameter(0)
+ public AsyncEnd asyncEnd;
+
+ @Parameter(1)
+ public EndTiming endTiming;
+
+ private ServletRequest servletRequest = null;
+ private AsyncContext asyncContext = null;
+ private AtomicBoolean failed = new AtomicBoolean();
+ private CountDownLatch threadLatch;
+ private CountDownLatch closeLatch;
+ private CountDownLatch endLatch;
+ private boolean dispatch;
+
+ @Test
+ public void testAsync() throws Exception {
+ dispatch = false;
+ servletRequest = null;
+ asyncContext = null;
+
+ // Initialise tracking fields
+ failed.set(true);
+ threadLatch = new CountDownLatch(1);
+ closeLatch = new CountDownLatch(1);
+ endLatch = new CountDownLatch(1);
+
+ // Setup Tomcat instance
+ Tomcat tomcat = getTomcatInstance();
+
+ // No file system docBase required
+ Context ctx = tomcat.addContext("", null);
+
+ AsyncServlet bug63816Servlet = new AsyncServlet();
+ Wrapper wrapper = Tomcat.addServlet(ctx, "bug63816Servlet", bug63816Servlet);
+ wrapper.setAsyncSupported(true);
+ ctx.addServletMappingDecoded("/*", "bug63816Servlet");
+
+ tomcat.start();
+
+ Client client = new Client();
+ client.setPort(getPort());
+ client.setRequest(new String[] { "GET / HTTP/1.1" + SimpleHttpClient.CRLF +
+ "Host: localhost:" + SimpleHttpClient.CRLF +
+ SimpleHttpClient.CRLF});
+ client.connect();
+ client.sendRequest();
+
+ if (asyncEnd.isError()) {
+ client.disconnect();
+ closeLatch.countDown();
+ try {
+ endLatch.await();
+ } catch (InterruptedException e) {
+ // Ignore
+ }
+ } else {
+ client.setUseContentLength(true);
+ client.readResponse(true);
+ }
+
+ Assert.assertFalse(failed.get());
+ }
+
+
+ private static final class Client extends SimpleHttpClient {
+
+ @Override
+ public boolean isResponseBodyOK() {
+ return true;
+ }
+ }
+
+
+ private final class AsyncServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
+ if (dispatch) {
+ return;
+ }
+
+ if (!asyncEnd.isError()) {
+ resp.setContentType("text/plain");
+ resp.setCharacterEncoding("UTF-8");
+ resp.setContentLength(2);
+ resp.getWriter().print("OK");
+ }
+
+ servletRequest = req;
+ asyncContext = req.startAsync();
+ asyncContext.addListener(new Listener());
+ asyncContext.setTimeout(1000);
+ Thread t = new NonContainerThread();
+
+ switch (endTiming) {
+ case INLINE: {
+ t.run();
+ break;
+ }
+ case THREAD_AFTER_EXIT: {
+ t.start();
+ threadLatch.countDown();
+ break;
+ }
+ case THREAD_BEFORE_EXIT: {
+ t.start();
+ try {
+ threadLatch.await();
+ } catch (InterruptedException e) {
+ // Ignore
+ }
+ endLatch.countDown();
+ break;
+ }
+ }
+ }
+ }
+
+
+ private final class NonContainerThread extends Thread {
+
+ @Override
+ public void run() {
+ if (endTiming == EndTiming.THREAD_AFTER_EXIT) {
+ try {
+ threadLatch.await();
+ } catch (InterruptedException e) {
+ // Ignore
+ }
+ }
+
+ // Trigger the error if necessary
+ if (asyncEnd.isError()) {
+ try {
+ closeLatch.await();
+ } catch (InterruptedException e) {
+ // Ignore
+ }
+ try {
+ ServletResponse resp = asyncContext.getResponse();
+ resp.setContentType("text/plain");
+ resp.setCharacterEncoding("UTF-8");
+ OutputStream os = resp.getOutputStream();
+ resp.setContentType("text/plain");
+ for (int i = 0; i < 16; i++) {
+ os.write(TestCoyoteAdapter.TEXT_8K.getBytes(StandardCharsets.UTF_8));
+ }
+ } catch (IOException e) {
+ // Expected
+ }
+ }
+
+ try {
+ switch (asyncEnd) {
+ case COMPLETE:
+ case ERROR_COMPLETE: {
+ asyncContext.complete();
+ break;
+ }
+ case DISPATCH:
+ case ERROR_DISPATCH: {
+ dispatch = true;
+ asyncContext.dispatch();
+ break;
+ }
+ case NONE:
+ case ERROR_NONE: {
+ break;
+ }
+ }
+
+ // The request must stay in async mode until doGet() exists
+ if (servletRequest.isAsyncStarted() != (endTiming == EndTiming.THREAD_AFTER_EXIT && !asyncEnd.isNone())) {
+ failed.set(false);
+ }
+ } finally {
+ if (endTiming == EndTiming.THREAD_BEFORE_EXIT) {
+ threadLatch.countDown();
+ } else if (endTiming == EndTiming.THREAD_AFTER_EXIT) {
+ endLatch.countDown();
+ }
+ }
+ }
+ }
+
+
+ private class Listener implements AsyncListener {
+
+ @Override
+ public void onComplete(AsyncEvent event) throws IOException {
+ if (endTiming == EndTiming.INLINE) {
+ endLatch.countDown();
+ }
+ }
+
+ @Override
+ public void onTimeout(AsyncEvent event) throws IOException {
+ // NO-OP
+ }
+
+ @Override
+ public void onError(AsyncEvent event) throws IOException {
+ // NO-OP
+ }
+
+ @Override
+ public void onStartAsync(AsyncEvent event) throws IOException {
+ // NO-OP
+ }
+ }
+
+
+ public enum AsyncEnd {
+
+ NONE ( true, false),
+ COMPLETE (false, false),
+ DISPATCH (false, false),
+ ERROR_NONE ( true, true),
+ ERROR_COMPLETE(false, true),
+ ERROR_DISPATCH(false, true);
+
+ final boolean none;
+ final boolean error;
+
+ private AsyncEnd(boolean none, boolean error) {
+ this.none = none;
+ this.error = error;
+ }
+
+ public boolean isNone() {
+ return none;
+ }
+
+ public boolean isError() {
+ return error;
+ }
+ }
+
+
+ public enum EndTiming {
+ INLINE,
+ THREAD_BEFORE_EXIT,
+ THREAD_AFTER_EXIT
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 924b71a..3f10e31 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -82,9 +82,9 @@
<code>AsyncListener.onError()</code>. (markt)
</fix>
<fix>
- <bug>63816</bug>: Correctly handle I/O errors on a non-container thread
- after asynchronous processing has been started but before the container
- thread that started asynchronous processing has completed processing the
+ <bug>63816</bug> and <bug>63817</bug>: Correctly handle I/O errors after
+ asynchronous processing has been started but before the container thread
+ that started asynchronous processing has completed processing the
current request/response. (markt)
</fix>
</changelog>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 03/07: Refactor
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit f353d63c7df7113b70b6c35b3d203879382e0efa
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Oct 15 21:16:11 2019 +0100
Refactor
---
java/org/apache/coyote/AsyncStateMachine.java | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/java/org/apache/coyote/AsyncStateMachine.java b/java/org/apache/coyote/AsyncStateMachine.java
index 98df79c..8363afa 100644
--- a/java/org/apache/coyote/AsyncStateMachine.java
+++ b/java/org/apache/coyote/AsyncStateMachine.java
@@ -232,11 +232,9 @@ public class AsyncStateMachine<S> {
*/
public synchronized SocketState asyncPostProcess() {
if (state == AsyncState.COMPLETE_PENDING) {
- clearNonBlockingListeners();
state = AsyncState.COMPLETING;
return SocketState.ASYNC_END;
} else if (state == AsyncState.DISPATCH_PENDING) {
- clearNonBlockingListeners();
state = AsyncState.DISPATCHING;
return SocketState.ASYNC_END;
} else if (state == AsyncState.STARTING) {
@@ -268,13 +266,8 @@ public class AsyncStateMachine<S> {
if (!ContainerThreadMarker.isContainerThread() && state == AsyncState.STARTING) {
state = AsyncState.COMPLETE_PENDING;
return false;
- } else {
- return doComplete();
}
- }
-
- private synchronized boolean doComplete() {
boolean triggerDispatch = false;
if (state == AsyncState.STARTING || state == AsyncState.MUST_ERROR) {
// Processing is on a container thread so no need to transfer
@@ -329,13 +322,8 @@ public class AsyncStateMachine<S> {
if (!ContainerThreadMarker.isContainerThread() && state == AsyncState.STARTING) {
state = AsyncState.DISPATCH_PENDING;
return false;
- } else {
- return doDispatch();
}
- }
-
- private synchronized boolean doDispatch() {
boolean triggerDispatch = false;
if (state == AsyncState.STARTING || state == AsyncState.MUST_ERROR) {
// Processing is on a container thread so no need to transfer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/07: Refactor
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 8e17930703c3d6c96cd2d0e8ff450c0ecf0d4519
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Oct 15 21:14:25 2019 +0100
Refactor
---
java/org/apache/coyote/AsyncStateMachine.java | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/coyote/AsyncStateMachine.java b/java/org/apache/coyote/AsyncStateMachine.java
index 6aa7a50..98df79c 100644
--- a/java/org/apache/coyote/AsyncStateMachine.java
+++ b/java/org/apache/coyote/AsyncStateMachine.java
@@ -232,10 +232,12 @@ public class AsyncStateMachine<S> {
*/
public synchronized SocketState asyncPostProcess() {
if (state == AsyncState.COMPLETE_PENDING) {
- doComplete();
+ clearNonBlockingListeners();
+ state = AsyncState.COMPLETING;
return SocketState.ASYNC_END;
} else if (state == AsyncState.DISPATCH_PENDING) {
- doDispatch();
+ clearNonBlockingListeners();
+ state = AsyncState.DISPATCHING;
return SocketState.ASYNC_END;
} else if (state == AsyncState.STARTING) {
state = AsyncState.STARTED;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org