From eff18f457d61d6d443cb08b4717d5b1b867904c4 Mon Sep 17 00:00:00 2001 From: spypsy Date: Thu, 7 Mar 2024 09:00:04 +0000 Subject: [PATCH 1/2] fix: sleep fn mem leak --- yarn-project/foundation/src/sleep/index.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/yarn-project/foundation/src/sleep/index.ts b/yarn-project/foundation/src/sleep/index.ts index 168e4578f1c..10d0024cf74 100644 --- a/yarn-project/foundation/src/sleep/index.ts +++ b/yarn-project/foundation/src/sleep/index.ts @@ -38,6 +38,9 @@ export class InterruptibleSleep { const promise = new Promise(resolve => (timeout = setTimeout(() => resolve(false), ms))); this.timeouts.push(timeout); const shouldThrow = await Promise.race([promise, this.interruptPromise]); + if (!shouldThrow) { + this.interruptPromise = new Promise(resolve => (this.interruptResolve = resolve)); + } clearTimeout(timeout); this.timeouts.splice(this.timeouts.indexOf(timeout), 1); if (shouldThrow) { From 5cc910876e8648825164cc5c031d5235e8586cc8 Mon Sep 17 00:00:00 2001 From: spypsy Date: Thu, 7 Mar 2024 15:14:49 +0000 Subject: [PATCH 2/2] support multiple sleep operations --- yarn-project/foundation/src/sleep/index.ts | 32 ++++++++-------- .../foundation/src/sleep/sleep.test.ts | 38 +++++++++++++++++++ 2 files changed, 54 insertions(+), 16 deletions(-) create mode 100644 yarn-project/foundation/src/sleep/sleep.test.ts diff --git a/yarn-project/foundation/src/sleep/index.ts b/yarn-project/foundation/src/sleep/index.ts index 10d0024cf74..3e22098d4d8 100644 --- a/yarn-project/foundation/src/sleep/index.ts +++ b/yarn-project/foundation/src/sleep/index.ts @@ -21,9 +21,7 @@ import { InterruptError } from '../errors/index.js'; * setTimeout(() =\> sleeper.interrupt(true), 1500); // Interrupt the sleep after 1.5 seconds */ export class InterruptibleSleep { - private interruptResolve: (shouldThrow: boolean) => void = () => {}; - private interruptPromise = new Promise(resolve => (this.interruptResolve = resolve)); - private timeouts: NodeJS.Timeout[] = []; + private interrupts: Array<(shouldThrow: boolean) => void> = []; /** * Sleep for a specified amount of time in milliseconds. @@ -33,16 +31,18 @@ export class InterruptibleSleep { * @param ms - The number of milliseconds to sleep. * @returns A Promise that resolves after the specified time has passed. */ - public async sleep(ms: number) { - let timeout!: NodeJS.Timeout; - const promise = new Promise(resolve => (timeout = setTimeout(() => resolve(false), ms))); - this.timeouts.push(timeout); - const shouldThrow = await Promise.race([promise, this.interruptPromise]); - if (!shouldThrow) { - this.interruptPromise = new Promise(resolve => (this.interruptResolve = resolve)); - } - clearTimeout(timeout); - this.timeouts.splice(this.timeouts.indexOf(timeout), 1); + public async sleep(ms: number): Promise { + let interruptResolve: (shouldThrow: boolean) => void; + const interruptPromise = new Promise(resolve => { + interruptResolve = resolve; + this.interrupts.push(resolve); + }); + + const timeoutPromise = new Promise(resolve => setTimeout(() => resolve(false), ms)); + const shouldThrow = await Promise.race([interruptPromise, timeoutPromise]); + + this.interrupts = this.interrupts.filter(res => res !== interruptResolve); + if (shouldThrow) { throw new InterruptError('Interrupted.'); } @@ -55,9 +55,9 @@ export class InterruptibleSleep { * * @param sleepShouldThrow - A boolean value indicating whether the sleep operation should throw an error when interrupted. Default is false. */ - public interrupt(sleepShouldThrow = false) { - this.interruptResolve(sleepShouldThrow); - this.interruptPromise = new Promise(resolve => (this.interruptResolve = resolve)); + public interrupt(sleepShouldThrow = false): void { + this.interrupts.forEach(resolve => resolve(sleepShouldThrow)); + this.interrupts = []; } } diff --git a/yarn-project/foundation/src/sleep/sleep.test.ts b/yarn-project/foundation/src/sleep/sleep.test.ts new file mode 100644 index 00000000000..0063a05bbf1 --- /dev/null +++ b/yarn-project/foundation/src/sleep/sleep.test.ts @@ -0,0 +1,38 @@ +import { jest } from '@jest/globals'; + +import { InterruptError } from '../errors/index.js'; +import { InterruptibleSleep } from './index.js'; + +describe('InterruptibleSleep', () => { + it('should sleep for 100ms', async () => { + const sleeper = new InterruptibleSleep(); + const start = Date.now(); + await sleeper.sleep(100); + const end = Date.now(); + // -1 ms wiggle room for rounding errors + expect(end - start).toBeGreaterThanOrEqual(99); + }); + + it('can start multiple sleeps', async () => { + const sleeper = new InterruptibleSleep(); + const start = Date.now(); + await Promise.all([sleeper.sleep(100), sleeper.sleep(150)]); + const end = Date.now(); + expect(end - start).toBeGreaterThanOrEqual(149); + }); + + it('can interrup multiple sleeps', async () => { + const stub = jest.fn(); + const sleeper = new InterruptibleSleep(); + const start = Date.now(); + let end1; + const sleep1 = sleeper.sleep(100).then(() => { + end1 = Date.now(); + }); + const sleep2 = sleeper.sleep(150).then(stub); + setTimeout(() => sleeper.interrupt(true), 125); + await Promise.all([sleep1, sleep2]).catch(e => expect(e).toBeInstanceOf(InterruptError)); + expect(end1! - start).toBeGreaterThanOrEqual(99); + expect(stub).not.toHaveBeenCalled(); + }); +});