fake-timers icon indicating copy to clipboard operation
fake-timers copied to clipboard

setInterval with delay 0 runs indefinitely, other scheduled tasks never execute

Open ford-retool opened this issue 1 year ago • 3 comments

  • FakeTimers version : 14.0.0
  • Environment : node (standalone and used as part of jest fake timers)
  • Example URL : reproduction repo
  • Other libraries you are using: Not necessary to reproduce, but issue is worse when used with jest and react testing library

What did you expect to happen? If an interval and multiple timers are all scheduled to execute at the same time, I would expect all of them to execute once, prior to intervals executing again

What actually happens The interval with a delay of 0 runs repeatedly, and timeouts that are due to run are never executed. This is especially problematic when used in conjunction with jest and react testing library. RTL's waitFor method, when used with fake timers, relies on calling an immediate setTimeout and the advancing timers by 0. If the code under test uses a setInterval with a delay of 0, the test hangs forever. The spec in the reproduction repo demonstrates this without the use of RTL.

How to reproduce

The following code using fake timers

const FakeTimers = require("@sinonjs/fake-timers");

var clock = FakeTimers.install();

let intervalCount = 0;
const intervalId = setInterval(() => {
  console.log("interval");
  if (++intervalCount > 4) {
    clearInterval(intervalId);
  }
}, 0);
setTimeout(() => console.log("timeout"), 0);

clock.tick(0);

prints

interval
interval
interval
interval
interval
timeout

An example without fake timers, such as

let intervalCount = 0;
const intervalId = setInterval(() => {
  console.log("interval");
  if (intervalCount++ > 4) {
    clearInterval(intervalId);
  }
}, 0);

setTimeout(() => console.log("timeout"), 0);

prints

interval
timeout
interval
interval
interval
interval
interval

Additional Information In debugging this, it seems that the issue is that when intervals are executed, their callAt time is adjusted by the interval, but their position in the list of timers is not adjusted, so the interval continues to be the first timer to run. https://github.com/sinonjs/fake-timers/blob/48f089fdc830e39fcec31dd23099cc360da0bab2/src/fake-timers-src.js#L794

ford-retool avatar Dec 13 '24 20:12 ford-retool

Excellent report! Thanks for taking the time. This seems doable, but I can't do this in the immediate future. Unless you can see a quick fix (which it seemed lik you did) and can supply a PR, I can only say I'll have a look at this in the start of January. Bit busy now just before X-mas :)

fatso83 avatar Dec 15 '24 22:12 fatso83

No rush at all on my end - it was easy enough to work around in this case since having an delay of 1 in the actual code was functionally equivalent.

ford-retool avatar Dec 16 '24 16:12 ford-retool

I started working on this last night, moving our current hashmap approach into one using lists instead. The idea being that I could then pop a timer of the list for the current time and add it to the end of a (sub)list to avoid the issue above. Before getting to fixing that, I first need to get to feature parity with the current approach - basically making the test suite pass. I got mosts of the internal tests to work, but pretty much all the /tests/issue*.js tests still breaks, so still some more work to do. I could not immediately see what was wrong, so if anyone has some time to have a look, it's here: https://github.com/sinonjs/fake-timers/tree/522-tasks-not-adjusted-on-settimeout-0

fatso83 avatar Dec 31 '24 12:12 fatso83