timers: fix this for unref'd timers

nodejs · loyd · Jan 13, 2015 · Jan 13, 2015 · Jan 13, 2015 · Mar 4, 2015
commit ad60db8e4efe78f09c016cae3946d68b3b2cd801
diff --git a/lib/timers.js b/lib/timers.js
@@ -198,7 +198,7 @@ exports.setTimeout = function(callback, after) {
      */
     var args = Array.prototype.slice.call(arguments, 2);
     timer._onTimeout = function() {
-      callback.apply(timer, args);
+      callback.apply(this, args);
     };
   }
 
@@ -242,12 +242,12 @@ exports.setInterval = function(callback, repeat) {
   function wrapper() {
     callback.apply(this, args);
     // If callback called clearInterval().
-    if (timer._repeat === false) return;
+    if (this._repeat === false) return;
     // If timer is unref'd (or was - it's permanently removed from the list.)
     if (this._handle) {
       this._handle.start(repeat, 0);
     } else {
-      timer._idleTimeout = repeat;
+      this._idleTimeout = repeat;
       exports.active(timer);
     }
   }
@@ -281,7 +281,7 @@ Timeout.prototype.unref = function() {
     if (delay < 0) delay = 0;
     exports.unenroll(this);
     this._handle = new Timer();
-    this._handle[kOnTimeout] = this._onTimeout;
+    this._handle[kOnTimeout] = this._onTimeout && this._onTimeout.bind(this);
     this._handle.start(delay, 0);
     this._handle.domain = this.domain;
     this._handle.unref();

diff --git a/test/parallel/test-timers-this.js b/test/parallel/test-timers-this.js
@@ -1,7 +1,8 @@
 var assert = require('assert');
 
-var immediateThis, intervalThis, timeoutThis,
-    immediateArgsThis, intervalArgsThis, timeoutArgsThis;
+var immediateThis, intervalThis, timeoutThis, intervalUnrefThis,
+    timeoutUnrefThis, immediateArgsThis, intervalArgsThis, timeoutArgsThis,
+    intervalUnrefArgsThis, timeoutUnrefArgsThis;
 
 var immediateHandler = setImmediate(function () {
   immediateThis = this;
@@ -11,6 +12,7 @@ var immediateArgsHandler = setImmediate(function () {
   immediateArgsThis = this;
 }, "args ...");
 
+
 var intervalHandler = setInterval(function () {
   clearInterval(intervalHandler);
 
@@ -23,6 +25,21 @@ var intervalArgsHandler = setInterval(function () {
   intervalArgsThis = this;
 }, 0, "args ...");
 
+var intervalUnrefHandler = setInterval(function () {
+  clearInterval(intervalUnrefHandler);
+
+  intervalUnrefThis = this;
+});
+intervalUnrefHandler.unref();
+
+var intervalUnrefArgsHandler = setInterval(function () {
+  clearInterval(intervalUnrefArgsHandler);
+
+  intervalUnrefArgsThis = this;
+}, 0, "args ...");
+intervalUnrefArgsHandler.unref();
+
+
 var timeoutHandler = setTimeout(function () {
   timeoutThis = this;
 });
@@ -31,13 +48,29 @@ var timeoutArgsHandler = setTimeout(function () {
   timeoutArgsThis = this;
 }, 0, "args ...");
 
+var timeoutUnrefHandler = setTimeout(function () {
+  timeoutUnrefThis = this;
+});
+timeoutUnrefHandler.unref();
+
+var timeoutUnrefArgsHandler = setTimeout(function () {
+  timeoutUnrefArgsThis = this;
+}, 0, "args ...");
+timeoutUnrefArgsHandler.unref();
+
+setTimeout(function() {}, 5);
+
 process.once('exit', function () {
   assert.strictEqual(immediateThis, immediateHandler);
   assert.strictEqual(immediateArgsThis, immediateArgsHandler);
 
   assert.strictEqual(intervalThis, intervalHandler);
   assert.strictEqual(intervalArgsThis, intervalArgsHandler);
+  assert.strictEqual(intervalUnrefThis, intervalUnrefHandler);
+  assert.strictEqual(intervalUnrefArgsThis, intervalUnrefArgsHandler);
 
   assert.strictEqual(timeoutThis, timeoutHandler);
   assert.strictEqual(timeoutArgsThis, timeoutArgsHandler);
+  assert.strictEqual(timeoutUnrefThis, timeoutUnrefHandler);
+  assert.strictEqual(timeoutUnrefArgsThis, timeoutUnrefArgsHandler);
 });