From 47335ce56fdede2219233100039981efe5c8efbe Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Wed, 26 Nov 2014 12:27:57 -0800 Subject: [PATCH 1/2] timers: fix unref() memory leak The destructor isn't being called for timers that have been unref'd. Fixes: https://github.com/joyent/node/issues/8364 --- lib/timers.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/timers.js b/lib/timers.js index 668d5536c8186c..302a11e18b5624 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -301,6 +301,14 @@ const Timeout = function(after) { this._repeat = null; }; + +function unrefdHandle() { + this.owner._onTimeout(); + if (!this.owner.repeat) + this.owner.close(); +} + + Timeout.prototype.unref = function() { if (this._handle) { this._handle.unref(); @@ -315,7 +323,8 @@ Timeout.prototype.unref = function() { if (this._called && !this._repeat) return; this._handle = new Timer(); - this._handle[kOnTimeout] = this._onTimeout; + this._handle.owner = this; + this._handle[kOnTimeout] = unrefdHandle; this._handle.start(delay, 0); this._handle.domain = this.domain; this._handle.unref(); From ba676bfff598a0731da2c9153a11c81ded0e5d18 Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Thu, 18 Dec 2014 16:51:08 -0800 Subject: [PATCH 2/2] timers: don't close interval timers when unrefd This change fixes a regression introduced by commit 0d051238be2e07e671d7d9f4f444e0cc1efadf1b, which contained a typo that would cause every unrefd interval to fire only once. Fixes: https://github.com/joyent/node/issues/8900 Reviewed-By: Timothy J Fontaine Reviewed-By: Colin Ihrig Reviewed-by: Trevor Norris --- lib/timers.js | 2 +- .../test-timers-unrefd-interval-still-fires.js | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-timers-unrefd-interval-still-fires.js diff --git a/lib/timers.js b/lib/timers.js index 302a11e18b5624..442789439bc7c2 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -304,7 +304,7 @@ const Timeout = function(after) { function unrefdHandle() { this.owner._onTimeout(); - if (!this.owner.repeat) + if (!this.owner._repeat) this.owner.close(); } diff --git a/test/parallel/test-timers-unrefd-interval-still-fires.js b/test/parallel/test-timers-unrefd-interval-still-fires.js new file mode 100644 index 00000000000000..3ea94454cfdb49 --- /dev/null +++ b/test/parallel/test-timers-unrefd-interval-still-fires.js @@ -0,0 +1,18 @@ +/* + * This test is a regression test for joyent/node#8900. + */ +var assert = require('assert'); + +var N = 5; +var nbIntervalFired = 0; +var timer = setInterval(function() { + ++nbIntervalFired; + if (nbIntervalFired === N) + clearInterval(timer); +}, 1); + +timer.unref(); + +setTimeout(function onTimeout() { + assert.strictEqual(nbIntervalFired, N); +}, 100);