diff --git a/src/api/callback.cc b/src/api/callback.cc index 6d4e28e1d9070f..238bf49a54f76d 100644 --- a/src/api/callback.cc +++ b/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, 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; } diff --git a/src/async_wrap-inl.h b/src/async_wrap-inl.h index 53972493052f5e..e3e48666e4fbd6 100644 --- a/src/async_wrap-inl.h +++ b/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 AsyncWrap::MakeCallback( const v8::Local symbol, int argc, diff --git a/src/async_wrap.h b/src/async_wrap.h index 2651b5a054d554..dd82497a259243 100644 --- a/src/async_wrap.h +++ b/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 resource, @@ -200,18 +199,6 @@ class AsyncWrap : public BaseObject { static v8::Local GetOwner(Environment* env, v8::Local 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: diff --git a/src/env-inl.h b/src/env-inl.h index d61ae8f8df1d17..79708080d1e634 100644 --- a/src/env-inl.h +++ b/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_; } diff --git a/src/env.h b/src/env.h index a649675c7cc902..d6dd50b70110c5 100644 --- a/src/env.h +++ b/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(); diff --git a/src/node.cc b/src/node.cc index f437ea4be96450..0b7fca544f820b 100644 --- a/src/node.cc +++ b/src/node.cc @@ -406,13 +406,8 @@ MaybeLocal StartExecution(Environment* env, const char* main_script_id) { ->GetFunction(env->context()) .ToLocalChecked()}; - Local result; - if (!ExecuteBootstrapper(env, main_script_id, ¶meters, &arguments) - .ToLocal(&result) || - !task_queue::RunNextTicksNative(env)) { - return MaybeLocal(); - } - return scope.Escape(result); + return scope.EscapeMaybe( + ExecuteBootstrapper(env, main_script_id, ¶meters, &arguments)); } MaybeLocal StartMainThreadExecution(Environment* env) { diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 33769869992f8f..bfc1582b468e17 100644 --- a/src/node_http_parser.cc +++ b/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 head_response = - MakeCallback(cb.As(), arraysize(argv), argv); + MaybeLocal head_response; + { + InternalCallbackScope callback_scope( + this, InternalCallbackScope::kSkipTaskQueues); + head_response = cb.As()->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 r = MakeCallback(cb.As(), 0, nullptr); + MaybeLocal r; + { + InternalCallbackScope callback_scope( + this, InternalCallbackScope::kSkipTaskQueues); + r = cb.As()->Call(env()->context(), object(), 0, nullptr); + } if (r.IsEmpty()) { got_exception_ = true; diff --git a/src/node_internals.h b/src/node_internals.h index 4ec883c8913e45..4daf1d55b5deab 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -199,14 +199,24 @@ v8::MaybeLocal InternalMakeCallback( class InternalCallbackScope { public: - // Tell the constructor whether its `object` parameter may be empty or not. - enum ResourceExpectation { kRequireResource, kAllowEmptyResource }; + enum Flags { + kNoFlags = 0, + // 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 object, const async_context& asyncContext, - ResourceExpectation expect = kRequireResource); + int flags = kNoFlags); // 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 +227,8 @@ class InternalCallbackScope { Environment* env_; async_context async_context_; v8::Local object_; - AsyncCallbackScope callback_scope_; + bool skip_hooks_; + bool skip_task_queues_; bool failed_ = false; bool pushed_ids_ = false; bool closed_ = false; diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 1c1a09280b70d8..b1e20c2b14d40d 100644 --- a/src/node_main_instance.cc +++ b/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(), + { 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); diff --git a/src/node_process.h b/src/node_process.h index 48d5aa704f71e9..5db7b004d6f939 100644 --- a/src/node_process.h +++ b/src/node_process.h @@ -34,15 +34,6 @@ v8::Maybe ProcessEmitDeprecationWarning(Environment* env, v8::MaybeLocal CreateProcessObject(Environment* env); void PatchProcessObject(const v8::FunctionCallbackInfo& 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_ diff --git a/src/node_task_queue.cc b/src/node_task_queue.cc index ef1aff6cd4138d..f418a272470e2b 100644 --- a/src/node_task_queue.cc +++ b/src/node_task_queue.cc @@ -41,22 +41,6 @@ static void EnqueueMicrotask(const FunctionCallbackInfo& args) { isolate->EnqueueMicrotask(args[0].As()); } -// 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 callback = env->tick_callback_function(); - CHECK(!callback.IsEmpty()); - return !callback->Call(env->context(), env->process_object(), 0, nullptr) - .IsEmpty(); -} - static void RunMicrotasks(const FunctionCallbackInfo& args) { MicrotasksScope::PerformCheckpoint(args.GetIsolate()); } diff --git a/src/node_worker.cc b/src/node_worker.cc index af79540631f153..24a37ccd47c7ad 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -273,8 +273,13 @@ 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(), + { 1, 0 }, + InternalCallbackScope::kAllowEmptyResource | + InternalCallbackScope::kSkipAsyncHooks); + if (!env_->RunBootstrapping().IsEmpty()) { CreateEnvMessagePort(env_.get()); if (is_stopped()) return; @@ -282,8 +287,6 @@ void Worker::Run() { USE(StartExecution(env_.get(), "internal/main/worker_thread")); } - env_->async_hooks()->pop_async_id(1); - Debug(this, "Loaded environment for worker %llu", thread_id_); } diff --git a/src/stream_pipe.cc b/src/stream_pipe.cc index be7fc882ea5c8a..832a20d324f0ea 100644 --- a/src/stream_pipe.cc +++ b/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,9 @@ 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); + HandleScope handle_scope(pipe->env()->isolate()); + InternalCallbackScope callback_scope(pipe, + InternalCallbackScope::kSkipTaskQueues); pipe->ShutdownWritable(); pipe->Unpipe(); return; @@ -206,7 +207,9 @@ 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); + HandleScope handle_scope(pipe->env()->isolate()); + InternalCallbackScope callback_scope(pipe, + InternalCallbackScope::kSkipTaskQueues); pipe->is_reading_ = true; pipe->source()->ReadStart(); }