nextTick: Preserve depth in error/reentry cases

When there is an error that is thrown in a nextTick function, which is
then handled by a domain or other process.on('uncaughtException')
handler, if the error handler *also* adds a nextTick and triggers
multiple MakeCallback events (ie, by doing some I/O), then it would
skip over the tickDepth check, resulting in an infinite spin.

Solution: Check the tickDepth at the start of the tick processing, and
preserve it when we are cleaning up in the error case or exiting early
in the re-entry case.

In order to make sure that tick callbacks are *eventually* handled, any
callback triggered by the underlying spinner in libuv will be processed
as if starting from a tick depth of 0.

src/node.cc

@@ -247,7 +247,9 @@ static void Tick(void) {
 
   TryCatch try_catch;
 
-  cb->Call(process, 0, NULL);
+  // Let the tick callback know that this is coming from the spinner
+  Handle<Value> argv[] = { True() };
+  cb->Call(process, ARRAY_SIZE(argv), argv);
 
   if (try_catch.HasCaught()) {
     FatalException(try_catch);

src/node.js

@@ -260,8 +260,8 @@
     // limit is hit, which is not ideal, but not terrible.
     process.maxTickDepth = 1000;
 
-    function tickDone() {
-      tickDepth = 0;
+    function tickDone(tickDepth_) {
+      tickDepth = tickDepth_ || 0;
       nextTickQueue.splice(0, nextTickIndex);
       nextTickIndex = 0;
       inTick = false;
@@ -270,12 +270,35 @@
       }
     }
 
-    process._tickCallback = function() {
+    process._tickCallback = function(fromSpinner) {
+
+      // if you add a nextTick in a domain's error handler, then
+      // it's possible to cycle indefinitely.  Normally, the tickDone
+      // in the finally{} block below will prevent this, however if
+      // that error handler ALSO triggers multiple MakeCallbacks, then
+      // it'll try to keep clearing the queue, since the finally block
+      // fires *before* the error hits the top level and is handled.
+      if (tickDepth >= process.maxTickDepth) {
+        if (fromSpinner) {
+          // coming in from the event queue.  reset.
+          tickDepth = 0;
+        } else {
+          if (nextTickQueue.length) {
+            process._needTickCallback();
+          }
+          return;
+        }
+      }
+
+      if (!nextTickQueue.length) return tickDone();
+
       if (inTick) return;
       inTick = true;
 
       // always do this at least once.  otherwise if process.maxTickDepth
-      // is set to some negative value, we'd never process any of them.
+      // is set to some negative value, or if there were repeated errors
+      // preventing tickDepth from being cleared, we'd never process any
+      // of them.
       do {
         tickDepth++;
         var nextTickLength = nextTickQueue.length;
@@ -292,7 +315,9 @@
             callback();
             threw = false;
           } finally {
-            if (threw) tickDone();
+            // finally blocks fire before the error hits the top level,
+            // so we can't clear the tickDepth at this point.
+            if (threw) tickDone(tickDepth);
           }
           if (tock.domain) {
             tock.domain.exit();
@@ -316,7 +341,9 @@
       var tock = { callback: callback };
       if (process.domain) tock.domain = process.domain;
       nextTickQueue.push(tock);
-      process._needTickCallback();
+      if (nextTickQueue.length) {
+        process._needTickCallback();
+      }
     };
   };
 

test/simple/test-domain-stack.js

@@ -44,3 +44,7 @@ var foo = a.bind(function() {
 for (var i = 0; i < 1000; i++) {
   process.nextTick(foo);
 }
+
+process.on('exit', function(c) {
+  if (!c) console.log('ok');
+});

test/simple/test-next-tick-error-spin.js

@@ -0,0 +1,69 @@
+// 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.
+
+var common = require('../common');
+var assert = require('assert');
+
+if (process.argv[2] !== 'child') {
+  var spawn = require('child_process').spawn;
+  var child = spawn(process.execPath, [__filename, 'child'], {
+    stdio: 'pipe'//'inherit'
+  });
+  var timer = setTimeout(function() {
+    throw new Error('child is hung');
+  }, 500);
+  child.on('exit', function(code) {
+    console.error('ok');
+    assert(!code);
+    clearTimeout(timer);
+  });
+} else {
+
+  var domain = require('domain');
+  var d = domain.create();
+  process.maxTickDepth = 10;
+
+  // in the error handler, we trigger several MakeCallback events
+  d.on('error', function(e) {
+    console.log('a')
+    console.log('b')
+    console.log('c')
+    console.log('d')
+    console.log('e')
+    f();
+  });
+
+  function f() {
+    process.nextTick(function() {
+      d.run(function() {
+        throw(new Error('x'));
+      });
+    });
+  }
+
+  f();
+  setTimeout(function () {
+    console.error('broke in!');
+    //process.stdout.close();
+    //process.stderr.close();
+    process.exit(0);
+  });
+}