Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cross library interleaving can cause node 10's nextTick limit to be hit. #337

Merged
merged 2 commits into from Dec 6, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .travis.yml
@@ -1,6 +1,7 @@
language: node_js
node_js:
- '0.10'
- '0.11'
after_success:
- "./bin/publish_to_s3.js"
env:
Expand Down
12 changes: 10 additions & 2 deletions lib/rsvp/asap.js
Expand Up @@ -15,6 +15,7 @@ export default function asap(callback, arg) {
var browserWindow = (typeof window !== 'undefined') ? window : undefined
var browserGlobal = browserWindow || {};
var BrowserMutationObserver = browserGlobal.MutationObserver || browserGlobal.WebKitMutationObserver;
var isNode = typeof process !== 'undefined' && {}.toString.call(process) === '[object process]';

// test for web worker but not in IE10
var isWorker = typeof Uint8ClampedArray !== 'undefined' &&
Expand All @@ -23,8 +24,15 @@ var isWorker = typeof Uint8ClampedArray !== 'undefined' &&

// node
function useNextTick() {
var nextTick = process.nextTick;
// node version 0.10.x displays a deprecation warning when nextTick is used recursively
// setImmediate should be used instead instead
var version = process.versions.node.match(/^(?:(\d+)\.)?(?:(\d+)\.)?(\*|\d+)$/);
if (Array.isArray(version) && version[1] === '0' && version[2] === '10') {
nextTick = setImmediate;
}
return function() {
process.nextTick(flush);
nextTick(flush);
};
}

Expand Down Expand Up @@ -88,7 +96,7 @@ function attemptVertex() {

var scheduleFlush;
// Decide what async method to use to triggering processing of queued callbacks:
if (typeof process !== 'undefined' && {}.toString.call(process) === '[object process]') {
if (isNode) {
scheduleFlush = useNextTick();
} else if (BrowserMutationObserver) {
scheduleFlush = useMutationObserver();
Expand Down
34 changes: 34 additions & 0 deletions test/tests/extension-test.js
Expand Up @@ -2520,3 +2520,37 @@ describe("Thenables should not be able to run code during assimilation", functio
assert.strictEqual(thenCalled, false);
});
});

describe("on node 0.10.x, using process.nextTick recursively shows deprecation warning. setImmediate should be used instead.", function() {
// (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.

it("should not cause 'RangeError: Maximum call stack size exceeded'", function (done) {
var total = 1000;

var resolved = 0;
var nextTick = function(depth) {
if (depth >= total) {
return RSVP.resolve();
}
var d = depth + 1;
//console.log('entered:', d);
return new RSVP.Promise(function(resolve){
process.nextTick(function(){
nextTick(d).then(function(){
resolved++;
//console.log('resolving:', d);
resolve();
});
});
});
};

nextTick(0)
.then(function(){
//console.log('nextTick: final');
assert.strictEqual(resolved, total);
done();
});
});

});