src: move AsyncCallbackScope out of Environment

PR-URL: nodejs#26824 Refs: nodejs#26776 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
targos · Mar 27, 2019 · 0fa0fe0 · 0fa0fe0
1 parent 16f90ef
commit 0fa0fe0
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 28 deletions.
diff --git a/src/api/callback.cc b/src/api/callback.cc
@@ -92,7 +92,7 @@ void InternalCallbackScope::Close() {
     AsyncWrap::EmitAfter(env_, async_context_.async_id);
   }
 
-  if (env_->makecallback_depth() > 1) {
+  if (env_->async_callback_scope_depth() > 1) {
     return;
   }
 
@@ -217,7 +217,7 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate,
   Context::Scope context_scope(env->context());
   MaybeLocal<Value> ret =
       InternalMakeCallback(env, recv, callback, argc, argv, asyncContext);
-  if (ret.IsEmpty() && env->makecallback_depth() == 0) {
+  if (ret.IsEmpty() && env->async_callback_scope_depth() == 0) {
     // This is only for legacy compatibility and we may want to look into
     // removing/adjusting it.
     return Undefined(env->isolate());

diff --git a/src/env-inl.h b/src/env-inl.h
@@ -212,18 +212,24 @@ Environment* Environment::ForAsyncHooks(AsyncHooks* hooks) {
   return ContainerOf(&Environment::async_hooks_, hooks);
 }
 
+inline AsyncCallbackScope::AsyncCallbackScope(Environment* env) : env_(env) {
+  env_->PushAsyncCallbackScope();
+}
 
-inline Environment::AsyncCallbackScope::AsyncCallbackScope(Environment* env)
-    : env_(env) {
-  env_->makecallback_cntr_++;
+inline AsyncCallbackScope::~AsyncCallbackScope() {
+  env_->PopAsyncCallbackScope();
+}
+
+inline size_t Environment::async_callback_scope_depth() const {
+  return async_callback_scope_depth_;
 }
 
-inline Environment::AsyncCallbackScope::~AsyncCallbackScope() {
-  env_->makecallback_cntr_--;
+inline void Environment::PushAsyncCallbackScope() {
+  async_callback_scope_depth_++;
 }
 
-inline size_t Environment::makecallback_depth() const {
-  return makecallback_cntr_;
+inline void Environment::PopAsyncCallbackScope() {
+  async_callback_scope_depth_--;
 }
 
 inline Environment::ImmediateInfo::ImmediateInfo(v8::Isolate* isolate)

diff --git a/src/env.h b/src/env.h
@@ -607,24 +607,26 @@ class AsyncHooks {
   void grow_async_ids_stack();
 };
 
+class AsyncCallbackScope {
+ public:
+  AsyncCallbackScope() = delete;
+  explicit AsyncCallbackScope(Environment* env);
+  ~AsyncCallbackScope();
+  AsyncCallbackScope(const AsyncCallbackScope&) = delete;
+  AsyncCallbackScope& operator=(const AsyncCallbackScope&) = delete;
+
+ private:
+  Environment* env_;
+};
+
 class Environment {
  public:
   Environment(const Environment&) = delete;
   Environment& operator=(const Environment&) = delete;
 
-  class AsyncCallbackScope {
-   public:
-    AsyncCallbackScope() = delete;
-    explicit AsyncCallbackScope(Environment* env);
-    ~AsyncCallbackScope();
-    AsyncCallbackScope(const AsyncCallbackScope&) = delete;
-    AsyncCallbackScope& operator=(const AsyncCallbackScope&) = delete;
-
-   private:
-    Environment* env_;
-  };
-
-  inline size_t makecallback_depth() const;
+  inline size_t async_callback_scope_depth() const;
+  inline void PushAsyncCallbackScope();
+  inline void PopAsyncCallbackScope();
 
   class ImmediateInfo {
    public:
@@ -1082,7 +1084,7 @@ class Environment {
   bool printed_error_ = false;
   bool emit_env_nonstring_warning_ = true;
   bool emit_err_name_warning_ = true;
-  size_t makecallback_cntr_ = 0;
+  size_t async_callback_scope_depth_ = 0;
   std::vector<double> destroy_async_id_list_;
 
   std::shared_ptr<EnvironmentOptions> options_;

diff --git a/src/node.cc b/src/node.cc
@@ -807,7 +807,7 @@ inline int StartNodeWithIsolate(Isolate* isolate,
 #endif  // HAVE_INSPECTOR && NODE_USE_V8_PLATFORM
 
   {
-    Environment::AsyncCallbackScope callback_scope(&env);
+    AsyncCallbackScope callback_scope(&env);
     env.async_hooks()->push_async_ids(1, 0);
     LoadEnvironment(&env);
     env.async_hooks()->pop_async_id(1);

diff --git a/src/node_http_parser_impl.h b/src/node_http_parser_impl.h
@@ -323,7 +323,7 @@ class Parser : public AsyncWrap, public StreamListener {
 
     argv[A_UPGRADE] = Boolean::New(env()->isolate(), parser_.upgrade);
 
-    Environment::AsyncCallbackScope callback_scope(env());
+    AsyncCallbackScope callback_scope(env());
 
     MaybeLocal<Value> head_response =
         MakeCallback(cb.As<Function>(), arraysize(argv), argv);
@@ -394,7 +394,7 @@ class Parser : public AsyncWrap, public StreamListener {
     if (!cb->IsFunction())
       return 0;
 
-    Environment::AsyncCallbackScope callback_scope(env());
+    AsyncCallbackScope callback_scope(env());
 
     MaybeLocal<Value> r = MakeCallback(cb.As<Function>(), 0, nullptr);
 

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -211,7 +211,7 @@ class InternalCallbackScope {
   Environment* env_;
   async_context async_context_;
   v8::Local<v8::Object> object_;
-  Environment::AsyncCallbackScope callback_scope_;
+  AsyncCallbackScope callback_scope_;
   bool failed_ = false;
   bool pushed_ids_ = false;
   bool closed_ = false;

diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -288,7 +288,7 @@ void Worker::Run() {
         inspector_started = true;
 
         HandleScope handle_scope(isolate_);
-        Environment::AsyncCallbackScope callback_scope(env_.get());
+        AsyncCallbackScope callback_scope(env_.get());
         env_->async_hooks()->push_async_ids(1, 0);
         if (!RunBootstrapping(env_.get()).IsEmpty()) {
           USE(StartExecution(env_.get(), "internal/main/worker_thread"));