eclipse-ee4j / eclipse-ee4j/tyrus

Only restart timeout idle timeout-timer if ping succeeds?

Open
#637 0 comments 1 reaction 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.