Use non-throttled timeouts for websockets

Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com>
signalapp · Oct 7, 2021 · 383e9af · 383e9af
1 parent b6c7c19
commit 383e9af
Show file tree

Hide file tree

Showing 7 changed files with 107 additions and 32 deletions.
diff --git a/ts/Timers.ts b/ts/Timers.ts
@@ -0,0 +1,18 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+const { timers } = window.SignalContext;
+
+export type { Timeout } from './context/Timers';
+
+export function setTimeout(
+  ...args: Parameters<typeof timers.setTimeout>
+): ReturnType<typeof timers.setTimeout> {
+  return timers.setTimeout(...args);
+}
+
+export function clearTimeout(
+  ...args: Parameters<typeof timers.clearTimeout>
+): ReturnType<typeof timers.clearTimeout> {
+  return timers.clearTimeout(...args);
+}
diff --git a/ts/background.ts b/ts/background.ts
@@ -18,12 +18,14 @@ import {
   ConversationAttributesType,
 } from './model-types.d';
 import * as Bytes from './Bytes';
+import * as Timers from './Timers';
 import { WhatIsThis, DeliveryReceiptBatcherItemType } from './window.d';
 import { getTitleBarVisibility, TitleBarVisibility } from './types/Settings';
 import { SocketStatus } from './types/SocketStatus';
 import { DEFAULT_CONVERSATION_COLOR } from './types/Colors';
 import { ChallengeHandler } from './challenge';
 import * as durations from './util/durations';
+import { explodePromise } from './util/explodePromise';
 import { isWindowDragElement } from './util/isWindowDragElement';
 import { assert, strictAssert } from './util/assert';
 import { dropNull } from './util/dropNull';
@@ -1926,8 +1928,8 @@ export async function startApp(): Promise<void> {
     return syncRequest;
   };
 
-  let disconnectTimer: NodeJS.Timeout | undefined;
-  let reconnectTimer: number | undefined;
+  let disconnectTimer: Timers.Timeout | undefined;
+  let reconnectTimer: Timers.Timeout | undefined;
   function onOffline() {
     log.info('offline');
 
@@ -1937,7 +1939,7 @@ export async function startApp(): Promise<void> {
     // We've received logs from Linux where we get an 'offline' event, then 30ms later
     //   we get an online event. This waits a bit after getting an 'offline' event
     //   before disconnecting the socket manually.
-    disconnectTimer = setTimeout(disconnect, 1000);
+    disconnectTimer = Timers.setTimeout(disconnect, 1000);
 
     if (challengeHandler) {
       challengeHandler.onOffline();
@@ -1952,12 +1954,12 @@ export async function startApp(): Promise<void> {
 
     if (disconnectTimer && isSocketOnline()) {
       log.warn('Already online. Had a blip in online/offline status.');
-      clearTimeout(disconnectTimer);
+      Timers.clearTimeout(disconnectTimer);
       disconnectTimer = undefined;
       return;
     }
     if (disconnectTimer) {
-      clearTimeout(disconnectTimer);
+      Timers.clearTimeout(disconnectTimer);
       disconnectTimer = undefined;
     }
 
@@ -2005,7 +2007,7 @@ export async function startApp(): Promise<void> {
       log.info('connect', { firstRun, connectCount });
 
       if (reconnectTimer) {
-        clearTimeout(reconnectTimer);
+        Timers.clearTimeout(reconnectTimer);
         reconnectTimer = undefined;
       }
 
@@ -2292,19 +2294,17 @@ export async function startApp(): Promise<void> {
       log.info(
         'waitForEmptyEventQueue: Waiting for MessageReceiver empty event...'
       );
-      let resolve: undefined | (() => void);
-      let reject: undefined | ((error: Error) => void);
-      const promise = new Promise<void>((innerResolve, innerReject) => {
-        resolve = innerResolve;
-        reject = innerReject;
-      });
+      const { resolve, reject, promise } = explodePromise<void>();
+
+      const timeout = Timers.setTimeout(() => {
+        reject(new Error('Empty queue never fired'));
+      }, FIVE_MINUTES);
 
-      const timeout = reject && setTimeout(reject, FIVE_MINUTES);
       const onEmptyOnce = () => {
         if (messageReceiver) {
           messageReceiver.removeEventListener('empty', onEmptyOnce);
         }
-        clearTimeout(timeout);
+        Timers.clearTimeout(timeout);
         if (resolve) {
           resolve();
         }
@@ -3435,7 +3435,7 @@ export async function startApp(): Promise<void> {
         const timeout = reconnectBackOff.getAndIncrement();
 
         log.info(`retrying in ${timeout}ms`);
-        reconnectTimer = setTimeout(connect, timeout);
+        reconnectTimer = Timers.setTimeout(connect, timeout);
 
         window.Whisper.events.trigger('reconnectTimer');
 

diff --git a/ts/context/Timers.ts b/ts/context/Timers.ts
@@ -0,0 +1,43 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { setTimeout, clearTimeout } from 'timers';
+
+export type Timeout = {
+  id: number;
+  __signalContext: never;
+};
+
+export class Timers {
+  private counter = 0;
+
+  private readonly timers = new Map<number, NodeJS.Timeout>();
+
+  public setTimeout(callback: () => void, delay: number): Timeout {
+    let id: number;
+    do {
+      id = this.counter;
+      // eslint-disable-next-line no-bitwise
+      this.counter = (this.counter + 1) >>> 0;
+    } while (this.timers.has(id));
+
+    const timer = setTimeout(() => {
+      this.timers.delete(id);
+      callback();
+    }, delay);
+
+    this.timers.set(id, timer);
+
+    return ({ id } as unknown) as Timeout;
+  }
+
+  public clearTimeout({ id }: Timeout): ReturnType<typeof clearTimeout> {
+    const timer = this.timers.get(id);
+    if (timer === undefined) {
+      return;
+    }
+
+    this.timers.delete(id);
+    return clearTimeout(timer);
+  }
+}
diff --git a/ts/context/index.ts b/ts/context/index.ts
@@ -3,6 +3,7 @@
 
 import { Bytes } from './Bytes';
 import { Crypto } from './Crypto';
+import { Timers } from './Timers';
 import {
   createNativeThemeListener,
   MinimalIPC,
@@ -13,6 +14,8 @@ export class Context {
 
   public readonly crypto = new Crypto();
 
+  public readonly timers = new Timers();
+
   public readonly nativeThemeListener;
 
   constructor(private readonly ipc: MinimalIPC) {

diff --git a/ts/test-electron/WebsocketResources_test.ts b/ts/test-electron/WebsocketResources_test.ts
@@ -32,6 +32,12 @@ describe('WebSocket-Resource', () => {
     this.clock = this.sandbox.useFakeTimers({
       now: NOW,
     });
+    this.sandbox
+      .stub(window.SignalContext.timers, 'setTimeout')
+      .callsFake(setTimeout);
+    this.sandbox
+      .stub(window.SignalContext.timers, 'clearTimeout')
+      .callsFake(clearTimeout);
   });
 
   afterEach(function afterEach() {

diff --git a/ts/textsecure/SocketManager.ts b/ts/textsecure/SocketManager.ts
@@ -18,6 +18,7 @@ import { sleep } from '../util/sleep';
 import { SocketStatus } from '../types/SocketStatus';
 import * as Errors from '../types/errors';
 import * as Bytes from '../Bytes';
+import * as Timers from '../Timers';
 import * as log from '../logging/log';
 
 import WebSocketResource, {
@@ -508,22 +509,22 @@ export class SocketManager extends EventListener {
 
     const { promise, resolve, reject } = explodePromise<WebSocketResource>();
 
-    const timer = setTimeout(() => {
+    const timer = Timers.setTimeout(() => {
       reject(new ConnectTimeoutError('Connection timed out'));
 
       client.abort();
     }, timeout);
 
     let resource: WebSocketResource | undefined;
     client.on('connect', socket => {
-      clearTimeout(timer);
+      Timers.clearTimeout(timer);
 
       resource = new WebSocketResource(socket, resourceOptions);
       resolve(resource);
     });
 
     client.on('httpResponse', async response => {
-      clearTimeout(timer);
+      Timers.clearTimeout(timer);
 
       const statusCode = response.statusCode || -1;
       await handleStatusCode(statusCode);
@@ -547,7 +548,7 @@ export class SocketManager extends EventListener {
     });
 
     client.on('connectFailed', e => {
-      clearTimeout(timer);
+      Timers.clearTimeout(timer);
 
       reject(
         new HTTPError('connectResource: connectFailed', {
@@ -568,7 +569,7 @@ export class SocketManager extends EventListener {
             resource.close(3000, 'aborted');
           } else {
             log.warn(`SocketManager aborting connection ${path}`);
-            clearTimeout(timer);
+            Timers.clearTimeout(timer);
             client.abort();
           }
         },

diff --git a/ts/textsecure/WebsocketResources.ts b/ts/textsecure/WebsocketResources.ts
@@ -35,6 +35,7 @@ import { normalizeNumber } from '../util/normalizeNumber';
 import * as Errors from '../types/errors';
 import { SignalService as Proto } from '../protobuf';
 import * as log from '../logging/log';
+import * as Timers from '../Timers';
 
 const THIRTY_SECONDS = 30 * durations.SECOND;
 
@@ -118,7 +119,7 @@ export default class WebSocketResource extends EventTarget {
 
   private shuttingDown = false;
 
-  private shutdownTimer?: NodeJS.Timeout;
+  private shutdownTimer?: Timers.Timeout;
 
   // Public for tests
   public readonly keepalive?: KeepAlive;
@@ -198,15 +199,15 @@ export default class WebSocketResource extends EventTarget {
     this.addActive(id);
     const promise = new Promise<SendRequestResult>((resolve, reject) => {
       let timer = options.timeout
-        ? setTimeout(() => {
+        ? Timers.setTimeout(() => {
             this.removeActive(id);
             reject(new Error('Request timed out'));
           }, options.timeout)
         : undefined;
 
       this.outgoingMap.set(id, result => {
         if (timer !== undefined) {
-          clearTimeout(timer);
+          Timers.clearTimeout(timer);
           timer = undefined;
         }
 
@@ -244,7 +245,7 @@ export default class WebSocketResource extends EventTarget {
     // On linux the socket can wait a long time to emit its close event if we've
     //   lost the internet connection. On the order of minutes. This speeds that
     //   process up.
-    setTimeout(() => {
+    Timers.setTimeout(() => {
       if (this.closed) {
         return;
       }
@@ -268,7 +269,7 @@ export default class WebSocketResource extends EventTarget {
     this.shuttingDown = true;
 
     log.info('WebSocketResource: shutting down');
-    this.shutdownTimer = setTimeout(() => {
+    this.shutdownTimer = Timers.setTimeout(() => {
       if (this.closed) {
         return;
       }
@@ -369,7 +370,7 @@ export default class WebSocketResource extends EventTarget {
     }
 
     if (this.shutdownTimer) {
-      clearTimeout(this.shutdownTimer);
+      Timers.clearTimeout(this.shutdownTimer);
       this.shutdownTimer = undefined;
     }
 
@@ -388,9 +389,9 @@ const KEEPALIVE_INTERVAL_MS = 55000; // 55 seconds + 5 seconds for closing the
 const MAX_KEEPALIVE_INTERVAL_MS = 5 * durations.MINUTE;
 
 class KeepAlive {
-  private keepAliveTimer: NodeJS.Timeout | undefined;
+  private keepAliveTimer: Timers.Timeout | undefined;
 
-  private disconnectTimer: NodeJS.Timeout | undefined;
+  private disconnectTimer: Timers.Timeout | undefined;
 
   private path: string;
 
@@ -431,7 +432,7 @@ class KeepAlive {
 
     if (this.disconnect) {
       // automatically disconnect if server doesn't ack
-      this.disconnectTimer = setTimeout(() => {
+      this.disconnectTimer = Timers.setTimeout(() => {
         log.info('WebSocketResources: disconnecting due to no response');
         this.clearTimers();
 
@@ -457,16 +458,19 @@ class KeepAlive {
 
     this.clearTimers();
 
-    this.keepAliveTimer = setTimeout(() => this.send(), KEEPALIVE_INTERVAL_MS);
+    this.keepAliveTimer = Timers.setTimeout(
+      () => this.send(),
+      KEEPALIVE_INTERVAL_MS
+    );
   }
 
   private clearTimers(): void {
     if (this.keepAliveTimer) {
-      clearTimeout(this.keepAliveTimer);
+      Timers.clearTimeout(this.keepAliveTimer);
       this.keepAliveTimer = undefined;
     }
     if (this.disconnectTimer) {
-      clearTimeout(this.disconnectTimer);
+      Timers.clearTimeout(this.disconnectTimer);
       this.disconnectTimer = undefined;
     }
   }