From 4a4ff6c08bd3ba15e85b2f3ee4dd6505f8e10a5e Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Thu, 5 Sep 2024 16:54:48 +0200 Subject: [PATCH] [Turbopack] fix serialization problems/bugs (#69662) ### What? * improve error * fix CachedTaskType serialization * fix next config serialization * fix serialization of Rope * fix serialization of AliasMap * more efficent rope serialization * add missing primitives * Disable serialization for RuntimeVersions --- Cargo.lock | 6 ++-- Cargo.toml | 2 ++ crates/next-core/src/next_config.rs | 4 +-- turbopack/crates/turbo-tasks-fs/Cargo.toml | 3 +- turbopack/crates/turbo-tasks-fs/src/rope.rs | 22 +++++++++++-- turbopack/crates/turbo-tasks/src/backend.rs | 32 ++++++++++++++----- .../turbo-tasks/src/task/shared_reference.rs | 2 +- turbopack/crates/turbo-tasks/src/trace.rs | 2 +- .../src/ecmascript/merged/update.rs | 1 + turbopack/crates/turbopack-core/Cargo.toml | 1 + .../crates/turbopack-core/src/environment.rs | 3 +- .../turbopack-core/src/resolve/alias_map.rs | 8 +++-- 12 files changed, 64 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 103add441231b..bc18ebc85def9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5928,9 +5928,9 @@ dependencies = [ [[package]] name = "serde_bytes" -version = "0.11.9" +version = "0.11.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "416bda436f9aab92e02c8e10d49a15ddd339cea90b6e340fe51ed97abb548294" +checksum = "387cc504cb06bb40a96c8e04e951fe01854cf6bc921053c954e4a606d9675c6a" dependencies = [ "serde", ] @@ -8684,6 +8684,7 @@ dependencies = [ "parking_lot", "rstest", "serde", + "serde_bytes", "serde_json", "serde_path_to_error", "sha2", @@ -8951,6 +8952,7 @@ dependencies = [ "regex", "rstest", "serde", + "serde_bytes", "serde_json", "sourcemap", "swc_core", diff --git a/Cargo.toml b/Cargo.toml index e52c45b1c15b6..73a5e167d570f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -187,6 +187,8 @@ rustc-hash = "1.1.0" semver = "1.0.16" serde = { version = "1.0.152", features = ["derive"] } serde_json = "1.0.93" +serde_bytes = "0.11.15" +serde_path_to_error = "0.1.9" serde_qs = "0.11.0" serde_with = "2.3.2" serde_yaml = "0.9.17" diff --git a/crates/next-core/src/next_config.rs b/crates/next-core/src/next_config.rs index a0947d48d597a..de7e93a84e849 100644 --- a/crates/next-core/src/next_config.rs +++ b/crates/next-core/src/next_config.rs @@ -314,10 +314,10 @@ pub struct ImageConfig { pub loader_file: Option, pub domains: Vec, pub disable_static_images: bool, - #[serde(rename(deserialize = "minimumCacheTTL"))] + #[serde(rename = "minimumCacheTTL")] pub minimum_cache_ttl: u64, pub formats: Vec, - #[serde(rename(deserialize = "dangerouslyAllowSVG"))] + #[serde(rename = "dangerouslyAllowSVG")] pub dangerously_allow_svg: bool, pub content_security_policy: String, pub remote_patterns: Vec, diff --git a/turbopack/crates/turbo-tasks-fs/Cargo.toml b/turbopack/crates/turbo-tasks-fs/Cargo.toml index 59c616c4bb054..c2d63dca2bf6d 100644 --- a/turbopack/crates/turbo-tasks-fs/Cargo.toml +++ b/turbopack/crates/turbo-tasks-fs/Cargo.toml @@ -40,8 +40,9 @@ mime = { workspace = true } notify = { workspace = true } parking_lot = { workspace = true } serde = { workspace = true, features = ["rc"] } +serde_bytes = { workspace = true } serde_json = { workspace = true } -serde_path_to_error = "0.1.9" +serde_path_to_error = { workspace = true } tokio = { workspace = true } tracing = { workspace = true } turbo-tasks = { workspace = true } diff --git a/turbopack/crates/turbo-tasks-fs/src/rope.rs b/turbopack/crates/turbo-tasks-fs/src/rope.rs index 95f03ab64df72..76e384f7f8ba8 100644 --- a/turbopack/crates/turbo-tasks-fs/src/rope.rs +++ b/turbopack/crates/turbo-tasks-fs/src/rope.rs @@ -14,6 +14,7 @@ use anyhow::{Context, Result}; use bytes::{Buf, Bytes}; use futures::Stream; use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use serde_bytes::ByteBuf; use tokio::io::{AsyncRead, ReadBuf}; use turbo_tasks_hash::{DeterministicHash, DeterministicHasher}; use RopeElem::{Local, Shared}; @@ -342,19 +343,34 @@ impl Serialize for Rope { /// possible owner of a individual "shared" data doesn't make sense). fn serialize(&self, serializer: S) -> Result { use serde::ser::Error; - let s = self.to_str().map_err(Error::custom)?; - serializer.serialize_str(&s) + let bytes = self.to_bytes().map_err(Error::custom)?; + match bytes { + Cow::Borrowed(b) => serde_bytes::Bytes::new(b).serialize(serializer), + Cow::Owned(b) => ByteBuf::from(b).serialize(serializer), + } } } impl<'de> Deserialize<'de> for Rope { /// Deserializes strings into a contiguous, immutable Rope. fn deserialize>(deserializer: D) -> Result { - let bytes = >::deserialize(deserializer)?; + let bytes = ByteBuf::deserialize(deserializer)?.into_vec(); Ok(Rope::from(bytes)) } } +pub mod ser_as_string { + use serde::{ser::Error, Serializer}; + + use super::Rope; + + /// Serializes a Rope into a string. + pub fn serialize(rope: &Rope, serializer: S) -> Result { + let s = rope.to_str().map_err(Error::custom)?; + serializer.serialize_str(&s) + } +} + impl PartialEq for Rope { // Ropes with similar contents are equals, regardless of their structure. fn eq(&self, other: &Self) -> bool { diff --git a/turbopack/crates/turbo-tasks/src/backend.rs b/turbopack/crates/turbo-tasks/src/backend.rs index a272bbad83572..017183814bd0c 100644 --- a/turbopack/crates/turbo-tasks/src/backend.rs +++ b/turbopack/crates/turbo-tasks/src/backend.rs @@ -164,23 +164,27 @@ mod ser { { match self { CachedTaskType::Native { fn_type, this, arg } => { - let mut s = serializer.serialize_seq(Some(3))?; + let mut s = serializer.serialize_tuple(5)?; s.serialize_element::(&0)?; s.serialize_element(&FunctionAndArg::Borrowed { fn_type: *fn_type, - arg, + arg: &**arg, })?; s.serialize_element(this)?; + s.serialize_element(&())?; + s.serialize_element(&())?; s.end() } CachedTaskType::ResolveNative { fn_type, this, arg } => { - let mut s = serializer.serialize_seq(Some(3))?; + let mut s = serializer.serialize_tuple(5)?; s.serialize_element::(&1)?; s.serialize_element(&FunctionAndArg::Borrowed { fn_type: *fn_type, arg: &**arg, })?; s.serialize_element(this)?; + s.serialize_element(&())?; + s.serialize_element(&())?; s.end() } CachedTaskType::ResolveTrait { @@ -236,6 +240,12 @@ mod ser { let this = seq .next_element()? .ok_or_else(|| serde::de::Error::invalid_length(2, &self))?; + let () = seq + .next_element()? + .ok_or_else(|| serde::de::Error::invalid_length(3, &self))?; + let () = seq + .next_element()? + .ok_or_else(|| serde::de::Error::invalid_length(4, &self))?; Ok(CachedTaskType::Native { fn_type, this, arg }) } 1 => { @@ -248,18 +258,24 @@ mod ser { let this = seq .next_element()? .ok_or_else(|| serde::de::Error::invalid_length(2, &self))?; + let () = seq + .next_element()? + .ok_or_else(|| serde::de::Error::invalid_length(3, &self))?; + let () = seq + .next_element()? + .ok_or_else(|| serde::de::Error::invalid_length(4, &self))?; Ok(CachedTaskType::ResolveNative { fn_type, this, arg }) } 2 => { let trait_type = seq .next_element()? - .ok_or_else(|| serde::de::Error::invalid_length(0, &self))?; + .ok_or_else(|| serde::de::Error::invalid_length(1, &self))?; let method_name = seq .next_element()? - .ok_or_else(|| serde::de::Error::invalid_length(1, &self))?; + .ok_or_else(|| serde::de::Error::invalid_length(2, &self))?; let this = seq .next_element()? - .ok_or_else(|| serde::de::Error::invalid_length(2, &self))?; + .ok_or_else(|| serde::de::Error::invalid_length(3, &self))?; let Some(method) = registry::get_trait(trait_type).methods.get(&method_name) else { @@ -267,7 +283,7 @@ mod ser { }; let arg = seq .next_element_seed(method.arg_deserializer)? - .ok_or_else(|| serde::de::Error::invalid_length(3, &self))?; + .ok_or_else(|| serde::de::Error::invalid_length(4, &self))?; Ok(CachedTaskType::ResolveTrait { trait_type, method_name, @@ -279,7 +295,7 @@ mod ser { } } } - deserializer.deserialize_seq(Visitor) + deserializer.deserialize_tuple(5, Visitor) } } } diff --git a/turbopack/crates/turbo-tasks/src/task/shared_reference.rs b/turbopack/crates/turbo-tasks/src/task/shared_reference.rs index cef099af92569..25383b8dbc640 100644 --- a/turbopack/crates/turbo-tasks/src/task/shared_reference.rs +++ b/turbopack/crates/turbo-tasks/src/task/shared_reference.rs @@ -108,7 +108,7 @@ impl Serialize for TypedSharedReference { } else { Err(serde::ser::Error::custom(format!( "{:?} is not serializable", - arc + registry::get_value_type_global_name(*ty) ))) } } diff --git a/turbopack/crates/turbo-tasks/src/trace.rs b/turbopack/crates/turbo-tasks/src/trace.rs index cbf1fc042d4c6..162b64cdf2050 100644 --- a/turbopack/crates/turbo-tasks/src/trace.rs +++ b/turbopack/crates/turbo-tasks/src/trace.rs @@ -58,7 +58,7 @@ macro_rules! ignore { } } -ignore!(i8, u8, i16, u16, i32, u32, i64, u64, f32, f64, char, bool, usize); +ignore!(i8, u8, i16, u16, i32, u32, i64, u64, i128, u128, f32, f64, char, bool, isize, usize); ignore!( AtomicI8, AtomicU8, diff --git a/turbopack/crates/turbopack-browser/src/ecmascript/merged/update.rs b/turbopack/crates/turbopack-browser/src/ecmascript/merged/update.rs index 3afac4632e67d..8e3fca803318c 100644 --- a/turbopack/crates/turbopack-browser/src/ecmascript/merged/update.rs +++ b/turbopack/crates/turbopack-browser/src/ecmascript/merged/update.rs @@ -75,6 +75,7 @@ struct EcmascriptMergedChunkPartial { #[derive(Serialize)] struct EcmascriptModuleEntry { + #[serde(with = "turbo_tasks_fs::rope::ser_as_string")] code: Rope, url: String, map: Option, diff --git a/turbopack/crates/turbopack-core/Cargo.toml b/turbopack/crates/turbopack-core/Cargo.toml index 0963dcd46f08c..429ef2b5f6faa 100644 --- a/turbopack/crates/turbopack-core/Cargo.toml +++ b/turbopack/crates/turbopack-core/Cargo.toml @@ -26,6 +26,7 @@ patricia_tree = "0.5.5" ref-cast = "1.0.20" regex = { workspace = true } serde = { workspace = true, features = ["rc"] } +serde_bytes = { workspace = true } serde_json = { workspace = true, features = ["preserve_order"] } sourcemap = { workspace = true } swc_core = { workspace = true, features = ["ecma_preset_env", "common"] } diff --git a/turbopack/crates/turbopack-core/src/environment.rs b/turbopack/crates/turbopack-core/src/environment.rs index 89f40160dab6f..3f612b192393e 100644 --- a/turbopack/crates/turbopack-core/src/environment.rs +++ b/turbopack/crates/turbopack-core/src/environment.rs @@ -299,7 +299,8 @@ pub struct BrowserEnvironment { #[turbo_tasks::value(shared)] pub struct EdgeWorkerEnvironment {} -#[turbo_tasks::value(transparent)] +// TODO preset_env_base::Version implements Serialize/Deserialize incorrectly +#[turbo_tasks::value(transparent, serialization = "none")] pub struct RuntimeVersions(#[turbo_tasks(trace_ignore)] pub Versions); #[turbo_tasks::function] diff --git a/turbopack/crates/turbopack-core/src/resolve/alias_map.rs b/turbopack/crates/turbopack-core/src/resolve/alias_map.rs index 7b6ae2acae92e..35ae8fc069e29 100644 --- a/turbopack/crates/turbopack-core/src/resolve/alias_map.rs +++ b/turbopack/crates/turbopack-core/src/resolve/alias_map.rs @@ -10,6 +10,7 @@ use serde::{ ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer, }; +use serde_bytes::{ByteBuf, Bytes}; use turbo_tasks::{ debug::{internal::PassthroughDebug, ValueDebugFormat, ValueDebugFormatString}, trace::{TraceRawVcs, TraceRawVcsContext}, @@ -61,7 +62,8 @@ where S: Serializer, { let mut map = serializer.serialize_map(Some(self.map.len()))?; - for (key, value) in self.map.iter() { + for (prefix, value) in self.map.iter() { + let key = ByteBuf::from(prefix); map.serialize_entry(&key, value)?; } map.end() @@ -87,8 +89,8 @@ where M: MapAccess<'de>, { let mut map = AliasMap::new(); - while let Some((key, value)) = access.next_entry()? { - map.insert(key, value); + while let Some((key, value)) = access.next_entry::<&Bytes, _>()? { + map.map.insert(key, value); } Ok(map) }