Repository: denoland/deno_core
Pull request #770 (2024-06-06 19:52:35)

feat: ExternalOpsTracker
Allows an embedder to track "external" operations which should still keep the event loop alive. Will be used to ref napi threadsafe functions.


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 2674abc..09f022e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2648,9 +2648,9 @@ dependencies = [
 
 [[package]]
 name = "v8"
-version = "0.92.0"
+version = "0.93.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "234589219e37a7496cbce73d971586db8369871be2420372c45a579b6a919b15"
+checksum = "abc3ff12243d345cc697c151b29fbb8ad1f898e3f8b7aa1386701ff9f2c7cbbf"
 dependencies = [
  "bitflags",
  "fslock",
@@ -2758,15 +2758,14 @@ dependencies = [
 
 [[package]]
 name = "which"
-version = "5.0.0"
+version = "6.0.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9bf3ea8596f3a0dd5980b46430f2058dfe2c36a27ccfbb1845d6fbfcd9ba6e14"
+checksum = "8211e4f58a2b2805adfbefbc07bab82958fc91e3836339b1ab7ae32465dce0d7"
 dependencies = [
  "either",
  "home",
- "once_cell",
  "rustix",
- "windows-sys 0.48.0",
+ "winsafe",
 ]
 
 [[package]]
@@ -2948,6 +2947,12 @@ dependencies = [
  "memchr",
 ]
 
+[[package]]
+name = "winsafe"
+version = "0.0.19"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d135d17ab770252ad95e9a872d365cf3090e3be864a34ab46f48555993efc904"
+
 [[package]]
 name = "wyz"
 version = "0.5.1"
diff --git a/Cargo.toml b/Cargo.toml
index fd79656..2caa451 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -24,7 +24,7 @@ deno_ops = { version = "0.160.0", path = "./ops" }
 serde_v8 = { version = "0.193.0", path = "./serde_v8" }
 deno_core_testing = { path = "./testing" }
 
-v8 = { version = "0.92.0", default-features = false }
+v8 = { version = "0.93.0", default-features = false }
 deno_ast = { version = "=0.35.3", features = ["transpiling"] }
 deno_unsync = "0.3.2"
 deno_core_icudata = "0.0.73"
diff --git a/core/lib.rs b/core/lib.rs
index 727f9a7..abe938c 100644
--- a/core/lib.rs
+++ b/core/lib.rs
@@ -127,6 +127,7 @@ pub use crate::modules::SourceCodeCacheInfo;
 pub use crate::modules::StaticModuleLoader;
 pub use crate::modules::ValidateImportAttributesCb;
 pub use crate::normalize_path::normalize_path;
+pub use crate::ops::ExternalOpsTracker;
 pub use crate::ops::OpId;
 pub use crate::ops::OpMetadata;
 pub use crate::ops::OpState;
diff --git a/core/ops.rs b/core/ops.rs
index 4709782..f071e01 100644
--- a/core/ops.rs
+++ b/core/ops.rs
@@ -16,6 +16,8 @@ use std::ops::Deref;
 use std::ops::DerefMut;
 use std::ptr::NonNull;
 use std::rc::Rc;
+use std::sync::atomic::AtomicUsize;
+use std::sync::atomic::Ordering;
 use std::sync::Arc;
 use v8::fast_api::CFunctionInfo;
 use v8::fast_api::CTypeInfo;
@@ -250,12 +252,43 @@ impl OpCtx {
   }
 }
 
+/// Allows an embedder to track operations which should
+/// keep the event loop alive.
+#[derive(Debug, Clone)]
+pub struct ExternalOpsTracker {
+  counter: Arc<AtomicUsize>,
+}
+
+impl ExternalOpsTracker {
+  pub fn ref_op(&self) {
+    self.counter.fetch_add(1, Ordering::Relaxed);
+  }
+
+  pub fn unref_op(&self) {
+    let _ =
+      self
+        .counter
+        .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |x| {
+          if x == 0 {
+            None
+          } else {
+            Some(x - 1)
+          }
+        });
+  }
+
+  pub(crate) fn has_pending_ops(&self) -> bool {
+    self.counter.load(Ordering::Relaxed) > 0
+  }
+}
+
 /// Maintains the resources and ops inside a JS runtime.
 pub struct OpState {
   pub resource_table: ResourceTable,
   pub(crate) gotham_state: GothamState,
   pub waker: Arc<AtomicWaker>,
   pub feature_checker: Arc<FeatureChecker>,
+  pub external_ops_tracker: ExternalOpsTracker,
 }
 
 impl OpState {
@@ -265,6 +298,9 @@ impl OpState {
       gotham_state: Default::default(),
       waker: Arc::new(AtomicWaker::new()),
       feature_checker: maybe_feature_checker.unwrap_or_default(),
+      external_ops_tracker: ExternalOpsTracker {
+        counter: Arc::new(AtomicUsize::new(0)),
+      },
     }
   }
 
diff --git a/core/runtime/jsrealm.rs b/core/runtime/jsrealm.rs
index ab8e15c..694f2bb 100644
--- a/core/runtime/jsrealm.rs
+++ b/core/runtime/jsrealm.rs
@@ -11,6 +11,7 @@ use crate::modules::ModuleCodeString;
 use crate::modules::ModuleId;
 use crate::modules::ModuleMap;
 use crate::modules::ModuleName;
+use crate::ops::ExternalOpsTracker;
 use crate::ops::OpCtx;
 use crate::stats::RuntimeActivityTraces;
 use crate::tasks::V8TaskSpawnerFactory;
@@ -68,6 +69,7 @@ pub struct ContextState {
   pub(crate) exception_state: Rc<ExceptionState>,
   pub(crate) has_next_tick_scheduled: Cell<bool>,
   pub(crate) get_error_class_fn: GetErrorClassFn,
+  pub(crate) external_ops_tracker: ExternalOpsTracker,
 }
 
 impl ContextState {
@@ -76,6 +78,7 @@ impl ContextState {
     isolate_ptr: *mut v8::OwnedIsolate,
     get_error_class_fn: GetErrorClassFn,
     op_ctxs: Box<[OpCtx]>,
+    external_ops_tracker: ExternalOpsTracker,
   ) -> Self {
     Self {
       isolate: Some(isolate_ptr),
@@ -91,6 +94,7 @@ impl ContextState {
       task_spawner_factory: Default::default(),
       timers: Default::default(),
       unrefed_ops: Default::default(),
+      external_ops_tracker,
     }
   }
 }
diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs
index 8768c3f..2ff5591 100644
--- a/core/runtime/jsruntime.rs
+++ b/core/runtime/jsruntime.rs
@@ -605,13 +605,9 @@ impl JsRuntime {
   }
 
   /// Only constructor, configuration is done through `options`.
-  pub fn new(mut options: RuntimeOptions) -> JsRuntime {
-    setup::init_v8(
-      options.v8_platform.take(),
-      cfg!(test),
-      options.unsafe_expose_natives_and_gc(),
-    );
-    match JsRuntime::new_inner(options, false) {
+  /// Panics if the runtime cannot be initialized.
+  pub fn new(options: RuntimeOptions) -> JsRuntime {
+    match Self::try_new(options) {
       Ok(runtime) => runtime,
       Err(err) => {
         panic!("Failed to initialize a JsRuntime: {:?}", err);
@@ -619,6 +615,17 @@ impl JsRuntime {
     }
   }
 
+  /// Only constructor, configuration is done through `options`.
+  /// Returns an error if the runtime cannot be initialized.
+  pub fn try_new(mut options: RuntimeOptions) -> Result<JsRuntime, Error> {
+    setup::init_v8(
+      options.v8_platform.take(),
+      cfg!(test),
+      options.unsafe_expose_natives_and_gc(),
+    );
+    JsRuntime::new_inner(options, false)
+  }
+
   pub(crate) fn state_from(isolate: &v8::Isolate) -> Rc<JsRuntimeState> {
     let state_ptr = isolate.get_data(STATE_DATA_OFFSET);
     let state_rc =
@@ -804,6 +811,7 @@ impl JsRuntime {
       isolate_ptr,
       options.get_error_class_fn.unwrap_or(&|_| "Error"),
       op_ctxs,
+      op_state.borrow().external_ops_tracker.clone(),
     ));
 
     // TODO(bartlomieju): factor out
@@ -1802,6 +1810,7 @@ impl JsRuntime {
         || pending_state.has_pending_dyn_imports
         || pending_state.has_pending_dyn_module_evaluation
         || pending_state.has_pending_background_tasks
+        || pending_state.has_pending_external_ops
         || pending_state.has_tick_scheduled
       {
         // pass, will be polled again
@@ -1816,6 +1825,7 @@ impl JsRuntime {
       if pending_state.has_pending_ops
         || pending_state.has_pending_dyn_imports
         || pending_state.has_pending_background_tasks
+        || pending_state.has_pending_external_ops
         || pending_state.has_tick_scheduled
       {
         // pass, will be polled again
@@ -1884,20 +1894,28 @@ fn create_context<'a>(
 }
 
 impl JsRuntimeForSnapshot {
-  pub fn new(mut options: RuntimeOptions) -> JsRuntimeForSnapshot {
+  /// Create a new runtime, panicking if the process fails.
+  pub fn new(options: RuntimeOptions) -> JsRuntimeForSnapshot {
+    match Self::try_new(options) {
+      Ok(runtime) => runtime,
+      Err(err) => {
+        panic!("Failed to initialize JsRuntime for snapshotting: {:?}", err);
+      }
+    }
+  }
+
+  /// Try to create a new runtime, returning an error if the process fails.
+  pub fn try_new(
+    mut options: RuntimeOptions,
+  ) -> Result<JsRuntimeForSnapshot, Error> {
     setup::init_v8(
       options.v8_platform.take(),
       true,
       options.unsafe_expose_natives_and_gc(),
     );
 
-    let runtime = match JsRuntime::new_inner(options, true) {
-      Ok(r) => r,
-      Err(err) => {
-        panic!("Failed to initialize JsRuntime for snapshotting: {:?}", err);
-      }
-    };
-    JsRuntimeForSnapshot(runtime)
+    let runtime = JsRuntime::new_inner(options, true)?;
+    Ok(JsRuntimeForSnapshot(runtime))
   }
 
   /// Takes a snapshot and consumes the runtime.
@@ -1996,6 +2014,7 @@ pub(crate) struct EventLoopPendingState {
   has_pending_background_tasks: bool,
   has_tick_scheduled: bool,
   has_pending_promise_events: bool,
+  has_pending_external_ops: bool,
 }
 
 impl EventLoopPendingState {
@@ -2038,6 +2057,7 @@ impl EventLoopPendingState {
       has_pending_background_tasks: scope.has_pending_background_tasks(),
       has_tick_scheduled: state.has_next_tick_scheduled.get(),
       has_pending_promise_events,
+      has_pending_external_ops: state.external_ops_tracker.has_pending_ops(),
     }
   }
 
@@ -2056,6 +2076,7 @@ impl EventLoopPendingState {
       || self.has_pending_background_tasks
       || self.has_tick_scheduled
       || self.has_pending_promise_events
+      || self.has_pending_external_ops
   }
 }
 


Test patch:
diff --git a/core/runtime/tests/misc.rs b/core/runtime/tests/misc.rs
index ba94cc2..1ba8ff8 100644
--- a/core/runtime/tests/misc.rs
+++ b/core/runtime/tests/misc.rs
@@ -1243,8 +1243,9 @@ async fn global_template_middleware() {
     _key: v8::Local<'s, v8::Name>,
     _args: v8::PropertyCallbackArguments<'s>,
     _rv: v8::ReturnValue,
-  ) {
-    unsafe { CALLS.push("descriptor".to_string()) }
+  ) -> v8::Intercepted {
+    unsafe { CALLS.push("descriptor".to_string()) };
+    v8::Intercepted::No
   }
 
   pub fn setter<'s>(
@@ -1253,10 +1254,11 @@ async fn global_template_middleware() {
     _value: v8::Local<'s, v8::Value>,
     _args: v8::PropertyCallbackArguments<'s>,
     _rv: v8::ReturnValue,
-  ) {
+  ) -> v8::Intercepted {
     unsafe {
       CALLS.push("setter".to_string());
-    }
+    };
+    v8::Intercepted::No
   }
 
   fn definer<'s>(
@@ -1265,10 +1267,11 @@ async fn global_template_middleware() {
     _descriptor: &v8::PropertyDescriptor,
     _args: v8::PropertyCallbackArguments<'s>,
     _rv: v8::ReturnValue,
-  ) {
+  ) -> v8::Intercepted {
     unsafe {
       CALLS.push("definer".to_string());
-    }
+    };
+    v8::Intercepted::No
   }
 
   pub fn gt_middleware<'s>(