From fd37d37f9daf8ea7bceca5b742f530b5ea3cc766 Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Fri, 10 Nov 2023 09:21:24 -0700 Subject: [PATCH] src: use own RequestInterrupt implementation If RunCommand succeeds then the callback should always be called. This way everything can be cleaned up properly. PR-URL: https://github.com/nodesource/nsolid/pull/24 Reviewed-by: Santiago Gimeno --- src/nsolid/nsolid_api.cc | 28 ++++++++++++++++++++++++---- src/nsolid/nsolid_api.h | 3 ++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/nsolid/nsolid_api.cc b/src/nsolid/nsolid_api.cc index a207ef7d51..9cef5e6a43 100644 --- a/src/nsolid/nsolid_api.cc +++ b/src/nsolid/nsolid_api.cc @@ -106,6 +106,8 @@ EnvInst::EnvInst(Environment* env) er = eloop_cmds_msg_.init(event_loop_, run_event_loop_cmds_, this); CHECK_EQ(er, 0); + er = interrupt_msg_.init(event_loop_, run_interrupt_msg_, this); + CHECK_EQ(er, 0); er = metrics_handle_.init(event_loop_); CHECK_EQ(er, 0); er = info_lock_.init(true); @@ -118,10 +120,14 @@ EnvInst::EnvInst(Environment* env) CHECK_EQ(er, 0); eloop_cmds_msg_.unref(); + interrupt_msg_.unref(); metrics_handle_.unref(); env->RegisterHandleCleanup(eloop_cmds_msg_.base_handle(), handle_cleanup_cb_, nullptr); + env->RegisterHandleCleanup(interrupt_msg_.base_handle(), + handle_cleanup_cb_, + nullptr); env->RegisterHandleCleanup(metrics_handle_.base_handle(), handle_cleanup_cb_, nullptr); @@ -151,9 +157,11 @@ int EnvInst::RunCommand(SharedEnvInst envinst_sp, if (type == CommandType::Interrupt) { EnvInst::CmdQueueStor cmd_stor = { envinst_sp, cb, data }; + // Send it off to both locations. The first to run will retrieve the data + // from the queue. envinst_sp->interrupt_cb_q_.enqueue(std::move(cmd_stor)); - node::RequestInterrupt(envinst_sp->env(), run_interrupt_, nullptr); - return 0; + envinst_sp->isolate()->RequestInterrupt(run_interrupt_, nullptr); + return envinst_sp->interrupt_msg_.send(); } if (type == CommandType::InterruptOnly) { @@ -452,14 +460,26 @@ void EnvInst::run_event_loop_cmds_(ns_async*, EnvInst* envinst) { void EnvInst::run_interrupt_msg_(ns_async*, EnvInst* envinst) { CmdQueueStor stor; + // Need to disable access to JS from here, but first need to make sure the + // Isolate still exists before trying to disable JS access. + if (envinst->env() != nullptr) { + Isolate::DisallowJavascriptExecutionScope scope( + envinst->isolate(), + Isolate::DisallowJavascriptExecutionScope::CRASH_ON_FAILURE); + while (envinst->interrupt_cb_q_.dequeue(stor)) { + stor.cb(stor.envinst_sp, stor.data); + } + return; + } + while (envinst->interrupt_cb_q_.dequeue(stor)) { stor.cb(stor.envinst_sp, stor.data); } } -void EnvInst::run_interrupt_(void*) { - SharedEnvInst envinst_sp = GetCurrent(Isolate::GetCurrent()); +void EnvInst::run_interrupt_(Isolate* isolate, void*) { + SharedEnvInst envinst_sp = GetCurrent(isolate); if (!envinst_sp) { return; } diff --git a/src/nsolid/nsolid_api.h b/src/nsolid/nsolid_api.h index d4a56f68a9..c6aa9f3ccd 100644 --- a/src/nsolid/nsolid_api.h +++ b/src/nsolid/nsolid_api.h @@ -282,7 +282,7 @@ class EnvInst { static void run_event_loop_cmds_(nsuv::ns_async*, EnvInst*); static void run_interrupt_msg_(nsuv::ns_async*, EnvInst*); - static void run_interrupt_(void* arg); + static void run_interrupt_(v8::Isolate* isolate, void* arg); static void run_interrupt_only_(v8::Isolate* isolate, void* arg); static void handle_cleanup_cb_(Environment* env, uv_handle_t* handle, @@ -320,6 +320,7 @@ class EnvInst { uint64_t thread_id_ = UINT64_MAX; uv_thread_t creation_thread_; bool is_main_thread_; + nsuv::ns_async interrupt_msg_; // This is what's used to queue commands that run on the Worker's event loop. nsuv::ns_async eloop_cmds_msg_; TSQueue eloop_cmds_q_;