From 1daaa7b4269a2f4c2728eca6a986c83612d27f11 Mon Sep 17 00:00:00 2001 From: Matvei Andrienko Date: Tue, 5 Nov 2024 16:49:57 +0100 Subject: [PATCH 1/3] fix: use worker to prevent timer throttling --- packages/client/generate-timer-worker.sh | 16 +++ packages/client/package.json | 5 +- packages/client/src/StreamSfuClient.ts | 10 +- .../src/coordinator/connection/connection.ts | 10 +- packages/client/src/timers/index.ts | 116 ++++++++++++++++++ packages/client/src/timers/types.ts | 15 +++ packages/client/src/timers/worker.build.ts | 9 ++ packages/client/src/timers/worker.ts | 37 ++++++ 8 files changed, 208 insertions(+), 10 deletions(-) create mode 100755 packages/client/generate-timer-worker.sh create mode 100644 packages/client/src/timers/index.ts create mode 100644 packages/client/src/timers/types.ts create mode 100644 packages/client/src/timers/worker.build.ts create mode 100644 packages/client/src/timers/worker.ts diff --git a/packages/client/generate-timer-worker.sh b/packages/client/generate-timer-worker.sh new file mode 100755 index 0000000000..157f20486a --- /dev/null +++ b/packages/client/generate-timer-worker.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash + +npx tsc src/timers/worker.ts \ + --skipLibCheck \ + --removeComments \ + --module preserve \ + --lib ES2020,WebWorker \ + --outDir worker-dist + +cat <src/timers/worker.build.ts +export const timerWorker = { + src: \`$( { this.signalWs.removeEventListener('close', this.handleWebSocketClose); - clearInterval(this.keepAliveInterval); + getTimers().clearInterval(this.keepAliveInterval); clearTimeout(this.connectionCheckTimeout); this.onSignalClose?.(); }; @@ -489,8 +490,9 @@ export class StreamSfuClient { }; private keepAlive = () => { - clearInterval(this.keepAliveInterval); - this.keepAliveInterval = setInterval(() => { + const timers = getTimers(); + timers.clearInterval(this.keepAliveInterval); + this.keepAliveInterval = timers.setInterval(() => { this.ping().catch((e) => { this.logger('error', 'Error sending healthCheckRequest to SFU', e); }); diff --git a/packages/client/src/coordinator/connection/connection.ts b/packages/client/src/coordinator/connection/connection.ts index 9a2f04bdbb..1ef011d6ca 100644 --- a/packages/client/src/coordinator/connection/connection.ts +++ b/packages/client/src/coordinator/connection/connection.ts @@ -22,6 +22,7 @@ import type { UR, } from './types'; import type { ConnectedEvent, WSAuthMessage } from '../../gen/coordinator'; +import { getTimers } from '../../timers'; // Type guards to check WebSocket error type const isCloseEvent = ( @@ -58,7 +59,7 @@ export class StableWSConnection { authenticationSent: boolean; consecutiveFailures: number; pingInterval: number; - healthCheckTimeoutRef?: NodeJS.Timeout; + healthCheckTimeoutRef?: number; isConnecting: boolean; isDisconnected: boolean; isHealthy: boolean; @@ -249,7 +250,7 @@ export class StableWSConnection { // start by removing all the listeners if (this.healthCheckTimeoutRef) { - clearInterval(this.healthCheckTimeoutRef); + getTimers().clearInterval(this.healthCheckTimeoutRef); } if (this.connectionCheckTimeoutRef) { clearInterval(this.connectionCheckTimeoutRef); @@ -757,12 +758,13 @@ export class StableWSConnection { * Schedules a next health check ping for websocket. */ scheduleNextPing = () => { + const timers = getTimers(); if (this.healthCheckTimeoutRef) { - clearTimeout(this.healthCheckTimeoutRef); + timers.clearTimeout(this.healthCheckTimeoutRef); } // 30 seconds is the recommended interval (messenger uses this) - this.healthCheckTimeoutRef = setTimeout(() => { + this.healthCheckTimeoutRef = timers.setTimeout(() => { // send the healthcheck..., server replies with a health check event const data = [{ type: 'health.check', client_id: this.client.clientID }]; // try to send on the connection diff --git a/packages/client/src/timers/index.ts b/packages/client/src/timers/index.ts new file mode 100644 index 0000000000..6ec4086ff2 --- /dev/null +++ b/packages/client/src/timers/index.ts @@ -0,0 +1,116 @@ +import { lazy } from '../helpers/lazy'; +import { getLogger } from '../logger'; +import { TimerWorkerEvent, TimerWorkerRequest } from './types'; +import { timerWorker } from './worker.build'; + +class TimerWorker { + private currentTimerId = 1; + private callbacks = new Map void>(); + private worker: Worker | undefined; + private fallback = false; + + setup(): void { + try { + const source = timerWorker.src; + const blob = new Blob([source], { + type: 'application/javascript; charset=utf-8', + }); + const script = URL.createObjectURL(blob); + this.worker = new Worker(script, { name: 'str-timer-worker' }); + this.worker.addEventListener('message', (event) => { + const { type, id } = event.data as TimerWorkerEvent; + if (type === 'tick') { + this.callbacks.get(id)?.(); + } + }); + } catch (err: any) { + getLogger(['timer-worker'])('error', err); + this.fallback = true; + } + } + + destroy(): void { + this.callbacks.clear(); + this.worker?.terminate(); + this.worker = undefined; + this.fallback = false; + } + + get ready() { + return this.fallback || Boolean(this.worker); + } + + setInterval(callback: () => void, timeout: number): number { + return this.setTimer('setInterval', callback, timeout); + } + + clearInterval(id?: number): void { + this.clearTimer('clearInterval', id); + } + + setTimeout(callback: () => void, timeout: number): number { + return this.setTimer('setTimeout', callback, timeout); + } + + clearTimeout(id?: number): void { + this.clearTimer('clearTimeout', id); + } + + private setTimer( + type: 'setTimeout' | 'setInterval', + callback: () => void, + timeout: number, + ) { + if (!this.ready) { + this.setup(); + } + + if (this.fallback) { + return (type === 'setTimeout' ? setTimeout : setInterval)( + callback, + timeout, + ) as unknown as number; + } + + const id = this.getTimerId(); + this.callbacks.set(id, callback); + this.sendMessage({ type, id, timeout }); + return id; + } + + private clearTimer(type: 'clearTimeout' | 'clearInterval', id?: number) { + if (!id) { + return; + } + + if (!this.ready) { + this.setup(); + } + + if (this.fallback) { + this.clearInterval(id); + return; + } + + this.callbacks.delete(id); + this.sendMessage({ type, id }); + } + + private getTimerId() { + return this.currentTimerId++; + } + + private sendMessage(message: TimerWorkerRequest) { + if (!this.worker) { + throw new Error("Cannot use timer worker before it's set up"); + } + + this.worker.postMessage(message); + } +} + +export const getTimers = lazy(() => { + const instance = new TimerWorker(); + instance.setup(); + return instance; +}); diff --git a/packages/client/src/timers/types.ts b/packages/client/src/timers/types.ts new file mode 100644 index 0000000000..40030c3a83 --- /dev/null +++ b/packages/client/src/timers/types.ts @@ -0,0 +1,15 @@ +export type TimerWorkerRequest = + | { + type: 'setInterval' | 'setTimeout'; + id: number; + timeout: number; + } + | { + type: 'clearInterval' | 'clearTimeout'; + id: number; + }; + +export type TimerWorkerEvent = { + type: 'tick'; + id: number; +}; diff --git a/packages/client/src/timers/worker.build.ts b/packages/client/src/timers/worker.build.ts new file mode 100644 index 0000000000..eeddcf28e0 --- /dev/null +++ b/packages/client/src/timers/worker.build.ts @@ -0,0 +1,9 @@ +// Do not modify this file manually. You can edit worker.ts if necessary +// and the run ./generate-timer-worker.sh +export const timerWorker = { + get src(): string { + throw new Error( + 'Timer worker source missing. Did you forget to run generate-timer-worker.sh?', + ); + }, +}; diff --git a/packages/client/src/timers/worker.ts b/packages/client/src/timers/worker.ts new file mode 100644 index 0000000000..47c7a248f6 --- /dev/null +++ b/packages/client/src/timers/worker.ts @@ -0,0 +1,37 @@ +/* eslint-disable */ + +import type { TimerWorkerEvent, TimerWorkerRequest } from './types'; + +const timerIdMapping = new Map(); + +self.addEventListener('message', (event: MessageEvent) => { + const request = event.data as TimerWorkerRequest; + + switch (request.type) { + case 'setTimeout': + case 'setInterval': + timerIdMapping.set( + request.id, + (request.type === 'setTimeout' ? setTimeout : setInterval)( + () => tick(request.id), + request.timeout, + ), + ); + break; + + case 'clearTimeout': + case 'clearInterval': + (request.type === 'clearTimeout' ? clearTimeout : clearInterval)( + timerIdMapping.get(request.id), + ); + timerIdMapping.delete(request.id); + break; + } +}); + +function tick(id: number) { + const message: TimerWorkerEvent = { type: 'tick', id }; + self.postMessage(message); +} + +/* eslint-enable */ From 67148d9bc1de67eb832062d8f78118dd83a84e93 Mon Sep 17 00:00:00 2001 From: Matvei Andrienko Date: Tue, 5 Nov 2024 17:12:07 +0100 Subject: [PATCH 2/3] prevent memory leak from setTimeout --- packages/client/src/timers/index.ts | 12 +++++++++++- packages/client/src/timers/worker.ts | 11 +++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/client/src/timers/index.ts b/packages/client/src/timers/index.ts index 6ec4086ff2..cadc3259e2 100644 --- a/packages/client/src/timers/index.ts +++ b/packages/client/src/timers/index.ts @@ -73,7 +73,17 @@ class TimerWorker { } const id = this.getTimerId(); - this.callbacks.set(id, callback); + + this.callbacks.set(id, () => { + callback(); + + // Timeouts are one-off operations, so no need to keep callback reference + // after timer has fired + if (type === 'setTimeout') { + this.callbacks.delete(id); + } + }); + this.sendMessage({ type, id, timeout }); return id; } diff --git a/packages/client/src/timers/worker.ts b/packages/client/src/timers/worker.ts index 47c7a248f6..4170b2f251 100644 --- a/packages/client/src/timers/worker.ts +++ b/packages/client/src/timers/worker.ts @@ -12,10 +12,13 @@ self.addEventListener('message', (event: MessageEvent) => { case 'setInterval': timerIdMapping.set( request.id, - (request.type === 'setTimeout' ? setTimeout : setInterval)( - () => tick(request.id), - request.timeout, - ), + (request.type === 'setTimeout' ? setTimeout : setInterval)(() => { + tick(request.id); + + if (request.type === 'setTimeout') { + timerIdMapping.delete(request.id); + } + }, request.timeout), ); break; From 790c60519b26794009c280b39c827bfc39dee2ec Mon Sep 17 00:00:00 2001 From: Matvei Andrienko Date: Wed, 6 Nov 2024 12:36:37 +0100 Subject: [PATCH 3/3] fix: typo --- packages/client/src/timers/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/client/src/timers/index.ts b/packages/client/src/timers/index.ts index cadc3259e2..7cb68b9553 100644 --- a/packages/client/src/timers/index.ts +++ b/packages/client/src/timers/index.ts @@ -98,7 +98,7 @@ class TimerWorker { } if (this.fallback) { - this.clearInterval(id); + (type === 'clearTimeout' ? clearTimeout : clearInterval)(id); return; }