From f579b79d2edad321e6abf640358903a13ada009a Mon Sep 17 00:00:00 2001 From: Clark Date: Thu, 13 Jul 2023 14:22:10 -0400 Subject: [PATCH] Change websocket keepalive response time to 20s. --- .../net/SignalWebSocketHealthMonitor.java | 50 +++++++++++++++---- .../websocket/WebSocketConnection.java | 8 +-- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/net/SignalWebSocketHealthMonitor.java b/app/src/main/java/org/thoughtcrime/securesms/net/SignalWebSocketHealthMonitor.java index f0d73b1194..9c7ed91dc1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/net/SignalWebSocketHealthMonitor.java +++ b/app/src/main/java/org/thoughtcrime/securesms/net/SignalWebSocketHealthMonitor.java @@ -30,8 +30,16 @@ public final class SignalWebSocketHealthMonitor implements HealthMonitor { private static final String TAG = Log.tag(SignalWebSocketHealthMonitor.class); - private static final long KEEP_ALIVE_SEND_CADENCE = TimeUnit.SECONDS.toMillis(WebSocketConnection.KEEPALIVE_TIMEOUT_SECONDS); - private static final long MAX_TIME_SINCE_SUCCESSFUL_KEEP_ALIVE = KEEP_ALIVE_SEND_CADENCE * 3; + /** + * This is the amount of time in between sent keep alives. Must be greater than {@link SignalWebSocketHealthMonitor#KEEP_ALIVE_TIMEOUT} + */ + private static final long KEEP_ALIVE_SEND_CADENCE = TimeUnit.SECONDS.toMillis(WebSocketConnection.KEEPALIVE_FREQUENCY_SECONDS); + + /** + * This is the amount of time we will wait for a response to the keep alive before we consider the websockets dead. + * It is required that this value be less than {@link SignalWebSocketHealthMonitor#KEEP_ALIVE_SEND_CADENCE} + */ + private static final long KEEP_ALIVE_TIMEOUT = TimeUnit.SECONDS.toMillis(20); private final Executor executor = Executors.newSingleThreadExecutor(); @@ -103,11 +111,12 @@ public final class SignalWebSocketHealthMonitor implements HealthMonitor { @Override public void onKeepAliveResponse(long sentTimestamp, boolean isIdentifiedWebSocket) { + final long keepAliveTime = System.currentTimeMillis(); executor.execute(() -> { if (isIdentifiedWebSocket) { - identified.lastKeepAliveReceived = System.currentTimeMillis(); + identified.lastKeepAliveReceived = keepAliveTime; } else { - unidentified.lastKeepAliveReceived = System.currentTimeMillis(); + unidentified.lastKeepAliveReceived = keepAliveTime; } }); } @@ -138,7 +147,7 @@ public final class SignalWebSocketHealthMonitor implements HealthMonitor { /** * Sends periodic heartbeats/keep-alives over both WebSockets to prevent connection timeouts. If - * either WebSocket fails 3 times to get a return heartbeat both are forced to be recreated. + * either WebSocket fails to get a return heartbeat after {@link SignalWebSocketHealthMonitor#KEEP_ALIVE_TIMEOUT} seconds, both are forced to be recreated. */ private class KeepAliveSender extends Thread { @@ -148,20 +157,26 @@ public final class SignalWebSocketHealthMonitor implements HealthMonitor { identified.lastKeepAliveReceived = System.currentTimeMillis(); unidentified.lastKeepAliveReceived = System.currentTimeMillis(); + long keepAliveSendTime = System.currentTimeMillis(); while (shouldKeepRunning && isKeepAliveNecessary()) { try { - sleepTimer.sleep(KEEP_ALIVE_SEND_CADENCE); + long nextKeepAliveSendTime = (keepAliveSendTime + KEEP_ALIVE_SEND_CADENCE); + sleepUntil(nextKeepAliveSendTime); if (shouldKeepRunning && isKeepAliveNecessary()) { - long keepAliveRequiredSinceTime = System.currentTimeMillis() - MAX_TIME_SINCE_SUCCESSFUL_KEEP_ALIVE; + keepAliveSendTime = System.currentTimeMillis(); + signalWebSocket.sendKeepAlive(); + } - if (identified.lastKeepAliveReceived < keepAliveRequiredSinceTime || unidentified.lastKeepAliveReceived < keepAliveRequiredSinceTime) { + final long responseRequiredTime = keepAliveSendTime + KEEP_ALIVE_TIMEOUT; + sleepUntil(responseRequiredTime); + + if (shouldKeepRunning && isKeepAliveNecessary()) { + if (identified.lastKeepAliveReceived < keepAliveSendTime || unidentified.lastKeepAliveReceived < keepAliveSendTime) { Log.w(TAG, "Missed keep alives, identified last: " + identified.lastKeepAliveReceived + " unidentified last: " + unidentified.lastKeepAliveReceived + - " needed by: " + keepAliveRequiredSinceTime); + " needed by: " + responseRequiredTime); signalWebSocket.forceNewWebSockets(); - } else { - signalWebSocket.sendKeepAlive(); } } } catch (Throwable e) { @@ -170,6 +185,19 @@ public final class SignalWebSocketHealthMonitor implements HealthMonitor { } } + private void sleepUntil(long timeMs) { + while (System.currentTimeMillis() < timeMs) { + long waitTime = timeMs - System.currentTimeMillis(); + if (waitTime > 0) { + try { + sleepTimer.sleep(waitTime); + } catch (InterruptedException e) { + Log.w(TAG, e); + } + } + } + } + public void shutdown() { shouldKeepRunning = false; } diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/websocket/WebSocketConnection.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/websocket/WebSocketConnection.java index dc6362d91a..a1d876e4d0 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/websocket/WebSocketConnection.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/websocket/WebSocketConnection.java @@ -58,8 +58,8 @@ import static org.whispersystems.signalservice.internal.websocket.WebSocketProto public class WebSocketConnection extends WebSocketListener { - private static final String TAG = WebSocketConnection.class.getSimpleName(); - public static final int KEEPALIVE_TIMEOUT_SECONDS = 30; + private static final String TAG = WebSocketConnection.class.getSimpleName(); + public static final int KEEPALIVE_FREQUENCY_SECONDS = 30; private final LinkedList incomingRequests = new LinkedList<>(); private final Map outgoingRequests = new HashMap<>(); @@ -143,9 +143,9 @@ public class WebSocketConnection extends WebSocketListener { OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder().sslSocketFactory(new Tls12SocketFactory(socketFactory.first()), socketFactory.second()) .connectionSpecs(serviceUrl.getConnectionSpecs().orElse(Util.immutableList(ConnectionSpec.RESTRICTED_TLS))) - .readTimeout(KEEPALIVE_TIMEOUT_SECONDS + 10, TimeUnit.SECONDS) + .readTimeout(KEEPALIVE_FREQUENCY_SECONDS + 10, TimeUnit.SECONDS) .dns(dns.orElse(Dns.SYSTEM)) - .connectTimeout(KEEPALIVE_TIMEOUT_SECONDS + 10, TimeUnit.SECONDS); + .connectTimeout(KEEPALIVE_FREQUENCY_SECONDS + 10, TimeUnit.SECONDS); for (Interceptor interceptor : interceptors) { clientBuilder.addInterceptor(interceptor);