From d789e5bb77e0337773c69ed9d4e24696c019c6ac Mon Sep 17 00:00:00 2001 From: Felipe Lalanne Date: Wed, 31 Jul 2024 17:41:41 -0400 Subject: [PATCH] Avoid leaking memory on deep promise recursions The following pattern ```ts async function longRunning() { // do something await setTimeout(delay); await longRunning(); } ``` Is regularly used for long running operations on the supervisor (e.g. polling target state). We have recently discovered that this pattern can slowly leak memory as it essentially creates an infinite promise chain. Using `void longRunning()` breaks the chain and avoids the issue. This commit fixes all those instances where the pattern was used. Change-type: patch --- src/api-binder/index.ts | 2 +- src/api-binder/poll.ts | 4 ++-- src/api-binder/report.ts | 3 ++- src/device-state/index.ts | 1 - src/lib/migration.ts | 2 +- src/logging/monitor.ts | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/api-binder/index.ts b/src/api-binder/index.ts index a5f6461b..f6584513 100644 --- a/src/api-binder/index.ts +++ b/src/api-binder/index.ts @@ -454,7 +454,7 @@ async function provisionOrRetry(retryDelay: number): Promise { delay: retryDelay, }); await setTimeout(retryDelay); - return provisionOrRetry(retryDelay); + void provisionOrRetry(retryDelay); } } diff --git a/src/api-binder/poll.ts b/src/api-binder/poll.ts index 6f0aa356..1cfb6ebd 100644 --- a/src/api-binder/poll.ts +++ b/src/api-binder/poll.ts @@ -174,8 +174,8 @@ const poll = async ( const delayedLoop = async (delayBy: number) => { // Wait until we want to poll again await setTimeout(delayBy); - // Poll again - await poll(false, fetchErrors); + // Poll again (use void to break recursion) + void poll(false, fetchErrors); }; // Check if we want to skip first request and just loop again diff --git a/src/api-binder/report.ts b/src/api-binder/report.ts index fefdd26b..cb370129 100644 --- a/src/api-binder/report.ts +++ b/src/api-binder/report.ts @@ -267,7 +267,8 @@ export async function startReporting() { // Wait until we want to report again await setTimeout(delayBy); // Try to report again - await recursivelyReport(delayBy); + // the void is necessary to break the recursion and avoid leaks + void recursivelyReport(delayBy); } } diff --git a/src/device-state/index.ts b/src/device-state/index.ts index 5a84657b..41829240 100644 --- a/src/device-state/index.ts +++ b/src/device-state/index.ts @@ -759,7 +759,6 @@ export function triggerApplyTarget({ scheduledApply = null; } }); - return null; } export async function applyIntermediateTarget( diff --git a/src/lib/migration.ts b/src/lib/migration.ts index 0d17dff0..440f74ed 100644 --- a/src/lib/migration.ts +++ b/src/lib/migration.ts @@ -88,6 +88,6 @@ export async function loadBackupFromMigration( log.error(`Error restoring migration backup, retrying: ${err}`); await setTimeout(retryDelay); - return loadBackupFromMigration(targetState, retryDelay); + void loadBackupFromMigration(targetState, retryDelay); } } diff --git a/src/logging/monitor.ts b/src/logging/monitor.ts index 09131371..d57640b3 100644 --- a/src/logging/monitor.ts +++ b/src/logging/monitor.ts @@ -118,7 +118,7 @@ class LogMonitor { }s`, ); await setTimeout(wait); - return this.start(); + void this.start(); } public isAttached(containerId: string): boolean {