src: add AsyncCallbackScope

Add a scope that will allow MakeCallback to know whether or not it's
currently running. This will prevent nextTickQueue and the
MicrotaskQueue from being processed recursively. It is also required to
wrap the bootloading stage since it doesn't run within a MakeCallback.

Ref: nodejs/node-v0.x-archive#9245
Ref: #7048
PR-URL: #4507
Reviewed-By: Fedor Indutny <[email protected]>

Author: trevnorris

src/async-wrap.cc

@@ -184,6 +184,8 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
   Local<Object> domain;
   bool has_domain = false;
 
+  Environment::AsyncCallbackScope callback_scope(env());
+
   if (env()->using_domains()) {
     Local<Value> domain_v = context->Get(env()->domain_string());
     has_domain = domain_v->IsObject();
@@ -236,7 +238,7 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
 
   Environment::TickInfo* tick_info = env()->tick_info();
 
-  if (tick_info->in_tick()) {
+  if (callback_scope.in_makecallback()) {
     return ret;
   }
 
@@ -249,12 +251,8 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
     return ret;
   }
 
-  tick_info->set_in_tick(true);
-
   env()->tick_callback_function()->Call(process, 0, nullptr);
 
-  tick_info->set_in_tick(false);
-
   if (try_catch.HasCaught()) {
     tick_info->set_last_threw(true);
     return Undefined(env()->isolate());

src/env-inl.h

@@ -88,6 +88,20 @@ inline void Environment::AsyncHooks::set_enable_callbacks(uint32_t flag) {
   fields_[kEnableCallbacks] = flag;
 }
 
+inline Environment::AsyncCallbackScope::AsyncCallbackScope(Environment* env)
+    : env_(env) {
+  env_->makecallback_cntr_++;
+}
+
+inline Environment::AsyncCallbackScope::~AsyncCallbackScope() {
+  env_->makecallback_cntr_--;
+  CHECK_GE(env_->makecallback_cntr_, 0);
+}
+
+inline bool Environment::AsyncCallbackScope::in_makecallback() {
+  return env_->makecallback_cntr_ > 1;
+}
+
 inline Environment::DomainFlag::DomainFlag() {
   for (int i = 0; i < kFieldsCount; ++i) fields_[i] = 0;
 }
@@ -210,6 +224,7 @@ inline Environment::Environment(v8::Local<v8::Context> context,
       using_domains_(false),
       printed_error_(false),
       trace_sync_io_(false),
+      makecallback_cntr_(0),
       async_wrap_uid_(0),
       debugger_agent_(this),
       http_parser_buffer_(nullptr),

src/env.cc

@@ -57,10 +57,10 @@ void Environment::PrintSyncTrace() const {
 }
 
 
-bool Environment::KickNextTick() {
+bool Environment::KickNextTick(Environment::AsyncCallbackScope* scope) {
   TickInfo* info = tick_info();
 
-  if (info->in_tick()) {
+  if (scope->in_makecallback()) {
     return true;
   }
 
@@ -73,15 +73,11 @@ bool Environment::KickNextTick() {
     return true;
   }
 
-  info->set_in_tick(true);
-
   // process nextTicks after call
   TryCatch try_catch;
   try_catch.SetVerbose(true);
   tick_callback_function()->Call(process_object(), 0, nullptr);
 
-  info->set_in_tick(false);
-
   if (try_catch.HasCaught()) {
     info->set_last_threw(true);
     return false;

src/env.h

@@ -294,6 +294,19 @@ class Environment {
     DISALLOW_COPY_AND_ASSIGN(AsyncHooks);
   };
 
+  class AsyncCallbackScope {
+   public:
+    explicit AsyncCallbackScope(Environment* env);
+    ~AsyncCallbackScope();
+
+    inline bool in_makecallback();
+
+   private:
+    Environment* env_;
+
+    DISALLOW_COPY_AND_ASSIGN(AsyncCallbackScope);
+  };
+
   class DomainFlag {
    public:
     inline uint32_t* fields();
@@ -446,7 +459,7 @@ class Environment {
 
   inline int64_t get_async_wrap_uid();
 
-  bool KickNextTick();
+  bool KickNextTick(AsyncCallbackScope* scope);
 
   inline uint32_t* heap_statistics_buffer() const;
   inline void set_heap_statistics_buffer(uint32_t* pointer);
@@ -541,6 +554,7 @@ class Environment {
   bool using_domains_;
   bool printed_error_;
   bool trace_sync_io_;
+  size_t makecallback_cntr_;
   int64_t async_wrap_uid_;
   debugger::Agent debugger_agent_;
 

src/node.cc

@@ -1132,6 +1132,8 @@ Local<Value> MakeCallback(Environment* env,
   bool ran_init_callback = false;
   bool has_domain = false;
 
+  Environment::AsyncCallbackScope callback_scope(env);
+
   // TODO(trevnorris): Adding "_asyncQueue" to the "this" in the init callback
   // is a horrible way to detect usage. Rethink how detection should happen.
   if (recv->IsObject()) {
@@ -1192,7 +1194,7 @@ Local<Value> MakeCallback(Environment* env,
     }
   }
 
-  if (!env->KickNextTick())
+  if (!env->KickNextTick(&callback_scope))
     return Undefined(env->isolate());
 
   return ret;
@@ -4100,7 +4102,10 @@ static void StartNodeInstance(void* arg) {
     if (instance_data->use_debug_agent())
       StartDebug(env, debug_wait_connect);
 
-    LoadEnvironment(env);
+    {
+      Environment::AsyncCallbackScope callback_scope(env);
+      LoadEnvironment(env);
+    }
 
     env->set_trace_sync_io(trace_sync_io);
 

src/node_http_parser.cc

@@ -579,6 +579,8 @@ class Parser : public BaseObject {
     if (!cb->IsFunction())
       return;
 
+    Environment::AsyncCallbackScope callback_scope(parser->env());
+
     // Hooks for GetCurrentBuffer
     parser->current_buffer_len_ = nread;
     parser->current_buffer_data_ = buf->base;
@@ -588,7 +590,7 @@ class Parser : public BaseObject {
     parser->current_buffer_len_ = 0;
     parser->current_buffer_data_ = nullptr;
 
-    parser->env()->KickNextTick();
+    parser->env()->KickNextTick(&callback_scope);
   }
 
 

src/node_internals.h

@@ -69,8 +69,6 @@ v8::Local<v8::Value> MakeCallback(Environment* env,
                                    int argc = 0,
                                    v8::Local<v8::Value>* argv = nullptr);
 
-bool KickNextTick();
-
 // Convert a struct sockaddr to a { address: '1.2.3.4', port: 1234 } JS object.
 // Sets address and port properties on the info object and returns it.
 // If |info| is omitted, a new object is returned.