kriskowal / asap

High-priority task queue for Node.js and browsers
MIT License
608 stars 46 forks source link

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

Open stefanpenner opened 9 years ago

stefanpenner commented 9 years ago

In node 0.10.x Cross library async interleaving can result in (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.

Although, asap typically avoids this with its own micro task queue:

https://github.com/kriskowal/asap/blob/master/raw.js#L86-L96

In some scenarios, it is possible to interleave two such micro tasks queues in a way that still results in the limit being hit.

In addition to interleaving between different libraries due to the node_module's dupe friendly module resolution strategy, it is common to have multiple copies of the same library present and interacting with one another.

A quick example:

var A = require('./path/to/rsvp_a').Promise;
var B = require('./path/to/rsvp_b').Promise;

A.resolve().then(function() {
  console.log('first nextTick');
  B.resolve().then(function() {
    console.log('second nextTick');
    A.resolve().then(function() {
      console.log('third nextTick');
    });
  });
});

An example failing test thanks to @taras https://github.com/tildeio/rsvp.js/pull/337/files#diff-e7e77ddad631a023d39a62f3ba8b7f17R2524

this limit has been removed in node 0.11.0

https://github.com/iojs/io.js/commit/0761c90204d7a0134c657e20f91bd83bfa6e677a https://github.com/iojs/io.js/commit/0761c90204d7a0134c657e20f91bd83bfa6e677a

Unfortunately our solution was to fallback to setImmediate in node 0.10.x.

I wasn't able to think of a better solution, but that doesn't mean a better one doesn't exist so if someone has one feel free to share :)

related: https://github.com/tildeio/rsvp.js/pull/337 https://github.com/cujojs/when/issues/410 https://github.com/petkaantonov/bluebird/issues/399

kriskowal commented 9 years ago

Sounds to me like we have to fall back to setImmediate on Node 0.10. Pull requests welcome.