diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 151006e5..adb2b9b3 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -48,7 +48,7 @@ "uses": "actions-rs/cargo@v1", "with": { "command": "check", - "args": "--all-features" + "args": "--features ci" }, "name": "Run `cargo check`" }, @@ -128,6 +128,32 @@ fi", } ] }, + "no_oom": { + "name": "Strict OOM checks", + "runs-on": "ubuntu-latest", + "steps": [ + { + "uses": "actions/checkout@v2", + "name": "Checkout" + }, + { + "uses": "actions-rs/toolchain@v1", + "with": { + "profile": "minimal", + "toolchain": "nightly", + "components": "rust-src", + "override": true + }, + "name": "Install Rust nightly" + }, + { + "run": "cargo build --no-default-features --features unstable-strict-oom-checks -Z build-std=core,alloc --target x86_64-unknown-linux-gnu", + "env": { + "RUSTFLAGS": "--cfg no_global_oom_handling" + } + } + ] + }, "lints": { "name": "Lints", "runs-on": "ubuntu-latest", @@ -158,7 +184,7 @@ fi", "uses": "actions-rs/cargo@v1", "with": { "command": "clippy", - "args": "--all-features -- -D warnings" + "args": "--features ci -- -D warnings" }, "name": "Run `cargo clippy`" } @@ -213,7 +239,7 @@ fi", "uses": "actions-rs/tarpaulin@v0.1", "with": { "version": "0.19.1", - "args": "--all --all-features" + "args": "--all --features ci" } }, { diff --git a/Cargo.toml b/Cargo.toml index ead432e9..0078fe55 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,16 +1,17 @@ [workspace] -members = [ - "derive", - "compatibility" -] +members = ["derive", "compatibility"] [package] name = "bincode" version = "2.0.0-rc.1" # remember to update html_root_url and bincode_derive -authors = ["Ty Overby ", "Zoey Riordan ", "Victor Koenders "] +authors = [ + "Ty Overby ", + "Zoey Riordan ", + "Victor Koenders ", +] exclude = ["logo.svg", "examples/*", ".gitignore", ".github/"] -publish = true +publish = true repository = "https://github.com/bincode-org/bincode" documentation = "https://docs.rs/bincode" @@ -29,6 +30,12 @@ std = ["alloc", "serde?/std"] alloc = ["serde?/alloc"] derive = ["bincode_derive"] +## !! Not for public use !! +# features to check again in CI +ci = ["std", "alloc", "derive", "serde"] +# experimental strict OOM checks, requires nightly features +unstable-strict-oom-checks = ["alloc"] + [dependencies] bincode_derive = { path = "derive", version = "2.0.0-rc.1", optional = true } serde = { version = "1.0", default-features = false, optional = true } @@ -42,7 +49,7 @@ criterion = "0.3" rand = "0.8" uuid = { version = "0.8", features = ["serde"] } chrono = { version = "0.4", features = ["serde"] } -glam = { version="0.20.5", features=["serde"] } +glam = { version = "0.20.5", features = ["serde"] } [[bench]] name = "varint" diff --git a/src/error.rs b/src/error.rs index a544f0cf..148ddf84 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,10 @@ //! Errors that can be encounting by Encoding and Decoding. +#[cfg(feature = "alloc")] +use alloc::collections::TryReserveError; +#[cfg(all(feature = "alloc", feature = "unstable-strict-oom-checks"))] +use core::alloc::AllocError; + /// Errors that can be encountered by encoding a type #[non_exhaustive] #[derive(Debug)] @@ -51,6 +56,10 @@ pub enum EncodeError { time: std::boxed::Box, }, + /// bincode failed to allocate enough memory + #[cfg(feature = "alloc")] + OutOfMemory(OutOfMemory), + #[cfg(feature = "serde")] /// A serde-specific error that occurred while decoding. Serde(crate::features::serde::EncodeError), @@ -171,6 +180,54 @@ pub enum DecodeError { #[cfg(feature = "serde")] /// A serde-specific error that occurred while decoding. Serde(crate::features::serde::DecodeError), + + /// bincode failed to allocate enough memory + #[cfg(feature = "alloc")] + OutOfMemory(OutOfMemory), +} + +/// A wrapper to make all the out of memory errors consistent +#[cfg(feature = "alloc")] +#[derive(Debug, PartialEq)] +pub enum OutOfMemory { + /// Failed to reserve an entry + TryReserve(TryReserveError), + #[cfg(feature = "unstable-strict-oom-checks")] + /// Failed to allocate memory + Alloc(AllocError), +} + +impl From for DecodeError { + fn from(e: TryReserveError) -> Self { + Self::OutOfMemory(OutOfMemory::TryReserve(e)) + } +} + +impl From for EncodeError { + fn from(e: TryReserveError) -> Self { + Self::OutOfMemory(OutOfMemory::TryReserve(e)) + } +} + +#[cfg(all(feature = "alloc", feature = "unstable-strict-oom-checks"))] +impl From for DecodeError { + fn from(e: AllocError) -> Self { + Self::OutOfMemory(OutOfMemory::Alloc(e)) + } +} + +#[cfg(all(feature = "alloc", feature = "unstable-strict-oom-checks"))] +impl From for EncodeError { + fn from(e: AllocError) -> Self { + Self::OutOfMemory(OutOfMemory::Alloc(e)) + } +} + +#[cfg(all(feature = "alloc", feature = "unstable-strict-oom-checks"))] +impl From for DecodeError { + fn from(e: AllocError) -> Self { + Self::OutOfMemory(OutOfMemory::Alloc(e)) + } } impl core::fmt::Display for DecodeError { diff --git a/src/features/impl_alloc.rs b/src/features/impl_alloc.rs index b705de96..2d8800f1 100644 --- a/src/features/impl_alloc.rs +++ b/src/features/impl_alloc.rs @@ -9,7 +9,6 @@ use alloc::sync::Arc; use alloc::{ borrow::{Cow, ToOwned}, boxed::Box, - collections::*, rc::Rc, string::String, vec::Vec, @@ -30,7 +29,21 @@ impl VecWriter { impl enc::write::Writer for VecWriter { fn write(&mut self, bytes: &[u8]) -> Result<(), EncodeError> { - self.inner.extend_from_slice(bytes); + self.inner.try_reserve(bytes.len())?; + + let start = self.inner.len(); + + // Get a slice of `&mut [MaybeUninit]` of the remaining capacity + let remaining = &mut self.inner.spare_capacity_mut()[..bytes.len()]; + for (i, b) in bytes.iter().copied().enumerate() { + // TODO: is there a better way to copy from `&mut [MaybeUninit]` to `&[u8]`? + remaining[i].write(b); + } + + unsafe { + // Safety: We reserved enough bytes, and the bytes have values written to them + self.inner.set_len(start + bytes.len()); + } Ok(()) } } @@ -46,217 +59,230 @@ pub fn encode_to_vec(val: E, config: C) -> Result Decode for BinaryHeap -where - T: Decode + Ord, -{ - fn decode(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; +// TODO: these collections straight up don't exist with `no_global_oom_handling` +#[cfg(not(feature = "unstable-strict-oom-checks"))] +mod collections { + use super::*; + use alloc::collections::{BTreeMap, BTreeSet, BinaryHeap, VecDeque}; - let mut map = BinaryHeap::with_capacity(len); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); + impl Decode for BinaryHeap + where + T: Decode + Ord, + { + fn decode(decoder: &mut D) -> Result { + let len = crate::de::decode_slice_len(decoder)?; + decoder.claim_container_read::(len)?; - let key = T::decode(decoder)?; - map.push(key); - } - Ok(map) - } -} -impl<'de, T> BorrowDecode<'de> for BinaryHeap -where - T: BorrowDecode<'de> + Ord, -{ - fn borrow_decode>(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; + let mut map = BinaryHeap::new(); + map.try_reserve(len)?; - let mut map = BinaryHeap::with_capacity(len); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); - let key = T::borrow_decode(decoder)?; - map.push(key); + let key = T::decode(decoder)?; + map.push(key); + } + Ok(map) } - Ok(map) } -} - -impl Encode for BinaryHeap -where - T: Encode + Ord, -{ - fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { - crate::enc::encode_slice_len(encoder, self.len())?; - for val in self.iter() { - val.encode(encoder)?; + impl<'de, T> BorrowDecode<'de> for BinaryHeap + where + T: BorrowDecode<'de> + Ord, + { + fn borrow_decode>(decoder: &mut D) -> Result { + let len = crate::de::decode_slice_len(decoder)?; + decoder.claim_container_read::(len)?; + + let mut map = BinaryHeap::new(); + map.try_reserve(len)?; + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); + + let key = T::borrow_decode(decoder)?; + map.push(key); + } + Ok(map) } - Ok(()) } -} - -impl Decode for BTreeMap -where - K: Decode + Ord, - V: Decode, -{ - fn decode(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::<(K, V)>(len)?; - let mut map = BTreeMap::new(); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::<(K, V)>()); - - let key = K::decode(decoder)?; - let value = V::decode(decoder)?; - map.insert(key, value); + impl Encode for BinaryHeap + where + T: Encode + Ord, + { + fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { + crate::enc::encode_slice_len(encoder, self.len())?; + for val in self.iter() { + val.encode(encoder)?; + } + Ok(()) } - Ok(map) } -} -impl<'de, K, V> BorrowDecode<'de> for BTreeMap -where - K: BorrowDecode<'de> + Ord, - V: BorrowDecode<'de>, -{ - fn borrow_decode>(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::<(K, V)>(len)?; - let mut map = BTreeMap::new(); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::<(K, V)>()); - - let key = K::borrow_decode(decoder)?; - let value = V::borrow_decode(decoder)?; - map.insert(key, value); + impl Decode for BTreeMap + where + K: Decode + Ord, + V: Decode, + { + fn decode(decoder: &mut D) -> Result { + let len = crate::de::decode_slice_len(decoder)?; + decoder.claim_container_read::<(K, V)>(len)?; + + let mut map = BTreeMap::new(); + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::<(K, V)>()); + + let key = K::decode(decoder)?; + let value = V::decode(decoder)?; + map.insert(key, value); + } + Ok(map) + } + } + impl<'de, K, V> BorrowDecode<'de> for BTreeMap + where + K: BorrowDecode<'de> + Ord, + V: BorrowDecode<'de>, + { + fn borrow_decode>(decoder: &mut D) -> Result { + let len = crate::de::decode_slice_len(decoder)?; + decoder.claim_container_read::<(K, V)>(len)?; + + let mut map = BTreeMap::new(); + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::<(K, V)>()); + + let key = K::borrow_decode(decoder)?; + let value = V::borrow_decode(decoder)?; + map.insert(key, value); + } + Ok(map) } - Ok(map) } -} -impl Encode for BTreeMap -where - K: Encode + Ord, - V: Encode, -{ - fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { - crate::enc::encode_slice_len(encoder, self.len())?; - for (key, val) in self.iter() { - key.encode(encoder)?; - val.encode(encoder)?; + impl Encode for BTreeMap + where + K: Encode + Ord, + V: Encode, + { + fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { + crate::enc::encode_slice_len(encoder, self.len())?; + for (key, val) in self.iter() { + key.encode(encoder)?; + val.encode(encoder)?; + } + Ok(()) } - Ok(()) } -} -impl Decode for BTreeSet -where - T: Decode + Ord, -{ - fn decode(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; + impl Decode for BTreeSet + where + T: Decode + Ord, + { + fn decode(decoder: &mut D) -> Result { + let len = crate::de::decode_slice_len(decoder)?; + decoder.claim_container_read::(len)?; - let mut map = BTreeSet::new(); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); + let mut map = BTreeSet::new(); + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); - let key = T::decode(decoder)?; - map.insert(key); + let key = T::decode(decoder)?; + map.insert(key); + } + Ok(map) } - Ok(map) } -} -impl<'de, T> BorrowDecode<'de> for BTreeSet -where - T: BorrowDecode<'de> + Ord, -{ - fn borrow_decode>(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; - - let mut map = BTreeSet::new(); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); - - let key = T::borrow_decode(decoder)?; - map.insert(key); + impl<'de, T> BorrowDecode<'de> for BTreeSet + where + T: BorrowDecode<'de> + Ord, + { + fn borrow_decode>(decoder: &mut D) -> Result { + let len = crate::de::decode_slice_len(decoder)?; + decoder.claim_container_read::(len)?; + + let mut map = BTreeSet::new(); + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); + + let key = T::borrow_decode(decoder)?; + map.insert(key); + } + Ok(map) } - Ok(map) } -} -impl Encode for BTreeSet -where - T: Encode + Ord, -{ - fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { - crate::enc::encode_slice_len(encoder, self.len())?; - for item in self.iter() { - item.encode(encoder)?; + impl Encode for BTreeSet + where + T: Encode + Ord, + { + fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { + crate::enc::encode_slice_len(encoder, self.len())?; + for item in self.iter() { + item.encode(encoder)?; + } + Ok(()) } - Ok(()) } -} -impl Decode for VecDeque -where - T: Decode, -{ - fn decode(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; + impl Decode for VecDeque + where + T: Decode, + { + fn decode(decoder: &mut D) -> Result { + let len = crate::de::decode_slice_len(decoder)?; + decoder.claim_container_read::(len)?; - let mut map = VecDeque::with_capacity(len); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); + let mut map = VecDeque::new(); + map.try_reserve(len)?; + + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); - let key = T::decode(decoder)?; - map.push_back(key); + let key = T::decode(decoder)?; + map.push_back(key); + } + Ok(map) } - Ok(map) } -} -impl<'de, T> BorrowDecode<'de> for VecDeque -where - T: BorrowDecode<'de>, -{ - fn borrow_decode>(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; - - let mut map = VecDeque::with_capacity(len); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); - - let key = T::borrow_decode(decoder)?; - map.push_back(key); + impl<'de, T> BorrowDecode<'de> for VecDeque + where + T: BorrowDecode<'de>, + { + fn borrow_decode>(decoder: &mut D) -> Result { + let len = crate::de::decode_slice_len(decoder)?; + decoder.claim_container_read::(len)?; + + let mut map = VecDeque::new(); + map.try_reserve(len)?; + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); + + let key = T::borrow_decode(decoder)?; + map.push_back(key); + } + Ok(map) } - Ok(map) } -} -impl Encode for VecDeque -where - T: Encode, -{ - fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { - crate::enc::encode_slice_len(encoder, self.len())?; - for item in self.iter() { - item.encode(encoder)?; + impl Encode for VecDeque + where + T: Encode, + { + fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { + crate::enc::encode_slice_len(encoder, self.len())?; + for item in self.iter() { + item.encode(encoder)?; + } + Ok(()) } - Ok(()) } } @@ -268,12 +294,25 @@ where let len = crate::de::decode_slice_len(decoder)?; decoder.claim_container_read::(len)?; - let mut vec = Vec::with_capacity(len); + let mut vec = Vec::new(); + vec.try_reserve(len)?; + + let slice = vec.spare_capacity_mut(); + let mut guard = DropGuard { slice, idx: 0 }; + for _ in 0..len { // See the documentation on `unclaim_bytes_read` as to why we're doing this here decoder.unclaim_bytes_read(core::mem::size_of::()); - vec.push(T::decode(decoder)?); + let t = T::decode(decoder)?; + guard.slice[guard.idx].write(t); + guard.idx += 1; + } + // Don't drop the guard + core::mem::forget(guard); + unsafe { + // All values are written, we can now set the length of the vec + vec.set_len(vec.len() + len) } Ok(vec) } @@ -287,12 +326,25 @@ where let len = crate::de::decode_slice_len(decoder)?; decoder.claim_container_read::(len)?; - let mut vec = Vec::with_capacity(len); + let mut vec = Vec::new(); + vec.try_reserve(len)?; + + let slice = vec.spare_capacity_mut(); + let mut guard = DropGuard { slice, idx: 0 }; + for _ in 0..len { // See the documentation on `unclaim_bytes_read` as to why we're doing this here decoder.unclaim_bytes_read(core::mem::size_of::()); - vec.push(T::borrow_decode(decoder)?); + let t = T::borrow_decode(decoder)?; + guard.slice[guard.idx].write(t); + guard.idx += 1; + } + // Don't drop the guard + core::mem::forget(guard); + unsafe { + // All values are written, we can now set the length of the vec + vec.set_len(vec.len() + len) } Ok(vec) } @@ -321,11 +373,20 @@ impl Decode for String { } impl_borrow_decode!(String); +// TODO +// String does not implement Into for Box because it allocates again +// we could do this manually with `Box::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl Decode for Box { fn decode(decoder: &mut D) -> Result { String::decode(decoder).map(String::into_boxed_str) } } + +// TODO +// String does not implement Into for Box because it allocates again +// we could do this manually with `Box::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl_borrow_decode!(Box); impl Encode for String { @@ -340,7 +401,11 @@ where { fn decode(decoder: &mut D) -> Result { let t = T::decode(decoder)?; - Ok(Box::new(t)) + #[cfg(feature = "unstable-strict-oom-checks")] + let b = Box::try_new(t)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let b = Box::new(t); + Ok(b) } } impl<'de, T> BorrowDecode<'de> for Box @@ -349,7 +414,11 @@ where { fn borrow_decode>(decoder: &mut D) -> Result { let t = T::borrow_decode(decoder)?; - Ok(Box::new(t)) + #[cfg(feature = "unstable-strict-oom-checks")] + let b = Box::try_new(t)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let b = Box::new(t); + Ok(b) } } @@ -362,16 +431,52 @@ where } } +#[cfg(feature = "unstable-strict-oom-checks")] impl Decode for Box<[T]> where T: Decode, { fn decode(decoder: &mut D) -> Result { - let vec = Vec::decode(decoder)?; - Ok(vec.into_boxed_slice()) + let len = decode_slice_len(decoder)?; + decoder.claim_container_read::(len)?; + + unsafe { + let mut result = Box::try_new_uninit_slice(len)?; + + let mut guard = DropGuard { + slice: &mut result, + idx: 0, + }; + + while guard.idx < len { + decoder.unclaim_bytes_read(core::mem::size_of::()); + let t = T::decode(decoder)?; + + guard.slice.get_unchecked_mut(guard.idx).write(t); + guard.idx += 1; + } + + core::mem::forget(guard); + Ok(result.assume_init()) + } + } +} + +#[cfg(not(feature = "unstable-strict-oom-checks"))] +impl Decode for Box<[T]> +where + T: Decode, +{ + fn decode(decoder: &mut D) -> Result { + let vec = Vec::::decode(decoder)?; + Ok(vec.into()) } } +// TODO +// Vec does not implement Into for Box<[T]> because it allocates again +// we could do this manually with `Box::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl<'de, T> BorrowDecode<'de> for Box<[T]> where T: BorrowDecode<'de> + 'de, @@ -419,7 +524,11 @@ where { fn decode(decoder: &mut D) -> Result { let t = T::decode(decoder)?; - Ok(Rc::new(t)) + #[cfg(feature = "unstable-strict-oom-checks")] + let rc = Rc::try_new(t)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let rc = Rc::new(t); + Ok(rc) } } @@ -429,7 +538,11 @@ where { fn borrow_decode>(decoder: &mut D) -> Result { let t = T::borrow_decode(decoder)?; - Ok(Rc::new(t)) + #[cfg(feature = "unstable-strict-oom-checks")] + let rc = Rc::try_new(t)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let rc = Rc::new(t); + Ok(rc) } } @@ -442,6 +555,10 @@ where } } +// TODO +// Vec does not implement Into for Rc<[T]> because it allocates again +// we could do this manually with `Rc::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl Decode for Rc<[T]> where T: Decode, @@ -452,6 +569,10 @@ where } } +// TODO +// Vec does not implement Into for Rc<[T]> because it allocates again +// we could do this manually with `Rc::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl<'de, T> BorrowDecode<'de> for Rc<[T]> where T: BorrowDecode<'de> + 'de, @@ -469,10 +590,18 @@ where { fn decode(decoder: &mut D) -> Result { let t = T::decode(decoder)?; - Ok(Arc::new(t)) + #[cfg(feature = "unstable-strict-oom-checks")] + let arc = Arc::try_new(t)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let arc = Arc::new(t); + Ok(arc) } } +// TODO +// String does not implement Into for Arc because it allocates again +// we could do this manually with `Arc::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] #[cfg(target_has_atomic = "ptr")] impl Decode for Arc { fn decode(decoder: &mut D) -> Result { @@ -488,10 +617,18 @@ where { fn borrow_decode>(decoder: &mut D) -> Result { let t = T::borrow_decode(decoder)?; - Ok(Arc::new(t)) + #[cfg(feature = "unstable-strict-oom-checks")] + let arc = Arc::try_new(t)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let arc = Arc::new(t); + Ok(arc) } } +// TODO +// String does not implement Into for Arc because it allocates again +// we could do this manually with `Arc::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] #[cfg(target_has_atomic = "ptr")] impl<'de> BorrowDecode<'de> for Arc { fn borrow_decode>(decoder: &mut D) -> Result { @@ -510,6 +647,10 @@ where } } +// TODO +// Vec does not implement Into for Arc<[T]> +// we could do this manually with `Arc::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] #[cfg(target_has_atomic = "ptr")] impl Decode for Arc<[T]> where @@ -521,6 +662,10 @@ where } } +// TODO +// Vec does not implement Into for Arc<[T]> +// we could do this manually with `Arc::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] #[cfg(target_has_atomic = "ptr")] impl<'de, T> BorrowDecode<'de> for Arc<[T]> where @@ -531,3 +676,20 @@ where Ok(vec.into()) } } + +/// A drop guard that will trigger when an item fails to decode. +/// If an item at index n fails to decode, we have to properly drop the 0..(n-1) values that have been read. +struct DropGuard<'a, T> { + slice: &'a mut [core::mem::MaybeUninit], + idx: usize, +} + +impl<'a, T> Drop for DropGuard<'a, T> { + fn drop(&mut self) { + unsafe { + for item in &mut self.slice[..self.idx] { + core::ptr::drop_in_place(item as *mut core::mem::MaybeUninit as *mut T); + } + } + } +} diff --git a/src/features/impl_std.rs b/src/features/impl_std.rs index 72330ed5..9ec91941 100644 --- a/src/features/impl_std.rs +++ b/src/features/impl_std.rs @@ -433,7 +433,9 @@ where decoder.claim_container_read::<(K, V)>(len)?; let hash_builder: S = Default::default(); - let mut map = HashMap::with_capacity_and_hasher(len, hash_builder); + let mut map = HashMap::with_hasher(hash_builder); + map.try_reserve(len)?; + for _ in 0..len { // See the documentation on `unclaim_bytes_read` as to why we're doing this here decoder.unclaim_bytes_read(core::mem::size_of::<(K, V)>()); @@ -445,16 +447,19 @@ where Ok(map) } } -impl<'de, K, V> BorrowDecode<'de> for HashMap +impl<'de, K, V, S> BorrowDecode<'de> for HashMap where K: BorrowDecode<'de> + Eq + std::hash::Hash, V: BorrowDecode<'de>, + S: std::hash::BuildHasher + Default, { fn borrow_decode>(decoder: &mut D) -> Result { let len = crate::de::decode_slice_len(decoder)?; decoder.claim_container_read::<(K, V)>(len)?; - let mut map = HashMap::with_capacity(len); + let mut map = HashMap::with_hasher(S::default()); + map.try_reserve(len)?; + for _ in 0..len { // See the documentation on `unclaim_bytes_read` as to why we're doing this here decoder.unclaim_bytes_read(core::mem::size_of::<(K, V)>()); diff --git a/src/lib.rs b/src/lib.rs index 9c13e474..5efc291b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,10 @@ #![no_std] #![warn(missing_docs, unused_lifetimes)] #![cfg_attr(docsrs, feature(doc_cfg))] +#![cfg_attr( + feature = "unstable-strict-oom-checks", + feature(allocator_api, new_uninit) +)] //! Bincode is a crate for encoding and decoding using a tiny binary //! serialization strategy. Using it, you can easily go from having