From 3eecdf9f1422640c7439507cb39ffb3dff57b3e4 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 16 Oct 2015 15:34:15 -0400 Subject: [PATCH] timers: reuse timer in `setTimeout().unref()` Instead of creating new timer - reuse the timer from the freelist. This won't make the freelist timer active for the duration of `uv_close()`, and will let the event-loop exit properly. Fix: #1264 PR-URL: https://github.com/nodejs/node/pull/3407 Reviewed-By: Trevor Norris Reviewed-By: Jeremiah Senkpiel --- lib/timers.js | 29 ++++++++++++++----- .../test-timers-unrefed-in-beforeexit.js | 20 +++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-timers-unrefed-in-beforeexit.js diff --git a/lib/timers.js b/lib/timers.js index 50ae477d59..b75b7ccfc0 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -119,16 +119,27 @@ function listOnTimeoutNT(list) { } -const unenroll = exports.unenroll = function(item) { +function reuse(item) { L.remove(item); var list = lists[item._idleTimeout]; - // if empty then stop the watcher - debug('unenroll'); + // if empty - reuse the watcher if (list && L.isEmpty(list)) { + debug('reuse hit'); + list.stop(); + delete lists[item._idleTimeout]; + return list; + } + + return null; +} + + +const unenroll = exports.unenroll = function(item) { + var list = reuse(item); + if (list) { debug('unenroll: list empty'); list.close(); - delete lists[item._idleTimeout]; } // if active is called later, then we want to make sure not to insert again item._idleTimeout = -1; @@ -312,12 +323,16 @@ Timeout.prototype.unref = function() { if (!this._idleStart) this._idleStart = now; var delay = this._idleStart + this._idleTimeout - now; if (delay < 0) delay = 0; - exports.unenroll(this); // Prevent running cb again when unref() is called during the same cb - if (this._called && !this._repeat) return; + if (this._called && !this._repeat) { + exports.unenroll(this); + return; + } + + var handle = reuse(this); - this._handle = new Timer(); + this._handle = handle || new Timer(); this._handle.owner = this; this._handle[kOnTimeout] = unrefdHandle; this._handle.start(delay, 0); diff --git a/test/parallel/test-timers-unrefed-in-beforeexit.js b/test/parallel/test-timers-unrefed-in-beforeexit.js new file mode 100644 index 0000000000..0ac3311816 --- /dev/null +++ b/test/parallel/test-timers-unrefed-in-beforeexit.js @@ -0,0 +1,20 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +var once = 0; + +process.on('beforeExit', () => { + if (once > 1) + throw new RangeError('beforeExit should only have been called once!'); + + setTimeout(() => {}, 1).unref(); + once++; +}); + +process.on('exit', (code) => { + if (code !== 0) return; + + assert.strictEqual(once, 1); +});