async_hooks: use scope for defaultTriggerAsyncId

Previously the getter would mutate the kDefaultTriggerAsncId value. This
refactor changes the setter to bind the current kDefaultTriggerAsncId to
a scope, such that the getter doesn't have to mutate its own value.

PR-URL: https://github.com/nodejs/node/pull/17273
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

9 files changed, 73 insertions, 57 deletions

diff --git a/src/async_wrap.cc b/src/async_wrap.cc
index 555d616426..79d54b1000 100644
--- a/src/async_wrap.cc
+++ b/src/async_wrap.cc
@@ -308,12 +308,13 @@ static void PromiseHook(PromiseHookType type, Local<Promise> promise,
       if (parent_wrap == nullptr) {
         parent_wrap = PromiseWrap::New(env, parent_promise, nullptr, true);
       }
-      // get id from parentWrap
-      double trigger_async_id = parent_wrap->get_async_id();
-      env->set_default_trigger_async_id(trigger_async_id);
-    }
 
-    wrap = PromiseWrap::New(env, promise, parent_wrap, silent);
+      AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(
+        env, parent_wrap->get_async_id());
+      wrap = PromiseWrap::New(env, promise, parent_wrap, silent);
+    } else {
+      wrap = PromiseWrap::New(env, promise, nullptr, silent);
+    }
   }
 
   CHECK_NE(wrap, nullptr);
diff --git a/src/connection_wrap.cc b/src/connection_wrap.cc
index b7c1949e11..8de77f361d 100644
--- a/src/connection_wrap.cc
+++ b/src/connection_wrap.cc
@@ -49,7 +49,6 @@ void ConnectionWrap<WrapType, UVType>::OnConnection(uv_stream_t* handle,
   };
 
   if (status == 0) {
-    env->set_default_trigger_async_id(wrap_data->get_async_id());
     // Instantiate the client javascript object and handle.
     Local<Object> client_obj = WrapType::Instantiate(env,
                                                      wrap_data,
diff --git a/src/env-inl.h b/src/env-inl.h
index 2d4c0c18e9..1451631728 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -176,23 +176,27 @@ inline void Environment::AsyncHooks::clear_async_id_stack() {
   async_id_fields_[kTriggerAsyncId] = 0;
 }
 
-inline Environment::AsyncHooks::InitScope::InitScope(
-    Environment* env, double init_trigger_async_id)
-        : env_(env),
-          async_id_fields_ref_(env->async_hooks()->async_id_fields()) {
-  if (env_->async_hooks()->fields()[AsyncHooks::kCheck] > 0) {
-    CHECK_GE(init_trigger_async_id, -1);
+inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope
+  ::DefaultTriggerAsyncIdScope(Environment* env,
+                               double default_trigger_async_id)
+    : async_id_fields_ref_(env->async_hooks()->async_id_fields()) {
+  if (env->async_hooks()->fields()[AsyncHooks::kCheck] > 0) {
+    CHECK_GE(default_trigger_async_id, 0);
   }
-  env->async_hooks()->push_async_ids(
-    async_id_fields_ref_[AsyncHooks::kExecutionAsyncId],
-    init_trigger_async_id);
+
+  old_default_trigger_async_id_ =
+    async_id_fields_ref_[AsyncHooks::kDefaultTriggerAsyncId];
+  async_id_fields_ref_[AsyncHooks::kDefaultTriggerAsyncId] =
+    default_trigger_async_id;
 }
 
-inline Environment::AsyncHooks::InitScope::~InitScope() {
-  env_->async_hooks()->pop_async_id(
-    async_id_fields_ref_[AsyncHooks::kExecutionAsyncId]);
+inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope
+  ::~DefaultTriggerAsyncIdScope() {
+  async_id_fields_ref_[AsyncHooks::kDefaultTriggerAsyncId] =
+    old_default_trigger_async_id_;
 }
 
+
 inline Environment::AsyncCallbackScope::AsyncCallbackScope(Environment* env)
     : env_(env) {
   env_->makecallback_cntr_++;
@@ -456,17 +460,12 @@ inline double Environment::trigger_async_id() {
 inline double Environment::get_default_trigger_async_id() {
   double default_trigger_async_id =
     async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId];
-  async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = -1;
   // If defaultTriggerAsyncId isn't set, use the executionAsyncId
   if (default_trigger_async_id < 0)
     default_trigger_async_id = execution_async_id();
   return default_trigger_async_id;
 }
 
-inline void Environment::set_default_trigger_async_id(const double id) {
-  async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = id;
-}
-
 inline double* Environment::heap_statistics_buffer() const {
   CHECK_NE(heap_statistics_buffer_, nullptr);
   return heap_statistics_buffer_;
diff --git a/src/env.h b/src/env.h
index 7d7986bce4..8146229943 100644
--- a/src/env.h
+++ b/src/env.h
@@ -402,22 +402,23 @@ class Environment {
     inline size_t stack_size();
     inline void clear_async_id_stack();  // Used in fatal exceptions.
 
-    // Used to propagate the trigger_async_id to the constructor of any newly
-    // created resources using RAII. Instead of needing to pass the
-    // trigger_async_id along with other constructor arguments.
-    class InitScope {
+    // Used to set the kDefaultTriggerAsyncId in a scope. This is instead of
+    // passing the trigger_async_id along with other constructor arguments.
+    class DefaultTriggerAsyncIdScope {
      public:
-      InitScope() = delete;
-      explicit InitScope(Environment* env, double init_trigger_async_id);
-      ~InitScope();
+      DefaultTriggerAsyncIdScope() = delete;
+      explicit DefaultTriggerAsyncIdScope(Environment* env,
+                                          double init_trigger_async_id);
+      ~DefaultTriggerAsyncIdScope();
 
      private:
-      Environment* env_;
       AliasedBuffer<double, v8::Float64Array> async_id_fields_ref_;
+      double old_default_trigger_async_id_;
 
-      DISALLOW_COPY_AND_ASSIGN(InitScope);
+      DISALLOW_COPY_AND_ASSIGN(DefaultTriggerAsyncIdScope);
     };
 
+
    private:
     friend class Environment;  // So we can call the constructor.
     inline explicit AsyncHooks(v8::Isolate* isolate);
@@ -559,7 +560,6 @@ class Environment {
   inline double execution_async_id();
   inline double trigger_async_id();
   inline double get_default_trigger_async_id();
-  inline void set_default_trigger_async_id(const double id);
 
   // List of id's that have been destroyed and need the destroy() cb called.
   inline std::vector<double>* destroy_async_id_list();
diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc
index c6dce756ce..c5958a2271 100644
--- a/src/pipe_wrap.cc
+++ b/src/pipe_wrap.cc
@@ -53,7 +53,8 @@ Local<Object> PipeWrap::Instantiate(Environment* env,
                                     AsyncWrap* parent,
                                     PipeWrap::SocketType type) {
   EscapableHandleScope handle_scope(env->isolate());
-  AsyncHooks::InitScope init_scope(env, parent->get_async_id());
+  AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env,
+                                                       parent->get_async_id());
   CHECK_EQ(false, env->pipe_constructor_template().IsEmpty());
   Local<Function> constructor = env->pipe_constructor_template()->GetFunction();
   CHECK_EQ(false, constructor.IsEmpty());
diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h
index cc89a11bac..cdcff67cc5 100644
--- a/src/stream_base-inl.h
+++ b/src/stream_base-inl.h
@@ -143,7 +143,8 @@ void StreamBase::JSMethod(const FunctionCallbackInfo<Value>& args) {
   if (!wrap->IsAlive())
     return args.GetReturnValue().Set(UV_EINVAL);
 
-  AsyncHooks::InitScope init_scope(handle->env(), handle->get_async_id());
+  AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(
+    handle->env(), handle->get_async_id());
   args.GetReturnValue().Set((wrap->*Method)(args));
 }
 
diff --git a/src/stream_base.cc b/src/stream_base.cc
index f4bfb72f3f..b1aea79d52 100644
--- a/src/stream_base.cc
+++ b/src/stream_base.cc
@@ -52,7 +52,7 @@ int StreamBase::Shutdown(const FunctionCallbackInfo<Value>& args) {
 
   AsyncWrap* wrap = GetAsyncWrap();
   CHECK_NE(wrap, nullptr);
-  env->set_default_trigger_async_id(wrap->get_async_id());
+  AsyncHooks::DefaultTriggerAsyncIdScope(env, wrap->get_async_id());
   ShutdownWrap* req_wrap = new ShutdownWrap(env,
                                             req_wrap_obj,
                                             this);
@@ -109,7 +109,6 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
   size_t storage_size = 0;
   uint32_t bytes = 0;
   size_t offset;
-  AsyncWrap* wrap;
   WriteWrap* req_wrap;
   int err;
 
@@ -153,10 +152,13 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
       goto done;
   }
 
-  wrap = GetAsyncWrap();
-  CHECK_NE(wrap, nullptr);
-  env->set_default_trigger_async_id(wrap->get_async_id());
-  req_wrap = WriteWrap::New(env, req_wrap_obj, this, storage_size);
+  {
+    AsyncWrap* wrap = GetAsyncWrap();
+    CHECK_NE(wrap, nullptr);
+    AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env,
+                                                         wrap->get_async_id());
+    req_wrap = WriteWrap::New(env, req_wrap_obj, this, storage_size);
+  }
 
   offset = 0;
   if (!all_buffers) {
@@ -227,7 +229,6 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo<Value>& args) {
   const char* data = Buffer::Data(args[1]);
   size_t length = Buffer::Length(args[1]);
 
-  AsyncWrap* wrap;
   WriteWrap* req_wrap;
   uv_buf_t buf;
   buf.base = const_cast<char*>(data);
@@ -243,11 +244,14 @@ int StreamBase::WriteBuffer(const FunctionCallbackInfo<Value>& args) {
     goto done;
   CHECK_EQ(count, 1);
 
-  wrap = GetAsyncWrap();
-  if (wrap != nullptr)
-    env->set_default_trigger_async_id(wrap->get_async_id());
   // Allocate, or write rest
-  req_wrap = WriteWrap::New(env, req_wrap_obj, this);
+  {
+    AsyncWrap* wrap = GetAsyncWrap();
+    CHECK_NE(wrap, nullptr);
+    AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env,
+                                                         wrap->get_async_id());
+    req_wrap = WriteWrap::New(env, req_wrap_obj, this);
+  }
 
   err = DoWrite(req_wrap, bufs, count, nullptr);
   if (HasWriteQueue())
@@ -278,7 +282,6 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
   Local<Object> req_wrap_obj = args[0].As<Object>();
   Local<String> string = args[1].As<String>();
   Local<Object> send_handle_obj;
-  AsyncWrap* wrap;
   if (args[2]->IsObject())
     send_handle_obj = args[2].As<Object>();
 
@@ -329,10 +332,13 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
     CHECK_EQ(count, 1);
   }
 
-  wrap = GetAsyncWrap();
-  if (wrap != nullptr)
-    env->set_default_trigger_async_id(wrap->get_async_id());
-  req_wrap = WriteWrap::New(env, req_wrap_obj, this, storage_size);
+  {
+    AsyncWrap* wrap = GetAsyncWrap();
+    CHECK_NE(wrap, nullptr);
+    AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(env,
+                                                         wrap->get_async_id());
+    req_wrap = WriteWrap::New(env, req_wrap_obj, this, storage_size);
+  }
 
   data = req_wrap->Extra();
 
diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc
index 233e3a1a15..3a0a3f295e 100644
--- a/src/tcp_wrap.cc
+++ b/src/tcp_wrap.cc
@@ -56,7 +56,8 @@ Local<Object> TCPWrap::Instantiate(Environment* env,
                                    AsyncWrap* parent,
                                    TCPWrap::SocketType type) {
   EscapableHandleScope handle_scope(env->isolate());
-  AsyncHooks::InitScope init_scope(env, parent->get_async_id());
+  AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(
+    env, parent->get_async_id());
   CHECK_EQ(env->tcp_constructor_template().IsEmpty(), false);
   Local<Function> constructor = env->tcp_constructor_template()->GetFunction();
   CHECK_EQ(constructor.IsEmpty(), false);
@@ -292,7 +293,8 @@ void TCPWrap::Connect(const FunctionCallbackInfo<Value>& args) {
   int err = uv_ip4_addr(*ip_address, port, &addr);
 
   if (err == 0) {
-    env->set_default_trigger_async_id(wrap->get_async_id());
+    AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(
+      env, wrap->get_async_id());
     ConnectWrap* req_wrap =
         new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP);
     err = uv_tcp_connect(req_wrap->req(),
@@ -328,7 +330,8 @@ void TCPWrap::Connect6(const FunctionCallbackInfo<Value>& args) {
   int err = uv_ip6_addr(*ip_address, port, &addr);
 
   if (err == 0) {
-    env->set_default_trigger_async_id(wrap->get_async_id());
+    AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(
+      env, wrap->get_async_id());
     ConnectWrap* req_wrap =
         new ConnectWrap(env, req_wrap_obj, AsyncWrap::PROVIDER_TCPCONNECTWRAP);
     err = uv_tcp_connect(req_wrap->req(),
diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc
index 55e12e4278..27be93abd0 100644
--- a/src/udp_wrap.cc
+++ b/src/udp_wrap.cc
@@ -357,8 +357,12 @@ void UDPWrap::DoSend(const FunctionCallbackInfo<Value>& args, int family) {
   node::Utf8Value address(env->isolate(), args[4]);
   const bool have_callback = args[5]->IsTrue();
 
-  env->set_default_trigger_async_id(wrap->get_async_id());
-  SendWrap* req_wrap = new SendWrap(env, req_wrap_obj, have_callback);
+  SendWrap* req_wrap;
+  {
+    AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(
+      env, wrap->get_async_id());
+    req_wrap = new SendWrap(env, req_wrap_obj, have_callback);
+  }
   size_t msg_size = 0;
 
   MaybeStackBuffer<uv_buf_t, 16> bufs(count);
@@ -507,7 +511,9 @@ Local<Object> UDPWrap::Instantiate(Environment* env,
                                    AsyncWrap* parent,
                                    UDPWrap::SocketType type) {
   EscapableHandleScope scope(env->isolate());
-  AsyncHooks::InitScope init_scope(env, parent->get_async_id());
+  AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(
+    env, parent->get_async_id());
+
   // If this assert fires then Initialize hasn't been called yet.
   CHECK_EQ(env->udp_constructor_function().IsEmpty(), false);
   Local<Object> instance = env->udp_constructor_function()