Fix process.nextTick throw call sites

felixge · isaacs · commit 814033365b14 · 2012-05-09T11:54:43.000-07:00
This patch now reports the proper throw call site for exceptions
triggered within process.nextTick. So instead of this:

node.js:201
        throw e; // process.nextTick error, or 'error' event on first tick
              ^

You will now see:

mydir/myscript.js:15
  throw new Error('My Error');
          ^

From my testing this patch causes no performance regressions, but does
greatly simplify processing the nextTickQueue.
diff --git a/src/node.cc b/src/node.cc
@@ -255,9 +255,7 @@ static void Spin(uv_idle_t* handle, int status) {
   Tick();
 }
 
-
-static Handle<Value> NeedTickCallback(const Arguments& args) {
-  HandleScope scope;
+static void StartTickSpinner() {
   need_tick_cb = true;
   // TODO: this tick_spinner shouldn't be necessary. An ev_prepare should be
   // sufficent, the problem is only in the case of the very last "tick" -
@@ -268,9 +266,12 @@ static Handle<Value> NeedTickCallback(const Arguments& args) {
     uv_idle_start(&tick_spinner, Spin);
     uv_ref(uv_default_loop());
   }
-  return Undefined();
 }
 
+static Handle<Value> NeedTickCallback(const Arguments& args) {
+  StartTickSpinner();
+  return Undefined();
+}
 
 static void PrepareTick(uv_prepare_t* handle, int status) {
   assert(handle == &prepare_tick_watcher);
@@ -1694,6 +1695,9 @@ void FatalException(TryCatch &try_catch) {
   emit->Call(process, 2, event_argv);
   // Decrement so we know if the next exception is a recursion or not
   uncaught_exception_counter--;
+
+  // This makes sure uncaught exceptions don't interfere with process.nextTick
+  StartTickSpinner();
 }
 
 
diff --git a/src/node.js b/src/node.js
@@ -180,26 +180,18 @@
 
   startup.processNextTick = function() {
     var nextTickQueue = [];
+    var nextTickIndex = 0;
 
     process._tickCallback = function() {
-      var l = nextTickQueue.length;
-      if (l === 0) return;
+      var nextTickLength = nextTickQueue.length;
+      if (nextTickLength === 0) return;
 
-      var q = nextTickQueue;
-      nextTickQueue = [];
-
-      try {
-        for (var i = 0; i < l; i++) q[i]();
-      }
-      catch (e) {
-        if (i + 1 < l) {
-          nextTickQueue = q.slice(i + 1).concat(nextTickQueue);
-        }
-        if (nextTickQueue.length) {
-          process._needTickCallback();
-        }
-        throw e; // process.nextTick error, or 'error' event on first tick
+      while (nextTickIndex < nextTickLength) {
+        nextTickQueue[nextTickIndex++]();
       }
+
+      nextTickQueue.splice(0, nextTickIndex);
+      nextTickIndex = 0;
     };
 
     process.nextTick = function(callback) {
diff --git a/test/message/stack_overflow.out b/test/message/stack_overflow.out
@@ -1,6 +1,6 @@
 before
 
-node.js:*
-        throw e; // process.nextTick error, or 'error' event on first tick
-              ^
+module.js:311
+    throw err;
+          ^
 RangeError: Maximum call stack size exceeded
diff --git a/test/message/throw_custom_error.out b/test/message/throw_custom_error.out
@@ -1,6 +1,6 @@
 before
 
-node.js:*
-        throw e; // process.nextTick error, or 'error' event on first tick
-              ^
+module.js:311
+    throw err;
+          ^
 MyCustomError: This is a custom message
diff --git a/test/message/throw_non_error.out b/test/message/throw_non_error.out
@@ -1,6 +1,6 @@
 before
 
-node.js:*
-        throw e; // process.nextTick error, or 'error' event on first tick
-              ^
+module.js:311
+    throw err;
+          ^
 [object Object]
diff --git a/test/message/undefined_reference_in_new_context.out b/test/message/undefined_reference_in_new_context.out
@@ -1,8 +1,8 @@
 before
 
-node.js:*
-        throw e; // process.nextTick error, or 'error' event on first tick
-              ^
+module.js:311
+    throw err;
+          ^
 ReferenceError: foo is not defined
     at evalmachine.<anonymous>:*
     at Object.<anonymous> (*test*message*undefined_reference_in_new_context.js:*)
