diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/stack-refresh.ts b/packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/stack-refresh.ts index 6cb2a3d5e..99e103169 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/stack-refresh.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/garbage-collection/stack-refresh.ts @@ -202,12 +202,30 @@ export class BackgroundStackRefresh { return Promise.resolve(); } - // The last refresh happened earlier than the time frame - // We will wait for the latest refresh to land or reject if it takes too long - return Promise.race([ - new Promise(resolve => this.queuedPromises.push(resolve)), - new Promise((_, reject) => setTimeout(() => reject(new ToolkitError('StackRefreshTimeout', 'refreshStacks took too long; the background thread likely threw an error')), ms)), - ]); + // The last refresh happened earlier than the time frame. + // We will wait for the latest refresh to land or reject if it takes too long. + // + // Whichever side wins, we must clean up after the loser: an uncleared timeout + // handle keeps the process event loop alive and its reject closure retained + // until it eventually fires, and a resolve callback left behind in + // queuedPromises after a timeout would sit there forever (justRefreshedStacks() + // only ever drains it, so on a `cdk gc` run with many concurrent timed-out + // callers this array would otherwise grow without bound). + return new Promise((resolve, reject) => { + const onRefresh = () => { + clearTimeout(timeoutHandle); + resolve(undefined); + }; + this.queuedPromises.push(onRefresh); + + const timeoutHandle = setTimeout(() => { + const index = this.queuedPromises.indexOf(onRefresh); + if (index !== -1) { + this.queuedPromises.splice(index, 1); + } + reject(new ToolkitError('StackRefreshTimeout', 'refreshStacks took too long; the background thread likely threw an error')); + }, ms); + }); } public stop() { diff --git a/packages/@aws-cdk/toolkit-lib/test/api/garbage-collection/garbage-collection.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/garbage-collection/garbage-collection.test.ts index de088dec6..6b2d769b2 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/garbage-collection/garbage-collection.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/garbage-collection/garbage-collection.test.ts @@ -1064,6 +1064,55 @@ describe('BackgroundStackRefresh', () => { expect(setTimeoutSpy).not.toHaveBeenCalled(); expect(jest.getTimerCount()).toBe(0); }); + + test('noOlderThan() clears its timeout handle once the refresh lands (no leaked timer)', async () => { + void backgroundRefresh.start(); + await jest.runOnlyPendingTimersAsync(); // first refresh lands; lastRefreshTime = T0, next refresh scheduled for T0+300000 + + jest.advanceTimersByTime(299000); // T0+299000: 1s before the next background refresh fires + + const timerCountBefore = jest.getTimerCount(); + + // 100s is shorter than the 299s elapsed since the last refresh, so this must take + // the "wait for it" branch rather than resolving immediately -- and it's much + // longer than the 1s until the next background refresh, so that refresh (not + // this call's own timeout) is what resolves it. + const waitPromise = backgroundRefresh.noOlderThan(100000); + // A new timer was armed for this call's timeout race. + expect(jest.getTimerCount()).toBeGreaterThan(timerCountBefore); + + jest.advanceTimersByTime(1000); // T0+300000: the next background refresh lands and resolves us + await expect(waitPromise).resolves.toBeUndefined(); + + // The timeout side of the race must have been cleared, not left pending. + expect(jest.getTimerCount()).toBe(timerCountBefore); + }); + + test('noOlderThan() does not leave a dangling entry in queuedPromises after it times out', async () => { + void backgroundRefresh.start(); + await jest.runOnlyPendingTimersAsync(); + jest.advanceTimersByTime(120000); + + const waitPromise = backgroundRefresh.noOlderThan(0); + jest.advanceTimersByTime(120000); + await expect(waitPromise).rejects.toThrow('refreshStacks took too long; the background thread likely threw an error'); + + // A stale resolver left behind here would sit in the queue forever (only + // justRefreshedStacks() drains it), growing unboundedly under repeated timeouts. + expect((backgroundRefresh as any).queuedPromises).toHaveLength(0); + }); + + test('many concurrent noOlderThan() timeouts do not accumulate in queuedPromises', async () => { + void backgroundRefresh.start(); + await jest.runOnlyPendingTimersAsync(); + jest.advanceTimersByTime(120000); + + const waitPromises = Array.from({ length: 25 }, () => backgroundRefresh.noOlderThan(0)); + jest.advanceTimersByTime(120000); + await Promise.allSettled(waitPromises); + + expect((backgroundRefresh as any).queuedPromises).toHaveLength(0); + }); }); describe('ProgressPrinter', () => {