diff --git a/lib/timers.js b/lib/timers.js index de3c9ed6c09f..ca0c201ec978 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -404,6 +404,31 @@ exports.clearImmediate = function(immediate) { var unrefList, unrefTimer; +function _makeTimerTimeout(timer) { + var domain = timer.domain; + + if (!timer._onTimeout) return; + + if (domain && domain._disposed) + return; + + try { + var threw = true; + + if (domain) domain.enter(); + + debug('unreftimer firing timeout'); + L.remove(timer); + timer._onTimeout(); + + threw = false; + + if (domain) + domain.exit(); + } finally { + if (threw) process.nextTick(unrefTimeout); + } +} function unrefTimeout() { var now = Timer.now(); @@ -414,7 +439,7 @@ function unrefTimeout() { var nextTimeoutTime; var nextTimeoutDuration; var minNextTimeoutTime; - var itemToDelete; + var timersToTimeout = []; // The actual timer fired and has not yet been rearmed, // let's consider its next firing time is invalid for now. @@ -445,44 +470,21 @@ function unrefTimeout() { // we scanned through the whole list. minNextTimeoutTime = nextTimeoutTime; } - - // This timer hasn't expired yet, skipping - cur = cur._idlePrev; - continue; + } else { + // We found a timer that expired. Do not call its _onTimeout callback + // right now, as it could mutate any item of the list (including itself). + // Instead, add it to another list that will be processed once the list + // of current timers has been fully traversed. + timersToTimeout.push(cur); } - // We found a timer that expired - var domain = cur.domain; - - if (!cur._onTimeout) continue; - - if (domain && domain._disposed) - continue; - - try { - var threw = true; - - if (domain) domain.enter(); - - itemToDelete = cur; - // Move to the previous item before calling the _onTimeout callback, - // as it can mutate the list. - cur = cur._idlePrev; - - // Remove the timeout from the list because it expired. - L.remove(itemToDelete); - - debug('unreftimer firing timeout'); - itemToDelete._onTimeout(); + cur = cur._idlePrev; + } - threw = false; + var nbTimersToTimeout = timersToTimeout.length; + for (var timerIdx = 0; timerIdx < nbTimersToTimeout; ++timerIdx) + _makeTimerTimeout(timersToTimeout[timerIdx]); - if (domain) - domain.exit(); - } finally { - if (threw) process.nextTick(unrefTimeout); - } - } // Rearm the actual timer with the timeout delay // of the earliest timeout found. diff --git a/test/simple/test-timers-unref-remove-other-unref-timers.js b/test/simple/test-timers-unref-remove-other-unref-timers.js new file mode 100644 index 000000000000..4aac760d08bd --- /dev/null +++ b/test/simple/test-timers-unref-remove-other-unref-timers.js @@ -0,0 +1,46 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +/* + * This test is a regression test for joyent/node#8897. + */ +var timers = require('timers'); + +var foo = new function() { + this._onTimeout = function() {} +}(); + +var bar = new function() { + this._onTimeout = function() { + timers.unenroll(foo); + } +}(); + +// We use timers with expiration times that are sufficiently apart to make +// sure that they're not fired at the same time on platforms where the timer +// resolution is a bit coarse (e.g Windows with a default resolution of ~15ms). +timers.enroll(bar, 1); +timers._unrefActive(bar); + +timers.enroll(foo, 50); +timers._unrefActive(foo); + +setTimeout(function() {}, 100);