[#118] Call adapter.close() from Grizzly WebSocket close callbacks#120
Merged
vharseko merged 1 commit intoJul 21, 2026
Merged
Conversation
…t close callbacks The Grizzly server endpoint (OpenICFWebSocketApplication.OpenICFWebSocket) and the client endpoint (ClientRemoteConnectorInfoManager.ICFWebSocket) drained their OperationMessageListeners on close but never called adapter.close(), so the closeListener registered by WebSocketConnectionGroup.handshake() never fired and every reconnect of the same session left a stale holder in the group: unbounded growth of webSockets, dead holders probed by the trySendMessage round-robin, and a half-closed (CLOSING) Grizzly socket still reported operational. Mirror the Jetty endpoint (OpenIdentityPlatform#115): fire the holder's close listeners from the transport close callback, in a finally so a throwing listener cannot skip the cleanup. Duplicate close deliveries stay harmless: both listener queues drain via poll() and SimpleWebSocket.close() is a no-op once the state has left CONNECTED. - ReconnectGroupTest (server): group-level regression with a stubbed ProtocolHandler; fails without the fix because the group keeps the stale holder and even reports it operational in the CLOSING state. - ClientReconnectGroupTest (client): real client/server pair; closes one connection at the transport level and waits for the group to return to exactly the permitted holder count after reconnect - times out without the fix.
maximthomas
approved these changes
Jul 21, 2026
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
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.
Fixes #118. Follow-up to #112 / #115.
Both Grizzly transports drained their
OperationMessageListeners on close but never calledadapter.close(), so thecloseListenerregistered byWebSocketConnectionGroup.handshake()never fired and every reconnect of the same session left a stale holder in the group:webSocketsfor a long-lived session (each stale pair pins the whole endpoint, itsProtocolHandlerand the GrizzlyConnection);trySendMessageround-robin on every send;onClosethe socket sits inCLOSING, whichisConnected()still counts as connected, so the group even reports the half-closed holder as operational.The existing mitigations never catch this: the server health checker only removes a whole group after 12 min of inactivity while a reconnecting session keeps its group operational forever, and the client heartbeat cannot close the stale holder because
onClosehas already removed it fromprivateConnections.Fix
Mirror the Jetty endpoint (#115): fire the holder's close listeners from the transport close callback, in a
finallyso a throwing listener cannot skip the cleanup.OpenICFWebSocketApplication.OpenICFWebSocket.onClose(connector-server-grizzly)ICFWebSocket.onCloseinClientRemoteConnectorInfoManager(connector-framework-server)Duplicate close deliveries (Grizzly emits
onCloseonce per direction of the close handshake) stay harmless: both listener queues drain viapoll()andSimpleWebSocket.close()is a no-op once the state has leftCONNECTED.Tests
ReconnectGroupTest(server, unit): group-level regression mirroring the JettyReconnectSendTest.testGroupDropsHolderOnClose— real endpoint with a stubbedProtocolHandler(a real one would try to write the close frame to the wire and throw on the null connection), two connect/handshake/close cycles plus duplicate-close suppression. Without the fix it fails on "the group must drop the holder of a closed connection".ClientReconnectGroupTest(client, integration):ICFWebSocketis a private inner class with transport-heavy dependencies, so a real client/server pair closes one connection at the transport level and waits for the group to return to exactly the permitted holder count after reconnect. Without the fix it times out on cycle 1.Verified both ways: each test fails with only its side of the fix reverted; with the fix the full suites pass (
connector-server-grizzly34/34,connector-framework-server29/29).