From 9f5e17b06448a1a96819ab4e5291a416b2a98bfa Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Sat, 22 Aug 2015 13:46:36 -0500 Subject: [PATCH 1/2] timers: fixing API refs to use safe internal refs Added safe internal references for 'clearTimeout(..)', 'active(..)', and 'unenroll(..)'. Changed various API refs from 'export.*' to use these safe internal references. Now, overwriting the global API identifiers does not create potential breakage and/or race conditions. See Issue #2493. --- lib/timers.js | 16 ++++++++-------- test/parallel/test-timers-api-refs.js | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 test/parallel/test-timers-api-refs.js diff --git a/lib/timers.js b/lib/timers.js index 478a05433912f3..dc2506e01e09a4 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -100,7 +100,7 @@ const unrefedLists = {}; // Schedule or re-schedule a timer. // The item must have been enroll()'d first. -exports.active = function(item) { +const active = exports.active = function(item) { insert(item, false); }; @@ -351,19 +351,19 @@ exports.setTimeout = function(callback, after) { if (process.domain) timer.domain = process.domain; - exports.active(timer); + active(timer); return timer; }; -exports.clearTimeout = function(timer) { +const clearTimeout = exports.clearTimeout = function(timer) { if (timer && (timer[kOnTimeout] || timer._onTimeout)) { timer[kOnTimeout] = timer._onTimeout = null; if (timer instanceof Timeout) { timer.close(); // for after === 0 } else { - exports.unenroll(timer); + unenroll(timer); } } }; @@ -409,7 +409,7 @@ exports.setInterval = function(callback, repeat) { timer._repeat = ontimeout; if (process.domain) timer.domain = process.domain; - exports.active(timer); + active(timer); return timer; @@ -425,7 +425,7 @@ exports.setInterval = function(callback, repeat) { this._handle.start(repeat, 0); } else { timer._idleTimeout = repeat; - exports.active(timer); + active(timer); } } }; @@ -468,7 +468,7 @@ Timeout.prototype.unref = function() { // Prevent running cb again when unref() is called during the same cb if (this._called && !this._repeat) { - exports.unenroll(this); + unenroll(this); return; } @@ -496,7 +496,7 @@ Timeout.prototype.close = function() { this._handle[kOnTimeout] = null; this._handle.close(); } else { - exports.unenroll(this); + unenroll(this); } return this; }; diff --git a/test/parallel/test-timers-api-refs.js b/test/parallel/test-timers-api-refs.js new file mode 100644 index 00000000000000..00d1c1355fe045 --- /dev/null +++ b/test/parallel/test-timers-api-refs.js @@ -0,0 +1,20 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +// don't verify the globals for this test +common.globalCheck = false; + +// try overriding global APIs to make sure +// they're not relied on by the timers +global.clearTimeout = assert.fail; + +// run timeouts/intervals through the paces +const intv = setInterval(function() {}, 1); + +setTimeout(function() { + clearInterval(intv); +}, 100); + +setTimeout(function() {}, 2); + From e1f5bb814dce9d9ac7fb3178a5a79fd7ccd3930c Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 24 Mar 2016 15:11:36 -0700 Subject: [PATCH 2/2] test: confirm globals not used internally --- test/parallel/test-timers-api-refs.js | 29 ++++++++++++++------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/test/parallel/test-timers-api-refs.js b/test/parallel/test-timers-api-refs.js index 00d1c1355fe045..c062369444b1e0 100644 --- a/test/parallel/test-timers-api-refs.js +++ b/test/parallel/test-timers-api-refs.js @@ -1,20 +1,21 @@ 'use strict'; const common = require('../common'); -const assert = require('assert'); +const timers = require('timers'); -// don't verify the globals for this test -common.globalCheck = false; +// delete global APIs to make sure they're not relied on by the internal timers +// code +delete global.setTimeout; +delete global.clearTimeout; +delete global.setInterval; +delete global.clearInterval; +delete global.setImmediate; +delete global.clearImmediate; -// try overriding global APIs to make sure -// they're not relied on by the timers -global.clearTimeout = assert.fail; +const timeoutCallback = () => { timers.clearTimeout(timeout); }; +const timeout = timers.setTimeout(common.mustCall(timeoutCallback), 1); -// run timeouts/intervals through the paces -const intv = setInterval(function() {}, 1); - -setTimeout(function() { - clearInterval(intv); -}, 100); - -setTimeout(function() {}, 2); +const intervalCallback = () => { timers.clearInterval(interval); }; +const interval = timers.setInterval(common.mustCall(intervalCallback), 1); +const immediateCallback = () => { timers.clearImmediate(immediate); }; +const immediate = timers.setImmediate(immediateCallback);