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);
 returnUndefined(env()->isolate());

src/env-inl.h

@@ -88,6 +88,20 @@ inline void Environment::AsyncHooks::set_enable_callbacks(uint32_t flag){
  fields_[kEnableCallbacks] = flag;
 }
 
+inlineEnvironment::AsyncCallbackScope::AsyncCallbackScope(Environment* env)
+ : env_(env){
+ env_->makecallback_cntr_++;
+}
+
+inlineEnvironment::AsyncCallbackScope::~AsyncCallbackScope(){
+ env_->makecallback_cntr_--;
+CHECK_GE(env_->makecallback_cntr_, 0);
+}
+
+inlineboolEnvironment::AsyncCallbackScope::in_makecallback(){
+return env_->makecallback_cntr_ > 1;
+}
+
 inlineEnvironment::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{
 }
 
 
-boolEnvironment::KickNextTick(){
+boolEnvironment::KickNextTick(Environment::AsyncCallbackScope* scope){
  TickInfo* info = tick_info();
 
-if (info->in_tick()){
+if (scope->in_makecallback()){
 returntrue;
  }
 
@@ -73,15 +73,11 @@ bool Environment::KickNextTick(){
 returntrue;
  }
 
- 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);
 returnfalse;

src/env.h

@@ -294,6 +294,19 @@ class Environment{
 DISALLOW_COPY_AND_ASSIGN(AsyncHooks);
  };
 
+classAsyncCallbackScope{
+public:
+explicitAsyncCallbackScope(Environment* env);
+~AsyncCallbackScope();
+
+inlineboolin_makecallback();
+
+private:
+ Environment* env_;
+
+DISALLOW_COPY_AND_ASSIGN(AsyncCallbackScope);
+ };
+
 classDomainFlag{
 public:
 inlineuint32_t* fields();
@@ -446,7 +459,7 @@ class Environment{
 
 inlineint64_tget_async_wrap_uid();
 
-boolKickNextTick();
+boolKickNextTick(AsyncCallbackScope* scope);
 
 inlineuint32_t* heap_statistics_buffer() const;
 inlinevoidset_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))
 returnUndefined(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);
 
-boolKickNextTick();
-
 // 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.