diff options
| -rw-r--r-- | Cargo.lock | 4 | ||||
| -rw-r--r-- | core/Cargo.toml | 2 | ||||
| -rw-r--r-- | core/bindings.rs | 19 | ||||
| -rw-r--r-- | core/runtime.rs | 58 | ||||
| -rw-r--r-- | serde_v8/Cargo.toml | 2 |
5 files changed, 23 insertions, 62 deletions
diff --git a/Cargo.lock b/Cargo.lock index 68212f05b..1799a10bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5465,9 +5465,9 @@ dependencies = [ [[package]] name = "v8" -version = "0.52.0" +version = "0.53.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "44e483583f1b8da53083a47335afa9b7196f35cb91f92d78452bf56508121da0" +checksum = "848795d431651537e90408672ec8960fe402522bc2dd7b7bf928fdc32025869a" dependencies = [ "bitflags", "fslock", diff --git a/core/Cargo.toml b/core/Cargo.toml index 57dafa0e5..a56b1b3db 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -34,7 +34,7 @@ serde_json = { version = "1.0.79", features = ["preserve_order"] } serde_v8 = { version = "0.65.0", path = "../serde_v8" } sourcemap = "6.1" url = { version = "2.3.1", features = ["serde", "expose_internals"] } -v8 = { version = "0.52.0", default-features = false } +v8 = { version = "0.53.0", default-features = false } [[example]] name = "http_bench_json_ops" diff --git a/core/bindings.rs b/core/bindings.rs index 52ecf7bac..fdd8abf1a 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -228,16 +228,13 @@ pub extern "C" fn wasm_async_resolve_promise_callback( } } -pub extern "C" fn host_import_module_dynamically_callback( - context: v8::Local<v8::Context>, - _host_defined_options: v8::Local<v8::Data>, - resource_name: v8::Local<v8::Value>, - specifier: v8::Local<v8::String>, - import_assertions: v8::Local<v8::FixedArray>, -) -> *mut v8::Promise { - // SAFETY: `CallbackScope` can be safely constructed from `Local<Context>` - let scope = &mut unsafe { v8::CallbackScope::new(context) }; - +pub fn host_import_module_dynamically_callback<'s>( + scope: &mut v8::HandleScope<'s>, + _host_defined_options: v8::Local<'s, v8::Data>, + resource_name: v8::Local<'s, v8::Value>, + specifier: v8::Local<'s, v8::String>, + import_assertions: v8::Local<'s, v8::FixedArray>, +) -> Option<v8::Local<'s, v8::Promise>> { // NOTE(bartlomieju): will crash for non-UTF-8 specifier let specifier_str = specifier .to_string(scope) @@ -298,7 +295,7 @@ pub extern "C" fn host_import_module_dynamically_callback( let promise = promise.catch(scope, map_err).unwrap(); - &*promise as *const _ as *mut _ + Some(promise) } pub extern "C" fn host_initialize_import_meta_object_callback( diff --git a/core/runtime.rs b/core/runtime.rs index f6d688186..2b824588c 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -38,7 +38,6 @@ use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; use std::ffi::c_void; -use std::mem::forget; use std::option::Option; use std::rc::Rc; use std::sync::Arc; @@ -81,8 +80,6 @@ pub struct JsRuntime { // This is an Option<OwnedIsolate> instead of just OwnedIsolate to workaround // a safety issue with SnapshotCreator. See JsRuntime::drop. v8_isolate: Option<v8::OwnedIsolate>, - snapshot_creator: Option<v8::SnapshotCreator>, - has_snapshotted: bool, built_from_snapshot: bool, allocations: IsolateAllocations, extensions: Vec<Extension>, @@ -190,28 +187,6 @@ pub(crate) struct JsRuntimeState { waker: AtomicWaker, } -impl Drop for JsRuntime { - fn drop(&mut self) { - if let Some(creator) = self.snapshot_creator.take() { - // TODO(ry): in rusty_v8, `SnapShotCreator::get_owned_isolate()` returns - // a `struct OwnedIsolate` which is not actually owned, hence the need - // here to leak the `OwnedIsolate` in order to avoid a double free and - // the segfault that it causes. - let v8_isolate = self.v8_isolate.take().unwrap(); - forget(v8_isolate); - - // TODO(ry) V8 has a strange assert which prevents a SnapshotCreator from - // being deallocated if it hasn't created a snapshot yet. - // https://github.com/v8/v8/blob/73212783fbd534fac76cc4b66aac899c13f71fc8/src/api.cc#L603 - // If that assert is removed, this if guard could be removed. - // WARNING: There may be false positive LSAN errors here. - if self.has_snapshotted { - drop(creator); - } - } - } -} - fn v8_init( v8_platform: Option<v8::SharedRef<v8::Platform>>, predictable: bool, @@ -354,15 +329,11 @@ impl JsRuntime { // SAFETY: we just asserted that layout has non-0 size. unsafe { std::alloc::alloc(layout) as *mut _ }; - let (mut isolate, maybe_snapshot_creator) = if options.will_snapshot { + let mut isolate = if options.will_snapshot { // TODO(ry) Support loading snapshots before snapshotting. assert!(options.startup_snapshot.is_none()); - let mut creator = v8::SnapshotCreator::new(Some(refs)); - // SAFETY: `get_owned_isolate` is unsafe because it may only be called - // once. This is the only place we call this function, so this call is - // safe. - let isolate = unsafe { creator.get_owned_isolate() }; - let mut isolate = JsRuntime::setup_isolate(isolate); + let snapshot_creator = v8::Isolate::snapshot_creator(Some(refs)); + let mut isolate = JsRuntime::setup_isolate(snapshot_creator); { // SAFETY: this is first use of `isolate_ptr` so we are sure we're // not overwriting an existing pointer. @@ -373,9 +344,9 @@ impl JsRuntime { let scope = &mut v8::HandleScope::new(&mut isolate); let context = bindings::initialize_context(scope, &op_ctxs, false); global_context = v8::Global::new(scope, context); - creator.set_default_context(context); + scope.set_default_context(context); } - (isolate, Some(creator)) + isolate } else { let mut params = options .create_params @@ -414,7 +385,7 @@ impl JsRuntime { global_context = v8::Global::new(scope, context); } - (isolate, None) + isolate }; op_state.borrow_mut().put(isolate_ptr); @@ -458,8 +429,6 @@ impl JsRuntime { let mut js_runtime = Self { v8_isolate: Some(isolate), - snapshot_creator: maybe_snapshot_creator, - has_snapshotted: false, built_from_snapshot: has_startup_snapshot, allocations: IsolateAllocations::default(), event_loop_middlewares: Vec::with_capacity(options.extensions.len()), @@ -745,9 +714,7 @@ impl JsRuntime { /// set to true. /// /// `Error` can usually be downcast to `JsError`. - pub fn snapshot(&mut self) -> v8::StartupData { - assert!(self.snapshot_creator.is_some()); - + pub fn snapshot(mut self) -> v8::StartupData { // Nuke Deno.core.ops.* to avoid ExternalReference snapshotting issues // TODO(@AaronO): make ops stable across snapshots { @@ -796,13 +763,10 @@ impl JsRuntime { state.js_nexttick_cbs.clear(); } - let snapshot_creator = self.snapshot_creator.as_mut().unwrap(); - let snapshot = snapshot_creator + let snapshot_creator = self.v8_isolate.unwrap(); + snapshot_creator .create_blob(v8::FunctionCodeHandling::Keep) - .unwrap(); - self.has_snapshotted = true; - - snapshot + .unwrap() } /// Returns the namespace object of a module. @@ -3938,7 +3902,7 @@ Deno.core.opAsync('op_async_serialize_object_with_numbers_as_keys', { #[test] fn js_realm_init_snapshot() { let snapshot = { - let mut runtime = JsRuntime::new(RuntimeOptions { + let runtime = JsRuntime::new(RuntimeOptions { will_snapshot: true, ..Default::default() }); diff --git a/serde_v8/Cargo.toml b/serde_v8/Cargo.toml index f497d9693..67bc5a597 100644 --- a/serde_v8/Cargo.toml +++ b/serde_v8/Cargo.toml @@ -18,7 +18,7 @@ derive_more = "0.99.17" serde = { version = "1.0.136", features = ["derive"] } serde_bytes = "0.11" smallvec = { version = "1.8", features = ["union"] } -v8 = { version = "0.52.0", default-features = false } +v8 = { version = "0.53.0", default-features = false } [dev-dependencies] bencher = "0.1" |