diff --git a/src/api/environment.cc b/src/api/environment.cc index 40aef133596e..a62b150cfa21 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -514,6 +514,9 @@ void FreeEnvironment(Environment* env) { Isolate* isolate = env->isolate(); Isolate::DisallowJavascriptExecutionScope disallow_js(isolate, Isolate::DisallowJavascriptExecutionScope::THROW_ON_FAILURE); + // A termination requested by Stop() targets this Environment; if no JS ran + // since, it is still pending and must not hit the isolate's next user. + isolate->CancelTerminateExecution(); { HandleScope handle_scope(isolate); // For env->context(). Context::Scope context_scope(env->context()); diff --git a/src/node_worker.cc b/src/node_worker.cc index 1a12946593ab..54427cef9032 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -326,11 +326,6 @@ void Worker::Run() { DeleteFnPtr env_; auto cleanup_env = OnScopeLeave([&]() { - // TODO(addaleax): This call is harmless but should not be necessary. - // Figure out why V8 is raising a DCHECK() here without it - // (in test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js). - isolate_->CancelTerminateExecution(); - if (!env_) return; env_->set_can_call_into_js(false); diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index 449d7d38750b..32f347c5c754 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -355,6 +355,27 @@ TEST_F(EnvironmentTest, MultipleEnvironmentsPerIsolate) { EXPECT_TRUE(called_cb_2); } +TEST_F(EnvironmentTest, StopFromExitHandlerDoesNotLeakIntoNextEnvironment) { + const v8::HandleScope handle_scope(isolate_); + const Argv argv; + { + Env env{handle_scope, argv}; + node::SetProcessExitHandler( + *env, [](node::Environment* env_, int) { node::Stop(env_); }); + // The uncaught exception runs the exit handler from C++ and does not + // re-enter JS afterwards, so nothing consumes the termination request. + EXPECT_TRUE( + node::LoadEnvironment(*env, "throw new Error('uncaught')").IsEmpty()); + EXPECT_TRUE(node::SpinEventLoop(*env).IsNothing()); + } + { + Env env{handle_scope, argv, node::EnvironmentFlags::kNoCreateInspector}; + v8::Local result = + node::LoadEnvironment(*env, "return 42;").ToLocalChecked(); + EXPECT_EQ(result->Int32Value(env.context()).FromJust(), 42); + } +} + TEST_F(EnvironmentTest, NoEnvironmentSanity) { const v8::HandleScope handle_scope(isolate_); v8::Local context = v8::Context::New(isolate_);