summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDivy Srivastava <dj.srivastava23@gmail.com>2023-03-08 22:21:14 +0530
committerGitHub <noreply@github.com>2023-03-08 22:21:14 +0530
commit0cce9c2bcc9667935a571d30847e66ef5d01a196 (patch)
tree3e188f47a7c8350a3f48fe5ec3a0b256351d41b9
parent06a12341c66e0a44436e1178603e0f398bfae10c (diff)
feat(core): prevent isolate drop for CLI main worker (#18059)
``` Benchmark 1: deno run -A ../empty.js Time (mean ± σ): 20.5 ms ± 0.5 ms [User: 13.4 ms, System: 5.1 ms] Range (min … max): 19.8 ms … 24.0 ms 119 runs Benchmark 2: target/release/deno run -A ../empty.js Time (mean ± σ): 18.8 ms ± 0.3 ms [User: 13.0 ms, System: 4.9 ms] Range (min … max): 18.3 ms … 19.9 ms 129 runs ``` --------- Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
-rw-r--r--cli/standalone.rs1
-rw-r--r--cli/worker.rs2
-rw-r--r--core/gotham_state.rs4
-rw-r--r--core/ops.rs4
-rw-r--r--core/runtime.rs18
-rw-r--r--runtime/examples/hello_runtime.rs1
-rw-r--r--runtime/worker.rs3
7 files changed, 33 insertions, 0 deletions
diff --git a/cli/standalone.rs b/cli/standalone.rs
index 352e04e57..7d13f91a8 100644
--- a/cli/standalone.rs
+++ b/cli/standalone.rs
@@ -302,6 +302,7 @@ pub async fn run(
shared_array_buffer_store: None,
compiled_wasm_module_store: None,
stdio: Default::default(),
+ leak_isolate: true,
};
let mut worker = MainWorker::bootstrap_from_options(
main_module.clone(),
diff --git a/cli/worker.rs b/cli/worker.rs
index 7ef90d79f..17d1d3c43 100644
--- a/cli/worker.rs
+++ b/cli/worker.rs
@@ -550,6 +550,7 @@ async fn create_main_worker_internal(
shared_array_buffer_store: Some(ps.shared_array_buffer_store.clone()),
compiled_wasm_module_store: Some(ps.compiled_wasm_module_store.clone()),
stdio,
+ leak_isolate: !bench_or_test && ps.options.coverage_dir().is_none(),
};
let mut worker = MainWorker::bootstrap_from_options(
@@ -780,6 +781,7 @@ mod tests {
shared_array_buffer_store: None,
compiled_wasm_module_store: None,
stdio: Default::default(),
+ leak_isolate: false,
};
MainWorker::bootstrap_from_options(main_module, permissions, options)
diff --git a/core/gotham_state.rs b/core/gotham_state.rs
index 422499f2f..43019eabb 100644
--- a/core/gotham_state.rs
+++ b/core/gotham_state.rs
@@ -76,6 +76,10 @@ impl GothamState {
pub fn take<T: 'static>(&mut self) -> T {
self.try_take().unwrap_or_else(|| missing::<T>())
}
+
+ pub fn clear(&mut self) {
+ self.data.clear();
+ }
}
fn missing<T: 'static>() -> ! {
diff --git a/core/ops.rs b/core/ops.rs
index ca465c821..b2442634e 100644
--- a/core/ops.rs
+++ b/core/ops.rs
@@ -179,6 +179,10 @@ impl OpState {
tracker: OpsTracker::new(ops_count),
}
}
+
+ pub fn clear_state(&mut self) {
+ self.gotham_state.clear();
+ }
}
impl Deref for OpState {
diff --git a/core/runtime.rs b/core/runtime.rs
index f751138a9..456dbe828 100644
--- a/core/runtime.rs
+++ b/core/runtime.rs
@@ -93,6 +93,9 @@ pub struct JsRuntime {
event_loop_middlewares: Vec<Box<OpEventLoopFn>>,
// Marks if this is considered the top-level runtime. Used only be inspector.
is_main: bool,
+ // Marks if it's OK to leak the current isolate. Use only by the
+ // CLI main worker.
+ leak_isolate: bool,
}
pub(crate) struct DynImportModEvaluate {
@@ -305,6 +308,10 @@ pub struct RuntimeOptions {
/// Describe if this is the main runtime instance, used by debuggers in some
/// situation - like disconnecting when program finishes running.
pub is_main: bool,
+
+ /// Whether it is OK to leak the V8 isolate. Only to be used by CLI
+ /// top-level runtime.
+ pub leak_isolate: bool,
}
#[derive(Copy, Clone, PartialEq, Eq)]
@@ -338,6 +345,16 @@ impl Drop for JsRuntime {
if let Some(v8_isolate) = self.v8_isolate.as_mut() {
Self::drop_state_and_module_map(v8_isolate);
}
+ if self.leak_isolate {
+ if let Some(v8_isolate) = self.v8_isolate.take() {
+ // Clear the GothamState. This allows final env cleanup hooks to run.
+ // Note: that OpState is cloned for every OpCtx, so we can't just drop
+ // one reference to it.
+ let rc_state = self.op_state();
+ rc_state.borrow_mut().clear_state();
+ std::mem::forget(v8_isolate);
+ }
+ }
}
}
@@ -674,6 +691,7 @@ impl JsRuntime {
state: state_rc,
module_map: Some(module_map_rc),
is_main: options.is_main,
+ leak_isolate: options.leak_isolate,
};
// Init resources and ops before extensions to make sure they are
diff --git a/runtime/examples/hello_runtime.rs b/runtime/examples/hello_runtime.rs
index fca7ff2c9..a47e9c908 100644
--- a/runtime/examples/hello_runtime.rs
+++ b/runtime/examples/hello_runtime.rs
@@ -66,6 +66,7 @@ async fn main() -> Result<(), AnyError> {
shared_array_buffer_store: None,
compiled_wasm_module_store: None,
stdio: Default::default(),
+ leak_isolate: true,
};
let js_path =
diff --git a/runtime/worker.rs b/runtime/worker.rs
index e624cb2b4..42ff6b0f9 100644
--- a/runtime/worker.rs
+++ b/runtime/worker.rs
@@ -141,6 +141,7 @@ pub struct WorkerOptions {
/// `WebAssembly.Module` objects cannot be serialized.
pub compiled_wasm_module_store: Option<CompiledWasmModuleStore>,
pub stdio: Stdio,
+ pub leak_isolate: bool,
}
impl Default for WorkerOptions {
@@ -177,6 +178,7 @@ impl Default for WorkerOptions {
startup_snapshot: Default::default(),
bootstrap: Default::default(),
stdio: Default::default(),
+ leak_isolate: false,
}
}
}
@@ -304,6 +306,7 @@ impl MainWorker {
extensions_with_js: options.extensions_with_js,
inspector: options.maybe_inspector_server.is_some(),
is_main: true,
+ leak_isolate: options.leak_isolate,
..Default::default()
});