From bea5dfb70f30559fe2c1323306494b8f5b23c75d Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Wed, 19 Aug 2026 20:42:07 +0000 Subject: [PATCH] node-api: enter env context for async callbacks `uvimpl::Work::AfterThreadPoolWork()` and the thread-safe function's `DispatchOne()` and `Finalize()` opened an `AsyncResource::CallbackScope` with only a `HandleScope`. `InternalCallbackScope` expects the resource's environment context to be entered and otherwise asserts that `Environment::GetCurrent(isolate)` is that environment, so with two environments on one isolate an addon's async work completion or thread-safe function call aborted the process whenever the other environment's context was current when the loop ran the callback. Enter the node-api env's context first, as `CallFinalizer()` and the zlib and WebCrypto thread pool callbacks already do. In `Finalize()` the scope covers only the finalizer call, since `MaybeDelete()` can free the env whose persistent handle `context()` returns. Signed-off-by: Shelley Vohr --- src/node_api.cc | 3 + test/cctest/test_node_api.cc | 106 +++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+) diff --git a/src/node_api.cc b/src/node_api.cc index d80569d9e92e..e0e7cca2a4ba 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -451,6 +451,7 @@ class ThreadSafeFunction { if (popped_value) { v8::HandleScope scope(env->isolate); + v8::Context::Scope context_scope(env->context()); AsyncResource::CallbackScope cb_scope(&*async_resource); napi_value js_callback = nullptr; if (!ref.IsEmpty()) { @@ -469,6 +470,7 @@ class ThreadSafeFunction { v8::HandleScope scope(env->isolate); EmptyQueue(); if (finalize_cb) { + v8::Context::Scope context_scope(env->context()); AsyncResource::CallbackScope cb_scope(&*async_resource); env->CallFinalizer(finalize_cb, finalize_data, context); } @@ -1236,6 +1238,7 @@ class Work : public node::AsyncResource, public node::ThreadPoolWork { // Establish a handle scope here so that every callback doesn't have to. // Also it is needed for the exception-handling below. v8::HandleScope scope(_env->isolate); + v8::Context::Scope context_scope(_env->context()); CallbackScope callback_scope(this); diff --git a/test/cctest/test_node_api.cc b/test/cctest/test_node_api.cc index 902776c157e9..29f1f3098fe4 100644 --- a/test/cctest/test_node_api.cc +++ b/test/cctest/test_node_api.cc @@ -40,3 +40,109 @@ TEST_F(NodeApiTest, CreateNodeApiEnv) { node_napi_env internal_env = reinterpret_cast(addon_env); EXPECT_EQ(internal_env->node_env(), env); } + +namespace { + +struct ContextCheckState { + napi_async_work work = nullptr; + bool complete_called = false; + bool complete_in_own_context = false; + bool call_js_called = false; + bool call_js_in_own_context = false; + bool tsfn_finalize_called = false; + bool tsfn_finalize_in_own_context = false; +}; + +bool InOwnContext(napi_env env) { + node_napi_env internal_env = reinterpret_cast(env); + return internal_env->isolate->GetCurrentContext() == internal_env->context(); +} + +} // namespace + +TEST_F(NodeApiTest, AsyncCallbacksEnterOwnContext) { + const v8::HandleScope handle_scope(isolate_); + Argv argv; + + Env env1{handle_scope, argv}; + node::LoadEnvironment(*env1, ""); + Env env2{handle_scope, argv, node::EnvironmentFlags::kNoFlags}; + node::LoadEnvironment(*env2, ""); + ASSERT_EQ(isolate_->GetCurrentContext(), env2.context()); + + ContextCheckState state; + { + v8::Context::Scope context_scope(env1.context()); + napi_addon_register_func init = [](napi_env env, napi_value exports) { + addon_env = env; + return exports; + }; + addon_env = nullptr; + napi_module_register_by_symbol(Object::New(isolate_), + Object::New(isolate_), + env1.context(), + init, + NAPI_VERSION); + ASSERT_NE(addon_env, nullptr); + + napi_value resource_name; + ASSERT_EQ(napi_create_string_utf8( + addon_env, "cctest", NAPI_AUTO_LENGTH, &resource_name), + napi_ok); + + ASSERT_EQ(napi_create_async_work( + addon_env, + nullptr, + resource_name, + [](napi_env env, void* data) {}, + [](napi_env env, napi_status status, void* data) { + auto* state = static_cast(data); + state->complete_called = true; + state->complete_in_own_context = InOwnContext(env); + napi_delete_async_work(env, state->work); + }, + &state, + &state.work), + napi_ok); + ASSERT_EQ(napi_queue_async_work(addon_env, state.work), napi_ok); + + napi_threadsafe_function tsfn; + ASSERT_EQ(napi_create_threadsafe_function( + addon_env, + nullptr, + nullptr, + resource_name, + 0, + 1, + &state, + [](napi_env env, void* finalize_data, void* hint) { + auto* state = + static_cast(finalize_data); + state->tsfn_finalize_called = true; + state->tsfn_finalize_in_own_context = InOwnContext(env); + }, + &state, + [](napi_env env, napi_value cb, void* context, void* data) { + auto* state = static_cast(context); + state->call_js_called = true; + state->call_js_in_own_context = InOwnContext(env); + }, + &tsfn), + napi_ok); + ASSERT_EQ(napi_call_threadsafe_function(tsfn, nullptr, napi_tsfn_blocking), + napi_ok); + ASSERT_EQ(napi_release_threadsafe_function(tsfn, napi_tsfn_release), + napi_ok); + } + + ASSERT_EQ(isolate_->GetCurrentContext(), env2.context()); + uv_run(¤t_loop, UV_RUN_DEFAULT); + + EXPECT_TRUE(state.complete_called); + EXPECT_TRUE(state.complete_in_own_context); + EXPECT_TRUE(state.call_js_called); + EXPECT_TRUE(state.call_js_in_own_context); + EXPECT_TRUE(state.tsfn_finalize_called); + EXPECT_TRUE(state.tsfn_finalize_in_own_context); + EXPECT_EQ(isolate_->GetCurrentContext(), env2.context()); +}