timer: make sure timer is cleared

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 {

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

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