[grid] Router WebSocket handle dropped close frames, idle disconnects, high-latency proxying#17197
Merged
Merged
Conversation
… high-latency proxying Signed-off-by: Viet Nguyen Duc <[email protected]>
Contributor
Review Summary by QodoImprove Grid Router WebSocket handling with TCP tunneling, keepalive pings, and idle detection
WalkthroughsDescription• Implement direct TCP tunneling for WebSocket connections to eliminate message parsing overhead • Add application-level WebSocket pings every 30s to prevent cloud load balancer idle timeouts • Handle node-initiated close frames and upstream errors with proper client channel closure • Install idle-state detection on tunnel channels to detect silently dropped connections • Add configurable --tcp-tunnel flag to disable direct tunneling for restricted network topologies File Changes1. java/src/org/openqa/selenium/grid/router/ProxyWebsocketsIntoGrid.java
|
Contributor
Code Review by Qodo
1. Tunnel hard idle timeout
|
Contributor
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
AutomatedTester
pushed a commit
that referenced
this pull request
Mar 11, 2026
…, high-latency proxying (#17197) Signed-off-by: Viet Nguyen Duc <[email protected]>
krishnamohan-kothapalli
pushed a commit
to krishnamohan-kothapalli/selenium
that referenced
this pull request
Mar 18, 2026
…, high-latency proxying (SeleniumHQ#17197) Signed-off-by: Viet Nguyen Duc <[email protected]>
This was referenced Apr 9, 2026
shs96c
added a commit
to shs96c/selenium
that referenced
this pull request
May 22, 2026
The Router has had a direct frame-forwarding path between the Netty pipeline and the upstream JDK WebSocket since db9b07a (2026-03-11, "[grid] Router WebSocket handle dropped close frames, idle disconnects, high-latency proxying", SeleniumHQ#17197). Once the client-side handshake completes, an inbound WebSocketFrameProxy forwards each Netty WebSocketFrame straight to the upstream WebSocket, and the outbound DirectForwardingListener writes upstream replies directly to the client channel. Together those removed the per-frame Message allocation and the executor hop in WebSocketMessageHandler on the Router side. The Node still did the full round-trip through MessageInboundConverter, WebSocketMessageHandler, the registered Consumer<Message>, and MessageOutboundConverter in both directions for every frame. Each frame allocated a TextMessage or BinaryMessage and hopped onto the channel executor on delivery. For a busy CDP or VNC session that is measurable allocation and executor-queue pressure on the Node. Apply the same PostUpgradeHook pattern on the Node side: the consumer returned from ProxyNodeWebsockets installs a WebSocketFrameProxy after the handshake so inbound frames forward straight to the browser-side WebSocket, and a DirectForwardingListener writes outbound frames directly to the client channel. Frames received before the handshake are buffered in arrival order and drained on handover, so a frame cannot land in a pipeline that has already had its Message-layer handlers removed. The hardening that the Router-side listener picked up in 8d8cf64 (2026-05-14, "[grid] Close pre-handshake race in WebSocket proxy", SeleniumHQ#17435) is mirrored on the Node listener: the pre-handshake buffer is capped at 128 frames with a 1009 close recorded on overflow; the close code and reason are recorded on pre-handshake close or error so a late onUpgrade can write a clean close frame to the client and tear the channel down rather than leaving it open; and the buffer is released on close so ref-counted frames cannot leak if the handshake never completes. The Node-specific behaviour is preserved: - Session-activity heartbeats (sessionConsumer.accept(sessionId)) fire per frame, both pre- and post-handshake. - The connectionReleased CAS still guards a single node.releaseConnection call across the close and error paths, including the overflow path introduced here. - VNC sessions still install a no-op heartbeat consumer so VNC traffic does not mark the session as recently active. The existing ProxyNodeWebsocketsTest continues to exercise the slot accounting, including the regression from SeleniumHQ#17197 where onError without a follow-on onClose used to leak the slot. New unit tests in NodeDirectForwardingListenerTest pin the per-frame heartbeat, the buffer-then-drain ordering, the surface-and-teardown behaviour on a pre-handshake close, and the overflow path's clean release of the session slot.
shs96c
added a commit
to shs96c/selenium
that referenced
this pull request
May 22, 2026
The Router has had a direct frame-forwarding path between the Netty pipeline and the upstream JDK WebSocket since db9b07a (2026-03-11, "[grid] Router WebSocket handle dropped close frames, idle disconnects, high-latency proxying", SeleniumHQ#17197). Once the client-side handshake completes, an inbound WebSocketFrameProxy forwards each Netty WebSocketFrame straight to the upstream WebSocket, and the outbound DirectForwardingListener writes upstream replies directly to the client channel. Together those removed the per-frame Message allocation and the executor hop in WebSocketMessageHandler on the Router side. The Node still did the full round-trip through MessageInboundConverter, WebSocketMessageHandler, the registered Consumer<Message>, and MessageOutboundConverter in both directions for every frame. Each frame allocated a TextMessage or BinaryMessage and hopped onto the channel executor on delivery. For a busy CDP or VNC session that is measurable allocation and executor-queue pressure on the Node. Apply the same PostUpgradeHook pattern on the Node side: the consumer returned from ProxyNodeWebsockets installs a WebSocketFrameProxy after the handshake so inbound frames forward straight to the browser-side WebSocket, and a DirectForwardingListener writes outbound frames directly to the client channel. Frames received before the handshake are buffered in arrival order and drained on handover, so a frame cannot land in a pipeline that has already had its Message-layer handlers removed. The hardening that the Router-side listener picked up in 8d8cf64 (2026-05-14, "[grid] Close pre-handshake race in WebSocket proxy", SeleniumHQ#17435) is mirrored on the Node listener: the pre-handshake buffer is capped at 128 frames with a 1009 close recorded on overflow; the close code and reason are recorded on pre-handshake close or error so a late onUpgrade can write a clean close frame to the client and tear the channel down rather than leaving it open; and the buffer is released on close so ref-counted frames cannot leak if the handshake never completes. The Node-specific behaviour is preserved: - Session-activity heartbeats (sessionConsumer.accept(sessionId)) fire per frame, both pre- and post-handshake. - The connectionReleased CAS still guards a single node.releaseConnection call across the close and error paths, including the overflow path introduced here. - VNC sessions still install a no-op heartbeat consumer so VNC traffic does not mark the session as recently active. The existing ProxyNodeWebsocketsTest continues to exercise the slot accounting, including the regression from SeleniumHQ#17197 where onError without a follow-on onClose used to leak the slot. New unit tests in NodeDirectForwardingListenerTest pin the per-frame heartbeat, the buffer-then-drain ordering, the surface-and-teardown behaviour on a pre-handshake close, and the overflow path's clean release of the session slot.
shs96c
added a commit
to shs96c/selenium
that referenced
this pull request
May 22, 2026
The Router has had a direct frame-forwarding path between the Netty pipeline and the upstream JDK WebSocket since db9b07a (2026-03-11, "[grid] Router WebSocket handle dropped close frames, idle disconnects, high-latency proxying", SeleniumHQ#17197). Once the client-side handshake completes, an inbound WebSocketFrameProxy forwards each Netty WebSocketFrame straight to the upstream WebSocket, and the outbound DirectForwardingListener writes upstream replies directly to the client channel. Together those removed the per-frame Message allocation and the executor hop in WebSocketMessageHandler on the Router side. The Node still did the full round-trip through MessageInboundConverter, WebSocketMessageHandler, the registered Consumer<Message>, and MessageOutboundConverter in both directions for every frame. Each frame allocated a TextMessage or BinaryMessage and hopped onto the channel executor on delivery. For a busy CDP or VNC session that is measurable allocation and executor-queue pressure on the Node. Apply the same PostUpgradeHook pattern on the Node side: the consumer returned from ProxyNodeWebsockets installs a WebSocketFrameProxy after the handshake so inbound frames forward straight to the browser-side WebSocket, and a DirectForwardingListener writes outbound frames directly to the client channel. Frames received before the handshake are buffered in arrival order and drained on handover, so a frame cannot land in a pipeline that has already had its Message-layer handlers removed. The hardening that the Router-side listener picked up in 8d8cf64 (2026-05-14, "[grid] Close pre-handshake race in WebSocket proxy", SeleniumHQ#17435) is mirrored on the Node listener: the pre-handshake buffer is capped at 128 frames with a 1009 close recorded on overflow; the close code and reason are recorded on pre-handshake close or error so a late onUpgrade can write a clean close frame to the client and tear the channel down rather than leaving it open; and the buffer is released on close so ref-counted frames cannot leak if the handshake never completes. Close-frame reasons coming from the upstream are now truncated to the 123-byte UTF-8 cap that RFC 6455 §5.5.1 imposes. The truncation uses a CharsetEncoder writing into a 120-byte buffer so it stops at a clean character boundary on overflow — a naive byte-truncate-then- decode could split a multi-byte sequence, produce a U+FFFD replacement on decode, and re-encode back over 123 bytes, breaking the close frame. The helper lives as a public static on WebSocketFrameProxy because both DirectForwardingListener classes already depend on that class. The Router-side listener that landed in SeleniumHQ#17435 had the same unchecked path; apply the helper there too so both proxies share the same safe behaviour. The Node-specific behaviour is preserved: - Session-activity heartbeats (sessionConsumer.accept(sessionId)) fire per frame, both pre- and post-handshake. - The connectionReleased CAS still guards a single node.releaseConnection call across the close and error paths, including the overflow path introduced here. - VNC sessions still install a no-op heartbeat consumer so VNC traffic does not mark the session as recently active. The existing ProxyNodeWebsocketsTest continues to exercise the slot accounting, including the regression from SeleniumHQ#17197 where onError without a follow-on onClose used to leak the slot. New unit tests in NodeDirectForwardingListenerTest pin the per-frame heartbeat, the buffer-then-drain ordering, the surface-and-teardown behaviour on a pre-handshake close, the overflow path's clean release of the session slot, and the safe truncation of an overlong upstream close reason that contains multi-byte UTF-8 characters. The shared helper has a focused unit test alongside it in WebSocketFrameProxyTest.
shs96c
added a commit
to shs96c/selenium
that referenced
this pull request
May 27, 2026
The Router has had a direct frame-forwarding path between the Netty pipeline and the upstream JDK WebSocket since db9b07a (2026-03-11, "[grid] Router WebSocket handle dropped close frames, idle disconnects, high-latency proxying", SeleniumHQ#17197). Once the client-side handshake completes, an inbound WebSocketFrameProxy forwards each Netty WebSocketFrame straight to the upstream WebSocket, and the outbound DirectForwardingListener writes upstream replies directly to the client channel. Together those removed the per-frame Message allocation and the executor hop in WebSocketMessageHandler on the Router side. The Node still did the full round-trip through MessageInboundConverter, WebSocketMessageHandler, the registered Consumer<Message>, and MessageOutboundConverter in both directions for every frame. Each frame allocated a TextMessage or BinaryMessage and hopped onto the channel executor on delivery. For a busy CDP or VNC session that is measurable allocation and executor-queue pressure on the Node. Apply the same PostUpgradeHook pattern on the Node side: the consumer returned from ProxyNodeWebsockets installs a WebSocketFrameProxy after the handshake so inbound frames forward straight to the browser-side WebSocket, and a DirectForwardingListener writes outbound frames directly to the client channel. Frames received before the handshake are buffered in arrival order and drained on handover, so a frame cannot land in a pipeline that has already had its Message-layer handlers removed. The hardening that the Router-side listener picked up in 8d8cf64 (2026-05-14, "[grid] Close pre-handshake race in WebSocket proxy", SeleniumHQ#17435) is mirrored on the Node listener: the pre-handshake buffer is capped at 128 frames with a 1009 close recorded on overflow; the close code and reason are recorded on pre-handshake close or error so a late onUpgrade can write a clean close frame to the client and tear the channel down rather than leaving it open; and the buffer is released on close so ref-counted frames cannot leak if the handshake never completes. Close-frame reasons coming from the upstream are now truncated to the 123-byte UTF-8 cap that RFC 6455 §5.5.1 imposes. The truncation uses a CharsetEncoder writing into a 120-byte buffer so it stops at a clean character boundary on overflow — a naive byte-truncate-then- decode could split a multi-byte sequence, produce a U+FFFD replacement on decode, and re-encode back over 123 bytes, breaking the close frame. The helper lives as a public static on WebSocketFrameProxy because both DirectForwardingListener classes already depend on that class. The Router-side listener that landed in SeleniumHQ#17435 had the same unchecked path; apply the helper there too so both proxies share the same safe behaviour. The Node-specific behaviour is preserved: - Session-activity heartbeats (sessionConsumer.accept(sessionId)) fire per frame, both pre- and post-handshake. - The connectionReleased CAS still guards a single node.releaseConnection call across the close and error paths, including the overflow path introduced here. - VNC sessions still install a no-op heartbeat consumer so VNC traffic does not mark the session as recently active. The existing ProxyNodeWebsocketsTest continues to exercise the slot accounting, including the regression from SeleniumHQ#17197 where onError without a follow-on onClose used to leak the slot. New unit tests in NodeDirectForwardingListenerTest pin the per-frame heartbeat, the buffer-then-drain ordering, the surface-and-teardown behaviour on a pre-handshake close, the overflow path's clean release of the session slot, and the safe truncation of an overlong upstream close reason that contains multi-byte UTF-8 characters. The shared helper has a focused unit test alongside it in WebSocketFrameProxyTest.
shs96c
added a commit
to shs96c/selenium
that referenced
this pull request
May 27, 2026
The Router has had a direct frame-forwarding path between the Netty pipeline and the upstream JDK WebSocket since db9b07a (2026-03-11, "[grid] Router WebSocket handle dropped close frames, idle disconnects, high-latency proxying", SeleniumHQ#17197). Once the client-side handshake completes, an inbound WebSocketFrameProxy forwards each Netty WebSocketFrame straight to the upstream WebSocket, and the outbound DirectForwardingListener writes upstream replies directly to the client channel. Together those removed the per-frame Message allocation and the executor hop in WebSocketMessageHandler on the Router side. The Node still did the full round-trip through MessageInboundConverter, WebSocketMessageHandler, the registered Consumer<Message>, and MessageOutboundConverter in both directions for every frame. Each frame allocated a TextMessage or BinaryMessage and hopped onto the channel executor on delivery. For a busy CDP or VNC session that is measurable allocation and executor-queue pressure on the Node. Apply the same PostUpgradeHook pattern on the Node side: the consumer returned from ProxyNodeWebsockets installs a WebSocketFrameProxy after the handshake so inbound frames forward straight to the browser-side WebSocket, and a DirectForwardingListener writes outbound frames directly to the client channel. Frames received before the handshake are buffered in arrival order and drained on handover, so a frame cannot land in a pipeline that has already had its Message-layer handlers removed. The hardening that the Router-side listener picked up in 8d8cf64 (2026-05-14, "[grid] Close pre-handshake race in WebSocket proxy", SeleniumHQ#17435) is mirrored on the Node listener: the pre-handshake buffer is capped at 128 frames with a 1009 close recorded on overflow; the close code and reason are recorded on pre-handshake close or error so a late onUpgrade can write a clean close frame to the client and tear the channel down rather than leaving it open; and the buffer is released on close so ref-counted frames cannot leak if the handshake never completes. Close-frame reasons coming from the upstream are now truncated to the 123-byte UTF-8 cap that RFC 6455 §5.5.1 imposes. The truncation uses a CharsetEncoder writing into a 120-byte buffer so it stops at a clean character boundary on overflow — a naive byte-truncate-then- decode could split a multi-byte sequence, produce a U+FFFD replacement on decode, and re-encode back over 123 bytes, breaking the close frame. The helper lives as a public static on WebSocketFrameProxy because both DirectForwardingListener classes already depend on that class. The Router-side listener that landed in SeleniumHQ#17435 had the same unchecked path; apply the helper there too so both proxies share the same safe behaviour. The Node-specific behaviour is preserved: - Session-activity heartbeats (sessionConsumer.accept(sessionId)) fire per frame, both pre- and post-handshake. - The connectionReleased CAS still guards a single node.releaseConnection call across the close and error paths, including the overflow path introduced here. - VNC sessions still install a no-op heartbeat consumer so VNC traffic does not mark the session as recently active. The existing ProxyNodeWebsocketsTest continues to exercise the slot accounting, including the regression from SeleniumHQ#17197 where onError without a follow-on onClose used to leak the slot. New unit tests in NodeDirectForwardingListenerTest pin the per-frame heartbeat, the buffer-then-drain ordering, the surface-and-teardown behaviour on a pre-handshake close, the overflow path's clean release of the session slot, and the safe truncation of an overlong upstream close reason that contains multi-byte UTF-8 characters. The shared helper has a focused unit test alongside it in WebSocketFrameProxyTest.
shs96c
added a commit
to shs96c/selenium
that referenced
this pull request
May 27, 2026
The read-idle close that TcpUpgradeTunnelHandler installs after the tunnel is established (introduced by db9b07a, 2026-03-11, "[grid] Router WebSocket handle dropped close frames, idle disconnects, high-latency proxying", SeleniumHQ#17197) tears down both tunnel channels when no bytes have been received for 120 seconds. That is the right behaviour when an intermediate load balancer has silently dropped the TCP connection, but it interacts badly with the backpressure mirroring added in 5cf2e2a (2026-05-26, "[grid] Apply TCP backpressure across the WebSocket tunnel handler", SeleniumHQ#17543): when the peer's outbound buffer crosses its high-water mark, TcpTunnelHandler sets autoRead=false on this side, no channelRead events fire, and the read-idle timer reaches its threshold even though the stall is by design. A sustained slow consumer for more than two minutes will therefore have the tunnel torn down underneath it. Gate the close in IdleCloseHandler.userEventTriggered on the channel's autoRead flag: while reads are paused by backpressure, log at FINE and ignore the event. As soon as the peer drains and TcpTunnelHandler restores autoRead=true the read-idle clock starts again from a fresh read, so a legitimately dropped connection is still detected within the same window once traffic resumes. IdleCloseHandler is promoted from a private nested class to package-private so a focused EmbeddedChannel unit test can exercise both branches: the close-both-channels behaviour on a normal idle event, and the ignored-while-paused behaviour with autoRead=false.
shs96c
added a commit
that referenced
this pull request
May 27, 2026
The Router has had a direct frame-forwarding path between the Netty pipeline and the upstream JDK WebSocket since db9b07a (2026-03-11, "[grid] Router WebSocket handle dropped close frames, idle disconnects, high-latency proxying", #17197). Once the client-side handshake completes, an inbound WebSocketFrameProxy forwards each Netty WebSocketFrame straight to the upstream WebSocket, and the outbound DirectForwardingListener writes upstream replies directly to the client channel. Together those removed the per-frame Message allocation and the executor hop in WebSocketMessageHandler on the Router side. The Node still did the full round-trip through MessageInboundConverter, WebSocketMessageHandler, the registered Consumer<Message>, and MessageOutboundConverter in both directions for every frame. Each frame allocated a TextMessage or BinaryMessage and hopped onto the channel executor on delivery. For a busy CDP or VNC session that is measurable allocation and executor-queue pressure on the Node. Apply the same PostUpgradeHook pattern on the Node side: the consumer returned from ProxyNodeWebsockets installs a WebSocketFrameProxy after the handshake so inbound frames forward straight to the browser-side WebSocket, and a DirectForwardingListener writes outbound frames directly to the client channel. Frames received before the handshake are buffered in arrival order and drained on handover, so a frame cannot land in a pipeline that has already had its Message-layer handlers removed. The hardening that the Router-side listener picked up in 8d8cf64 (2026-05-14, "[grid] Close pre-handshake race in WebSocket proxy", #17435) is mirrored on the Node listener: the pre-handshake buffer is capped at 128 frames with a 1009 close recorded on overflow; the close code and reason are recorded on pre-handshake close or error so a late onUpgrade can write a clean close frame to the client and tear the channel down rather than leaving it open; and the buffer is released on close so ref-counted frames cannot leak if the handshake never completes. Close-frame reasons coming from the upstream are now truncated to the 123-byte UTF-8 cap that RFC 6455 §5.5.1 imposes. The truncation uses a CharsetEncoder writing into a 120-byte buffer so it stops at a clean character boundary on overflow — a naive byte-truncate-then- decode could split a multi-byte sequence, produce a U+FFFD replacement on decode, and re-encode back over 123 bytes, breaking the close frame. The helper lives as a public static on WebSocketFrameProxy because both DirectForwardingListener classes already depend on that class. The Router-side listener that landed in #17435 had the same unchecked path; apply the helper there too so both proxies share the same safe behaviour. The Node-specific behaviour is preserved: - Session-activity heartbeats (sessionConsumer.accept(sessionId)) fire per frame, both pre- and post-handshake. - The connectionReleased CAS still guards a single node.releaseConnection call across the close and error paths, including the overflow path introduced here. - VNC sessions still install a no-op heartbeat consumer so VNC traffic does not mark the session as recently active. The existing ProxyNodeWebsocketsTest continues to exercise the slot accounting, including the regression from #17197 where onError without a follow-on onClose used to leak the slot. New unit tests in NodeDirectForwardingListenerTest pin the per-frame heartbeat, the buffer-then-drain ordering, the surface-and-teardown behaviour on a pre-handshake close, the overflow path's clean release of the session slot, and the safe truncation of an overlong upstream close reason that contains multi-byte UTF-8 characters. The shared helper has a focused unit test alongside it in WebSocketFrameProxyTest.
shs96c
added a commit
to shs96c/selenium
that referenced
this pull request
May 27, 2026
The read-idle close that TcpUpgradeTunnelHandler installs after the tunnel is established (introduced by db9b07a, 2026-03-11, "[grid] Router WebSocket handle dropped close frames, idle disconnects, high-latency proxying", SeleniumHQ#17197) tears down both tunnel channels when no bytes have been received for 120 seconds. That is the right behaviour when an intermediate load balancer has silently dropped the TCP connection, but it interacts badly with the backpressure mirroring added in 5cf2e2a (2026-05-26, "[grid] Apply TCP backpressure across the WebSocket tunnel handler", SeleniumHQ#17543): when the peer's outbound buffer crosses its high-water mark, TcpTunnelHandler sets autoRead=false on this side, no channelRead events fire, and the read-idle timer reaches its threshold even though the stall is by design. A sustained slow consumer for more than two minutes will therefore have the tunnel torn down underneath it. Gate the close in IdleCloseHandler.userEventTriggered on the channel's autoRead flag: while reads are paused by backpressure, log at FINE and ignore the event. As soon as the peer drains and TcpTunnelHandler restores autoRead=true the read-idle clock starts again from a fresh read, so a legitimately dropped connection is still detected within the same window once traffic resumes. IdleCloseHandler is promoted from a private nested class to package-private so a focused EmbeddedChannel unit test can exercise both branches: the close-both-channels behaviour on a normal idle event, and the ignored-while-paused behaviour with autoRead=false.
shs96c
added a commit
to shs96c/selenium
that referenced
this pull request
May 28, 2026
The read-idle close that TcpUpgradeTunnelHandler installs after the tunnel is established (introduced by db9b07a, 2026-03-11, "[grid] Router WebSocket handle dropped close frames, idle disconnects, high-latency proxying", SeleniumHQ#17197) tears down both tunnel channels when no bytes have been received for 120 seconds. That is the right behaviour when an intermediate load balancer has silently dropped the TCP connection, but it interacts badly with the backpressure mirroring added in 5cf2e2a (2026-05-26, "[grid] Apply TCP backpressure across the WebSocket tunnel handler", SeleniumHQ#17543): when the peer's outbound buffer crosses its high-water mark, TcpTunnelHandler sets autoRead=false on this side, no channelRead events fire, and the read-idle timer reaches its threshold even though the stall is by design. A sustained slow consumer for more than two minutes will therefore have the tunnel torn down underneath it. Gate the close in IdleCloseHandler.userEventTriggered on the channel's autoRead flag: while reads are paused by backpressure, log at FINE and ignore the event. As soon as the peer drains and TcpTunnelHandler restores autoRead=true the read-idle clock starts again from a fresh read, so a legitimately dropped connection is still detected within the same window once traffic resumes. IdleCloseHandler is promoted from a private nested class to package-private so a focused EmbeddedChannel unit test can exercise both branches: the close-both-channels behaviour on a normal idle event, and the ignored-while-paused behaviour with autoRead=false.
shs96c
added a commit
that referenced
this pull request
May 28, 2026
) The read-idle close that TcpUpgradeTunnelHandler installs after the tunnel is established (introduced by db9b07a, 2026-03-11, "[grid] Router WebSocket handle dropped close frames, idle disconnects, high-latency proxying", #17197) tears down both tunnel channels when no bytes have been received for 120 seconds. That is the right behaviour when an intermediate load balancer has silently dropped the TCP connection, but it interacts badly with the backpressure mirroring added in 5cf2e2a (2026-05-26, "[grid] Apply TCP backpressure across the WebSocket tunnel handler", #17543): when the peer's outbound buffer crosses its high-water mark, TcpTunnelHandler sets autoRead=false on this side, no channelRead events fire, and the read-idle timer reaches its threshold even though the stall is by design. A sustained slow consumer for more than two minutes will therefore have the tunnel torn down underneath it. Gate the close in IdleCloseHandler.userEventTriggered on the channel's autoRead flag: while reads are paused by backpressure, log at FINE and ignore the event. As soon as the peer drains and TcpTunnelHandler restores autoRead=true the read-idle clock starts again from a fresh read, so a legitimately dropped connection is still detected within the same window once traffic resumes. IdleCloseHandler is promoted from a private nested class to package-private so a focused EmbeddedChannel unit test can exercise both branches: the close-both-channels behaviour on a normal idle event, and the ignored-while-paused behaviour with autoRead=false.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Related Issues
💥 What does this PR do?
Improve three user-visible problems with WebSocket connections (BiDi, CDP) routed through Selenium Grid:
BiDi/CDP sessions hang after the browser crashes or the Node kills the session. When the Node closed the WebSocket, the client was never notified — it would sit open until the next keepalive cycle (up to 30 s) rather than receiving a clean close.
Cloud load balancers (AWS ALB, GCP, k8s ingress-nginx) silently drop idle WebSocket connections mid-session. These LBs have a 60 s idle timeout and ignore OS-level TCP keepalives. Long-running Playwright or BiDi tests that go quiet for more than 60 s would have their connection dropped without any signal. Fixed by sending application-level WebSocket pings every 30 s.
Every BiDi/CDP message was parsed and repackaged by the Router even though the Router has nothing to do with the message content. This added latency and CPU overhead proportional to message rate. The Router now bridges the connection at the TCP level, removing itself from the data path entirely after the initial handshake. Falls back to the old message-proxying path for network topologies where a direct TCP connection to the Node is not possible (e.g. Kubernetes port-forward).
🔧 Implementation Notes
💡 Additional Considerations
🔄 Types of changes