feature: WebSocket ping/pong heartbeat — JVM client (PR 4)#587
Conversation
The last backend in the heartbeat series. Wires the pingIntervalMillis param (added to WebSocketClient.connect in #584) into the JVM java.net.http client. - JavaWebSocketListener drives the shared WebSocketHeartbeat from a shared daemon ScheduledExecutorService (one for all client connections): sendPing on an idle tick, close(1011) on an unanswered ping. Resets on any inbound frame (onText/onBinary/onPong). Cancels the scheduled future in notifyClose. - JavaWebSocketContext.sendPing enqueues WebSocket.sendPing through the existing serialized send chain. The JS client remains the only backend without a heartbeat (the browser/Node global WebSocket can't send protocol pings) — documented on the connect API. Test: a Netty WebSocketClientTest case — the client pings every 150ms, the server auto-pongs, the live-but-idle connection is not reaped. uniJVM/test 1640, netty/test 38 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a client-side heartbeat mechanism for Java WebSocket connections to keep idle connections alive using ping/pong frames. It introduces a daemon scheduler to manage heartbeat ticks, records activity on incoming messages, and adds a test to verify the behavior. The review feedback highlights two important reliability issues: a potential race condition between scheduling and cancelling the heartbeat task that could lead to resource leaks, and the risk of silent cancellation of the heartbeat task if an exception is thrown during execution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request implements a client heartbeat mechanism for JavaWebSocketClient to keep idle connections alive using ping/pong frames, driven by a daemon scheduler. The feedback highlights critical concurrency and robustness improvements: synchronizing the scheduling and cancellation of the heartbeat task to prevent a potential race condition and task leak, wrapping the heartbeat tick execution in a try-catch block to avoid silent suppression of the scheduler on exceptions, and overriding onPing to ensure inbound ping frames also reset the idle timer.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- Close the onOpen/notifyClose schedule-vs-cancel race: after assigning heartbeatFuture, re-check `closed` and cancel if notifyClose ran during scheduling (it would have seen a null future and couldn't cancel). - Wrap the scheduled tick in try/catch: scheduleAtFixedRate suppresses all future runs once a task throws, which would silently kill the heartbeat. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Why
PR 4 (final) of the WebSocket heartbeat series (after #584 shared core + Native, #585 Node server, #586 Netty server). Wires the heartbeat into the JVM
java.net.httpclient — the last backend that can do it.What
JavaWebSocketListenerdrives the sharedWebSocketHeartbeatfrom a shared daemonScheduledExecutorService(one for all client connections):sendPingon an idle tick,close(1011, "ping timeout")on an unanswered ping. Resets on any inbound frame (onText/onBinary/onPong); cancels the scheduled future innotifyClose.JavaWebSocketContext.sendPingenqueuesWebSocket.sendPingthrough the existing serialized send chain (the JDK forbids overlapping sends).pingIntervalMillisparam added toWebSocketClient.connectin feature: WebSocket ping/pong heartbeat — shared core + Native server & client #584 (previously accept-and-ignored on the JVM client).The JS client stays the only backend without a heartbeat — the browser/Node global
WebSocketcan't send protocol pings from JS — documented on theconnectAPI.Testing
A Netty
WebSocketClientTestcase: the client pings every 150 ms, the server auto-pongs, and the live-but-idle connection is not reaped.uniJVM/test: 1640,netty/test: 38 pass.Heartbeat series complete
WebSocketHeartbeat+ Native server & client🤖 Generated with Claude Code