Bug 56739 - Error response body generated only occasionally
Summary: Error response body generated only occasionally
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 7.0.55
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-17 21:02 UTC by marko asplund
Modified: 2014-09-24 19:52 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description marko asplund 2014-07-17 21:02:14 UTC
When using the following approach in an asynchronous servlet for reporting an error condition, an error response body is generated only occasionally:

((HttpServletResponse) asyncContext.getResponse()).sendError(statusCode, message);
asyncContext.complete()

Based on a discussion on tomcat-users this should be a valid approach:

http://mail-archives.apache.org/mod_mbox/tomcat-users/201407.mbox/%3c53BD085C.7010205@apache.org%3e
(subject: "Servlet 3.1 asynchronous processing API")

Here's how to reproduce the issue:

marko@ubuntu:~$ ab -v 2 -l -n 100 'http://localhost:8080/servlet3-async/error1?fail=true' |grep ^Content-Length| sort | uniq -c
     95 Content-Length: 0
      5 Content-Length: 1082

The code and more information about the test can be found here:
https://github.com/marko-asplund/servlet3-async

Verified on:
Ubuntu 14.04 / OpenJDK 1.7.0_55
Mac OS X 10.8.5 / Oracle Java 1.7.0_55

The same thing occurs with Tomcat 7.0.54.
Comment 1 Mark Thomas 2014-08-06 13:25:16 UTC
The good news is that I now know what is going on here.

When the request is received it is process on container Thread C. By the time the thread reaches the Servlet, it has passed through Tomcat's request processing pipeline including the ErrorReportValve. If the Servlet's doGet() method, an application thread A is created just before the end of doGet(). Thread C then starts to exit Tomcat's request processing pipeline.

Thread A calls sendError() and complete().

If thread C exits the ErrorReportValve before thread A calls sendError() then no response body will be generated.

If thread A calls sendError() before thread C exits the ErrorReportValve then a response body will be generated.

In short, this comes down to a timing issue between the container and the application thread and the way that Tomcat reports unhandled errors with the ErrorReportValve. It would also be the case the a custom error page (handled by the StandardHostValve) is unlikely to be called in this case.

The bad news is that I don't see an obvious fix. It looks like the error page handling is going to need some refactoring but doing that without breaking the existing API (particularly for the ErrorReportVlave) is going to be tricky.
Comment 2 marko asplund 2014-08-06 19:35:58 UTC
Thanks for the update and investigating this issue!

What's the Tomcat dev team's policy to fixing issues like this?
Is it something that can be addressed in a TC 7.0 / 8.0 maintenance releases?
Comment 3 Mark Thomas 2014-08-06 20:34:49 UTC
Generally we like to close every open bug (not enahncement request) for a branch (e.g. 8.0.x) before we do the next 8.0.x release.

The excpetion is API breaking changes. Tomcat doesn't have a black and white view of what is the public API and what isn't. It is very much a judgement call based on how likely we think it is that someone will have written a custom component that is using the API that needs to change. How we handle these sorts of changes depends on the severity of the bug, the scale of the API change and what those API changes are.

For this particular bug I think I can see a solution but I'm not there yet. There are API changes but they aren't in an area where I'd expect folks to be writing custom components (I don't think I'll need to change the ErrorReportValve for example) so - assuming my solution works - this should be in the next 8.0.x release. It should also make it into the next 7.0.x release.
Comment 4 Mark Thomas 2014-08-07 08:53:39 UTC
This has been fixed in 8.0.x for 8.0.11 onwards.

I'm going to leave it a little while to give folks a chance to review the changes before back-porting it to 7.0.x.
Comment 5 marko asplund 2014-08-07 14:22:37 UTC
That's great news, thanks!

Almost forgot: do you think the "java.lang.IllegalStateException: Calling [asyncTimeout()] is not valid …" issue with TC 8 described on https://github.com/marko-asplund/servlet3-async is related to this?
Or should I report it as a separate issue?
Comment 6 Mark Thomas 2014-08-07 14:36:11 UTC
(In reply to marko asplund from comment #5)
> That's great news, thanks!
> 
> Almost forgot: do you think the "java.lang.IllegalStateException: Calling
> [asyncTimeout()] is not valid …" issue with TC 8 described on
> https://github.com/marko-asplund/servlet3-async is related to this?
> Or should I report it as a separate issue?

Didn't you already report that as Bug 56736 ?
Comment 7 marko asplund 2014-08-07 15:29:27 UTC
yeah, so it seems :-) sorry about the confusion.
Comment 8 Violeta Georgieva 2014-09-24 19:52:10 UTC
This has been back-ported in 7.0.x for 7.0.56 onwards.