From bc344d2b142965b994b7b0cd8fe894638c3baabb Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Sat, 11 Dec 2021 10:28:41 +0100 Subject: [PATCH] Added a pipeline to deny 'no_global_oom_handling' Added try_reserve to VecWriter, Vec, VecDeque and HashMap Made the OOM test succeed on the latest nightly Made the OOM check work on nightly Disabled stable checks for now Removed feature that is stable in current nightly Fixed issue where Box<[T]> would not do the size limit check Added a drop guard to `impl Decode for Vec` to make sure memory is not leaked when T::Decode panics Made the implementation of `VecWriter` use less lines of unsfe Fixed errors while merging --- .github/workflows/rust.yml | 32 ++- Cargo.toml | 21 +- src/error.rs | 57 ++++ src/features/impl_alloc.rs | 540 ++++++++++++++++++++++++------------- src/features/impl_std.rs | 11 +- src/lib.rs | 4 + 6 files changed, 463 insertions(+), 202 deletions(-) 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