From 848dabd31c4a475d3d578940faa0b3beb444c789 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 24 May 2024 20:01:29 +0530 Subject: [PATCH 01/19] reuse vm snapshot context --- ext/node/ops/vm_internal.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 9c641954e260c5..ea3595465c05c6 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -70,9 +70,9 @@ impl ContextifyContext { scope: &mut v8::HandleScope, sandbox_obj: v8::Local, ) { - let tmp = init_global_template(scope); + init_global_template(scope); - let context = create_v8_context(scope, tmp, None); + let context = create_v8_context(scope); Self::from_context(scope, context, sandbox_obj); } @@ -175,17 +175,10 @@ pub const VM_CONTEXT_INDEX: usize = 0; fn create_v8_context<'a>( scope: &mut v8::HandleScope<'a>, - object_template: v8::Local, - snapshot_data: Option<&'static [u8]>, ) -> v8::Local<'a, v8::Context> { let scope = &mut v8::EscapableHandleScope::new(scope); - - let context = if let Some(_snapshot_data) = snapshot_data { - v8::Context::from_snapshot(scope, VM_CONTEXT_INDEX).unwrap() - } else { - v8::Context::new_from_template(scope, object_template) - }; - + let context = v8::Context::from_snapshot(scope, VM_CONTEXT_INDEX) + .expect("Failed to create VM context from snapshot"); scope.escape(context) } From 21e16de6ae4adfb82955e2fd50590b5cedb4cf51 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 29 May 2024 11:40:55 +0530 Subject: [PATCH 02/19] pogress --- ext/node/lib.rs | 39 ++++++++++++++- ext/node/ops/vm.rs | 2 + ext/node/ops/vm_internal.rs | 94 +++++++++++++++++++++++-------------- leak.mjs | 8 ++++ runtime/snapshot.rs | 8 +++- 5 files changed, 112 insertions(+), 39 deletions(-) create mode 100644 leak.mjs diff --git a/ext/node/lib.rs b/ext/node/lib.rs index 2b31f704f6f4e5..a935ba00f1c28e 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -33,9 +33,12 @@ mod path; mod polyfill; mod resolution; +use ops::vm; pub use ops::ipc::ChildPipeFd; pub use ops::ipc::IpcJsonStreamResource; pub use ops::v8::VM_CONTEXT_INDEX; +pub use ops::vm::create_v8_context; +pub use ops::vm::init_global_template; pub use package_json::PackageJson; pub use path::PathClean; pub use polyfill::is_builtin_node_module; @@ -599,7 +602,41 @@ deno_core::extension!(deno_node, global_template_middleware = global_template_middleware, global_object_middleware = global_object_middleware, customizer = |ext: &mut deno_core::Extension| { - let mut external_references = Vec::with_capacity(7); + let mut external_references = Vec::with_capacity(14); + use deno_core::v8::MapFnTo; + external_references.push(ExternalReference { + function: vm::c_noop + }); + vm::GETTER_MAP_FN.with(|getter| { + external_references.push(ExternalReference { + named_getter: *getter, + }); + }); + vm::SETTER_MAP_FN.with(|setter| { + external_references.push(ExternalReference { + named_setter: *setter, + }); + }); + vm::DELETER_MAP_FN.with(|deleter| { + external_references.push(ExternalReference { + named_getter: *deleter, + },); + }); + vm::ENUMERATOR_MAP_FN.with(|enumerator| { + external_references.push(ExternalReference { + enumerator: *enumerator, + }); + }); + vm::DEFINER_MAP_FN.with(|definer| { + external_references.push(ExternalReference { + named_definer: *definer, + }); + }); + vm::DESCRIPTOR_MAP_FN.with(|descriptor| { + external_references.push(ExternalReference { + named_getter: *descriptor, + }); + }); global::GETTER_MAP_FN.with(|getter| { external_references.push(ExternalReference { diff --git a/ext/node/ops/vm.rs b/ext/node/ops/vm.rs index f18038f8f05ab2..e252f64561bf9d 100644 --- a/ext/node/ops/vm.rs +++ b/ext/node/ops/vm.rs @@ -7,6 +7,8 @@ use deno_core::v8; use super::vm_internal as i; +pub use i::*; + pub struct Script { inner: i::ContextifyScript, } diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index ea3595465c05c6..4c6b5d4d961c3c 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -70,9 +70,9 @@ impl ContextifyContext { scope: &mut v8::HandleScope, sandbox_obj: v8::Local, ) { - init_global_template(scope); + let tmp = init_global_template(scope, true); - let context = create_v8_context(scope); + let context = create_v8_context(scope, tmp, true); Self::from_context(scope, context, sandbox_obj); } @@ -88,7 +88,8 @@ impl ContextifyContext { .clone(); v8_context.set_security_token(main_context.get_security_token(scope)); - v8_context.set_slot(scope, context_state); + // This segfaults :( + // v8_context.set_slot(scope, context_state); let context = v8::Global::new(scope, v8_context); let sandbox = v8::Global::new(scope, sandbox_obj); @@ -165,6 +166,9 @@ impl ContextifyContext { let context = object.get_creation_context(scope)?; let context_ptr = context.get_aligned_pointer_from_embedder_data(1); + if context_ptr.is_null() { + return None; + } // SAFETY: We are storing a pointer to the ContextifyContext // in the embedder data of the v8::Context during creation. Some(unsafe { &*(context_ptr as *const ContextifyContext) }) @@ -173,36 +177,54 @@ impl ContextifyContext { pub const VM_CONTEXT_INDEX: usize = 0; -fn create_v8_context<'a>( - scope: &mut v8::HandleScope<'a>, +pub fn create_v8_context<'a>( + scope: &mut v8::HandleScope<'a, ()>, + object_template: v8::Local, + use_snapshot: bool, ) -> v8::Local<'a, v8::Context> { let scope = &mut v8::EscapableHandleScope::new(scope); - let context = v8::Context::from_snapshot(scope, VM_CONTEXT_INDEX) - .expect("Failed to create VM context from snapshot"); + + let context = if use_snapshot { + v8::Context::from_snapshot(scope, VM_CONTEXT_INDEX).unwrap() + } else { + let ctx = v8::Context::new_from_template(scope, object_template); + // ContextifyContexts will update this to a pointer to the native object + unsafe { ctx.set_aligned_pointer_in_embedder_data(1, std::ptr::null_mut()) }; + ctx + }; + scope.escape(context) } #[derive(Debug, Clone)] struct SlotContextifyGlobalTemplate(v8::Global); -fn init_global_template<'a>( - scope: &mut v8::HandleScope<'a>, +pub fn init_global_template<'a>( + scope: &mut v8::HandleScope<'a, ()>, + cache: bool, ) -> v8::Local<'a, v8::ObjectTemplate> { let mut maybe_object_template_slot = scope.get_slot::(); if maybe_object_template_slot.is_none() { - init_global_template_inner(scope); - maybe_object_template_slot = - scope.get_slot::(); - } + let global_object_template = init_global_template_inner(scope); + + if cache { + let contextify_global_template_slot = SlotContextifyGlobalTemplate( + v8::Global::new(scope, global_object_template), + ); + scope.set_slot(contextify_global_template_slot); + } + global_object_template + } else { let object_template_slot = maybe_object_template_slot .expect("ContextifyGlobalTemplate slot should be already populated.") .clone(); v8::Local::new(scope, object_template_slot.0) + } } -extern "C" fn c_noop(_: *const v8::FunctionCallbackInfo) {} +pub extern "C" fn c_noop(_: *const v8::FunctionCallbackInfo) {} // Using thread_local! to get around compiler bug. // @@ -224,7 +246,9 @@ thread_local! { pub static INDEXED_DESCRIPTOR_MAP_FN: v8::IndexedPropertyGetterCallback<'static> = indexed_property_descriptor.map_fn_to(); } -fn init_global_template_inner(scope: &mut v8::HandleScope) { +pub fn init_global_template_inner<'a>( + scope: &mut v8::HandleScope<'a, ()>, +) -> v8::Local<'a, v8::ObjectTemplate> { let global_func_template = v8::FunctionTemplate::builder_raw(c_noop).build(scope); let global_object_template = global_func_template.instance_template(scope); @@ -250,16 +274,16 @@ fn init_global_template_inner(scope: &mut v8::HandleScope) { let mut config = v8::IndexedPropertyHandlerConfiguration::new() .flags(v8::PropertyHandlerFlags::HAS_NO_SIDE_EFFECT); - config = INDEXED_GETTER_MAP_FN.with(|getter| config.getter_raw(*getter)); - config = INDEXED_SETTER_MAP_FN.with(|setter| config.setter_raw(*setter)); - config = - INDEXED_DELETER_MAP_FN.with(|deleter| config.deleter_raw(*deleter)); - config = - ENUMERATOR_MAP_FN.with(|enumerator| config.enumerator_raw(*enumerator)); - config = - INDEXED_DEFINER_MAP_FN.with(|definer| config.definer_raw(*definer)); - config = INDEXED_DESCRIPTOR_MAP_FN - .with(|descriptor| config.descriptor_raw(*descriptor)); +// config = INDEXED_GETTER_MAP_FN.with(|getter| config.getter_raw(*getter)); +// config = INDEXED_SETTER_MAP_FN.with(|setter| config.setter_raw(*setter)); +// config = +// INDEXED_DELETER_MAP_FN.with(|deleter| config.deleter_raw(*deleter)); +// config = +// ENUMERATOR_MAP_FN.with(|enumerator| config.enumerator_raw(*enumerator)); +// config = +// INDEXED_DEFINER_MAP_FN.with(|definer| config.definer_raw(*definer)); +// config = INDEXED_DESCRIPTOR_MAP_FN +// .with(|descriptor| config.descriptor_raw(*descriptor)); config }; @@ -268,10 +292,8 @@ fn init_global_template_inner(scope: &mut v8::HandleScope) { .set_named_property_handler(named_property_handler_config); global_object_template .set_indexed_property_handler(indexed_property_handler_config); - let contextify_global_template_slot = SlotContextifyGlobalTemplate( - v8::Global::new(scope, global_object_template), - ); - scope.set_slot(contextify_global_template_slot); + + global_object_template } fn property_getter<'s>( @@ -280,7 +302,7 @@ fn property_getter<'s>( args: v8::PropertyCallbackArguments<'s>, mut ret: v8::ReturnValue, ) { - let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; let sandbox = ctx.sandbox(scope); @@ -311,7 +333,7 @@ fn property_setter<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { - let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; let (attributes, is_declared_on_global_proxy) = match ctx .global_proxy(scope) @@ -399,7 +421,7 @@ fn property_deleter<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { - let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; let context = ctx.context(scope); let sandbox = ctx.sandbox(scope); @@ -416,7 +438,7 @@ fn property_enumerator<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { - let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; let context = ctx.context(scope); let sandbox = ctx.sandbox(scope); @@ -437,7 +459,7 @@ fn property_definer<'s>( args: v8::PropertyCallbackArguments<'s>, _: v8::ReturnValue, ) { - let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; let context = ctx.context(scope); let (attributes, is_declared) = match ctx @@ -513,7 +535,7 @@ fn property_descriptor<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { - let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; let context = ctx.context(scope); let sandbox = ctx.sandbox(scope); @@ -562,7 +584,7 @@ fn indexed_property_deleter<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { - let ctx = ContextifyContext::get(scope, args.this()).unwrap(); + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; let context = ctx.context(scope); let sandbox = ctx.sandbox(scope); diff --git a/leak.mjs b/leak.mjs new file mode 100644 index 00000000000000..d4eb1ad6430cf6 --- /dev/null +++ b/leak.mjs @@ -0,0 +1,8 @@ +import vm from "node:vm"; + +const script = new vm.Script("returnValue = 2+2"); +console.log("Running script 1000 times"); + +for (let i = 0; i < 1000; i++) { + script.runInNewContext({}, { timeout: 10000 }); +} diff --git a/runtime/snapshot.rs b/runtime/snapshot.rs index 923ea0b7596d4f..1ba334cec6f5b5 100644 --- a/runtime/snapshot.rs +++ b/runtime/snapshot.rs @@ -285,8 +285,12 @@ pub fn create_runtime_snapshot( let isolate = rt.v8_isolate(); let scope = &mut v8::HandleScope::new(isolate); - let ctx = v8::Context::new(scope); - assert_eq!(scope.add_context(ctx), deno_node::VM_CONTEXT_INDEX); + // let main_context = v8::Context::new(scope); + // scope.add_context(main_context); + + let tmpl = deno_node::init_global_template(scope, false); + let ctx = deno_node::create_v8_context(scope, tmpl, false); + assert_eq!(scope.add_context(ctx), 0); })), skip_op_registration: false, }, From 2b94abc80f1c34b0751c8e588972170dfe1248eb Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 7 Jun 2024 11:28:31 +0530 Subject: [PATCH 03/19] Fixes --- Cargo.lock | 6 ------ ext/node/ops/vm_internal.rs | 25 ++++++++++++++++++------- module.mjs | 3 +++ 3 files changed, 21 insertions(+), 13 deletions(-) create mode 100644 module.mjs diff --git a/Cargo.lock b/Cargo.lock index 142788eb30b4a7..2755ff2aa51290 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1304,8 +1304,6 @@ dependencies = [ [[package]] name = "deno_core" version = "0.284.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f4a5c809e81be26fcfbbce4275573251f6a156137b67059889e9e38f73e75b63" dependencies = [ "anyhow", "bincode", @@ -1760,8 +1758,6 @@ dependencies = [ [[package]] name = "deno_ops" version = "0.160.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "517e54d41a2da6a69b8f534294334d79d9115ddd43aea88a5ceefdb717e6d85e" dependencies = [ "proc-macro-rules", "proc-macro2", @@ -5764,8 +5760,6 @@ dependencies = [ [[package]] name = "serde_v8" version = "0.193.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "21ec612dfc7ab70330b5405e8015b25e637bbfe1d79c4bd173557933aea66e76" dependencies = [ "num-bigint", "serde", diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 4c6b5d4d961c3c..2202ad6b3e4ffa 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -83,13 +83,22 @@ impl ContextifyContext { ) { let main_context = scope.get_current_context(); let context_state = main_context - .get_slot::>(scope) - .unwrap() - .clone(); + .get_aligned_pointer_from_embedder_data(1); + let module_map = main_context + .get_aligned_pointer_from_embedder_data(2); v8_context.set_security_token(main_context.get_security_token(scope)); - // This segfaults :( - // v8_context.set_slot(scope, context_state); + // SAFETY: set embedder data from the creation context + unsafe { + v8_context.set_aligned_pointer_in_embedder_data( + 1, + context_state, + ); + v8_context.set_aligned_pointer_in_embedder_data( + 2, + module_map, + ); + } let context = v8::Global::new(scope, v8_context); let sandbox = v8::Global::new(scope, sandbox_obj); @@ -103,7 +112,7 @@ impl ContextifyContext { // lives longer than the execution context, so this should be safe. unsafe { v8_context.set_aligned_pointer_in_embedder_data( - 1, + 3, ptr as *const ContextifyContext as _, ); } @@ -165,7 +174,7 @@ impl ContextifyContext { ) -> Option<&'c ContextifyContext> { let context = object.get_creation_context(scope)?; - let context_ptr = context.get_aligned_pointer_from_embedder_data(1); + let context_ptr = context.get_aligned_pointer_from_embedder_data(3); if context_ptr.is_null() { return None; } @@ -190,6 +199,8 @@ pub fn create_v8_context<'a>( let ctx = v8::Context::new_from_template(scope, object_template); // ContextifyContexts will update this to a pointer to the native object unsafe { ctx.set_aligned_pointer_in_embedder_data(1, std::ptr::null_mut()) }; + unsafe { ctx.set_aligned_pointer_in_embedder_data(2, std::ptr::null_mut()) }; + unsafe { ctx.set_aligned_pointer_in_embedder_data(3, std::ptr::null_mut()) }; ctx }; diff --git a/module.mjs b/module.mjs new file mode 100644 index 00000000000000..2e0931952eb31d --- /dev/null +++ b/module.mjs @@ -0,0 +1,3 @@ +import vm from "node:vm"; +const script = new vm.Script("import('node:process')"); +console.log(await script.runInNewContext()); From e248d089a0b566c54490d3ed99375ef438466af0 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 7 Jun 2024 12:33:15 +0530 Subject: [PATCH 04/19] x --- Cargo.lock | 4 ++++ ext/node/ops/vm_internal.rs | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2755ff2aa51290..982541ef4a9226 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8044,3 +8044,7 @@ dependencies = [ "cc", "pkg-config", ] + +[[patch.unused]] +name = "v8" +version = "0.93.0" diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 2202ad6b3e4ffa..8b8b0505d640ad 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -198,8 +198,6 @@ pub fn create_v8_context<'a>( } else { let ctx = v8::Context::new_from_template(scope, object_template); // ContextifyContexts will update this to a pointer to the native object - unsafe { ctx.set_aligned_pointer_in_embedder_data(1, std::ptr::null_mut()) }; - unsafe { ctx.set_aligned_pointer_in_embedder_data(2, std::ptr::null_mut()) }; unsafe { ctx.set_aligned_pointer_in_embedder_data(3, std::ptr::null_mut()) }; ctx }; From e4adc96b38954b56d45ac1644847e1bf7ae8554b Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 7 Jun 2024 13:46:28 +0530 Subject: [PATCH 05/19] use indexes --- ext/node/ops/vm_internal.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 8b8b0505d640ad..d464f65c3719b0 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -83,19 +83,19 @@ impl ContextifyContext { ) { let main_context = scope.get_current_context(); let context_state = main_context - .get_aligned_pointer_from_embedder_data(1); + .get_aligned_pointer_from_embedder_data(deno_core::CONTEXT_STATE_SLOT_INDEX); let module_map = main_context - .get_aligned_pointer_from_embedder_data(2); + .get_aligned_pointer_from_embedder_data(deno_core::MODULE_MAP_SLOT_INDEX); v8_context.set_security_token(main_context.get_security_token(scope)); // SAFETY: set embedder data from the creation context unsafe { v8_context.set_aligned_pointer_in_embedder_data( - 1, + deno_core::CONTEXT_STATE_SLOT_INDEX, context_state, ); v8_context.set_aligned_pointer_in_embedder_data( - 2, + deno_core::MODULE_MAP_SLOT_INDEX, module_map, ); } From 36c264a807514a8032acb5c98aa68c88f2b7de55 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 7 Jun 2024 14:24:47 +0530 Subject: [PATCH 06/19] fixes --- ext/node/lib.rs | 30 ++++++++++++++- ext/node/ops/vm_internal.rs | 76 ++++++++++++++++++++++--------------- runtime/snapshot.rs | 5 +-- 3 files changed, 75 insertions(+), 36 deletions(-) diff --git a/ext/node/lib.rs b/ext/node/lib.rs index 7d2799728897ec..a854a7829d56cd 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -32,10 +32,10 @@ mod path; mod polyfill; mod resolution; -use ops::vm; pub use ops::ipc::ChildPipeFd; pub use ops::ipc::IpcJsonStreamResource; pub use ops::v8::VM_CONTEXT_INDEX; +use ops::vm; pub use ops::vm::create_v8_context; pub use ops::vm::init_global_template; pub use package_json::PackageJson; @@ -647,10 +647,10 @@ deno_core::extension!(deno_node, global_object_middleware = global_object_middleware, customizer = |ext: &mut deno_core::Extension| { let mut external_references = Vec::with_capacity(14); - use deno_core::v8::MapFnTo; external_references.push(ExternalReference { function: vm::c_noop }); + vm::GETTER_MAP_FN.with(|getter| { external_references.push(ExternalReference { named_getter: *getter, @@ -682,6 +682,32 @@ deno_core::extension!(deno_node, }); }); + vm::INDEXED_GETTER_MAP_FN.with(|getter| { + external_references.push(ExternalReference { + indexed_getter: *getter, + }); + }); + vm::INDEXED_SETTER_MAP_FN.with(|setter| { + external_references.push(ExternalReference { + indexed_setter: *setter, + }); + }); + vm::INDEXED_DELETER_MAP_FN.with(|deleter| { + external_references.push(ExternalReference { + indexed_getter: *deleter, + }); + }); + vm::INDEXED_DEFINER_MAP_FN.with(|definer| { + external_references.push(ExternalReference { + indexed_definer: *definer, + }); + }); + vm::INDEXED_DESCRIPTOR_MAP_FN.with(|descriptor| { + external_references.push(ExternalReference { + indexed_getter: *descriptor, + }); + }); + global::GETTER_MAP_FN.with(|getter| { external_references.push(ExternalReference { named_getter: *getter, diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index d464f65c3719b0..d3713abb262933 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -4,7 +4,6 @@ use deno_core::error::type_error; use deno_core::error::AnyError; use deno_core::v8; use deno_core::v8::MapFnTo; -use std::rc::Rc; pub const PRIVATE_SYMBOL_NAME: v8::OneByteConst = v8::String::create_external_onebyte_const(b"node:contextify:context"); @@ -82,8 +81,9 @@ impl ContextifyContext { sandbox_obj: v8::Local, ) { let main_context = scope.get_current_context(); - let context_state = main_context - .get_aligned_pointer_from_embedder_data(deno_core::CONTEXT_STATE_SLOT_INDEX); + let context_state = main_context.get_aligned_pointer_from_embedder_data( + deno_core::CONTEXT_STATE_SLOT_INDEX, + ); let module_map = main_context .get_aligned_pointer_from_embedder_data(deno_core::MODULE_MAP_SLOT_INDEX); @@ -198,7 +198,9 @@ pub fn create_v8_context<'a>( } else { let ctx = v8::Context::new_from_template(scope, object_template); // ContextifyContexts will update this to a pointer to the native object - unsafe { ctx.set_aligned_pointer_in_embedder_data(3, std::ptr::null_mut()) }; + unsafe { + ctx.set_aligned_pointer_in_embedder_data(3, std::ptr::null_mut()) + }; ctx }; @@ -212,24 +214,24 @@ pub fn init_global_template<'a>( scope: &mut v8::HandleScope<'a, ()>, cache: bool, ) -> v8::Local<'a, v8::ObjectTemplate> { - let mut maybe_object_template_slot = + let maybe_object_template_slot = scope.get_slot::(); if maybe_object_template_slot.is_none() { let global_object_template = init_global_template_inner(scope); if cache { - let contextify_global_template_slot = SlotContextifyGlobalTemplate( - v8::Global::new(scope, global_object_template), - ); - scope.set_slot(contextify_global_template_slot); + let contextify_global_template_slot = SlotContextifyGlobalTemplate( + v8::Global::new(scope, global_object_template), + ); + scope.set_slot(contextify_global_template_slot); } global_object_template } else { - let object_template_slot = maybe_object_template_slot - .expect("ContextifyGlobalTemplate slot should be already populated.") - .clone(); - v8::Local::new(scope, object_template_slot.0) + let object_template_slot = maybe_object_template_slot + .expect("ContextifyGlobalTemplate slot should be already populated.") + .clone(); + v8::Local::new(scope, object_template_slot.0) } } @@ -283,16 +285,16 @@ pub fn init_global_template_inner<'a>( let mut config = v8::IndexedPropertyHandlerConfiguration::new() .flags(v8::PropertyHandlerFlags::HAS_NO_SIDE_EFFECT); -// config = INDEXED_GETTER_MAP_FN.with(|getter| config.getter_raw(*getter)); -// config = INDEXED_SETTER_MAP_FN.with(|setter| config.setter_raw(*setter)); -// config = -// INDEXED_DELETER_MAP_FN.with(|deleter| config.deleter_raw(*deleter)); -// config = -// ENUMERATOR_MAP_FN.with(|enumerator| config.enumerator_raw(*enumerator)); -// config = -// INDEXED_DEFINER_MAP_FN.with(|definer| config.definer_raw(*definer)); -// config = INDEXED_DESCRIPTOR_MAP_FN -// .with(|descriptor| config.descriptor_raw(*descriptor)); + config = INDEXED_GETTER_MAP_FN.with(|getter| config.getter_raw(*getter)); + config = INDEXED_SETTER_MAP_FN.with(|setter| config.setter_raw(*setter)); + config = + INDEXED_DELETER_MAP_FN.with(|deleter| config.deleter_raw(*deleter)); + config = + ENUMERATOR_MAP_FN.with(|enumerator| config.enumerator_raw(*enumerator)); + config = + INDEXED_DEFINER_MAP_FN.with(|definer| config.definer_raw(*definer)); + config = INDEXED_DESCRIPTOR_MAP_FN + .with(|descriptor| config.descriptor_raw(*descriptor)); config }; @@ -311,7 +313,9 @@ fn property_getter<'s>( args: v8::PropertyCallbackArguments<'s>, mut ret: v8::ReturnValue, ) { - let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { + return; + }; let sandbox = ctx.sandbox(scope); @@ -342,7 +346,9 @@ fn property_setter<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { - let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { + return; + }; let (attributes, is_declared_on_global_proxy) = match ctx .global_proxy(scope) @@ -430,7 +436,9 @@ fn property_deleter<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { - let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { + return; + }; let context = ctx.context(scope); let sandbox = ctx.sandbox(scope); @@ -447,7 +455,9 @@ fn property_enumerator<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { - let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { + return; + }; let context = ctx.context(scope); let sandbox = ctx.sandbox(scope); @@ -468,7 +478,9 @@ fn property_definer<'s>( args: v8::PropertyCallbackArguments<'s>, _: v8::ReturnValue, ) { - let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { + return; + }; let context = ctx.context(scope); let (attributes, is_declared) = match ctx @@ -544,7 +556,9 @@ fn property_descriptor<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { - let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { + return; + }; let context = ctx.context(scope); let sandbox = ctx.sandbox(scope); @@ -593,7 +607,9 @@ fn indexed_property_deleter<'s>( args: v8::PropertyCallbackArguments<'s>, mut rv: v8::ReturnValue, ) { - let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; + let Some(ctx) = ContextifyContext::get(scope, args.this()) else { + return; + }; let context = ctx.context(scope); let sandbox = ctx.sandbox(scope); diff --git a/runtime/snapshot.rs b/runtime/snapshot.rs index 65576a5d678085..424596fe5e2a97 100644 --- a/runtime/snapshot.rs +++ b/runtime/snapshot.rs @@ -285,12 +285,9 @@ pub fn create_runtime_snapshot( let isolate = rt.v8_isolate(); let scope = &mut v8::HandleScope::new(isolate); - // let main_context = v8::Context::new(scope); - // scope.add_context(main_context); - let tmpl = deno_node::init_global_template(scope, false); let ctx = deno_node::create_v8_context(scope, tmpl, false); - assert_eq!(scope.add_context(ctx), 0); + assert_eq!(scope.add_context(ctx), deno_node::VM_CONTEXT_INDEX); })), skip_op_registration: false, }, From 8fbf099c010c7e7e914509e9ce2f421cdaa81ebe Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Sat, 8 Jun 2024 08:29:14 +0530 Subject: [PATCH 07/19] x --- ext/node/ops/vm_internal.rs | 6 ++++-- leak.mjs | 8 -------- tests/config/deno.lock | 15 +++++++++++++++ tests/unit_node/vm_test.ts | 9 +++++++++ 4 files changed, 28 insertions(+), 10 deletions(-) delete mode 100644 leak.mjs create mode 100644 tests/config/deno.lock diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index d3713abb262933..013ecbc1f0b2d7 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -263,7 +263,7 @@ pub fn init_global_template_inner<'a>( let global_func_template = v8::FunctionTemplate::builder_raw(c_noop).build(scope); let global_object_template = global_func_template.instance_template(scope); - global_object_template.set_internal_field_count(2); + global_object_template.set_internal_field_count(3); let named_property_handler_config = { let mut config = v8::NamedPropertyHandlerConfiguration::new() @@ -316,7 +316,7 @@ fn property_getter<'s>( let Some(ctx) = ContextifyContext::get(scope, args.this()) else { return; }; - + let sandbox = ctx.sandbox(scope); let tc_scope = &mut v8::TryCatch::new(scope); @@ -335,6 +335,8 @@ fn property_getter<'s>( rv = ctx.global_proxy(tc_scope).into(); } + println!("property_getter: {:?}", rv.to_rust_string_lossy(tc_scope)); + ret.set(rv); } } diff --git a/leak.mjs b/leak.mjs deleted file mode 100644 index d4eb1ad6430cf6..00000000000000 --- a/leak.mjs +++ /dev/null @@ -1,8 +0,0 @@ -import vm from "node:vm"; - -const script = new vm.Script("returnValue = 2+2"); -console.log("Running script 1000 times"); - -for (let i = 0; i < 1000; i++) { - script.runInNewContext({}, { timeout: 10000 }); -} diff --git a/tests/config/deno.lock b/tests/config/deno.lock new file mode 100644 index 00000000000000..3b51561a131d04 --- /dev/null +++ b/tests/config/deno.lock @@ -0,0 +1,15 @@ +{ + "version": "3", + "packages": { + "specifiers": { + "npm:@types/node": "npm:@types/node@18.16.19" + }, + "npm": { + "@types/node@18.16.19": { + "integrity": "sha512-IXl7o+R9iti9eBW4Wg2hx1xQDig183jj7YLn8F7udNceyfkbn1ZxmzZXuak20gR40D7pIkIY1kYGx5VIGbaHKA==", + "dependencies": {} + } + } + }, + "remote": {} +} diff --git a/tests/unit_node/vm_test.ts b/tests/unit_node/vm_test.ts index 21e56ee94313f8..cdb2d18a2361b5 100644 --- a/tests/unit_node/vm_test.ts +++ b/tests/unit_node/vm_test.ts @@ -148,3 +148,12 @@ reject().catch(() => {}) script.runInNewContext(); }, }); + +Deno.test({ + name: "vm runInNewContext module loader", + fn() { + const code = "import('node:process')"; + const script = new Script(code); + script.runInNewContext(); + }, +}); From 865b99f46d8f057256491204ffa6dd37ebb176b3 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Tue, 11 Jun 2024 22:46:29 +0530 Subject: [PATCH 08/19] x --- Cargo.lock | 10 ++++++---- tests/node_compat/runner/suite | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 982541ef4a9226..142788eb30b4a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1304,6 +1304,8 @@ dependencies = [ [[package]] name = "deno_core" version = "0.284.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f4a5c809e81be26fcfbbce4275573251f6a156137b67059889e9e38f73e75b63" dependencies = [ "anyhow", "bincode", @@ -1758,6 +1760,8 @@ dependencies = [ [[package]] name = "deno_ops" version = "0.160.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "517e54d41a2da6a69b8f534294334d79d9115ddd43aea88a5ceefdb717e6d85e" dependencies = [ "proc-macro-rules", "proc-macro2", @@ -5760,6 +5764,8 @@ dependencies = [ [[package]] name = "serde_v8" version = "0.193.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "21ec612dfc7ab70330b5405e8015b25e637bbfe1d79c4bd173557933aea66e76" dependencies = [ "num-bigint", "serde", @@ -8044,7 +8050,3 @@ dependencies = [ "cc", "pkg-config", ] - -[[patch.unused]] -name = "v8" -version = "0.93.0" diff --git a/tests/node_compat/runner/suite b/tests/node_compat/runner/suite index b114fad0ec952f..d12a68fc493006 160000 --- a/tests/node_compat/runner/suite +++ b/tests/node_compat/runner/suite @@ -1 +1 @@ -Subproject commit b114fad0ec952fddddefc8972c43d2959388bbc1 +Subproject commit d12a68fc4930062c0bed26447a6b5245697e77c1 From a358b6978642e2e5d724787fb4adf6c683c928f7 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Tue, 11 Jun 2024 22:51:30 +0530 Subject: [PATCH 09/19] rm lock --- tests/config/deno.lock | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 tests/config/deno.lock diff --git a/tests/config/deno.lock b/tests/config/deno.lock deleted file mode 100644 index 3b51561a131d04..00000000000000 --- a/tests/config/deno.lock +++ /dev/null @@ -1,15 +0,0 @@ -{ - "version": "3", - "packages": { - "specifiers": { - "npm:@types/node": "npm:@types/node@18.16.19" - }, - "npm": { - "@types/node@18.16.19": { - "integrity": "sha512-IXl7o+R9iti9eBW4Wg2hx1xQDig183jj7YLn8F7udNceyfkbn1ZxmzZXuak20gR40D7pIkIY1kYGx5VIGbaHKA==", - "dependencies": {} - } - } - }, - "remote": {} -} From 216436ee86bf066669ac0598c33744647e70f5ed Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 12 Jun 2024 09:12:13 +0530 Subject: [PATCH 10/19] Fixes --- ext/node/global.rs | 2 +- ext/node/ops/vm_internal.rs | 7 ++++--- tests/unit_node/vm_test.ts | 34 ++++++++++++++++++++++++++++------ 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/ext/node/global.rs b/ext/node/global.rs index 5c95b2f8fba02a..e01fca95e1dc55 100644 --- a/ext/node/global.rs +++ b/ext/node/global.rs @@ -275,7 +275,7 @@ fn current_mode(scope: &mut v8::HandleScope) -> Mode { }; let mut buffer = [MaybeUninit::uninit(); 2048]; let str = v8_string.to_rust_cow_lossy(scope, &mut buffer); - if node_resolver.in_npm_package_with_cache(str) { + if str.starts_with("node:") || node_resolver.in_npm_package_with_cache(str) { Mode::Node } else { Mode::Deno diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 95893f6f8c23b4..381058c69dc885 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -199,7 +199,10 @@ pub fn create_v8_context<'a>( let ctx = v8::Context::new_from_template(scope, object_template); // ContextifyContexts will update this to a pointer to the native object unsafe { - ctx.set_aligned_pointer_in_embedder_data(3, std::ptr::null_mut()) + ctx.set_aligned_pointer_in_embedder_data(1, std::ptr::null_mut()); + ctx.set_aligned_pointer_in_embedder_data(2, std::ptr::null_mut()); + ctx.set_aligned_pointer_in_embedder_data(3, std::ptr::null_mut()); + ctx.clear_all_slots(scope); }; ctx }; @@ -335,8 +338,6 @@ fn property_getter<'s>( rv = ctx.global_proxy(tc_scope).into(); } - println!("property_getter: {:?}", rv.to_rust_string_lossy(tc_scope)); - ret.set(rv); return v8::Intercepted::Yes; } diff --git a/tests/unit_node/vm_test.ts b/tests/unit_node/vm_test.ts index cdb2d18a2361b5..46e231c6ca72bf 100644 --- a/tests/unit_node/vm_test.ts +++ b/tests/unit_node/vm_test.ts @@ -149,11 +149,33 @@ reject().catch(() => {}) }, }); +// https://github.com/denoland/deno/issues/22441 Deno.test({ - name: "vm runInNewContext module loader", - fn() { - const code = "import('node:process')"; - const script = new Script(code); - script.runInNewContext(); - }, + name: "vm runInNewContext module loader", + fn() { + const code = "import('node:process')"; + const script = new Script(code); + script.runInNewContext(); + }, +}); + +// https://github.com/denoland/deno/issues/23913 +Deno.test({ + name: "vm memory leak crash", + fn() { + const script = new Script("returnValue = 2+2"); + + for (let i = 0; i < 1000; i++) { + script.runInNewContext({}, { timeout: 10000 }); + } + }, +}); + +// https://github.com/denoland/deno/issues/23852 +Deno.test({ + name: "vm runInThisContext global.foo", + fn() { + const result = runInThisContext(`global.foo = 1`); + assertEquals(result, 1); + }, }); From 1463137fb18868c652101f9edac5cc1fbd54ad1c Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 12 Jun 2024 09:52:47 +0530 Subject: [PATCH 11/19] lint --- ext/node/ops/vm_internal.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 381058c69dc885..0795cf3d57e422 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -197,10 +197,8 @@ pub fn create_v8_context<'a>( v8::Context::from_snapshot(scope, VM_CONTEXT_INDEX).unwrap() } else { let ctx = v8::Context::new_from_template(scope, object_template); - // ContextifyContexts will update this to a pointer to the native object + // SAFETY: ContextifyContexts will update this to a pointer to the native object unsafe { - ctx.set_aligned_pointer_in_embedder_data(1, std::ptr::null_mut()); - ctx.set_aligned_pointer_in_embedder_data(2, std::ptr::null_mut()); ctx.set_aligned_pointer_in_embedder_data(3, std::ptr::null_mut()); ctx.clear_all_slots(scope); }; From 9d645465b43fdcc0b534b441cb834407edb36794 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 12 Jun 2024 09:53:03 +0530 Subject: [PATCH 12/19] cleanup --- module.mjs | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 module.mjs diff --git a/module.mjs b/module.mjs deleted file mode 100644 index 2e0931952eb31d..00000000000000 --- a/module.mjs +++ /dev/null @@ -1,3 +0,0 @@ -import vm from "node:vm"; -const script = new vm.Script("import('node:process')"); -console.log(await script.runInNewContext()); From b3d8fced2d2d39ec25a13ed17487bbbe83153940 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 12 Jun 2024 10:10:17 +0530 Subject: [PATCH 13/19] x --- ext/node/ops/vm_internal.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index 0795cf3d57e422..d05e86f4a20c16 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -199,6 +199,8 @@ pub fn create_v8_context<'a>( let ctx = v8::Context::new_from_template(scope, object_template); // SAFETY: ContextifyContexts will update this to a pointer to the native object unsafe { + ctx.set_aligned_pointer_in_embedder_data(1, std::ptr::null_mut()); + ctx.set_aligned_pointer_in_embedder_data(2, std::ptr::null_mut()); ctx.set_aligned_pointer_in_embedder_data(3, std::ptr::null_mut()); ctx.clear_all_slots(scope); }; From b92c236ed8091c488592af259de8d98ea0184c7b Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 12 Jun 2024 10:47:57 +0530 Subject: [PATCH 14/19] update cache key --- .github/workflows/ci.generate.ts | 2 +- .github/workflows/ci.yml | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.generate.ts b/.github/workflows/ci.generate.ts index e86e13971e179f..ff0c4953d27f22 100755 --- a/.github/workflows/ci.generate.ts +++ b/.github/workflows/ci.generate.ts @@ -5,7 +5,7 @@ import { stringify } from "jsr:@std/yaml@^0.221/stringify"; // Bump this number when you want to purge the cache. // Note: the tools/release/01_bump_crate_versions.ts script will update this version // automatically via regex, so ensure that this line maintains this format. -const cacheVersion = 95; +const cacheVersion = 96; const ubuntuX86Runner = "ubuntu-22.04"; const ubuntuX86XlRunner = "ubuntu-22.04-xl"; diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 81af5dc28dbca2..d084528ec73286 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -367,8 +367,8 @@ jobs: path: |- ~/.cargo/registry/index ~/.cargo/registry/cache - key: '95-cargo-home-${{ matrix.os }}-${{ matrix.arch }}-${{ hashFiles(''Cargo.lock'') }}' - restore-keys: '95-cargo-home-${{ matrix.os }}-${{ matrix.arch }}' + key: '96-cargo-home-${{ matrix.os }}-${{ matrix.arch }}-${{ hashFiles(''Cargo.lock'') }}' + restore-keys: '96-cargo-home-${{ matrix.os }}-${{ matrix.arch }}' if: '!(matrix.skip)' - name: Restore cache build output (PR) uses: actions/cache/restore@v4 @@ -380,7 +380,7 @@ jobs: !./target/*/*.zip !./target/*/*.tar.gz key: never_saved - restore-keys: '95-cargo-target-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.profile }}-${{ matrix.job }}-' + restore-keys: '96-cargo-target-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.profile }}-${{ matrix.job }}-' - name: Apply and update mtime cache if: '!(matrix.skip) && (!startsWith(github.ref, ''refs/tags/''))' uses: ./.github/mtime_cache @@ -669,7 +669,7 @@ jobs: !./target/*/gn_out !./target/*/*.zip !./target/*/*.tar.gz - key: '95-cargo-target-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.profile }}-${{ matrix.job }}-${{ github.sha }}' + key: '96-cargo-target-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.profile }}-${{ matrix.job }}-${{ github.sha }}' publish-canary: name: publish canary runs-on: ubuntu-22.04 From 4d33b556246bd4d2587e9f0287fbbb090d270feb Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 12 Jun 2024 15:15:47 +0530 Subject: [PATCH 15/19] Enable tmux session --- .github/workflows/ci.generate.ts | 9 +++++++++ .github/workflows/ci.yml | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/.github/workflows/ci.generate.ts b/.github/workflows/ci.generate.ts index ff0c4953d27f22..48aad12d7d4a26 100755 --- a/.github/workflows/ci.generate.ts +++ b/.github/workflows/ci.generate.ts @@ -688,6 +688,15 @@ const ci = { ].join("\n"), env: { CARGO_PROFILE_DEV_DEBUG: 0 }, }, + { + name: "Setup tmate session", + if: [ + "(matrix.job == 'test' || matrix.job == 'bench') &&", + "matrix.profile == 'debug' && (matrix.use_sysroot ||", + "github.repository == 'denoland/deno')", + ].join("\n"), + uses: "mxschmitt/action-tmate@v3", + }, { name: "Build release", if: [ diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d084528ec73286..ce0161f7cf209b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -411,6 +411,12 @@ jobs: df -h env: CARGO_PROFILE_DEV_DEBUG: 0 + - name: Setup tmate session + if: |- + !(matrix.skip) && ((matrix.job == 'test' || matrix.job == 'bench') && + matrix.profile == 'debug' && (matrix.use_sysroot || + github.repository == 'denoland/deno')) + uses: mxschmitt/action-tmate@v3 - name: Build release if: |- !(matrix.skip) && ((matrix.job == 'test' || matrix.job == 'bench') && From 1674c8f4d1a855ff58a632faa4e98f78205a635d Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 12 Jun 2024 15:19:53 +0530 Subject: [PATCH 16/19] fix --- .github/workflows/ci.generate.ts | 2 +- .github/workflows/ci.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.generate.ts b/.github/workflows/ci.generate.ts index 48aad12d7d4a26..9b8d4b4b02e430 100755 --- a/.github/workflows/ci.generate.ts +++ b/.github/workflows/ci.generate.ts @@ -692,7 +692,7 @@ const ci = { name: "Setup tmate session", if: [ "(matrix.job == 'test' || matrix.job == 'bench') &&", - "matrix.profile == 'debug' && (matrix.use_sysroot ||", + "matrix.profile == 'release' && (matrix.use_sysroot ||", "github.repository == 'denoland/deno')", ].join("\n"), uses: "mxschmitt/action-tmate@v3", diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ce0161f7cf209b..fe5478a250fec4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -414,7 +414,7 @@ jobs: - name: Setup tmate session if: |- !(matrix.skip) && ((matrix.job == 'test' || matrix.job == 'bench') && - matrix.profile == 'debug' && (matrix.use_sysroot || + matrix.profile == 'release' && (matrix.use_sysroot || github.repository == 'denoland/deno')) uses: mxschmitt/action-tmate@v3 - name: Build release From a8746ab570c513884cea84a35f522b771f0f7364 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 12 Jun 2024 16:20:29 +0530 Subject: [PATCH 17/19] X --- .github/workflows/ci.generate.ts | 19 ++++++++++--------- ext/node/lib.rs | 3 --- ext/node/ops/vm_internal.rs | 6 +----- 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.generate.ts b/.github/workflows/ci.generate.ts index 9b8d4b4b02e430..95218dae79b93b 100755 --- a/.github/workflows/ci.generate.ts +++ b/.github/workflows/ci.generate.ts @@ -688,15 +688,16 @@ const ci = { ].join("\n"), env: { CARGO_PROFILE_DEV_DEBUG: 0 }, }, - { - name: "Setup tmate session", - if: [ - "(matrix.job == 'test' || matrix.job == 'bench') &&", - "matrix.profile == 'release' && (matrix.use_sysroot ||", - "github.repository == 'denoland/deno')", - ].join("\n"), - uses: "mxschmitt/action-tmate@v3", - }, + // Uncomment for remote debugging + // { + // name: "Setup tmate session", + // if: [ + // "(matrix.job == 'test' || matrix.job == 'bench') &&", + // "matrix.profile == 'release' && (matrix.use_sysroot ||", + // "github.repository == 'denoland/deno')", + // ].join("\n"), + // uses: "mxschmitt/action-tmate@v3", + // }, { name: "Build release", if: [ diff --git a/ext/node/lib.rs b/ext/node/lib.rs index 89fa3d7e8d2088..6462c4beec7a0d 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -645,9 +645,6 @@ deno_core::extension!(deno_node, global_object_middleware = global_object_middleware, customizer = |ext: &mut deno_core::Extension| { let mut external_references = Vec::with_capacity(14); - external_references.push(ExternalReference { - function: vm::c_noop - }); vm::GETTER_MAP_FN.with(|getter| { external_references.push(ExternalReference { diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index d05e86f4a20c16..d223c5a77ad901 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -238,8 +238,6 @@ pub fn init_global_template<'a>( } } -pub extern "C" fn c_noop(_: *const v8::FunctionCallbackInfo) {} - // Using thread_local! to get around compiler bug. // // See NOTE in ext/node/global.rs#L12 @@ -263,9 +261,7 @@ thread_local! { pub fn init_global_template_inner<'a>( scope: &mut v8::HandleScope<'a, ()>, ) -> v8::Local<'a, v8::ObjectTemplate> { - let global_func_template = - v8::FunctionTemplate::builder_raw(c_noop).build(scope); - let global_object_template = global_func_template.instance_template(scope); + let global_object_template = v8::ObjectTemplate::new(scope); global_object_template.set_internal_field_count(3); let named_property_handler_config = { From b3f2282dfcee5556fbe055682ca7fc4db1f50633 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 12 Jun 2024 16:26:10 +0530 Subject: [PATCH 18/19] x --- .github/workflows/ci.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe5478a250fec4..d084528ec73286 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -411,12 +411,6 @@ jobs: df -h env: CARGO_PROFILE_DEV_DEBUG: 0 - - name: Setup tmate session - if: |- - !(matrix.skip) && ((matrix.job == 'test' || matrix.job == 'bench') && - matrix.profile == 'release' && (matrix.use_sysroot || - github.repository == 'denoland/deno')) - uses: mxschmitt/action-tmate@v3 - name: Build release if: |- !(matrix.skip) && ((matrix.job == 'test' || matrix.job == 'bench') && From 0ea52597206052fb982414c311a3095519842f1a Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 12 Jun 2024 18:35:07 +0530 Subject: [PATCH 19/19] review --- ext/node/lib.rs | 1 + ext/node/ops/vm.rs | 17 ++++++++++++++++- ext/node/ops/vm_internal.rs | 18 ++++++++++++------ runtime/snapshot.rs | 11 +++++++++-- 4 files changed, 38 insertions(+), 9 deletions(-) diff --git a/ext/node/lib.rs b/ext/node/lib.rs index 6462c4beec7a0d..8a20b3109b87a4 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -38,6 +38,7 @@ pub use ops::v8::VM_CONTEXT_INDEX; use ops::vm; pub use ops::vm::create_v8_context; pub use ops::vm::init_global_template; +pub use ops::vm::ContextInitMode; pub use package_json::PackageJson; pub use path::PathClean; pub use polyfill::is_builtin_node_module; diff --git a/ext/node/ops/vm.rs b/ext/node/ops/vm.rs index e252f64561bf9d..a0a63cc6e5a2d6 100644 --- a/ext/node/ops/vm.rs +++ b/ext/node/ops/vm.rs @@ -7,7 +7,22 @@ use deno_core::v8; use super::vm_internal as i; -pub use i::*; +pub use i::create_v8_context; +pub use i::init_global_template; +pub use i::ContextInitMode; + +pub use i::DEFINER_MAP_FN; +pub use i::DELETER_MAP_FN; +pub use i::DESCRIPTOR_MAP_FN; +pub use i::ENUMERATOR_MAP_FN; +pub use i::GETTER_MAP_FN; +pub use i::SETTER_MAP_FN; + +pub use i::INDEXED_DEFINER_MAP_FN; +pub use i::INDEXED_DELETER_MAP_FN; +pub use i::INDEXED_DESCRIPTOR_MAP_FN; +pub use i::INDEXED_GETTER_MAP_FN; +pub use i::INDEXED_SETTER_MAP_FN; pub struct Script { inner: i::ContextifyScript, diff --git a/ext/node/ops/vm_internal.rs b/ext/node/ops/vm_internal.rs index d223c5a77ad901..4af1825954300e 100644 --- a/ext/node/ops/vm_internal.rs +++ b/ext/node/ops/vm_internal.rs @@ -69,9 +69,9 @@ impl ContextifyContext { scope: &mut v8::HandleScope, sandbox_obj: v8::Local, ) { - let tmp = init_global_template(scope, true); + let tmp = init_global_template(scope, ContextInitMode::UseSnapshot); - let context = create_v8_context(scope, tmp, true); + let context = create_v8_context(scope, tmp, ContextInitMode::UseSnapshot); Self::from_context(scope, context, sandbox_obj); } @@ -186,14 +186,20 @@ impl ContextifyContext { pub const VM_CONTEXT_INDEX: usize = 0; +#[derive(PartialEq)] +pub enum ContextInitMode { + ForSnapshot, + UseSnapshot, +} + pub fn create_v8_context<'a>( scope: &mut v8::HandleScope<'a, ()>, object_template: v8::Local, - use_snapshot: bool, + mode: ContextInitMode, ) -> v8::Local<'a, v8::Context> { let scope = &mut v8::EscapableHandleScope::new(scope); - let context = if use_snapshot { + let context = if mode == ContextInitMode::UseSnapshot { v8::Context::from_snapshot(scope, VM_CONTEXT_INDEX).unwrap() } else { let ctx = v8::Context::new_from_template(scope, object_template); @@ -215,7 +221,7 @@ struct SlotContextifyGlobalTemplate(v8::Global); pub fn init_global_template<'a>( scope: &mut v8::HandleScope<'a, ()>, - cache: bool, + mode: ContextInitMode, ) -> v8::Local<'a, v8::ObjectTemplate> { let maybe_object_template_slot = scope.get_slot::(); @@ -223,7 +229,7 @@ pub fn init_global_template<'a>( if maybe_object_template_slot.is_none() { let global_object_template = init_global_template_inner(scope); - if cache { + if mode == ContextInitMode::UseSnapshot { let contextify_global_template_slot = SlotContextifyGlobalTemplate( v8::Global::new(scope, global_object_template), ); diff --git a/runtime/snapshot.rs b/runtime/snapshot.rs index 424596fe5e2a97..2144ff07a2927a 100644 --- a/runtime/snapshot.rs +++ b/runtime/snapshot.rs @@ -285,8 +285,15 @@ pub fn create_runtime_snapshot( let isolate = rt.v8_isolate(); let scope = &mut v8::HandleScope::new(isolate); - let tmpl = deno_node::init_global_template(scope, false); - let ctx = deno_node::create_v8_context(scope, tmpl, false); + let tmpl = deno_node::init_global_template( + scope, + deno_node::ContextInitMode::ForSnapshot, + ); + let ctx = deno_node::create_v8_context( + scope, + tmpl, + deno_node::ContextInitMode::ForSnapshot, + ); assert_eq!(scope.add_context(ctx), deno_node::VM_CONTEXT_INDEX); })), skip_op_registration: false,