async_hooks: emitAfter correctly on fatalException

Previously calling emitAfter() from _fatalException would skipt the
first asyncId. Instead use the size() of the std::stack to determine how
many times to loop and call emitAfter().

PR-URL: #14914
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>

Author: trevnorris

lib/internal/bootstrap_node.js

@@ -347,7 +347,7 @@
     // Arrays containing hook flags and ids for async_hook calls.
     const { async_hook_fields, async_uid_fields } = async_wrap;
     // Internal functions needed to manipulate the stack.
-    const { clearIdStack, popAsyncIds } = async_wrap;
+    const { clearIdStack, asyncIdStackSize } = async_wrap;
     const { kAfter, kCurrentAsyncId, kInitTriggerId } = async_wrap.constants;
 
     process._fatalException = function(er) {
@@ -384,8 +384,7 @@
           do {
             NativeModule.require('async_hooks').emitAfter(
               async_uid_fields[kCurrentAsyncId]);
-          // popAsyncIds() returns true if there are more ids on the stack.
-          } while (popAsyncIds(async_uid_fields[kCurrentAsyncId]));
+          } while (asyncIdStackSize() > 0);
         // Or completely empty the id stack.
         } else {
           clearIdStack();

src/async-wrap.cc

@@ -474,6 +474,13 @@ void AsyncWrap::PopAsyncIds(const FunctionCallbackInfo<Value>& args) {
 }
 
 
+void AsyncWrap::AsyncIdStackSize(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  args.GetReturnValue().Set(
+      static_cast<double>(env->async_hooks()->stack_size()));
+}
+
+
 void AsyncWrap::ClearIdStack(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   env->async_hooks()->clear_id_stack();
@@ -503,6 +510,7 @@ void AsyncWrap::Initialize(Local<Object> target,
   env->SetMethod(target, "setupHooks", SetupHooks);
   env->SetMethod(target, "pushAsyncIds", PushAsyncIds);
   env->SetMethod(target, "popAsyncIds", PopAsyncIds);
+  env->SetMethod(target, "asyncIdStackSize", AsyncIdStackSize);
   env->SetMethod(target, "clearIdStack", ClearIdStack);
   env->SetMethod(target, "addIdToDestroyList", QueueDestroyId);
   env->SetMethod(target, "enablePromiseHook", EnablePromiseHook);

src/async-wrap.h

@@ -102,6 +102,7 @@ class AsyncWrap : public BaseObject {
   static void GetAsyncId(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void PushAsyncIds(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void PopAsyncIds(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void AsyncIdStackSize(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void ClearIdStack(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void AsyncReset(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void QueueDestroyId(const v8::FunctionCallbackInfo<v8::Value>& args);

src/env-inl.h

@@ -166,6 +166,10 @@ inline bool Environment::AsyncHooks::pop_ids(double async_id) {
   return !ids_stack_.empty();
 }
 
+inline size_t Environment::AsyncHooks::stack_size() {
+  return ids_stack_.size();
+}
+
 inline void Environment::AsyncHooks::clear_id_stack() {
   while (!ids_stack_.empty())
     ids_stack_.pop();

src/env.h

@@ -396,6 +396,7 @@ class Environment {
 
     inline void push_ids(double async_id, double trigger_id);
     inline bool pop_ids(double async_id);
+    inline size_t stack_size();
     inline void clear_id_stack();  // Used in fatal exceptions.
 
     // Used to propagate the trigger_id to the constructor of any newly created

test/parallel/test-emit-after-uncaught-exception.js

@@ -0,0 +1,40 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const async_hooks = require('async_hooks');
+
+const id_obj = {};
+let collect = true;
+
+const hook = async_hooks.createHook({
+  before(id) { if (collect) id_obj[id] = true; },
+  after(id) { delete id_obj[id]; },
+}).enable();
+
+process.once('uncaughtException', common.mustCall((er) => {
+  assert.strictEqual(er.message, 'bye');
+  collect = false;
+}));
+
+setImmediate(common.mustCall(() => {
+  process.nextTick(common.mustCall(() => {
+    assert.strictEqual(Object.keys(id_obj).length, 0);
+    hook.disable();
+  }));
+
+  // Create a stack of async ids that will need to be emitted in the case of
+  // an uncaught exception.
+  const ar1 = new async_hooks.AsyncResource('Mine');
+  ar1.emitBefore();
+
+  const ar2 = new async_hooks.AsyncResource('Mine');
+  ar2.emitBefore();
+
+  throw new Error('bye');
+
+  // TODO(trevnorris): This test shows that the after() hooks are always called
+  // correctly, but it doesn't solve where the emitDestroy() is missed because
+  // of the uncaught exception. Simple solution is to always call emitDestroy()
+  // before the emitAfter(), but how to codify this?
+}));