Author: markt
Date: Wed Feb 15 20:27:21 2017
New Revision: 1783144
URL: http://svn.apache.org/viewvc?rev=1783144&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60718
Improve error handling for asynchronous processing and correct a number of
cases where the <requestDestroyed() event was not being fired and an entry
wasn't being made in the access logs.
Modified:
tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
tomcat/trunk/java/org/apache/catalina/core/StandardHostValve.java
tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1783144&r1=1783143&r2=1783144&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Wed Feb
15 20:27:21 2017
@@ -140,14 +140,7 @@ public class CoyoteAdapter implements Ad
req.getRequestProcessor().setWorkerThreadName(Thread.currentThread().getName());
try {
if (!request.isAsync()) {
- // Error or timeout - need to tell listeners the request is
over
- // Have to test this first since state may change while in this
- // method and this is only required if entering this method in
- // this state
- Context ctxt = request.getMappingData().context;
- if (ctxt != null) {
- ctxt.fireRequestDestroyEvent(request);
- }
+ // Error or timeout
// Lift any suspension (e.g. if sendError() was used by an
async
// request) to allow the response to be written to the client
response.setSuspended(false);
@@ -382,6 +375,17 @@ public class CoyoteAdapter implements Ad
} catch (IOException e) {
// Ignore
} finally {
+ AtomicBoolean error = new AtomicBoolean(false);
+ res.action(ActionCode.IS_ERROR, error);
+
+ if (request.isAsyncCompleting() && error.get()) {
+ // Connection will be forcibly closed which will prevent
+ // completion happening at the usual point. Need to trigger
+ // call to onComplete() here.
+ res.action(ActionCode.ASYNC_POST_PROCESS, null);
+ async = false;
+ }
+
// Access log
if (!async && postParseSuccess) {
// Log only if processing was invoked.
@@ -391,11 +395,9 @@ public class CoyoteAdapter implements Ad
}
req.getRequestProcessor().setWorkerThreadName(null);
- AtomicBoolean error = new AtomicBoolean(false);
- res.action(ActionCode.IS_ERROR, error);
// Recycle the wrapper request and response
- if (!async || error.get()) {
+ if (!async) {
request.recycle();
response.recycle();
}
Modified: tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=1783144&r1=1783143&r2=1783144&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Wed Feb 15
20:27:21 2017
@@ -111,6 +111,7 @@ public class AsyncContextImpl implements
}
}
} finally {
+ context.fireRequestDestroyEvent(request);
clearServletRequestResponse();
context.unbind(Globals.IS_SECURITY_ENABLED, oldCL);
}
Modified: tomcat/trunk/java/org/apache/catalina/core/StandardHostValve.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardHostValve.java?rev=1783144&r1=1783143&r2=1783144&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardHostValve.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardHostValve.java Wed Feb
15 20:27:21 2017
@@ -178,7 +178,7 @@ final class StandardHostValve extends Va
}
}
- if (!request.isAsync() && (!asyncAtStart ||
!response.isErrorReportRequired())) {
+ if (!request.isAsync() && !asyncAtStart) {
context.fireRequestDestroyEvent(request);
}
} finally {
Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1783144&r1=1783143&r2=1783144&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java Wed Feb 15
20:27:21 2017
@@ -80,19 +80,19 @@ public abstract class AbstractProcessor
protected void setErrorState(ErrorState errorState, Throwable t) {
boolean blockIo = this.errorState.isIoAllowed() &&
!errorState.isIoAllowed();
this.errorState = this.errorState.getMostSevere(errorState);
+ if (response.getStatus() < 400) {
+ response.setStatus(500);
+ }
+ 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.
- if (response.getStatus() < 400) {
- response.setStatus(500);
- }
getLog().info(sm.getString("abstractProcessor.nonContainerThreadError"), t);
- // Set the request attribute so that the async onError() event is
- // fired when the error event is processed
- request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, t);
processSocketEvent(SocketEvent.ERROR, true);
}
}
Modified: tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java?rev=1783144&r1=1783143&r2=1783144&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java [UTF-8]
(original)
+++ tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java [UTF-8] Wed Feb
15 20:27:21 2017
@@ -69,15 +69,15 @@ import org.apache.tomcat.util.security.P
* ERROR - Something went wrong.
*
* |-----------------»------|
- * | \|/
/-----------------------------------«------------------------------|
- * |
|----------«-------ERROR----------------------------«-------------------------------|
|
- * | | complete() /|\/|\\
| |
- * | | | | \
| |
- * | | |-----»-------| | \-----------»----------|
| |
- * | | | | |dispatch()
| |
- * | | | | \|/
| |
- * | | | | |--|timeout() |
| |
- * | | | post() | | \|/ | post()
| |
+ * | \|/
/---«-------------------------------«------------------------------|
+ * | |----------«-----E R R O
R--«-----------------------«-------------------------------| |
+ * | | complete() /|\/|\\ \-«--------------------------------«-------|
| |
+ * | | | | \ |
| |
+ * | | |-----»-------| | \-----------»----------| |
| |
+ * | | | | |dispatch() |
| |
+ * | | | | \|/ ^
| |
+ * | | | | |--|timeout() | |
| |
+ * | | | post() | | \|/ | post() |
| |
* | | | |---------- | --»DISPATCHED«---------- |
--------------COMPLETING«-----| | |
* | | | | | /|\/|\ | | | /|\
/|\ | | |
* | | | | |---»- | ---| | |startAsync() | timeout()|--|
| | | |
@@ -390,7 +390,8 @@ public class AsyncStateMachine {
state == AsyncState.DISPATCHED ||
state == AsyncState.TIMING_OUT ||
state == AsyncState.MUST_COMPLETE ||
- state == AsyncState.READ_WRITE_OP) {
+ state == AsyncState.READ_WRITE_OP ||
+ state == AsyncState.COMPLETING) {
clearNonBlockingListeners();
state = AsyncState.ERROR;
} else {
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1783144&r1=1783143&r2=1783144&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Wed Feb 15 20:27:21 2017
@@ -89,6 +89,12 @@
does not include TRACE in the returned Allow header. (markt)
</fix>
<fix>
+ <bug>60718</bug>: Improve error handling for asynchronous processing
and
+ correct a number of cases where the <code>requestDestroyed()</code>
+ event was not being fired and an entry wasn't being made in the access
+ logs. (markt)
+ </fix>
+ <fix>
<bug>60720</bug>: Replace "WWW-Authenticate" literal with static final
AUTH_HEADER_NAME in SpnegoAuthenticator. Patch provided by Michael
Osipov. (violetagg)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
