| Summary: | CLOSE_CONNECTION_NOW is done when servlet/filter makes a response.flush(); and fails in HTTP/2 | ||
|---|---|---|---|
| Product: | Tomcat 9 | Reporter: | Alejandro Anadon <aag> |
| Component: | Connectors | Assignee: | Tomcat Developers Mailing List <dev> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | ||
| Priority: | P2 | ||
| Version: | 9.0.31 | ||
| Target Milestone: | ----- | ||
| Hardware: | PC | ||
| OS: | All | ||
I don't see any reason why not to do this (although I want to think this through a little more before committing a fix). It also looks like COMMIT needs the same treatment. Fixed in: - master for 10.0.0-M2 onwards - 9.0.x for 9.0.32 onwards - 8.5.x for 8.5.52 onwards |
When HTTP/2 is configured and a servlet/response makes a "response.flushBuffer()" (response is a ServletResponse object), sometimes it closes all channels of the connections. In the source code of org.apache.coyote.AbstractProcessor.java, 'public final void action(ActionCode actionCode, Object param)' method, I can see "CLOSE" case: --- case CLOSE: { action(ActionCode.COMMIT, null); try { finishResponse(); } catch (CloseNowException cne) { setErrorState(ErrorState.CLOSE_NOW, cne); } catch (IOException e) { setErrorState(ErrorState.CLOSE_CONNECTION_NOW, e); } break; } --- It is easy to see that with "finishResponse()", if it throws an exception, it tries first to capture "CloseNowException". If it is the case, then it "setErrorState(ErrorState.CLOSE_NOW, cne);"; so it close only the channel; not the whole connection. But this behavior is not the same with "case CLIENT_FLUSH:". If " flush();" throws an exception, it closes the whole connection with " setErrorState(ErrorState.CLOSE_CONNECTION_NOW, e);". My suggestion is to try catch first 'CloseNowException' (as in CLOSE case); and if it is catched, then close only the channel, not the whole connection. So the code could be something like this: ----- case CLIENT_FLUSH: { action(ActionCode.COMMIT, null); try { flush(); } catch (CloseNowException cne) { setErrorState(ErrorState.CLOSE_NOW, cne); response.setErrorException(cne); //I am not sure about this line } catch (IOException e) { setErrorState(ErrorState.CLOSE_CONNECTION_NOW, e); response.setErrorException(e); } break; } ----- Then, if an error is producced, it lets other chnnels to completed. I implemented it in my enviroment, and it works; but I do know know if it "breaks" some kind of standard or RFC or what else.