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: #24 Reviewed-by: Santiago Gimeno <[email protected]>
nodesource · Nov 13, 2023 · 1dca94b · 1dca94b
1 parent 3ac8239
commit 1dca94b
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 5 deletions.
diff --git 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
@@ -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<CmdQueueStor> eloop_cmds_q_;