Repository navigation
v11.11.0 timeout.refresh() does not seem to prevent event loop from exiting #26642
Description
- Version:
v11.11.0 - Platform:
Alpine Linux v3.9run inside Docker onLinux 03891e4323dc 5.0.0-arch1-1-ARCH #1 SMP PREEMPT Mon Mar 4 14:11:43 UTC 2019 x86_64 Linux - Subsystem:
Timers
When using timeout.refresh to keep re-running some code, Node.js v11.x exits process, even though timeout is supposed to be "ref'ed" by default (and it is: timeout.hasRef() returns true every time).
I am not 100% sure, but i think it might be caused by this line:
Line 232 in 32853c0
Shouldn't it look like this instead?
if (refed !== item[kRefed]) {Problem is 100% reproducible with following example (save it to file and run, e.g., node test-timer.js or DEBUG=1 node test-timer.js):
process.on('exit', () => console.log('Bye!'));
const DEBUG = Number(process.env.DEBUG) || false;
var countdown = 2;
var timer = setTimeout(run, 0);
function run () {
console.log('run', countdown--);
if (countdown < 1) {
console.log('all done');
clearTimeout(timer);
return;
}
timer.refresh();
// Uncomment next line to make this work on Node.js v11.x
// timer = setTimeout(run, 0);
if (DEBUG) {
console.debug('has ref?', timer.hasRef ? timer.hasRef() : timer);
}
}When run on v11.11.0, actual output is incorrect:
run 2
Bye!When run on v10.15.3, actual output is correct:
run 2
run 1
all done
Bye!Changing amount of time, e.g., from 0 to 1000, does not change the output.
Number assigned to countdown does not matter either, as long as it's greater than 1 ;).
Activity
ahwayakchih commentedon Mar 13, 2019
My guess about possible buggy line was wrong. I changed it but it did not help. Sorry.
But that line is part of the problem. When commented out,
refCountis correctly updated and process does not exit. With that line,refCountis incremented only once, wheninsertis called for the first time.But
refCountis decremented every time timeout function is called:node/lib/timers.js
Lines 345 to 346 in 32853c0
Shouldn't call to
timeout.refresh()somehow incrementrefCount? At least, if timeout function was called before. If it was not called yet,refCountwas not decremented, so it should not be incremented.At the moment, very ugly and hacky workaround is to useNevermind: that would keep repeating the function even when it shouldn't.timer._repeat = trueinstead of callingtimer.refresh(), andtimer._repeat = falsewhen it's time to stop it :(.starkwang commentedon Mar 14, 2019
Reproduced in 11.11.0 and the master.
starkwang commentedon Mar 14, 2019
cc @nodejs/timers
timers: fix refresh inside callback
timers: fix refresh inside callback