diff --git a/src/env.cc b/src/env.cc index 5f9a6acb461f97..9034c4e06971f3 100644 --- a/src/env.cc +++ b/src/env.cc @@ -527,6 +527,11 @@ void Environment::RegisterHandleCleanups() { } void Environment::CleanupHandles() { + Isolate::DisallowJavascriptExecutionScope disallow_js(isolate(), + Isolate::DisallowJavascriptExecutionScope::THROW_ON_FAILURE); + + RunAndClearNativeImmediates(true /* skip SetUnrefImmediate()s */); + for (ReqWrapBase* request : req_wrap_queue_) request->Cancel(); @@ -642,7 +647,7 @@ void Environment::AtExit(void (*cb)(void* arg), void* arg) { at_exit_functions_.push_front(ExitCallback{cb, arg}); } -void Environment::RunAndClearNativeImmediates() { +void Environment::RunAndClearNativeImmediates(bool only_refed) { TraceEventScope trace_scope(TRACING_CATEGORY_NODE1(environment), "RunAndClearNativeImmediates", this); size_t ref_count = 0; @@ -659,9 +664,11 @@ void Environment::RunAndClearNativeImmediates() { if (head->is_refed()) ref_count++; - head->Call(this); + if (head->is_refed() || !only_refed) + head->Call(this); + if (UNLIKELY(try_catch.HasCaught())) { - if (!try_catch.HasTerminated()) + if (!try_catch.HasTerminated() && can_call_into_js()) errors::TriggerUncaughtException(isolate(), try_catch); // We are done with the current callback. Move one iteration along, diff --git a/src/env.h b/src/env.h index 495d92471a336f..78724cebc1e519 100644 --- a/src/env.h +++ b/src/env.h @@ -1415,7 +1415,7 @@ class Environment : public MemoryRetainer { std::unique_ptr native_immediate_callbacks_head_; NativeImmediateCallback* native_immediate_callbacks_tail_ = nullptr; - void RunAndClearNativeImmediates(); + void RunAndClearNativeImmediates(bool only_refed = false); static void CheckImmediate(uv_check_t* handle); // Use an unordered_set, so that we have efficient insertion and removal. diff --git a/src/node_errors.cc b/src/node_errors.cc index e094fe4681fc56..17c1bd7f55c0fb 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -278,6 +278,9 @@ static void ReportFatalException(Environment* env, Local error, Local message, EnhanceFatalException enhance_stack) { + if (!env->can_call_into_js()) + enhance_stack = EnhanceFatalException::kDontEnhance; + Isolate* isolate = env->isolate(); CHECK(!error.IsEmpty()); CHECK(!message.IsEmpty()); @@ -956,7 +959,7 @@ void TriggerUncaughtException(Isolate* isolate, } MaybeLocal handled; - { + if (env->can_call_into_js()) { // We do not expect the global uncaught exception itself to throw any more // exceptions. If it does, exit the current Node.js instance. errors::TryCatchScope try_catch(env, diff --git a/src/node_process_events.cc b/src/node_process_events.cc index 440e67d412322b..06096226625bb6 100644 --- a/src/node_process_events.cc +++ b/src/node_process_events.cc @@ -34,6 +34,8 @@ Maybe ProcessEmitWarningGeneric(Environment* env, const char* warning, const char* type, const char* code) { + if (!env->can_call_into_js()) return Just(false); + HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); diff --git a/src/stream_pipe.cc b/src/stream_pipe.cc index 6e339378ceb374..d405c4d5cbeaed 100644 --- a/src/stream_pipe.cc +++ b/src/stream_pipe.cc @@ -42,7 +42,7 @@ StreamPipe::StreamPipe(StreamBase* source, } StreamPipe::~StreamPipe() { - Unpipe(); + Unpipe(true); } StreamBase* StreamPipe::source() { @@ -53,7 +53,7 @@ StreamBase* StreamPipe::sink() { return static_cast(writable_listener_.stream()); } -void StreamPipe::Unpipe() { +void StreamPipe::Unpipe(bool is_in_deletion) { if (is_closed_) return; @@ -68,11 +68,13 @@ void StreamPipe::Unpipe() { source()->RemoveStreamListener(&readable_listener_); sink()->RemoveStreamListener(&writable_listener_); + if (is_in_deletion) return; + // Delay the JS-facing part with SetImmediate, because this might be from // inside the garbage collector, so we can’t run JS here. HandleScope handle_scope(env()->isolate()); BaseObjectPtr strong_ref{this}; - env()->SetImmediate([this](Environment* env) { + env()->SetImmediate([this, strong_ref](Environment* env) { HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); Local object = this->object(); diff --git a/src/stream_pipe.h b/src/stream_pipe.h index 061ad9842e8f6d..0e155006102eaa 100644 --- a/src/stream_pipe.h +++ b/src/stream_pipe.h @@ -12,7 +12,7 @@ class StreamPipe : public AsyncWrap { StreamPipe(StreamBase* source, StreamBase* sink, v8::Local obj); ~StreamPipe() override; - void Unpipe(); + void Unpipe(bool is_in_deletion = false); static void New(const v8::FunctionCallbackInfo& args); static void Start(const v8::FunctionCallbackInfo& args); diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index cc9b8e4531f6ef..0db2963acc9ba3 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -185,3 +185,26 @@ static void at_exit_js(void* arg) { assert(obj->IsObject()); called_at_exit_js = true; } + +TEST_F(EnvironmentTest, SetImmediateCleanup) { + int called = 0; + int called_unref = 0; + + { + const v8::HandleScope handle_scope(isolate_); + const Argv argv; + Env env {handle_scope, argv}; + + (*env)->SetImmediate([&](node::Environment* env_arg) { + EXPECT_EQ(env_arg, *env); + called++; + }); + (*env)->SetUnrefImmediate([&](node::Environment* env_arg) { + EXPECT_EQ(env_arg, *env); + called_unref++; + }); + } + + EXPECT_EQ(called, 1); + EXPECT_EQ(called_unref, 0); +}