diff --git a/lib/timers.js b/lib/timers.js index 24dc7e1c263c7e..b2c1c3202e2fcb 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -276,7 +276,7 @@ exports.setInterval = function(callback, repeat) { timer._repeat(); // Timer might be closed - no point in restarting it - if (!timer._repeat) + if (timer._idleTimeout === -1) return; // If timer is unref'd (or was - it's permanently removed from the list.) @@ -352,6 +352,7 @@ Timeout.prototype.ref = function() { Timeout.prototype.close = function() { this._onTimeout = null; if (this._handle) { + this._idleTimeout = -1; this._handle[kOnTimeout] = null; this._handle.close(); } else { diff --git a/test/fixtures/test-timer-disarm.js b/test/fixtures/test-timer-disarm.js new file mode 100644 index 00000000000000..60a51d2cd9269b --- /dev/null +++ b/test/fixtures/test-timer-disarm.js @@ -0,0 +1,38 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const T = require('timers'); + +// This test is to make sure the timer will not fire again +// when it is disarmed in callback. +// This script is called by +// ../parallel/test-timers-disarmed-in-callbak-not-fire-anymore.js +// with NODE_DEBUG=timer to make sure 'active' and 'listOnTimeout' +// will not be called again after the timer is disarmed. +// Before fix https://github.com/nodejs/node/pull/4303, +// When disarm with clearInterval, it works fine. +// When disarm with close, active and listOnTimeout will be called again. +// When disarm with unenroll, timer still works. +// After this fix, timer behaves the same as clearTimeout/clearInterval +// when it is disared by close/unenroll in callback. + +var nbIntervalFired1 = 0; +var nbIntervalFired2 = 0; +var nbIntervalFired3 = 0; +const timer1 = setInterval(() => { + nbIntervalFired1++; + clearInterval(timer1); +}, 11); +const timer2 = setInterval(() => { + nbIntervalFired2++; + timer2.close(); +}, 12); +const timer3 = setInterval(() => { + nbIntervalFired3++; + T.unenroll(timer3); +}, 13); +setTimeout(() => { + assert.strictEqual(nbIntervalFired1, 1); + assert.strictEqual(nbIntervalFired2, 1); + assert.strictEqual(nbIntervalFired3, 1); +}, 100); diff --git a/test/parallel/test-timers-disarmed-in-callback-not-fire-anymore.js b/test/parallel/test-timers-disarmed-in-callback-not-fire-anymore.js new file mode 100644 index 00000000000000..4ebc94c70304de --- /dev/null +++ b/test/parallel/test-timers-disarmed-in-callback-not-fire-anymore.js @@ -0,0 +1,31 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const exec = require('child_process').exec; + +const testPath = './test/fixtures/test-timer-disarm.js'; +const testCommand = 'NODE_DEBUG=timer ' + process.execPath + ' ' + testPath; + +setTimeout(() => { + exec(testCommand, (err, stdout, stderr) => { + const o = stderr.split('\n'); + let timer1Fired = 0; + let timer2Fired = 0; + let timer3Fired = 0; + for (var i = 0; i < o.length; i++) { + var line = o[i]; + if (line.indexOf('timeout callback 11') >= 0) { + timer1Fired++; + } + if (line.indexOf('timeout callback 12') >= 0) { + timer2Fired++; + } + if (line.indexOf('timeout callback 13') >= 0) { + timer3Fired++; + } + } + assert.strictEqual(timer1Fired, 1); + assert.strictEqual(timer2Fired, 1); + assert.strictEqual(timer3Fired, 1); + }); +}, 100);