From bcb324c3ffd74147041cf892a0b2840aa340c248 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 20 Sep 2017 14:43:19 +0200 Subject: [PATCH] src: add can_call_into_js flag This prevents calls back into JS from the shutdown phase. Many thanks for Stephen Belanger for reviewing the original version of this commit in the Ayo.js project. Refs: https://github.com/ayojs/ayo/pull/82 PR-URL: https://github.com/nodejs/node/pull/19377 Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell --- src/async_wrap.cc | 7 +++++-- src/env-inl.h | 8 ++++++++ src/env.h | 7 +++++++ src/node.cc | 9 +++++++++ src/node_contextify.cc | 2 ++ 5 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/async_wrap.cc b/src/async_wrap.cc index f7a6d4e68dd..b20e2b746f7 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -141,6 +141,7 @@ static void DestroyAsyncIdsCallback(Environment* env, void* data) { do { std::vector destroy_async_id_list; destroy_async_id_list.swap(*env->destroy_async_id_list()); + if (!env->can_call_into_js()) return; for (auto async_id : destroy_async_id_list) { // Want each callback to be cleaned up after itself, instead of cleaning // them all up after the while() loop completes. @@ -166,7 +167,7 @@ void Emit(Environment* env, double async_id, AsyncHooks::Fields type, Local fn) { AsyncHooks* async_hooks = env->async_hooks(); - if (async_hooks->fields()[type] == 0) + if (async_hooks->fields()[type] == 0 || !env->can_call_into_js()) return; v8::HandleScope handle_scope(env->isolate()); @@ -625,8 +626,10 @@ void AsyncWrap::EmitTraceEventDestroy() { } void AsyncWrap::EmitDestroy(Environment* env, double async_id) { - if (env->async_hooks()->fields()[AsyncHooks::kDestroy] == 0) + if (env->async_hooks()->fields()[AsyncHooks::kDestroy] == 0 || + !env->can_call_into_js()) { return; + } if (env->destroy_async_id_list()->empty()) { env->SetUnrefImmediate(DestroyAsyncIdsCallback, nullptr); diff --git a/src/env-inl.h b/src/env-inl.h index f115656353c..0268879f5c7 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -559,6 +559,14 @@ void Environment::SetUnrefImmediate(native_immediate_callback cb, CreateImmediate(cb, data, obj, false); } +inline bool Environment::can_call_into_js() const { + return can_call_into_js_; +} + +inline void Environment::set_can_call_into_js(bool can_call_into_js) { + can_call_into_js_ = can_call_into_js; +} + inline performance::performance_state* Environment::performance_state() { return performance_state_.get(); } diff --git a/src/env.h b/src/env.h index de3014249ea..15d417ba606 100644 --- a/src/env.h +++ b/src/env.h @@ -679,6 +679,12 @@ class Environment { const char* path = nullptr, const char* dest = nullptr); + // If this flag is set, calls into JS (if they would be observable + // from userland) must be avoided. This flag does not indicate whether + // calling into JS is allowed from a VM perspective at this point. + inline bool can_call_into_js() const; + inline void set_can_call_into_js(bool can_call_into_js); + inline void ThrowError(const char* errmsg); inline void ThrowTypeError(const char* errmsg); inline void ThrowRangeError(const char* errmsg); @@ -821,6 +827,7 @@ class Environment { std::unique_ptr performance_state_; std::unordered_map performance_marks_; + bool can_call_into_js_ = true; #if HAVE_INSPECTOR std::unique_ptr inspector_agent_; diff --git a/src/node.cc b/src/node.cc index 5cb90d9d06e..6411eafb8cb 100644 --- a/src/node.cc +++ b/src/node.cc @@ -954,6 +954,11 @@ InternalCallbackScope::InternalCallbackScope(Environment* env, CHECK(!object.IsEmpty()); } + if (!env->can_call_into_js()) { + failed_ = true; + return; + } + HandleScope handle_scope(env->isolate()); // If you hit this assertion, you forgot to enter the v8::Context first. CHECK_EQ(Environment::GetCurrent(env->isolate()), env); @@ -997,6 +1002,7 @@ void InternalCallbackScope::Close() { Environment::TickInfo* tick_info = env_->tick_info(); + if (!env_->can_call_into_js()) return; if (!tick_info->has_scheduled()) { env_->isolate()->RunMicrotasks(); } @@ -1014,6 +1020,8 @@ void InternalCallbackScope::Close() { Local process = env_->process_object(); + if (!env_->can_call_into_js()) return; + if (env_->tick_callback_function()->Call(process, 0, nullptr).IsEmpty()) { env_->tick_info()->set_has_thrown(true); failed_ = true; @@ -4552,6 +4560,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, WaitForInspectorDisconnect(&env); + env.set_can_call_into_js(false); env.RunCleanup(); RunAtExit(&env); diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 4db82d4d7ca..23582454cd6 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -820,6 +820,8 @@ class ContextifyScript : public BaseObject { const bool display_errors, const bool break_on_sigint, const FunctionCallbackInfo& args) { + if (!env->can_call_into_js()) + return false; if (!ContextifyScript::InstanceOf(env, args.Holder())) { env->ThrowTypeError( "Script methods can only be called on script instances.");