Repository navigation
Conversation
WebSocketClient created its own Netty event loop group in every instance, next to the one of the core HTTP client connector, with a thread count that could not be configured. - Add NettyClientHelper.getEventLoopGroup(), an engine extension point exposing the client connector's event loop group. - Add WebSocketClient.setClient(Client): a borrowed, started Netty HTTP client whose event loop is shared. Without it, the client still creates its own threads. - Track the WebSocket connections in a channel group, so stopping the client closes its sessions without shutting down a shared event loop. - Store the helper in the "org.restlet.engine.helper" context attribute in Client.setContext(), as the constructors do. Clients added with Component.getClients().add(Protocol) get their context this way, and did not expose their helper. Fixes #1567 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Jerome Louvel <374450+jlouvel@users.noreply.github.com>
WebSocketClient.setClient found the borrowed client's NettyClientHelper through the "org.restlet.engine.helper" context attribute, so a client created with new Client(Protocol.HTTP), which has no context, couldn't be borrowed. Client.getHelper() is now public, as an engine extension point, and the WebSocket client uses it. The context requirement is removed from the setClient Javadoc and the error message. Fixes #1599. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Jerome Louvel <374450+jlouvel@users.noreply.github.com>
|
jlouvel
force-pushed
the
fix/websocket-client-shared-event-loop
branch
from
October 31, 2026 07:17
c24fe96 to
eec9acc
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A critical connect/stop race allows a late-registered channel to remain open on a borrowed event loop, leaving a WebSocket session active after the client stops.
1 open issue
What changed in this PR
This PR lets WebSocketClient borrow a started Netty HTTP client without requiring a context, addressing #1599. It builds on the shared-event-loop support from #1568.
Changes:
- Exposes
Client.getHelper()and uses it to access the borrowed client's connector directly. - Adds shared event-loop ownership and WebSocket session cleanup.
- Adds lifecycle and context-free borrowing tests, plus changelog entries.
| File | Description |
|---|---|
org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java |
Exposes the connector's event loop for extensions. |
org.restlet/src/main/java/org/restlet/Client.java |
Makes helper access public and updates context registration. |
org.restlet.ext.websocket/src/test/java/org/restlet/ext/websocket/WebSocketSharedEventLoopTestCase.java |
Tests borrowing, lifecycle behavior, and unsuitable clients. |
org.restlet.ext.websocket/src/main/java/org/restlet/ext/websocket/WebSocketClient.java |
Supports borrowing through direct helper access. |
org.restlet.ext.websocket/src/main/java/org/restlet/ext/websocket/internal/NettyWebSocketConnector.java |
Tracks connections and distinguishes owned from borrowed event loops. |
changes.md |
Documents borrowing support and core extension points. |
Review effort: Balanced
|
|
||
| /** The open connections, closed with the connector. */ | ||
| private final ChannelGroup channels = | ||
| new DefaultChannelGroup("restlet-websocket-client", GlobalEventExecutor.INSTANCE); |
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 #1599.
Summary
WebSocketClient.setClientfound the borrowed client'sNettyClientHelperthrough theorg.restlet.engine.helpercontext attribute. A client created withnew Client(Protocol.HTTP)has no context, so it couldn't be borrowed, and the error didn't say what to change.Client.getHelper()is now public, documented as an engine extension point for extensions that share the connector's resources. It was private, and it doesn't depend on a context.WebSocketClientusesclient.getHelper(). The context requirement is removed from thesetClientJavadoc and from the error message, which now reads "The borrowed client must use the Netty HTTP connector".Serverkeeps its helper private. It could get the same accessor later if an extension needs it.Testing
WebSocketSharedEventLoopTestCase.acceptsAClientWithoutContext: a startednew Client(Protocol.HTTP)is borrowed and serves a session. The existing tests still reject unstarted clients and clients without a Netty HTTP connector.org.restlet.ext.websockettests pass locally: 33 tests. SpotBugs and the full core suite couldn't run locally because the machine was short of memory; CI runs them.🤖 Generated with Claude Code