Issue #3806 async sendError

[gregw]

Issue #3806 async sendError

Avoid using isHandled as a test withing sendError as this can be
called asynchronously and is in a race with the normal dispatch of the
request, which could also be setting handled status.

Signed-off-by: Greg Wilkins [email protected]

[gregw]

@sbordet @janbartel This PR now fixes both problems: using isHandled and dispatching from within the ErrorHandler.

[sbordet]

@gregw I am not sure I understand the fix. Would be great if you add a comment describing the design of your solution, and how it fixes the problem.

[gregw]

See discussion starting here.

I think we need a more involved solution. Both sync and async error dispatches need to be delayed until either sync returns or async completes... only then can an error dispatch be done - ie never from within sendError.
We will need a different resetBuffer (or one with an argument) that leaves the outputStream closed, to be reopened only just before the error dispatch.

[gregw]

Extending the delayed error dispatch approach to handle async sendError calls is promising, but it still needs careful though.

Consider this sequence:

• Thread A Normal servlet dispatch
• Thread A startAsync called
• Thread A returns from servlet dispatch
• Thread B sendError called (state set, output temporarily closed)
• Thread B complete called (state update - no further action at this time)
• Thread B ERROR dispatch generates response and returns
• Thread B calls onComplete

This looks OK and could also play out as:

• Thread A Normal servlet dispatch
• Thread A startAsync called
• Thread B sendError called (state set, output temporarily closed)
• Thread B complete called (state update - no further action at this time)
• Thread A returns from servlet dispatch
• Thread A ERROR dispatch generates response and returns
• Thread A calls onComplete

But what if ERROR dispatch is itself async?

• Thread A Normal servlet dispatch
• Thread A startAsync called
• Thread A returns from servlet dispatch
• Thread B sendError called (state set, output temporarily closed)
• Thread B complete called (state update - no further action at this time)
• Thread B ERROR dispatch
• Thread B calls startAsync and returns
• Thread C generates error response
• Thread C calls complete
• Thread C calls onComplete

OK as well I think.... and with async Dispatch:

• Thread A Normal servlet dispatch
• Thread A startAsync called
• Thread A returns from servlet dispatch
• Thread B sendError called (state set, output temporarily closed)
• Thread B complete called (state update - no further action at this time)
• Thread B ERROR dispatch
• Thread B calls startAsync and returns
• Thread C calls AsyncContext.dispatch
• Thread C(or D) ASYNC dispatch
• Thread C(or D) generates response and returns
• Thread C(or D) calls onComplete

OK still. I think this can work.

[joakime]

This PR somehow broke the HTTP/2 layer in this branch?

[gregw]

See discussion in jakartaee/servlet#256

[gregw]

closing in favour of #3912