src: remove AsyncScope, AsyncCallbackScope, RunNextTicksNative

src/api/callback.cc

@@ -34,22 +34,25 @@ CallbackScope::~CallbackScope() {
   delete private_;
 }
 
-InternalCallbackScope::InternalCallbackScope(AsyncWrap* async_wrap)
+InternalCallbackScope::InternalCallbackScope(AsyncWrap* async_wrap, int flags)
     : InternalCallbackScope(async_wrap->env(),
                             async_wrap->object(),
                             { async_wrap->get_async_id(),
-                              async_wrap->get_trigger_async_id() }) {}
+                              async_wrap->get_trigger_async_id() },
+                            flags) {}
 
 InternalCallbackScope::InternalCallbackScope(Environment* env,
                                              Local<Object> object,
                                              const async_context& asyncContext,
-                                             ResourceExpectation expect)
+                                             int flags)
   : env_(env),
     async_context_(asyncContext),
     object_(object),
-    callback_scope_(env) {
-  CHECK_IMPLIES(expect == kRequireResource, !object.IsEmpty());
+    skip_hooks_(flags & kSkipAsyncHooks),
+    skip_task_queues_(flags & kSkipTaskQueues) {
+  CHECK_IMPLIES(!(flags & kAllowEmptyResource), !object.IsEmpty());
   CHECK_NOT_NULL(env);
+  env->PushAsyncCallbackScope();
 
   if (!env->can_call_into_js()) {
     failed_ = true;
@@ -60,7 +63,7 @@ InternalCallbackScope::InternalCallbackScope(Environment* env,
   // If you hit this assertion, you forgot to enter the v8::Context first.
   CHECK_EQ(Environment::GetCurrent(env->isolate()), env);
 
-  if (asyncContext.async_id != 0) {
+  if (asyncContext.async_id != 0 && !skip_hooks_) {
     // No need to check a return value because the application will exit if
     // an exception occurs.
     AsyncWrap::EmitBefore(env, asyncContext.async_id);
@@ -73,6 +76,7 @@ InternalCallbackScope::InternalCallbackScope(Environment* env,
 
 InternalCallbackScope::~InternalCallbackScope() {
   Close();
+  env_->PopAsyncCallbackScope();
 }
 
 void InternalCallbackScope::Close() {
@@ -89,11 +93,11 @@ void InternalCallbackScope::Close() {
 
   if (failed_) return;
 
-  if (async_context_.async_id != 0) {
+  if (async_context_.async_id != 0 && !skip_hooks_) {
     AsyncWrap::EmitAfter(env_, async_context_.async_id);
   }
 
-  if (env_->async_callback_scope_depth() > 1) {
+  if (env_->async_callback_scope_depth() > 1 || skip_task_queues_) {
     return;
   }
 

src/async_wrap-inl.h

@@ -50,20 +50,6 @@ inline double AsyncWrap::get_trigger_async_id() const {
 }
 
 
-inline AsyncWrap::AsyncScope::AsyncScope(AsyncWrap* wrap)
-    : wrap_(wrap) {
-  Environment* env = wrap->env();
-  if (env->async_hooks()->fields()[AsyncHooks::kBefore] == 0) return;
-  EmitBefore(env, wrap->get_async_id());
-}
-
-inline AsyncWrap::AsyncScope::~AsyncScope() {
-  Environment* env = wrap_->env();
-  if (env->async_hooks()->fields()[AsyncHooks::kAfter] == 0) return;
-  EmitAfter(env, wrap_->get_async_id());
-}
-
-
 inline v8::MaybeLocal<v8::Value> AsyncWrap::MakeCallback(
     const v8::Local<v8::String> symbol,
     int argc,

src/async_wrap.h

@@ -162,7 +162,6 @@ class AsyncWrap : public BaseObject {
   inline ProviderType set_provider_type(ProviderType provider);
 
   inline double get_async_id() const;
-
   inline double get_trigger_async_id() const;
 
   void AsyncReset(v8::Local<v8::Object> resource,
@@ -200,18 +199,6 @@ class AsyncWrap : public BaseObject {
   static v8::Local<v8::Object> GetOwner(Environment* env,
                                         v8::Local<v8::Object> obj);
 
-  // This is a simplified version of InternalCallbackScope that only runs
-  // the `before` and `after` hooks. Only use it when not actually calling
-  // back into JS; otherwise, use InternalCallbackScope.
-  class AsyncScope {
-   public:
-    explicit inline AsyncScope(AsyncWrap* wrap);
-    ~AsyncScope();
-
-   private:
-    AsyncWrap* wrap_ = nullptr;
-  };
-
   bool IsDoneInitializing() const override;
 
  private:

src/env-inl.h

@@ -211,14 +211,6 @@ Environment* Environment::ForAsyncHooks(AsyncHooks* hooks) {
   return ContainerOf(&Environment::async_hooks_, hooks);
 }
 
-inline AsyncCallbackScope::AsyncCallbackScope(Environment* env) : env_(env) {
-  env_->PushAsyncCallbackScope();
-}
-
-inline AsyncCallbackScope::~AsyncCallbackScope() {
-  env_->PopAsyncCallbackScope();
-}
-
 inline size_t Environment::async_callback_scope_depth() const {
   return async_callback_scope_depth_;
 }

src/env.h

@@ -721,20 +721,6 @@ class AsyncHooks : public MemoryRetainer {
   void grow_async_ids_stack();
 };
 
-class AsyncCallbackScope {
- public:
-  AsyncCallbackScope() = delete;
-  explicit AsyncCallbackScope(Environment* env);
-  ~AsyncCallbackScope();
-  AsyncCallbackScope(const AsyncCallbackScope&) = delete;
-  AsyncCallbackScope& operator=(const AsyncCallbackScope&) = delete;
-  AsyncCallbackScope(AsyncCallbackScope&&) = delete;
-  AsyncCallbackScope& operator=(AsyncCallbackScope&&) = delete;
-
- private:
-  Environment* env_;
-};
-
 class ImmediateInfo : public MemoryRetainer {
  public:
   inline AliasedUint32Array& fields();

src/node.cc

@@ -406,13 +406,8 @@ MaybeLocal<Value> StartExecution(Environment* env, const char* main_script_id) {
           ->GetFunction(env->context())
           .ToLocalChecked()};
 
-  Local<Value> result;
-  if (!ExecuteBootstrapper(env, main_script_id, &parameters, &arguments)
-           .ToLocal(&result) ||
-      !task_queue::RunNextTicksNative(env)) {
-    return MaybeLocal<Value>();
-  }
-  return scope.Escape(result);
+  return scope.EscapeMaybe(
+      ExecuteBootstrapper(env, main_script_id, &parameters, &arguments));
 }
 
 MaybeLocal<Value> StartMainThreadExecution(Environment* env) {

src/node_http_parser.cc

@@ -310,10 +310,13 @@ class Parser : public AsyncWrap, public StreamListener {
 
     argv[A_UPGRADE] = Boolean::New(env()->isolate(), parser_.upgrade);
 
-    AsyncCallbackScope callback_scope(env());
-
-    MaybeLocal<Value> head_response =
-        MakeCallback(cb.As<Function>(), arraysize(argv), argv);
+    MaybeLocal<Value> head_response;
+    {
+      InternalCallbackScope callback_scope(
+          this, InternalCallbackScope::kSkipTaskQueues);
+      head_response = cb.As<Function>()->Call(
+          env()->context(), object(), arraysize(argv), argv);
+    }
 
     int64_t val;
 
@@ -379,9 +382,12 @@ class Parser : public AsyncWrap, public StreamListener {
     if (!cb->IsFunction())
       return 0;
 
-    AsyncCallbackScope callback_scope(env());
-
-    MaybeLocal<Value> r = MakeCallback(cb.As<Function>(), 0, nullptr);
+    MaybeLocal<Value> r;
+    {
+      InternalCallbackScope callback_scope(
+          this, InternalCallbackScope::kSkipTaskQueues);
+      r = cb.As<Function>()->Call(env()->context(), object(), 0, nullptr);
+    }
 
     if (r.IsEmpty()) {
       got_exception_ = true;

src/node_internals.h

@@ -199,14 +199,23 @@ v8::MaybeLocal<v8::Value> InternalMakeCallback(
 
 class InternalCallbackScope {
  public:
-  // Tell the constructor whether its `object` parameter may be empty or not.
-  enum ResourceExpectation { kRequireResource, kAllowEmptyResource };
+  enum Flags {
+    // Tell the constructor whether its `object` parameter may be empty or not.
+    kAllowEmptyResource = 1,
+    // Indicates whether 'before' and 'after' hooks should be skipped.
+    kSkipAsyncHooks = 2,
+    // Indicates whether nextTick and microtask queues should be skipped.
+    // This should only be used when there is no call into JS in this scope.
+    // (The HTTP parser also uses it for some weird backwards
+    // compatibility issues, but it shouldn't.)
+    kSkipTaskQueues = 4
+  };
   InternalCallbackScope(Environment* env,
                         v8::Local<v8::Object> object,
                         const async_context& asyncContext,
-                        ResourceExpectation expect = kRequireResource);
+                        int flags = 0);
   // Utility that can be used by AsyncWrap classes.
-  explicit InternalCallbackScope(AsyncWrap* async_wrap);
+  explicit InternalCallbackScope(AsyncWrap* async_wrap, int flags = 0);
   ~InternalCallbackScope();
   void Close();
 
@@ -217,7 +226,8 @@ class InternalCallbackScope {
   Environment* env_;
   async_context async_context_;
   v8::Local<v8::Object> object_;
-  AsyncCallbackScope callback_scope_;
+  bool skip_hooks_;
+  bool skip_task_queues_;
   bool failed_ = false;
   bool pushed_ids_ = false;
   bool closed_ = false;

src/node_main_instance.cc

@@ -14,6 +14,7 @@ using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
 using v8::Locker;
+using v8::Object;
 using v8::SealHandleScope;
 
 NodeMainInstance::NodeMainInstance(Isolate* isolate,
@@ -111,10 +112,13 @@ int NodeMainInstance::Run() {
 
   if (exit_code == 0) {
     {
-      AsyncCallbackScope callback_scope(env.get());
-      env->async_hooks()->push_async_ids(1, 0);
+      InternalCallbackScope callback_scope(
+          env.get(),
+          Local<Object>(),
+          { 1, 0 },
+          InternalCallbackScope::kAllowEmptyResource |
+              InternalCallbackScope::kSkipAsyncHooks);
       LoadEnvironment(env.get());
-      env->async_hooks()->pop_async_id(1);
     }
 
     env->set_trace_sync_io(env->options()->trace_sync_io);

src/node_process.h

@@ -34,15 +34,6 @@ v8::Maybe<bool> ProcessEmitDeprecationWarning(Environment* env,
 v8::MaybeLocal<v8::Object> CreateProcessObject(Environment* env);
 void PatchProcessObject(const v8::FunctionCallbackInfo<v8::Value>& args);
 
-namespace task_queue {
-// Handle any nextTicks added in the first tick of the program.
-// We use the native version here for once so that any microtasks
-// created by the main module is then handled from C++, and
-// the call stack of the main script does not show up in the async error
-// stack trace.
-bool RunNextTicksNative(Environment* env);
-}  // namespace task_queue
-
 }  // namespace node
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 #endif  // SRC_NODE_PROCESS_H_

src/node_task_queue.cc

@@ -41,22 +41,6 @@ static void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {
   isolate->EnqueueMicrotask(args[0].As<Function>());
 }
 
-// Should be in sync with runNextTicks in internal/process/task_queues.js
-bool RunNextTicksNative(Environment* env) {
-  OnScopeLeave weakref_cleanup([&]() { env->RunWeakRefCleanup(); });
-
-  TickInfo* tick_info = env->tick_info();
-  if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn())
-    MicrotasksScope::PerformCheckpoint(env->isolate());
-  if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn())
-    return true;
-
-  Local<Function> callback = env->tick_callback_function();
-  CHECK(!callback.IsEmpty());
-  return !callback->Call(env->context(), env->process_object(), 0, nullptr)
-              .IsEmpty();
-}
-
 static void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {
   MicrotasksScope::PerformCheckpoint(args.GetIsolate());
 }

src/node_worker.cc

@@ -273,17 +273,20 @@ void Worker::Run() {
         inspector_started = true;
 #endif
         HandleScope handle_scope(isolate_);
-        AsyncCallbackScope callback_scope(env_.get());
-        env_->async_hooks()->push_async_ids(1, 0);
+        InternalCallbackScope callback_scope(
+            env_.get(),
+            Local<Object>(),
+            { 1, 0 },
+            InternalCallbackScope::kAllowEmptyResource |
+                InternalCallbackScope::kSkipAsyncHooks);
+
         if (!env_->RunBootstrapping().IsEmpty()) {
           CreateEnvMessagePort(env_.get());
           if (is_stopped()) return;
           Debug(this, "Created message port for worker %llu", thread_id_);
           USE(StartExecution(env_.get(), "internal/main/worker_thread"));
         }
 
-        env_->async_hooks()->pop_async_id(1);
-
         Debug(this, "Loaded environment for worker %llu", thread_id_);
       }
 

src/stream_pipe.cc

@@ -119,7 +119,6 @@ void StreamPipe::ReadableListener::OnStreamRead(ssize_t nread,
                                                 const uv_buf_t& buf_) {
   StreamPipe* pipe = ContainerOf(&StreamPipe::readable_listener_, this);
   AllocatedBuffer buf(pipe->env(), buf_);
-  AsyncScope async_scope(pipe);
   if (nread < 0) {
     // EOF or error; stop reading and pass the error to the previous listener
     // (which might end up in JS).
@@ -162,7 +161,8 @@ void StreamPipe::WritableListener::OnStreamAfterWrite(WriteWrap* w,
   StreamPipe* pipe = ContainerOf(&StreamPipe::writable_listener_, this);
   pipe->is_writing_ = false;
   if (pipe->is_eof_) {
-    AsyncScope async_scope(pipe);
+    InternalCallbackScope callback_scope(pipe,
+        InternalCallbackScope::kSkipTaskQueues);
     pipe->ShutdownWritable();
     pipe->Unpipe();
     return;
@@ -206,7 +206,8 @@ void StreamPipe::WritableListener::OnStreamWantsWrite(size_t suggested_size) {
   pipe->wanted_data_ = suggested_size;
   if (pipe->is_reading_ || pipe->is_closed_)
     return;
-  AsyncScope async_scope(pipe);
+  InternalCallbackScope callback_scope(pipe,
+      InternalCallbackScope::kSkipTaskQueues);
   pipe->is_reading_ = true;
   pipe->source()->ReadStart();
 }