There are a couple of problems relating to error handling during exception processing. Basically, Tomcat should ensure that the AsyncListener onError and onComplete methods are correctly called if an exception occurs during async processing in order to allow the listener code to release resources and do clean-up. This does not seem to happen. A sample servlet demonstrating the problem, AsyncDebugListener, is provided at the following location: https://github.com/msnicklous/AsyncDebug Build the AsyncDebugListener module using 'mvn install' and deploy it on Tomcat. The service method and each of the AsyncListener methods logs output to the AsyncListener.log file. You can follow execution by looking at the logs. Assuming Tomcat is installed locally, you can use the following URLs: 1) No error - works correctly: http://localhost:8080/AsyncDebugListener/ltest?reps=3 2) Exception during service method http://localhost:8080/AsyncDebugListener/ltest?err If async processing is started and an exception is thrown before the end of the service method, AsyncListener#onError is not called, however onTimeout followed by onComplete are both called. This is incorrect, as onError followed by onComplete should be called. 3) Exception during processing of AsyncContext#dispatch() target http://localhost:8080/AsyncDebugListener/ltest?reps=3&err Precondition: Async processing is started and the request is dispatched using AsyncContext#dispatch(). The service method returns to the container. If the container performs the resulting async dispatch and an exception is thrown, Tomcat drops the connection to the browser and no AsyncListener method is called. This is incorrect. Tomcat should flush the contents of the buffer to the browser and call the AsyncListener onError followed by the onComplete methods. Thanks for having a look at this!
I've made some changes in 9.0.x to address point 2. The provided test case (thanks for that it makes life a lot easier) now passes. I want to look at point 3 before thinking about back-porting.
Point 3 is fixed as well in 9.0.x for 9.0.0.M5 onwards. I'll look at back-porting this next.
Backports complete. - 8.5.x for 8.5.1 onwards - 8.0.x for 8.0.34 onwards - 7.0.x for 7.0.70 onwards - 6.0.x for 6.0.46 onwards
(In reply to Mark Thomas from comment #3) > - 6.0.x for 6.0.46 onwards Scratch the above one. Tomcat 6 was not affected by this fix. AsyncListener is a feature of Servlet 3.0 API / Tomcat 7 onwards.
Created attachment 33782 [details] Unit test of bug 59219 Unit test patch of the bug 59219 (point 2).
Created attachment 33783 [details] Unit test of bug 59219 This unit test checks 2 and 3 points.
Created attachment 33784 [details] Unit test of bug 59219 Unit test which checks point 2 and 3.
Thanks for the patch. It is being looked at. Currently I'm seeing odd behaviour on Windows so I want to test it on another platform. I do have some basic feedback at this point. Expect some more when once I have test test working: 1. Use separate methods for separate tests. It makes it easier to determine which tests are failing. 2. We aim to keep the code warning free. If you use Eclipse, checkout the configuration settings we use under res/ide-support/eclipse 3. It looks like you should be able to re-use the existing tracking listener.
And some more: 4. The wait loop logic is wrong. - It will always wait 5s even if the output is correct - If output in incorrect it will enter an infinite loop 5. It is clearer to define an explicit Wrapper insatnce and then call setAsyncSupported(true) on that. 6. The test doesn't pass. This actually highlights a bug in the fix (regression in the fix for point 2 when the fix for point 3 was made) which just goes to demonstrate the usefulness of test cases. I have all this fixed locally but I think it would be useful for you to try and fix these issues yourself.
I've fixed the regression. I held back on committing the test case to give you a chance to update your proposed patch.
Created attachment 33796 [details] Unit test of bug 59219 Thank you for your useful feedback and especialy your patience and your time :), i use IntelliJ IDEA Community edition because i'm more familiar with its debugger and its features, i run ant to convert the project to eclipse project then import it inside IDEA as eclipse project (ant buildfile doesn't offer support for IDEA and IDEA supports importing eclipse projects). The test case pass for point 2, but it doesn't for point 3, i noticed that when an exception occurs without a dispatch the onError run once, but if the exception occurs with no matter numbder of dispatch the onError run twice, i think it a bug in the fix as onError should run only once or the same as the dispatch has been called (i didn't read the servlet spec).
Comment on attachment 33796 [details] Unit test of bug 59219 Nice. That is very, very close to what I have. The differences are: - refactoring to remove duplicate code - setting the timeout on the async context to speed up some failure modes - different logic around loop tracking I'll get this committed shortly so you can see the differences. You'll get a credit in the commit message.
Created attachment 33802 [details] Unit test of bug 59219 Inserting timeout. About refactoring, of course there's some duplicate code as the servlets, ServletB can do what ServletA do, but i think like its better, because its more clear and more explicit for point 2, or not?? :)