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/10 12:47:56 UTC
[tomcat] branch master updated (7afcf7a -> 71d671e)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from 7afcf7a Add debug logging of read/write interest registration
new 830050c Expand async tests
new 0726d82 Fix instance where pipelined data may be missed after an async request
new 71d671e Don't trigger an additional dispatch with async I/O and complete
The 3 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/AbstractProcessorLight.java | 27 +++-
java/org/apache/coyote/AsyncStateMachine.java | 26 ++--
.../apache/catalina/core/TestAsyncContextImpl.java | 163 +++++++++++++++++++++
3 files changed, 199 insertions(+), 17 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/03: Expand async tests
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 830050ca05ff78393e93e0e7e6fb5adcdf6e7e93
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Oct 9 19:04:48 2019 +0100
Expand async tests
While reviewing the AsyncStateMachine I identified several
inconsistencies. These tests are for one of these inconsistencies. When
performing asyncIO, complete() and dispatch() were transitioning to
inconsistent states.
---
.../apache/catalina/core/TestAsyncContextImpl.java | 163 +++++++++++++++++++++
1 file changed, 163 insertions(+)
diff --git a/test/org/apache/catalina/core/TestAsyncContextImpl.java b/test/org/apache/catalina/core/TestAsyncContextImpl.java
index fb8b30f..d748086 100644
--- a/test/org/apache/catalina/core/TestAsyncContextImpl.java
+++ b/test/org/apache/catalina/core/TestAsyncContextImpl.java
@@ -20,6 +20,7 @@ import java.io.IOException;
import java.io.PrintWriter;
import java.net.URI;
import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
@@ -35,12 +36,14 @@ import javax.servlet.GenericServlet;
import javax.servlet.RequestDispatcher;
import javax.servlet.Servlet;
import javax.servlet.ServletException;
+import javax.servlet.ServletOutputStream;
import javax.servlet.ServletRequest;
import javax.servlet.ServletRequestEvent;
import javax.servlet.ServletRequestListener;
import javax.servlet.ServletRequestWrapper;
import javax.servlet.ServletResponse;
import javax.servlet.ServletResponseWrapper;
+import javax.servlet.WriteListener;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -2653,4 +2656,164 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
}
}
+
+
+ @Test
+ public void testAsyncIoEnd00() throws Exception {
+ doTestAsyncIoEnd(false, false);
+ }
+
+
+ @Test
+ public void testAsyncIoEnd01() throws Exception {
+ doTestAsyncIoEnd(false, true);
+ }
+
+
+ @Test
+ public void testAsyncIoEnd02() throws Exception {
+ doTestAsyncIoEnd(true, false);
+ }
+
+
+ @Test
+ public void testAsyncIoEnd03() throws Exception {
+ doTestAsyncIoEnd(true, true);
+ }
+
+
+ private void doTestAsyncIoEnd(boolean useThread, boolean useComplete) throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+
+ // No file system docBase required
+ Context ctx = tomcat.addContext("", null);
+
+ AsyncIoEndServlet asyncIoEndServlet = new AsyncIoEndServlet(useThread, useComplete);
+ Wrapper wrapper = Tomcat.addServlet(ctx, "asyncIoEndServlet", asyncIoEndServlet);
+ wrapper.setAsyncSupported(true);
+ ctx.addServletMappingDecoded("/asyncIoEndServlet", "asyncIoEndServlet");
+
+ SimpleServlet simpleServlet = new SimpleServlet();
+ Tomcat.addServlet(ctx, "simpleServlet", simpleServlet);
+ ctx.addServletMappingDecoded("/simpleServlet", "simpleServlet");
+
+ tomcat.start();
+
+ ByteChunk body = new ByteChunk();
+ int rc = getUrl("http://localhost:" + getPort() + "/asyncIoEndServlet", body, null);
+
+ Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+ Assert.assertEquals("OK", body.toString());
+
+ Assert.assertFalse(asyncIoEndServlet.getInvalidStateDetected());
+ }
+
+
+ private static class AsyncIoEndServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ private final boolean useThread;
+ private final boolean useComplete;
+ private AsyncIoEndWriteListener asyncIoEndWriteListener;
+
+ public AsyncIoEndServlet(boolean useThread, boolean useComplete) {
+ this.useThread = useThread;
+ this.useComplete = useComplete;
+ }
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
+
+ if (useComplete) {
+ // Write expected body here
+ resp.setContentType("text/plain");
+ resp.setCharacterEncoding("UTF-8");
+ resp.getOutputStream().write("OK".getBytes(StandardCharsets.UTF_8));
+ }
+ AsyncContext ac = req.startAsync();
+ ServletOutputStream sos = resp.getOutputStream();
+ asyncIoEndWriteListener= new AsyncIoEndWriteListener(ac, useThread, useComplete);
+ sos.setWriteListener(asyncIoEndWriteListener);
+ }
+
+ public boolean getInvalidStateDetected() {
+ if (asyncIoEndWriteListener != null) {
+ return asyncIoEndWriteListener.getInvalidStateDetected();
+ }
+ return false;
+ }
+ }
+
+
+ private static class AsyncIoEndWriteListener implements WriteListener {
+
+ private final AsyncContext ac;
+ private final boolean useThread;
+ private final boolean useComplete;
+ private boolean invalidStateDetected = false;
+
+ public AsyncIoEndWriteListener(AsyncContext ac, boolean useThread,
+ boolean useComplete) {
+ this.ac = ac;
+ this.useThread = useThread;
+ this.useComplete = useComplete;
+ }
+
+
+ @Override
+ public void onWritePossible() throws IOException {
+ if (useThread) {
+ (new Thread() {
+ @Override
+ public void run() {
+ doOnWritePossible();
+ }
+ }).start();
+ } else {
+ doOnWritePossible();
+ }
+ }
+
+
+ public void doOnWritePossible() {
+ // Hack to avoid ISE if we try gettign the request after complete/dispatch
+ ServletRequest req = ac.getRequest();
+ if (useComplete) {
+ ac.complete();
+ } else {
+ ac.dispatch("/simpleServlet");
+ }
+ if (req.isAsyncStarted()) {
+ invalidStateDetected = true;
+ }
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ throw new RuntimeException(throwable);
+ }
+
+
+ public boolean getInvalidStateDetected() {
+ return invalidStateDetected;
+ }
+ }
+
+
+ private static class SimpleServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
+
+ // Write expected body here
+ resp.setContentType("text/plain");
+ resp.setCharacterEncoding("UTF-8");
+ resp.getOutputStream().write("OK".getBytes(StandardCharsets.UTF_8));
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 03/03: Don't trigger an additional dispatch with async I/O
and complete
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 71d671eab810c5bf1272f1fccd1deff3e8606d28
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Oct 9 21:54:37 2019 +0100
Don't trigger an additional dispatch with async I/O and complete
---
java/org/apache/coyote/AsyncStateMachine.java | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/java/org/apache/coyote/AsyncStateMachine.java b/java/org/apache/coyote/AsyncStateMachine.java
index d9a887a..9f8a489 100644
--- a/java/org/apache/coyote/AsyncStateMachine.java
+++ b/java/org/apache/coyote/AsyncStateMachine.java
@@ -320,19 +320,27 @@ class AsyncStateMachine {
private synchronized boolean doComplete() {
clearNonBlockingListeners();
- boolean doComplete = false;
+ boolean triggerDispatch = false;
if (state == AsyncState.STARTING || state == AsyncState.TIMING_OUT ||
- state == AsyncState.ERROR || state == AsyncState.READ_WRITE_OP) {
+ state == AsyncState.ERROR) {
state = AsyncState.MUST_COMPLETE;
} else if (state == AsyncState.STARTED || state == AsyncState.COMPLETE_PENDING) {
state = AsyncState.COMPLETING;
- doComplete = true;
+ triggerDispatch = true;
+ } else if (state == AsyncState.READ_WRITE_OP) {
+ // Read/write operations can happen on or off a container thread but
+ // the call to listener that triggers the read/write will always be
+ // on a container thread and the socket will be added to the poller
+ // when the thread exits the AbstractConnectionHandler.process()
+ // method so don't do a dispatch here which would add it to the
+ // poller a second time.
+ state = AsyncState.COMPLETING;
} else {
throw new IllegalStateException(
sm.getString("asyncStateMachine.invalidAsyncState",
"asyncComplete()", state));
}
- return doComplete;
+ return triggerDispatch;
}
@@ -366,7 +374,7 @@ class AsyncStateMachine {
private synchronized boolean doDispatch() {
clearNonBlockingListeners();
- boolean doDispatch = false;
+ boolean triggerDispatch = false;
if (state == AsyncState.STARTING ||
state == AsyncState.TIMING_OUT ||
state == AsyncState.ERROR) {
@@ -381,22 +389,22 @@ class AsyncStateMachine {
// If on a container thread the current request/response are not the
// request/response associated with the AsyncContext so need a new
// container thread to process the different request/response.
- doDispatch = true;
+ triggerDispatch = true;
} else if (state == AsyncState.READ_WRITE_OP) {
state = AsyncState.DISPATCHING;
// If on a container thread then the socket will be added to the
- // poller poller when the thread exits the
+ // poller when the thread exits the
// AbstractConnectionHandler.process() method so don't do a dispatch
// here which would add it to the poller a second time.
if (!ContainerThreadMarker.isContainerThread()) {
- doDispatch = true;
+ triggerDispatch = true;
}
} else {
throw new IllegalStateException(
sm.getString("asyncStateMachine.invalidAsyncState",
"asyncDispatch()", state));
}
- return doDispatch;
+ return triggerDispatch;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/03: Fix instance where pipelined data may be missed
after an async request
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 0726d82ac2627a505f2be32712f1816b51dad4f4
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Oct 9 21:52:31 2019 +0100
Fix instance where pipelined data may be missed after an async request
---
java/org/apache/coyote/AbstractProcessorLight.java | 27 +++++++++++++++-------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/coyote/AbstractProcessorLight.java b/java/org/apache/coyote/AbstractProcessorLight.java
index 049797d..b26b41f 100644
--- a/java/org/apache/coyote/AbstractProcessorLight.java
+++ b/java/org/apache/coyote/AbstractProcessorLight.java
@@ -50,18 +50,14 @@ public abstract class AbstractProcessorLight implements Processor {
getLog().debug("Processing dispatch type: [" + nextDispatch + "]");
}
state = dispatch(nextDispatch.getSocketStatus());
+ if (!dispatches.hasNext()) {
+ state = checkForPipelinedData(state, socketWrapper);
+ }
} else if (status == SocketEvent.DISCONNECT) {
// Do nothing here, just wait for it to get recycled
} else if (isAsync() || isUpgrade() || state == SocketState.ASYNC_END) {
state = dispatch(status);
- if (state == SocketState.OPEN) {
- // There may be pipe-lined data to read. If the data isn't
- // processed now, execution will exit this loop and call
- // release() which will recycle the processor (and input
- // buffer) deleting any pipe-lined data. To avoid this,
- // process it now.
- state = service(socketWrapper);
- }
+ state = checkForPipelinedData(state, socketWrapper);
} else if (status == SocketEvent.OPEN_WRITE) {
// Extra write event likely after async, ignore
state = SocketState.LONG;
@@ -101,6 +97,21 @@ public abstract class AbstractProcessorLight implements Processor {
}
+ private SocketState checkForPipelinedData(SocketState inState, SocketWrapperBase<?> socketWrapper)
+ throws IOException {
+ if (inState == SocketState.OPEN) {
+ // There may be pipe-lined data to read. If the data isn't
+ // processed now, execution will exit this loop and call
+ // release() which will recycle the processor (and input
+ // buffer) deleting any pipe-lined data. To avoid this,
+ // process it now.
+ return service(socketWrapper);
+ } else {
+ return inState;
+ }
+ }
+
+
public void addDispatch(DispatchType dispatchType) {
synchronized (dispatches) {
dispatches.add(dispatchType);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org