Skip to content

Commit 5b0d2e7

Browse files
committed
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: ayojs/ayo#82 PR-URL: #19377 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 9e2554c commit 5b0d2e7

File tree

5 files changed

+31
-2
lines changed

5 files changed

+31
-2
lines changed

src/async_wrap.cc

+5-2
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ static void DestroyAsyncIdsCallback(Environment* env, void* data) {
141141
do {
142142
std::vector<double> destroy_async_id_list;
143143
destroy_async_id_list.swap(*env->destroy_async_id_list());
144+
if (!env->can_call_into_js()) return;
144145
for (auto async_id : destroy_async_id_list) {
145146
// Want each callback to be cleaned up after itself, instead of cleaning
146147
// them all up after the while() loop completes.
@@ -166,7 +167,7 @@ void Emit(Environment* env, double async_id, AsyncHooks::Fields type,
166167
Local<Function> fn) {
167168
AsyncHooks* async_hooks = env->async_hooks();
168169

169-
if (async_hooks->fields()[type] == 0)
170+
if (async_hooks->fields()[type] == 0 || !env->can_call_into_js())
170171
return;
171172

172173
v8::HandleScope handle_scope(env->isolate());
@@ -625,8 +626,10 @@ void AsyncWrap::EmitTraceEventDestroy() {
625626
}
626627

627628
void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
628-
if (env->async_hooks()->fields()[AsyncHooks::kDestroy] == 0)
629+
if (env->async_hooks()->fields()[AsyncHooks::kDestroy] == 0 ||
630+
!env->can_call_into_js()) {
629631
return;
632+
}
630633

631634
if (env->destroy_async_id_list()->empty()) {
632635
env->SetUnrefImmediate(DestroyAsyncIdsCallback, nullptr);

src/env-inl.h

+8
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,14 @@ void Environment::SetUnrefImmediate(native_immediate_callback cb,
559559
CreateImmediate(cb, data, obj, false);
560560
}
561561

562+
inline bool Environment::can_call_into_js() const {
563+
return can_call_into_js_;
564+
}
565+
566+
inline void Environment::set_can_call_into_js(bool can_call_into_js) {
567+
can_call_into_js_ = can_call_into_js;
568+
}
569+
562570
inline performance::performance_state* Environment::performance_state() {
563571
return performance_state_.get();
564572
}

src/env.h

+7
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,12 @@ class Environment {
679679
const char* path = nullptr,
680680
const char* dest = nullptr);
681681

682+
// If this flag is set, calls into JS (if they would be observable
683+
// from userland) must be avoided. This flag does not indicate whether
684+
// calling into JS is allowed from a VM perspective at this point.
685+
inline bool can_call_into_js() const;
686+
inline void set_can_call_into_js(bool can_call_into_js);
687+
682688
inline void ThrowError(const char* errmsg);
683689
inline void ThrowTypeError(const char* errmsg);
684690
inline void ThrowRangeError(const char* errmsg);
@@ -821,6 +827,7 @@ class Environment {
821827

822828
std::unique_ptr<performance::performance_state> performance_state_;
823829
std::unordered_map<std::string, uint64_t> performance_marks_;
830+
bool can_call_into_js_ = true;
824831

825832
#if HAVE_INSPECTOR
826833
std::unique_ptr<inspector::Agent> inspector_agent_;

src/node.cc

+9
Original file line numberDiff line numberDiff line change
@@ -958,6 +958,11 @@ InternalCallbackScope::InternalCallbackScope(Environment* env,
958958
CHECK(!object.IsEmpty());
959959
}
960960

961+
if (!env->can_call_into_js()) {
962+
failed_ = true;
963+
return;
964+
}
965+
961966
HandleScope handle_scope(env->isolate());
962967
// If you hit this assertion, you forgot to enter the v8::Context first.
963968
CHECK_EQ(Environment::GetCurrent(env->isolate()), env);
@@ -1001,6 +1006,7 @@ void InternalCallbackScope::Close() {
10011006

10021007
Environment::TickInfo* tick_info = env_->tick_info();
10031008

1009+
if (!env_->can_call_into_js()) return;
10041010
if (!tick_info->has_scheduled()) {
10051011
env_->isolate()->RunMicrotasks();
10061012
}
@@ -1018,6 +1024,8 @@ void InternalCallbackScope::Close() {
10181024

10191025
Local<Object> process = env_->process_object();
10201026

1027+
if (!env_->can_call_into_js()) return;
1028+
10211029
if (env_->tick_callback_function()->Call(process, 0, nullptr).IsEmpty()) {
10221030
env_->tick_info()->set_has_thrown(true);
10231031
failed_ = true;
@@ -4580,6 +4588,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
45804588

45814589
WaitForInspectorDisconnect(&env);
45824590

4591+
env.set_can_call_into_js(false);
45834592
env.RunCleanup();
45844593
RunAtExit(&env);
45854594

src/node_contextify.cc

+2
Original file line numberDiff line numberDiff line change
@@ -819,6 +819,8 @@ class ContextifyScript : public BaseObject {
819819
const bool display_errors,
820820
const bool break_on_sigint,
821821
const FunctionCallbackInfo<Value>& args) {
822+
if (!env->can_call_into_js())
823+
return false;
822824
if (!ContextifyScript::InstanceOf(env, args.Holder())) {
823825
env->ThrowTypeError(
824826
"Script methods can only be called on script instances.");

0 commit comments

Comments
 (0)