-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Specification does not specify who should send Pings and how often #176
Comments
@glassfishrobot Commented |
@glassfishrobot Commented Its not clear from that description if websocket applications will want ping/pongs at some regular interval, or use them only very rarely, or something imbetween. So that's why the API has just provided the API to send and receive. I believe the API on RemoteEndpoint.sendPing/sendPong is relatively clear on how to send them manually. We may well revist this in the next version of the API - perhaps define some options for using pings and pongs in a more automated way. |
@glassfishrobot Commented First, it is the server's (not necessarily the container's, just the "server"'s in general) responsibility to send pings. This is supported by the fact that the HTML5 WebSocket API provides no mechanism for sending pings or receiving pongs ... the spec says that browsers must simply respond to pings with pongs when the server sends them. Thus, you literally can't send pings from JavaScript. So, as the spec is written, every server endpoint must implement its own mechanism for periodically sending pings. Because there is an instance of a server endpoint for each connected session, the "easiest path" that most developers will take is to create a new thread for each new session and use that thread to send a ping every X seconds. Terror and chaos will reign. AT THE VERY LEAST, the container should be mandated to automatically send pings (for server endpoints only) at some fixed, pre-defined interval (of less than 30 seconds, since Chrome and FireFox close inactive WS connections after 30 seconds). Then, perhaps, in some later version we can add configurability for this (though I would argue the configurability is needed now, I know that's not going to happen). But NOT requiring the container to send pings for server endpoints WILL result in severe difficulties and problems for developers, and WILL open a can of worms in web applications. |
@glassfishrobot Commented |
@glassfishrobot Commented |
|
I think there is consensus that the WebSocket needs an API that;
The details of the API are open to discussion. Thoughts? |
I don't know if this is a good thing to have in the spec/api. I could easily see someone wanting to know duration of ping/pong round trip as well. Perhaps an event instead? Let the application decide what to do? @OnPongResponse
public void onPongResponse(Sesssion session, ByteBuffer pingPayload, Duration roundTrip)
@OnPongTimeout
public void onPongTimeout(Sesssion session, ByteBuffer pingPayload, Duration timeout) I don't like the fact that this change would force a scheduler for all websocket implementations. (something not required right now) |
I don't recall any requests for Tomcat to implement any sort of regular ping mechanism. On that basis, I have proposed a PR that clarifies the responsibilities for ping/pong but does not an any new API to configure regular pings. |
Jetty has had no requests for auto-ping, but several for auto-pong-reply (which is implemented). There are serveral libraries built on top of the jakarta websocket spec that handle send ping (listen for pong) on their own, and each one i've seen does it differently. |
The WebSocket spec does not specifically say who is responsible for sending ping messages and how often they should be sent. I think this is a serious problem, as it could be confusing to application developers. I assumed the container did that, but the Tomcat implementation does not (and I do not know whether that's a bug in the Tomcat implementation or part of the spec). The Javadoc for PongMessage and @OnMessage talk about receiving pongs, but nothing there says "you have to manually send pings in order to receive pongs" or something like that.
Assuming application developers are required to manually send pings, I think that's a potential problem. If users are solely responsible for sending ping messages, then we have immediately created a situation where (in many/most cases) the developer is responsible for creating and maintaining one or more threads to manage the sending of pings. This can add quite a lot of development overhead and testing to applications. Additionally, in some environments (such as Servlet containers) an application creating its own threads can be problematic (memory leaks, etc.). Ideally, the container would manage the sending of pings in order to avoid this situation.
To be the most flexible, @serverendpoint, @ClientEndpoint, ServerEndpointConfig, and ClientEndpointConfig should all probably have options to enable/disable, or configure the interval of, pinging, with it defaulting to on (or some number of seconds) for server endpoints and off (or negative seconds) for client endpoints.
Affected Versions
[1.0]
The text was updated successfully, but these errors were encountered: