From 4da2531c05f6e83864f1c16d6430f6c6da8ac81a Mon Sep 17 00:00:00 2001 From: Matt Broadstone Date: Sat, 26 Sep 2020 13:47:34 -0400 Subject: [PATCH] fix: permit waking async interval in unreliable clock environments The logic for waking the AsyncInterruptibleInterval sooner than its interval is dependent on an ability to reliably mark the last call made to the wrapped function. In environments like AWS Lambda where instances can be frozen and later thawed, it's possible for the last call to be in a distant past even though the internal timer has not completed yet. This change ensures that we immediately reschedule in these situations. NODE-2829 --- src/utils.ts | 20 ++++++++++++++---- test/unit/utils.test.js | 46 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index e395a448b51..8275ed8d13c 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1036,6 +1036,9 @@ export interface InterruptableAsyncIntervalOptions { minInterval: number; /** Whether the method should be called immediately when the interval is started */ immediate: boolean; + + /* @internal only used for testing unreliable timer environments */ + clock: () => number; } /** @internal */ @@ -1066,12 +1069,13 @@ export function makeInterruptableAsyncInterval( const interval = options.interval || 1000; const minInterval = options.minInterval || 500; const immediate = typeof options.immediate === 'boolean' ? options.immediate : false; + const clock = typeof options.clock === 'function' ? options.clock : now; function wake() { - const currentTime = now(); + const currentTime = clock(); const timeSinceLastWake = currentTime - lastWakeTime; const timeSinceLastCall = currentTime - lastCallTime; - const timeUntilNextCall = Math.max(interval - timeSinceLastCall, 0); + const timeUntilNextCall = interval - timeSinceLastCall; lastWakeTime = currentTime; // For the streaming protocol: there is nothing obviously stopping this @@ -1090,6 +1094,14 @@ export function makeInterruptableAsyncInterval( if (timeUntilNextCall > minInterval) { reschedule(minInterval); } + + // This is possible in virtualized environments like AWS Lambda where our + // clock is unreliable. In these cases the timer is "running" but never + // actually completes, so we want to execute immediately and then attempt + // to reschedule. + if (timeUntilNextCall < 0) { + executeAndReschedule(); + } } function stop() { @@ -1114,7 +1126,7 @@ export function makeInterruptableAsyncInterval( function executeAndReschedule() { lastWakeTime = 0; - lastCallTime = now(); + lastCallTime = clock(); fn(err => { if (err) throw err; @@ -1125,7 +1137,7 @@ export function makeInterruptableAsyncInterval( if (immediate) { executeAndReschedule(); } else { - lastCallTime = now(); + lastCallTime = clock(); reschedule(undefined); } diff --git a/test/unit/utils.test.js b/test/unit/utils.test.js index a701a02fa85..bae78fd89f4 100644 --- a/test/unit/utils.test.js +++ b/test/unit/utils.test.js @@ -114,5 +114,51 @@ describe('utils', function () { this.clock.tick(250); }); + + it('should immediately schedule if the clock is unreliable', function (done) { + let clockCalled = 0; + let lastTime = now(); + const marks = []; + const executor = makeInterruptableAsyncInterval( + callback => { + marks.push(now() - lastTime); + lastTime = now(); + callback(); + }, + { + interval: 50, + minInterval: 10, + immediate: true, + clock() { + clockCalled += 1; + + // needs to happen on the third call because `wake` checks + // the `currentTime` at the beginning of the function + if (clockCalled === 3) { + return now() - 100000; + } + + return now(); + } + } + ); + + // force mark at 20ms, and then the unreliable system clock + // will report a very stale `lastCallTime` on this mark. + setTimeout(() => executor.wake(), 10); + + // try to wake again in another `minInterval + immediate`, now + // using a very old `lastCallTime`. This should result in an + // immediate scheduling: 0ms (immediate), 20ms (wake with minIterval) + // and then 10ms for another immediate. + setTimeout(() => executor.wake(), 30); + + setTimeout(() => { + executor.stop(); + expect(marks).to.eql([0, 20, 10, 50, 50, 50, 50]); + done(); + }, 250); + this.clock.tick(250); + }); }); });