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/14 15:20:22 UTC
[tomcat] 03/03: Fix
https://bz.apache.org/bugzilla/show_bug.cgi?id=63816 async errors
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 327c5532f3f3123c937572541e56175851d73c48
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Oct 14 16:18:17 2019 +0100
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63816 async errors
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 current
request/response.
---
java/org/apache/coyote/AbstractProcessor.java | 13 +++--------
java/org/apache/coyote/AsyncStateMachine.java | 32 ++++-----------------------
webapps/docs/changelog.xml | 6 +++++
3 files changed, 13 insertions(+), 38 deletions(-)
diff --git a/java/org/apache/coyote/AbstractProcessor.java b/java/org/apache/coyote/AbstractProcessor.java
index fc32745..d5c3ee1 100644
--- a/java/org/apache/coyote/AbstractProcessor.java
+++ b/java/org/apache/coyote/AbstractProcessor.java
@@ -110,17 +110,10 @@ public abstract class AbstractProcessor extends AbstractProcessorLight implement
if (t != null) {
request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, t);
}
- if (blockIo && !ContainerThreadMarker.isContainerThread() && isAsync()) {
- // The error occurred on a non-container thread during async
- // processing which means not all of the necessary clean-up will
- // have been completed. Dispatch to a container thread to do the
- // clean-up. Need to do it this way to ensure that all the necessary
- // clean-up is performed.
- asyncStateMachine.asyncMustError();
- if (getLog().isDebugEnabled()) {
- getLog().debug(sm.getString("abstractProcessor.nonContainerThreadError"), t);
+ if (blockIo && isAsync()) {
+ if (asyncStateMachine.asyncError()) {
+ processSocketEvent(SocketEvent.ERROR, true);
}
- processSocketEvent(SocketEvent.ERROR, true);
}
}
diff --git a/java/org/apache/coyote/AsyncStateMachine.java b/java/org/apache/coyote/AsyncStateMachine.java
index 5fe1ecc..00191ba 100644
--- a/java/org/apache/coyote/AsyncStateMachine.java
+++ b/java/org/apache/coyote/AsyncStateMachine.java
@@ -154,7 +154,6 @@ class AsyncStateMachine {
DISPATCH_PENDING(true, true, false, false),
DISPATCHING (true, false, false, true),
READ_WRITE_OP (true, true, false, false),
- MUST_ERROR (true, true, false, false),
ERROR (true, true, false, false);
private final boolean isAsync;
@@ -434,36 +433,13 @@ class AsyncStateMachine {
}
- synchronized void asyncMustError() {
- if (state == AsyncState.STARTED) {
- clearNonBlockingListeners();
- state = AsyncState.MUST_ERROR;
- } else {
- throw new IllegalStateException(
- sm.getString("asyncStateMachine.invalidAsyncState",
- "asyncMustError()", state));
- }
+ synchronized boolean asyncError() {
+ clearNonBlockingListeners();
+ state = AsyncState.ERROR;
+ return !ContainerThreadMarker.isContainerThread();
}
- synchronized void asyncError() {
- if (state == AsyncState.STARTING ||
- state == AsyncState.STARTED ||
- state == AsyncState.DISPATCHED ||
- state == AsyncState.TIMING_OUT ||
- state == AsyncState.MUST_COMPLETE ||
- state == AsyncState.READ_WRITE_OP ||
- state == AsyncState.COMPLETING ||
- state == AsyncState.MUST_ERROR) {
- clearNonBlockingListeners();
- state = AsyncState.ERROR;
- } else {
- throw new IllegalStateException(
- sm.getString("asyncStateMachine.invalidAsyncState",
- "asyncError()", state));
- }
- }
-
synchronized void asyncRun(Runnable runnable) {
if (state == AsyncState.STARTING || state == AsyncState.STARTED ||
state == AsyncState.READ_WRITE_OP) {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a860c45..c36ce45 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -66,6 +66,12 @@
<code>AsyncListener.onTimeout()</code> or
<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
+ current request/response. (markt)
+ </fix>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 03/03: Fix
https://bz.apache.org/bugzilla/show_bug.cgi?id=63816 async errors
Posted by Mark Thomas <ma...@apache.org>.
On 14/10/2019 16:20, markt@apache.org wrote:
> 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 327c5532f3f3123c937572541e56175851d73c48
> Author: Mark Thomas <ma...@apache.org>
> AuthorDate: Mon Oct 14 16:18:17 2019 +0100
>
> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63816 async errors
>
> 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 current
> request/response.
Just a heads up that, while all the test cases pass, I think this patch
may over-simplify some edge cases. I'm planning on working on BZ 63817
next and the fix for that issue may end up re-instating some of the code
removed by this fix.
I plan to update/replace the state diagram once the code changes are
complete.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org