v11.11.0 `timeout.refresh()` does not seem to prevent event loop from exiting

[ahwayakchih]

• Version: v11.11.0
• Platform: Alpine Linux v3.9 run inside Docker on Linux 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:

node/lib/timers.js

Line 232 in 32853c0

if (refed === !item[kRefed]) {

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 ;).

[ahwayakchih]

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, refCount is correctly updated and process does not exit. With that line, refCount is incremented only once, when insert is called for the first time.
But refCount is decremented every time timeout function is called:

node/lib/timers.js

Lines 345 to 346 in 32853c0

	if (timer[kRefed])
	refCount--;

Shouldn't call to timeout.refresh() somehow increment refCount? At least, if timeout function was called before. If it was not called yet, refCount was not decremented, so it should not be incremented.

At the moment, very ugly and hacky workaround is to use timer._repeat = true instead of calling timer.refresh(), and timer._repeat = false when it's time to stop it :(. Nevermind: that would keep repeating the function even when it shouldn't.

[starkwang]

Reproduced in 11.11.0 and the master.

[starkwang]

cc @nodejs/timers