From 263b92907db706e1cfacad5104d204e1eb776aa2 Mon Sep 17 00:00:00 2001 From: digifox03 Date: Tue, 23 Jan 2024 23:31:01 +0100 Subject: [PATCH 1/4] Disable V8's PKU feature for tests --- core/Cargo.toml | 1 + core/runtime/jsruntime.rs | 12 ++++++++++-- serde_v8/magic/v8slice.rs | 3 ++- serde_v8/utils.rs | 2 +- testing/Cargo.toml | 1 + 5 files changed, 15 insertions(+), 4 deletions(-) diff --git a/core/Cargo.toml b/core/Cargo.toml index 0566d0a43..bbd3931a7 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -19,6 +19,7 @@ include_icu_data = [] v8_use_custom_libcxx = ["v8/use_custom_libcxx"] include_js_files_for_snapshotting = [] unsafe_runtime_options = [] +testing = [] # Use the old, slower (but better tested) JoinSet driver op_driver_joinset = [] diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs index 1eb0b1ad4..f83a9013f 100644 --- a/core/runtime/jsruntime.rs +++ b/core/runtime/jsruntime.rs @@ -413,8 +413,16 @@ fn v8_init( }; v8::V8::set_flags_from_string(&flags); - let v8_platform = v8_platform - .unwrap_or_else(|| v8::new_default_platform(0, false).make_shared()); + let v8_platform = v8_platform.unwrap_or_else(|| { + if cfg!(any(test, feature = "testing")) { + // We don't want to use the default platform in tests, because it + // uses memory protection keys which break multi-threaded tests. + v8::new_unprotected_default_platform(0, false) + } else { + v8::new_default_platform(0, false) + } + .make_shared() + }); v8::V8::initialize_platform(v8_platform.clone()); v8::V8::initialize(); diff --git a/serde_v8/magic/v8slice.rs b/serde_v8/magic/v8slice.rs index c2f0be22d..d04e8b9a3 100644 --- a/serde_v8/magic/v8slice.rs +++ b/serde_v8/magic/v8slice.rs @@ -107,7 +107,8 @@ where static V8_ONCE: std::sync::Once = std::sync::Once::new(); V8_ONCE.call_once(|| { - let platform = v8::new_default_platform(0, false).make_shared(); + let platform = + v8::new_unprotected_default_platform(0, false).make_shared(); v8::V8::initialize_platform(platform); v8::V8::initialize(); }); diff --git a/serde_v8/utils.rs b/serde_v8/utils.rs index 6a9732400..733dcceb2 100644 --- a/serde_v8/utils.rs +++ b/serde_v8/utils.rs @@ -11,7 +11,7 @@ pub fn js_exec<'s>( } pub fn v8_init() { - let platform = v8::new_default_platform(0, false).make_shared(); + let platform = v8::new_unprotected_default_platform(0, false).make_shared(); v8::V8::initialize_platform(platform); v8::V8::initialize(); } diff --git a/testing/Cargo.toml b/testing/Cargo.toml index 6181e2fde..d3661a775 100644 --- a/testing/Cargo.toml +++ b/testing/Cargo.toml @@ -15,6 +15,7 @@ path = "./lib.rs" [dev-dependencies] deno_core.workspace = true +deno_core.features = ["testing"] pretty_assertions.workspace = true prettyplease.workspace = true testing_macros.workspace = true From 16d927de7b2b119882a1277dd5516d3c78701d0d Mon Sep 17 00:00:00 2001 From: digifox03 Date: Wed, 24 Jan 2024 09:16:07 +0100 Subject: [PATCH 2/4] Rename feature flag to unsafe_use_unprotected_platform --- core/Cargo.toml | 2 +- core/runtime/jsruntime.rs | 5 ++--- testing/Cargo.toml | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/core/Cargo.toml b/core/Cargo.toml index bbd3931a7..cecd08550 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -19,7 +19,7 @@ include_icu_data = [] v8_use_custom_libcxx = ["v8/use_custom_libcxx"] include_js_files_for_snapshotting = [] unsafe_runtime_options = [] -testing = [] +unsafe_use_unprotected_platform = [] # Use the old, slower (but better tested) JoinSet driver op_driver_joinset = [] diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs index f83a9013f..a77d44063 100644 --- a/core/runtime/jsruntime.rs +++ b/core/runtime/jsruntime.rs @@ -414,9 +414,8 @@ fn v8_init( v8::V8::set_flags_from_string(&flags); let v8_platform = v8_platform.unwrap_or_else(|| { - if cfg!(any(test, feature = "testing")) { - // We don't want to use the default platform in tests, because it - // uses memory protection keys which break multi-threaded tests. + if cfg!(any(test, feature = "unsafe_use_unprotected_platform")) { + // We want to use the unprotected platform for unit tests v8::new_unprotected_default_platform(0, false) } else { v8::new_default_platform(0, false) diff --git a/testing/Cargo.toml b/testing/Cargo.toml index d3661a775..62b5a99b6 100644 --- a/testing/Cargo.toml +++ b/testing/Cargo.toml @@ -15,7 +15,7 @@ path = "./lib.rs" [dev-dependencies] deno_core.workspace = true -deno_core.features = ["testing"] +deno_core.features = ["unsafe_use_unprotected_platform"] pretty_assertions.workspace = true prettyplease.workspace = true testing_macros.workspace = true From 54b8233dd6c9db6cc85367a9080246e4b68fb9a2 Mon Sep 17 00:00:00 2001 From: digifox03 Date: Wed, 24 Jan 2024 17:54:12 +0100 Subject: [PATCH 3/4] Move utilities to internal crate --- Cargo.lock | 8 ++++++++ serde_v8/Cargo.toml | 1 + serde_v8/benches/de.rs | 4 ++-- serde_v8/benches/ser.rs | 2 +- serde_v8/lib.rs | 1 - serde_v8/tests/de.rs | 4 ++-- serde_v8/tests/magic.rs | 4 ++-- serde_v8/tests/ser.rs | 4 ++-- serde_v8/utilities/Cargo.toml | 9 +++++++++ serde_v8/{utils.rs => utilities/lib.rs} | 0 10 files changed, 27 insertions(+), 10 deletions(-) create mode 100644 serde_v8/utilities/Cargo.toml rename serde_v8/{utils.rs => utilities/lib.rs} (100%) diff --git a/Cargo.lock b/Cargo.lock index 800d9ef8c..422174675 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1230,11 +1230,19 @@ dependencies = [ "serde", "serde_bytes", "serde_json", + "serde_v8_utilities", "smallvec", "thiserror", "v8", ] +[[package]] +name = "serde_v8_utilities" +version = "0.0.0" +dependencies = [ + "v8", +] + [[package]] name = "sha-1" version = "0.10.0" diff --git a/serde_v8/Cargo.toml b/serde_v8/Cargo.toml index 9f99aea76..3bdd66cbf 100644 --- a/serde_v8/Cargo.toml +++ b/serde_v8/Cargo.toml @@ -23,6 +23,7 @@ thiserror.workspace = true v8.workspace = true [dev-dependencies] +serde_v8_utilities = { path = "./utilities" } bencher.workspace = true serde_json.workspace = true serde_bytes.workspace = true diff --git a/serde_v8/benches/de.rs b/serde_v8/benches/de.rs index b7cd0bf32..a355e9875 100644 --- a/serde_v8/benches/de.rs +++ b/serde_v8/benches/de.rs @@ -5,9 +5,9 @@ use bencher::Bencher; use serde::Deserialize; -use serde_v8::utils::js_exec; -use serde_v8::utils::v8_do; use serde_v8::ByteString; +use serde_v8_utilities::js_exec; +use serde_v8_utilities::v8_do; #[derive(Debug, Deserialize, PartialEq)] struct MathOp { diff --git a/serde_v8/benches/ser.rs b/serde_v8/benches/ser.rs index 83274fae3..faf96dea3 100644 --- a/serde_v8/benches/ser.rs +++ b/serde_v8/benches/ser.rs @@ -5,8 +5,8 @@ use bencher::Bencher; use serde::Serialize; -use serde_v8::utils::v8_do; use serde_v8::ByteString; +use serde_v8_utilities::v8_do; #[derive(Serialize)] struct MathOp { diff --git a/serde_v8/lib.rs b/serde_v8/lib.rs index 3bce2e5c2..5dabe486f 100644 --- a/serde_v8/lib.rs +++ b/serde_v8/lib.rs @@ -6,7 +6,6 @@ mod magic; mod payload; mod ser; mod serializable; -pub mod utils; pub use de::from_v8; pub use de::from_v8_cached; diff --git a/serde_v8/tests/de.rs b/serde_v8/tests/de.rs index 122f96f1c..f8f925ac9 100644 --- a/serde_v8/tests/de.rs +++ b/serde_v8/tests/de.rs @@ -2,13 +2,13 @@ use serde::Deserialize; use serde::Deserializer; -use serde_v8::utils::js_exec; -use serde_v8::utils::v8_do; use serde_v8::BigInt; use serde_v8::ByteString; use serde_v8::Error; use serde_v8::JsBuffer; use serde_v8::U16String; +use serde_v8_utilities::js_exec; +use serde_v8_utilities::v8_do; #[derive(Debug, Deserialize, PartialEq)] struct MathOp { diff --git a/serde_v8/tests/magic.rs b/serde_v8/tests/magic.rs index e3ed1d330..bec937fa0 100644 --- a/serde_v8/tests/magic.rs +++ b/serde_v8/tests/magic.rs @@ -2,9 +2,9 @@ use serde::Deserialize; use serde::Serialize; -use serde_v8::utils::js_exec; -use serde_v8::utils::v8_do; use serde_v8::Result; +use serde_v8_utilities::js_exec; +use serde_v8_utilities::v8_do; #[derive(Deserialize)] struct MagicOp<'s> { diff --git a/serde_v8/tests/ser.rs b/serde_v8/tests/ser.rs index b61a758f9..b183166b0 100644 --- a/serde_v8/tests/ser.rs +++ b/serde_v8/tests/ser.rs @@ -1,9 +1,9 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use serde::Serialize; use serde_json::json; -use serde_v8::utils::js_exec; -use serde_v8::utils::v8_do; use serde_v8::BigInt; +use serde_v8_utilities::js_exec; +use serde_v8_utilities::v8_do; #[derive(Debug, Serialize, PartialEq)] struct MathOp { diff --git a/serde_v8/utilities/Cargo.toml b/serde_v8/utilities/Cargo.toml new file mode 100644 index 000000000..8a011d87e --- /dev/null +++ b/serde_v8/utilities/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "serde_v8_utilities" +version = "0.0.0" + +[lib] +path = "lib.rs" + +[dependencies] +v8.workspace = true diff --git a/serde_v8/utils.rs b/serde_v8/utilities/lib.rs similarity index 100% rename from serde_v8/utils.rs rename to serde_v8/utilities/lib.rs From 1b7446e3bfce0d0f74b8e3aa9aac9a4bc61def06 Mon Sep 17 00:00:00 2001 From: digifox03 Date: Wed, 24 Jan 2024 18:25:02 +0100 Subject: [PATCH 4/4] Add copyright notice to Cargo.toml --- serde_v8/utilities/Cargo.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/serde_v8/utilities/Cargo.toml b/serde_v8/utilities/Cargo.toml index 8a011d87e..fbdceda80 100644 --- a/serde_v8/utilities/Cargo.toml +++ b/serde_v8/utilities/Cargo.toml @@ -1,3 +1,5 @@ +# Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. + [package] name = "serde_v8_utilities" version = "0.0.0"