From 0fa0fe0e7703169cd0fe55efd0bcf755a2469615 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 20 Mar 2019 19:19:02 +0800 Subject: [PATCH] src: move AsyncCallbackScope out of Environment PR-URL: https://github.com/nodejs/node/pull/26824 Refs: https://github.com/nodejs/node/issues/26776 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- src/api/callback.cc | 4 ++-- src/env-inl.h | 20 +++++++++++++------- src/env.h | 30 ++++++++++++++++-------------- src/node.cc | 2 +- src/node_http_parser_impl.h | 4 ++-- src/node_internals.h | 2 +- src/node_worker.cc | 2 +- 7 files changed, 36 insertions(+), 28 deletions(-) diff --git a/src/api/callback.cc b/src/api/callback.cc index 7c6e8d8697ebf3..4083ae84879059 100644 --- 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 MakeCallback(Isolate* isolate, Context::Scope context_scope(env->context()); MaybeLocal 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 index d211d46997cd41..0cc086ce4128fa 100644 --- 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 index ec126e6d2a0599..f047a1f1c77eae 100644 --- 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 destroy_async_id_list_; std::shared_ptr options_; diff --git a/src/node.cc b/src/node.cc index b90dbbfb5f3709..3d587ced167821 100644 --- 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 index a154af5cfbaf4e..a2eea3bb3d7ce7 100644 --- 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 head_response = MakeCallback(cb.As(), 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 r = MakeCallback(cb.As(), 0, nullptr); diff --git a/src/node_internals.h b/src/node_internals.h index cd9b2590dffd91..59e95dca8e6e5b 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -211,7 +211,7 @@ class InternalCallbackScope { Environment* env_; async_context async_context_; v8::Local 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 index 1e19e529bd2ad5..24bd76c1ff400b 100644 --- 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"));