meteor / promise

ES6 Promise polyfill with Fiber support
MIT License
63 stars 15 forks source link

Infinite loop in Node.js 7.0.0 and above #18

Closed corwin-of-amber closed 7 years ago

corwin-of-amber commented 7 years ago

I have been investigating this for a few days. As it seems, in Node starting version 7.0.0 (perhaps even earlier?), when a resolving function returns a Promise, .then() is called on that promise with new resolving functions (per http://www.ecma-international.org/ecma-262/6.0/#sec-promise-resolve-functions). This causes an infinite loop that keeps creating fibers trying to resolve the promise. Here is a short snippet that shows it:

f = require('fibers');
require('meteor-promise').makeCompatible(Promise, f);

new Promise(function(r) { r() }).then(function(x) { return x; });

With Meteor's Node.js 4.8.0, this program finishes immediately. When run with Node.js 7.0.0, it never terminates. I edited promise_server.js and added a printf in wrapCallback:

function wrapCallback(callback, Promise, dynamics) {
  console.log("[promise] wrapCallback", callback);
  ...

Sure enough, the output is:

[promise] wrapCallback function () {}
[promise] wrapCallback undefined
[promise] wrapCallback function t() { [native code] }
[promise] wrapCallback function u() { [native code] }
[promise] wrapCallback function t() { [native code] }
[promise] wrapCallback function u() { [native code] }
[promise] wrapCallback function t() { [native code] }
[promise] wrapCallback function u() { [native code] }
[promise] wrapCallback function t() { [native code] }
[promise] wrapCallback function u() { [native code] }
[promise] wrapCallback function t() { [native code] }
[promise] wrapCallback function u() { [native code] }
[promise] wrapCallback function t() { [native code] }
 ·
 ·
 ·

These pesky function t() and function u() are the new resolving functions. The fix from pull request #11 does not help in this case because they are created afresh every time. I suggest to fix it by attaching the same expando _meteorPromiseAlreadyWrapped to Promise objects created by FiberPool.

I am embarrassed to make a pull request for this since I did not write an appropriate Mocha test case, so for the time being I am just attaching the diff.

meteor-promise.diff

benjamn commented 7 years ago

Thanks for investigating this problem, @corwin-of-amber! The callback spew you're talking about reminds me of what I saw (but didn't really understand) the last time I was trying to run Meteor on Node 6 (https://github.com/meteor/meteor/pull/6923), so I'm hoping your fix moves that effort forward.

corwin-of-amber commented 7 years ago

You are very welcome! BTW I am running (an experimental version of) Meteor's master branch on my Node 7 right now. I am going to check whether any of Meteor's functionality has been affected. I patched a few source files to bypass the dev_bundle and use installed versions of node, npm, mongod etc. I even used Chrome Dev-tools to debug the artifact. I will be happy to help if you are considering to continue the porting efforts.

benjamn commented 7 years ago

@corwin-of-amber Please do create a pull request, so that the existing tests will run. We can always push additional tests to the PR branch later.

jedwards1211 commented 7 years ago

I just independently discovered this bug in the structure of the code, but I still don't understand why this issue wasn't happening on Node 5 and below...