eclipse-ee4j / eclipse-ee4j/tyrus
Only restart timeout idle timeout-timer if ping succeeds?
- Dominant language
- Java
- Stars
- 128
- Forks
- 49
- PR merge metrics
- No merged PRs in 30d
Description
Hi,
See https://github.com/eclipse-ee4j/tyrus/blob/master/core/src/main/java/org/glassfish/tyrus/core/TyrusRemoteEndpoint.java#L433 for the following code snippet:
```
public void sendPing(ByteBuffer applicationData) throws IOException {
if (applicationData != null && applicationData.remaining() > 125) {
throw new IllegalArgumentException(LocalizationMessages.APPLICATION_DATA_TOO_LONG("Ping"));
}
session.restartIdleTimeoutExecutor();
webSocket.sendPing(Utils.getRemainingArray(applicationData));
}
```
Wouldn't it be better to only restart the idle timeout-timer if the ping frame was successfully sent? Now it is restarted regardless of the result of the ping frame.
For example, if you have a session with `.setMaxIdleTimeout(60)`, that session will never be closed if you are sending ping packets every -60 seconds, even if the ping packets are not correctly sent and/or correctly received on the other side.
I would say that an approach similar as in https://github.com/eclipse-ee4j/tyrus/blob/master/core/src/main/java/org/glassfish/tyrus/core/TyrusRemoteEndpoint.java#L156 should also be applied to `sendPing`/`sendPong`.
Contributor guide
Assessment
This issue has not been assessed yet.