Issue #13349 - only hard close the websocket connection if abnormal close code #13557
Conversation
…lose code Signed-off-by: Lachlan Roberts <[email protected]>
| else | ||
| { | ||
| connection.cancelDemand(); | ||
| connection.getEndPoint().shutdownOutput(); |
There was a problem hiding this comment.
Now the EndPoint is not closed here.
We cannot rely on the other peer to behave properly.
How do we close the EndPoint now?
…case Signed-off-by: Lachlan Roberts <[email protected]>
Signed-off-by: Lachlan Roberts <[email protected]>
Signed-off-by: Lachlan Roberts <[email protected]>
The OnWebSocketClose notification can be called in two cases, if a close frame is received, or if the connection is notified as closed through FrameHandler.onClosed. The callback should only be succeeded not failed if this is called twice, because this is a normal case. Received a response close frame and OnWebSocketClose is called through JettyWebSocketFrameHandler.onFrame. Then the connection is closed so JettyWebSocketFrameHandler.onClosed is called, but we cannot call OnWebSocketClose again. Signed-off-by: Lachlan Roberts <[email protected]>
…s called Signed-off-by: Lachlan Roberts <[email protected]>
Signed-off-by: Lachlan Roberts <[email protected]>
Signed-off-by: Lachlan Roberts <[email protected]>
|
@sbordet please review this PR again. |
| enum EndPointState | ||
| { | ||
| OPEN, | ||
| ISHUT, | ||
| OSHUT, | ||
| CLOSED | ||
| } |
There was a problem hiding this comment.
I'm very VERY cautious about this!
Firstly it is duplicating state from inside the endpoint, which itself knows if it is ISHUT or OSHUT, and you can query that state with endPoint.isInputShutdown() and endpoint.isOutputShutdown(). Duplicating state is never a good idea and always results in at least small period of incoherent state.
But then I do not think that EndPointState is truly an orthogonal state to WebSocketState. Can the endpoint really be in any of those 4 states whilst the session is in any of its states?
3 participants
Closes #13349
Closes #12235
This change avoids hard closing the websocket connection in cases with a normal close code.
This will avoid a race in HTTP/2 where the RST_STREAM frame is racing against the DATA frame holding the WebSocket close.
See comment #13349 for more details.