Close a WebSocket when it is idle

[patcher454]

Related: #5696

Motivation:

WebSocket is mostly a long-running request,
it is not easy to limit when it will end with a request timeout.
It would make more sense to terminate the request when no messages are sent or received for a certain period of time.

The existing idle timeout handler operates at the connection level so a new scheduler that can perform at the WebSocket level is necessary.

Modifications:

• WebSocketServiceChannelHandler
  • Add ServerConfig parameter to the constructor
  • Add a feature closes channels using ScheduledFuture
• Http1RequestDecoder
  • Modify new WebSocketServiceChannelHandler() Parameter in pipeline.replace()

Result:

• Closes Close a WebSocket when it is idle #5696 .
• Set idleTimeoutMillis in ServerConfig to a non-zero value, WebSockets will close after the idle timeout period.

[CLAassistant]

All committers have signed the CLA.

[github-actions]

🔍 Build Scan® (commit: 0b2a046)

Job name	Status	Build Scan®
build-windows-latest-jdk-21	❌ (failure)	https://ge.armeria.dev/s/4fap7l6zhrofi
build-self-hosted-unsafe-jdk-8	❌ (failure)	https://ge.armeria.dev/s/gadklpoujtt34
build-self-hosted-unsafe-jdk-21-snapshot-blockhound	❌ (failure)	https://ge.armeria.dev/s/afsfl4rxm3bk2
build-self-hosted-unsafe-jdk-17-min-java-17-coverage	❌ (failure)	https://ge.armeria.dev/s/olswiuviqrp7q
build-self-hosted-unsafe-jdk-17-min-java-11	❌ (failure)	https://ge.armeria.dev/s/o36zpv2crchui
build-self-hosted-unsafe-jdk-17-leak	❌ (failure)	https://ge.armeria.dev/s/smuti3l4ha2rg
build-self-hosted-unsafe-jdk-11	❌ (failure)	https://ge.armeria.dev/s/ctunwep7trw26
build-macos-12-jdk-21	❌ (failure)	https://ge.armeria.dev/s/3szyrwibczovk

[jrhee17]

The approach looks good overall 👍 Can you fix the failing test?

[jrhee17]

nit; we probably don't want a 10 second test since it will slow down our build.

[jrhee17]

@ikhoon Did you imagine that only HTTP1 needs this kind of behavior when creating the issue? I'm curious if we don't need to consider H2C as well.

[patcher454]

• Thank you for your quick review.
• In case of a failed test, it may be a little late because I'm in the military. But I'll fix it ASAP.
• Below is a summary of why I didn't consider H2C, and what I understood about the WebSocket code.
• Please let me know if there's anything wrong.

Reason

1. In HTTP2 protocol, close the idle channel by the AbstractKeepAliveHandler class, the parent of the HTTP2ServerKeepAliveHandler generated by the HTTP2ServerConnectionHandler

2. HTTP2 idleTimeout features already tested in KeepAliveHandlerTest

3. I modified, the WebSocketServiceChannelHandler, is registered in Channel Pipeline only for HTTP1 protocol

4. AbstractKeepAliveHandler was using idleTimeoutMillis in ServerConfig to close the idle channel, WebSocketServiceChannelHandler also uses idleTimeoutMillis in ServerConfig for the same result

About Web Socket Operation

HTTP/2(H2C)

1. When connected by SessionProtocol.H2C, the Http2ServerConnectionHandler is registered in the channel pipeline as shown in the picture.

2. When the Http2ServerConnectionHandler is created, if the idleTimeoutMillis is not 0, create the Http2ServerKeepAliveHandler as shown in the picture below.

3. In AbstractKeepAliveHandler, parent of the Http2ServerKeepAliveHandler, close the idle channel like the log below.

16:20:27.830 [armeria-common-worker-nio-3-2] DEBUG c.l.a.i.c.AbstractKeepAliveHandler - [id: 0x0af5558b, L:/127.0.0.1:55809 - R:/127.0.0.1:55811] Closing an idle server connection

HTTP/1(H1C)

1. When you connect to SessionProtocol.H1C, decode the request in the Http1RequestDecoder.

2. If the request is WebSocketUpgradeRequest, replace the Http1RequestDecoder with the WebSocketServiceChannelHandler as shown in the picture below.

3. However, there was no idleTimeout-related code in the existing WebSocketServiceChannelHandler

4. So when connected with the HTTP1 protocol, the channel did not close if the idle state continued.

[jrhee17]

In HTTP2 protocol, close the idle channel by the AbstractKeepAliveHandler class, the parent of the HTTP2ServerKeepAliveHandler generated by the HTTP2ServerConnectionHandler

Sure, but I thought Http2ServerKeepAliveHandler also operated on the connection level. I was asking the original issue creator because I was looking at the following line which seemed to imply an idle timeout for requests which h2c doesn't have at the moment.

The existing idle timeout handler operates at the connection level so a new scheduler that can perform at the WebSocket level is necessary.

[trustin]

What do you think about solving this issue in a more generic way? We could add idle timeout settings to any HttpService, like:

• "Abort the stream if a peer doesn't send anything for N milliseconds."
• "Abort the stream if the service doesn't send anything for N milliseconds."
• "Abort the stream if neither the peer nor the service doesn't send anything for N milliseconds."

[patcher454]

I think I was just too focused on shutting down the connection in idle state.
I will solve it at the higher level and request it again.

@jrhee17 @trustin
Thank you for your review.