stack overflow running tests in node 0.10

[aredridel]

stack overflow running tests in node 0.10

[cscott]

I know mocha had similar issues, see mochajs/mocha#777 -- so this might be a bug in the test runner, not in html5 itself.

[metamatt]

Note that html5 is using node-tap, not mocha, as its test runner.

I expect it is a similar issue where nextTick should be replaced with setImmediate, though.

I find stack overflows really painful to troubleshoot in the current Node releases because you get no context (call stack) at all. This is actually due to V8, not Node, and V8 fixed this a while ago, and I've been eagerly awaiting it showing up in Node (it showed up in V8 3.15, Node 0.10 is using V8 3.14, Node 0.11 unstable cycle releases are now at V8 3.16.17) so I built node from current 0.11 bleeding edge source the other day to try this out. For this issue I get this stack trace:

RangeError: Maximum call stack size exceeded
    at node.js
    at maxTickWarn (node.js:379:9)
    at process.nextTick (node.js:480:9)
    at onwrite (_stream_writable.js:250:15)
    at WritableState.onwrite (_stream_writable.js:92:5)
    at WriteStream.Socket._write (net.js:634:5)
    at doWrite (_stream_writable.js:211:10)
    at writeOrBuffer (_stream_writable.js:201:5)
    at WriteStream.Writable.write (_stream_writable.js:172:11)
    at WriteStream.Socket.write (net.js:596:40)

which is something, but not enough to go on. Invoking node with --throw-deprecation turns out to be more helpful, as it gives us a call stack from the first offense (when process.nextTick finds recursive invocations), not the last offense (when it's happened so many times the stack overflows). That gives us

Error: (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
    at maxTickWarn (node.js:375:15)
    at process.nextTick (node.js:480:9)
    at onwrite (_stream_writable.js:250:15)
    at WritableState.onwrite (_stream_writable.js:92:5)
    at WriteStream.Socket._write (net.js:634:5)
    at doWrite (_stream_writable.js:211:10)
    at writeOrBuffer (_stream_writable.js:201:5)
    at WriteStream.Writable.write (_stream_writable.js:172:11)
    at WriteStream.Socket.write (net.js:596:40)
    at TapProducer.ondata (stream.js:51:26)
    at TapProducer.EventEmitter.emit (events.js:95:17)
    at TapProducer.write (/Users/magi/src/open/html5/node_modules/tap/lib/tap-producer.js:37:8)

Still not enough frames to see all the way from one nextTick call to the recursive one, but it does point the finger generally in tap's direction. Without taking the time to deeply understand the tap code, I found two uses of process.nextTick in its codebase proper, changed both to setImmediate, and reran the tree-construction-test.js, and it does survive the test25.dat-19 test case that stack-overflowed before.

However the tests run much more slowly, and basically tie up the CPU. So this doesn't seem like a good idea.

[aredridel]

Oh, excellent! Thank you!

[aredridel]

This should be mostly better now -- can you try?