From 27573e6dce38373844cb2b8df7d892dffdba31cd Mon Sep 17 00:00:00 2001 From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Date: Thu, 7 Oct 2021 11:18:22 -0700 Subject: [PATCH] Use non-throttled timeouts for websockets --- ts/Timers.ts | 18 +++++++++ ts/background.ts | 30 +++++++------- ts/context/Timers.ts | 43 +++++++++++++++++++++ ts/context/index.ts | 3 ++ ts/test-electron/WebsocketResources_test.ts | 6 +++ ts/textsecure/SocketManager.ts | 11 +++--- ts/textsecure/WebsocketResources.ts | 28 ++++++++------ 7 files changed, 107 insertions(+), 32 deletions(-) create mode 100644 ts/Timers.ts create mode 100644 ts/context/Timers.ts diff --git a/ts/Timers.ts b/ts/Timers.ts new file mode 100644 index 000000000..ec26422fb --- /dev/null +++ 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 +): ReturnType { + return timers.setTimeout(...args); +} + +export function clearTimeout( + ...args: Parameters +): ReturnType { + return timers.clearTimeout(...args); +} diff --git a/ts/background.ts b/ts/background.ts index ddf13ee3f..82755ff39 100644 --- 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'; @@ -1925,8 +1927,8 @@ export async function startApp(): Promise { 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'); @@ -1936,7 +1938,7 @@ export async function startApp(): Promise { // 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(); @@ -1951,12 +1953,12 @@ export async function startApp(): Promise { 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; } @@ -2004,7 +2006,7 @@ export async function startApp(): Promise { log.info('connect', { firstRun, connectCount }); if (reconnectTimer) { - clearTimeout(reconnectTimer); + Timers.clearTimeout(reconnectTimer); reconnectTimer = undefined; } @@ -2291,19 +2293,17 @@ export async function startApp(): Promise { log.info( 'waitForEmptyEventQueue: Waiting for MessageReceiver empty event...' ); - let resolve: undefined | (() => void); - let reject: undefined | ((error: Error) => void); - const promise = new Promise((innerResolve, innerReject) => { - resolve = innerResolve; - reject = innerReject; - }); + const { resolve, reject, promise } = explodePromise(); + + 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(); } @@ -3434,7 +3434,7 @@ export async function startApp(): Promise { 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 new file mode 100644 index 000000000..d65bd4f91 --- /dev/null +++ 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(); + + 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 { + 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 index 4738c8931..943ecbf21 100644 --- 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 index a95d74785..1da306f21 100644 --- 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 index de4e7d9a0..e06a33254 100644 --- 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,7 +509,7 @@ export class SocketManager extends EventListener { const { promise, resolve, reject } = explodePromise(); - const timer = setTimeout(() => { + const timer = Timers.setTimeout(() => { reject(new ConnectTimeoutError('Connection timed out')); client.abort(); @@ -516,14 +517,14 @@ export class SocketManager extends EventListener { 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 index 3cca90042..caa58f4d7 100644 --- 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,7 +199,7 @@ export default class WebSocketResource extends EventTarget { this.addActive(id); const promise = new Promise((resolve, reject) => { let timer = options.timeout - ? setTimeout(() => { + ? Timers.setTimeout(() => { this.removeActive(id); reject(new Error('Request timed out')); }, options.timeout) @@ -206,7 +207,7 @@ export default class WebSocketResource extends EventTarget { 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; } }