Better protect against concurrent error handling for async requests #32340
Description
Section 3.2.2.4 of the Servlet spec indicates the request and response objects are not guaranteed to be thread safe, and the application must ensure that access to the request and response objects are thread safe. This is reasonably straight forward for applications to handle, if they even write concurrently, which may not be the case. However, the Servlet container may also trigger an AsyncListener#onError
notification at any time as a result of network issues, which Spring MVC delegates to CallableProcessingInterceptor
s and DeferredProcessingInterceptor
s before performing an ASYNC dispatch for final handling of the error.
The onError
comes on a Servlet container thread and may compete with the application write that would also fail. Currently, WebAsyncManager
is not sufficiently well protected. For example, the failed write may be first to set the concurrentResult, and soon after perform an ASYNC dispatch. Meanwhile onError
may return quickly, noticing the result is set and not taking any action in which case the Servlet container performs an ERROR dispatch that would race with the ASYNC dispatch. Another possibility is that onError
is first to set the concurrentResult, which is later cleared during handling in the resulting dispatch, and that could trigger an additional ASYNC dispatch if the write fails after that.
The recently reported #32042 is related to such a race. The fix prevented an ASYNC dispatch if the error implies a disconnected client. However, that depends on the exception being recognized as such (also not wrapped by the application), and also precludes any ASYNC dispatch for final error handling where at least one should go through. We need a more conclusive fix for this.
In addition to that, the spec requires the request and response objects to only be accessed within the object’s life cycle. So as part of this work, we should also ensure compliance with this rule by wrapping the ServletOutputStream
to reject further use, rather than expect all applications to synchronize with onError
notifications in order to be compliant with the rule.