From f16f52487782474c3cba41af42c1a91cb86a734d Mon Sep 17 00:00:00 2001 From: maniac103 Date: Wed, 11 Jan 2023 15:05:19 +0100 Subject: [PATCH] [gardena] Fix server ping timeout logic (#14203) On server timeouts, it's possible for multiple minutes to elapse between receiving the last pong on the old connection and attempting to send the next ping on the new connection. If that time span exceeded 5 minutes, the binding went into a minutely reconnection loop, because it never attempted to send a ping anymore, which led to lastPong not being updated anymore. To fix this, replace the timing dependent timestamp handling by a simple counter which counts how many consecutive ping attempts didn't receive an answer, and closing the connection after 5 unsuccessful ping attempts. That new counter is now also reset whenever the connection is restarted. Fixes #14188 Signed-off-by: Danny Baumann --- .../gardena/internal/GardenaSmartWebSocket.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/bundles/org.openhab.binding.gardena/src/main/java/org/openhab/binding/gardena/internal/GardenaSmartWebSocket.java b/bundles/org.openhab.binding.gardena/src/main/java/org/openhab/binding/gardena/internal/GardenaSmartWebSocket.java index 0999feda38b..9ef2ca5078b 100644 --- a/bundles/org.openhab.binding.gardena/src/main/java/org/openhab/binding/gardena/internal/GardenaSmartWebSocket.java +++ b/bundles/org.openhab.binding.gardena/src/main/java/org/openhab/binding/gardena/internal/GardenaSmartWebSocket.java @@ -16,7 +16,6 @@ import java.io.IOException; import java.net.URI; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; -import java.time.Instant; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -49,12 +48,12 @@ import org.slf4j.LoggerFactory; public class GardenaSmartWebSocket { private final Logger logger = LoggerFactory.getLogger(GardenaSmartWebSocket.class); private final GardenaSmartWebSocketListener socketEventListener; - private final long WEBSOCKET_IDLE_TIMEOUT = 300; + private final int MAX_UNANSWERED_PINGS = 5; private WebSocketSession session; private WebSocketClient webSocketClient; private boolean closing; - private Instant lastPong = Instant.now(); + private int unansweredPings = 0; private ScheduledExecutorService scheduler; private @Nullable ScheduledFuture connectionTracker; private ByteBuffer pingPayload = ByteBuffer.wrap("ping".getBytes(StandardCharsets.UTF_8)); @@ -115,8 +114,9 @@ public class GardenaSmartWebSocket { } @OnWebSocketConnect - public void onConnect(Session session) { + public synchronized void onConnect(Session session) { closing = false; + unansweredPings = 0; logger.debug("Connected to Gardena Webservice ({})", socketId); ScheduledFuture connectionTracker = this.connectionTracker; @@ -129,9 +129,9 @@ public class GardenaSmartWebSocket { } @OnWebSocketFrame - public void onFrame(Frame pong) { + public synchronized void onFrame(Frame pong) { if (pong instanceof PongFrame) { - lastPong = Instant.now(); + unansweredPings = 0; logger.trace("Pong received ({})", socketId); } } @@ -170,14 +170,14 @@ public class GardenaSmartWebSocket { */ private synchronized void sendKeepAlivePing() { final PostOAuth2Response accessToken = token; - if ((Instant.now().getEpochSecond() - lastPong.getEpochSecond() > WEBSOCKET_IDLE_TIMEOUT) || accessToken == null - || accessToken.isAccessTokenExpired()) { + if (unansweredPings > MAX_UNANSWERED_PINGS || accessToken == null || accessToken.isAccessTokenExpired()) { session.close(1000, "Timeout manually closing dead connection (" + socketId + ")"); } else { if (session.isOpen()) { try { logger.trace("Sending ping ({})", socketId); session.getRemote().sendPing(pingPayload); + ++unansweredPings; } catch (IOException ex) { logger.debug("Error while sending ping: {}", ex.getMessage()); }