Skip to content

Commit

Permalink
timers: fixing API refs to use safe internal refs
Browse files Browse the repository at this point in the history
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.

PR-URL: #5882
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Fixes: #2493
  • Loading branch information
getify authored and evanlucas committed Mar 31, 2016
1 parent cb676cf commit 7ec5397
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
16 changes: 8 additions & 8 deletions lib/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};

Expand Down Expand Up @@ -346,19 +346,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);
}
}
};
Expand Down Expand Up @@ -400,7 +400,7 @@ exports.setInterval = function(callback, repeat) {
timer._repeat = ontimeout;

if (process.domain) timer.domain = process.domain;
exports.active(timer);
active(timer);

return timer;

Expand All @@ -416,7 +416,7 @@ exports.setInterval = function(callback, repeat) {
this._handle.start(repeat, 0);
} else {
timer._idleTimeout = repeat;
exports.active(timer);
active(timer);
}
}
};
Expand Down Expand Up @@ -459,7 +459,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;
}

Expand Down Expand Up @@ -487,7 +487,7 @@ Timeout.prototype.close = function() {
this._handle[kOnTimeout] = null;
this._handle.close();
} else {
exports.unenroll(this);
unenroll(this);
}
return this;
};
Expand Down
20 changes: 20 additions & 0 deletions test/parallel/test-timers-api-refs.js
Original file line number Diff line number Diff line change
@@ -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);

0 comments on commit 7ec5397

Please sign in to comment.