process: do not directly schedule _tickCallback in _fatalException

apapirovski · apapirovski · commit 54910929865d · 2017-12-27T14:09:09.000-05:00
When a process encounters a _fatalException that is caught, it should
schedule execution of nextTicks but not in an arbitrary place of the
next Immediates queue. Instead, add a no-op function to the queue
that will ensure processImmediate runs, which will then ensure
that nextTicks are processed at the end.
diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js
@@ -362,6 +362,8 @@
     }
   }
 
+  function noop() {}
+
   function setupProcessFatal() {
     const async_wrap = process.binding('async_wrap');
     // Arrays containing hook flags and ids for async_hook calls.
@@ -372,23 +374,15 @@
             kDefaultTriggerAsyncId, kStackLength } = async_wrap.constants;
 
     process._fatalException = function(er) {
-      var caught;
-
       // It's possible that kDefaultTriggerAsyncId was set for a constructor
       // call that threw and was never cleared. So clear it now.
       async_id_fields[kDefaultTriggerAsyncId] = -1;
 
       if (exceptionHandlerState.captureFn !== null) {
         exceptionHandlerState.captureFn(er);
-        caught = true;
-      }
-
-      if (!caught)
-        caught = process.emit('uncaughtException', er);
-
-      // If someone handled it, then great.  otherwise, die in C++ land
-      // since that means that we'll exit the process, emit the 'exit' event
-      if (!caught) {
+      } else if (!process.emit('uncaughtException', er)) {
+        // If someone handled it, then great.  otherwise, die in C++ land
+        // since that means that we'll exit the process, emit the 'exit' event
         try {
           if (!process._exiting) {
             process._exiting = true;
@@ -397,24 +391,25 @@
         } catch (er) {
           // nothing to be done about it at this point.
         }
+        return false;
+      }
 
+      // If we handled an error, then make sure any ticks get processed
+      // by ensuring that the next Immediate cycle isn't empty
+      NativeModule.require('timers').setImmediate(noop);
+
+      // Emit the after() hooks now that the exception has been handled.
+      if (async_hook_fields[kAfter] > 0) {
+        const { emitAfter } = NativeModule.require('internal/async_hooks');
+        do {
+          emitAfter(async_id_fields[kExecutionAsyncId]);
+        } while (async_hook_fields[kStackLength] > 0);
+      // Or completely empty the id stack.
       } else {
-        // If we handled an error, then make sure any ticks get processed
-        NativeModule.require('timers').setImmediate(process._tickCallback);
-
-        // Emit the after() hooks now that the exception has been handled.
-        if (async_hook_fields[kAfter] > 0) {
-          do {
-            NativeModule.require('internal/async_hooks').emitAfter(
-              async_id_fields[kExecutionAsyncId]);
-          } while (async_hook_fields[kStackLength] > 0);
-        // Or completely empty the id stack.
-        } else {
-          clearAsyncIdStack();
-        }
+        clearAsyncIdStack();
       }
 
-      return caught;
+      return true;
     };
   }
 
diff --git a/test/parallel/test-process-fatal-exception-tick.js b/test/parallel/test-process-fatal-exception-tick.js
@@ -0,0 +1,24 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+// If a process encounters an uncaughtException, it should schedule
+// processing of nextTicks on the next Immediates cycle but not
+// before all Immediates are handled
+
+let stage = 0;
+
+process.once('uncaughtException', common.expectsError({
+  type: Error,
+  message: 'caughtException'
+}));
+
+setImmediate(() => {
+  stage++;
+  process.nextTick(() => assert.strictEqual(stage, 2));
+});
+const now = Date.now();
+setTimeout(() => setImmediate(() => stage++), 1);
+while (now + 10 >= Date.now());
+throw new Error('caughtException');
