array de bin leak

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "nanoserde"
-version = "0.1.34"
+version = "0.1.36"
 authors = ["makepad <info@makepad.nl>", "Fedor <not.fl3@gmail.com>"]
 license = "MIT OR Apache-2.0"
 description = """

src/serde_bin.rs

@@ -402,14 +402,32 @@ where
     T: DeBin,
 {
     fn de_bin(o: &mut usize, d: &[u8]) -> Result<Self, DeBinErr> {
-        unsafe {
-            let mut to = core::mem::MaybeUninit::<[T; N]>::uninit();
-            let top: *mut T = core::mem::transmute(&mut to);
-            for c in 0..N {
-                top.add(c).write(DeBin::de_bin(o, d)?);
+        use core::mem::MaybeUninit;
+
+        // waiting for uninit_array(or for array::try_from_fn) stabilization
+        // https://github.com/rust-lang/rust/issues/96097
+        // https://github.com/rust-lang/rust/issues/89379
+        let mut to: [MaybeUninit<T>; N] =
+            unsafe { MaybeUninit::<[MaybeUninit<T>; N]>::uninit().assume_init() };
+
+        for index in 0..N {
+            to[index] = match DeBin::de_bin(o, d) {
+                Ok(v) => MaybeUninit::new(v),
+                Err(e) => {
+                    // drop all the MaybeUninit values which we've already
+                    // successfully deserialized so we don't leak memory.
+                    // See https://github.com/not-fl3/nanoserde/issues/79
+                    for (_, to_drop) in (0..index).zip(to) {
+                        unsafe { to_drop.assume_init() };
+                    }
+                    return Err(e);
+                }
             }
-            Ok(to.assume_init())
         }
+
+        // waiting for array_assume_init or core::array::map optimizations
+        // https://github.com/rust-lang/rust/issues/61956
+        Ok(unsafe { (&*(&to as *const _ as *const MaybeUninit<_>)).assume_init_read() })
     }
 }
 

src/serde_json.rs

@@ -999,31 +999,42 @@ where
     T: DeJson,
 {
     fn de_json(o: &mut DeJsonState, d: &mut Chars) -> Result<Self, DeJsonErr> {
-        unsafe {
-            let mut to = core::mem::MaybeUninit::<[T; N]>::uninit();
-            let top: *mut T = core::mem::transmute(&mut to);
-            de_json_array_impl_inner(top, N, o, d)?;
-            Ok(to.assume_init())
+        use core::mem::MaybeUninit;
+
+        // waiting for uninit_array(or for array::try_from_fn) stabilization
+        // https://github.com/rust-lang/rust/issues/96097
+        // https://github.com/rust-lang/rust/issues/89379
+        let mut to: [MaybeUninit<T>; N] =
+            unsafe { MaybeUninit::<[MaybeUninit<T>; N]>::uninit().assume_init() };
+        o.block_open(d)?;
+
+        for index in 0..N {
+            to[index] = match DeJson::de_json(o, d).and_then(|ret| {
+                o.eat_comma_block(d)?;
+                Ok(ret)
+            }) {
+                Ok(v) => MaybeUninit::new(v),
+                Err(e) => {
+                    // drop all the MaybeUninit values which we've already
+                    // successfully deserialized so we don't leak memory.
+                    // See https://github.com/not-fl3/nanoserde/issues/79
+                    for (_, to_drop) in (0..index).zip(to) {
+                        unsafe { to_drop.assume_init() };
+                    }
+                    return Err(e);
+                }
+            }
         }
-    }
-}
 
-unsafe fn de_json_array_impl_inner<T>(
-    top: *mut T,
-    count: usize,
-    s: &mut DeJsonState,
-    i: &mut Chars,
-) -> Result<(), DeJsonErr>
-where
-    T: DeJson,
-{
-    s.block_open(i)?;
-    for c in 0..count {
-        top.add(c).write(DeJson::de_json(s, i)?);
-        s.eat_comma_block(i)?;
+        // waiting for array_assume_init or core::array::map optimizations
+        // https://github.com/rust-lang/rust/issues/61956
+        // initializing before block close so that drop will run automatically if err encountered there
+        let initialized =
+            unsafe { (&*(&to as *const _ as *const MaybeUninit<_>)).assume_init_read() };
+        o.block_close(d)?;
+
+        Ok(initialized)
     }
-    s.block_close(i)?;
-    Ok(())
 }
 
 fn de_json_comma_block<T>(s: &mut DeJsonState, i: &mut Chars) -> Result<T, DeJsonErr>

src/serde_ron.rs

@@ -1021,35 +1021,46 @@ where
     }
 }
 
-unsafe fn de_ron_array_impl_inner<T>(
-    top: *mut T,
-    count: usize,
-    s: &mut DeRonState,
-    i: &mut Chars,
-) -> Result<(), DeRonErr>
-where
-    T: DeRon,
-{
-    s.paren_open(i)?;
-    for c in 0..count {
-        top.add(c).write(DeRon::de_ron(s, i)?);
-        s.eat_comma_paren(i)?;
-    }
-    s.paren_close(i)?;
-    Ok(())
-}
-
 impl<T, const N: usize> DeRon for [T; N]
 where
     T: DeRon,
 {
-    fn de_ron(s: &mut DeRonState, i: &mut Chars) -> Result<Self, DeRonErr> {
-        unsafe {
-            let mut to = core::mem::MaybeUninit::<[T; N]>::uninit();
-            let top: *mut T = core::mem::transmute(&mut to);
-            de_ron_array_impl_inner(top, N, s, i)?;
-            Ok(to.assume_init())
+    fn de_ron(o: &mut DeRonState, d: &mut Chars) -> Result<Self, DeRonErr> {
+        use core::mem::MaybeUninit;
+
+        // waiting for uninit_array(or for array::try_from_fn) stabilization
+        // https://github.com/rust-lang/rust/issues/96097
+        // https://github.com/rust-lang/rust/issues/89379
+        let mut to: [MaybeUninit<T>; N] =
+            unsafe { MaybeUninit::<[MaybeUninit<T>; N]>::uninit().assume_init() };
+        o.paren_open(d)?;
+
+        for index in 0..N {
+            to[index] = match DeRon::de_ron(o, d).and_then(|ret| {
+                o.eat_comma_paren(d)?;
+                Ok(ret)
+            }) {
+                Ok(v) => MaybeUninit::new(v),
+                Err(e) => {
+                    // drop all the MaybeUninit values which we've already
+                    // successfully deserialized so we don't leak memory.
+                    // See https://github.com/not-fl3/nanoserde/issues/79
+                    for (_, to_drop) in (0..index).zip(to) {
+                        unsafe { to_drop.assume_init() };
+                    }
+                    return Err(e);
+                }
+            }
         }
+
+        // waiting for array_assume_init or core::array::map optimizations
+        // https://github.com/rust-lang/rust/issues/61956
+        // initializing before block close so that drop will run automatically if err encountered there
+        let initialized =
+            unsafe { (&*(&to as *const _ as *const MaybeUninit<_>)).assume_init_read() };
+        o.paren_close(d)?;
+
+        Ok(initialized)
     }
 }
 

tests/bin.rs

@@ -1,4 +1,8 @@
-use std::collections::{BTreeSet, HashMap, HashSet, LinkedList};
+use std::{
+    array,
+    collections::{BTreeSet, HashMap, HashSet, LinkedList},
+    sync::atomic::AtomicBool,
+};
 
 use nanoserde::{DeBin, SerBin};
 
@@ -238,3 +242,30 @@ fn collections() {
 
     assert_eq!(test, test_deserialized);
 }
+
+#[test]
+fn array_leak_test() {
+    static TOGGLED_ON_DROP: AtomicBool = AtomicBool::new(false);
+
+    #[derive(Default, Clone, SerBin, DeBin)]
+    struct IncrementOnDrop {
+        inner: u128,
+    }
+
+    impl Drop for IncrementOnDrop {
+        fn drop(&mut self) {
+            TOGGLED_ON_DROP.store(true, std::sync::atomic::Ordering::SeqCst);
+        }
+    }
+
+    let items: [_; 2] = array::from_fn(|_| IncrementOnDrop::default());
+    let serialized = nanoserde::SerBin::serialize_bin(&items);
+    let corrupted_serialized = &serialized[..serialized.len() - 1];
+
+    if let Ok(_) = <[IncrementOnDrop; 2] as nanoserde::DeBin>::deserialize_bin(corrupted_serialized)
+    {
+        panic!("Unexpected success")
+    }
+
+    assert!(TOGGLED_ON_DROP.load(std::sync::atomic::Ordering::SeqCst))
+}

tests/json.rs

@@ -1,6 +1,9 @@
 use nanoserde::{DeJson, SerJson};
 
-use std::collections::{BTreeSet, HashMap, HashSet, LinkedList};
+use std::{
+    collections::{BTreeSet, HashMap, HashSet, LinkedList},
+    sync::atomic::AtomicBool,
+};
 
 #[test]
 fn de() {
@@ -558,6 +561,7 @@ fn test_various_escapes() {
 
 // there are only 1024*1024 surrogate pairs, so we can do an exhautive test.
 #[test]
+#[cfg_attr(miri, ignore)]
 fn test_surrogate_pairs_exhaustively() {
     for lead in 0xd800..0xdc00 {
         for trail in 0xdc00..0xe000 {
@@ -804,3 +808,31 @@ fn ser_str() {
         SerJson::serialize_json(&a_str)
     );
 }
+
+#[test]
+fn array_leak_test() {
+    static TOGGLED_ON_DROP: AtomicBool = AtomicBool::new(false);
+
+    #[derive(Default, Clone, SerJson, DeJson)]
+    struct IncrementOnDrop {
+        inner: u64,
+    }
+
+    impl Drop for IncrementOnDrop {
+        fn drop(&mut self) {
+            TOGGLED_ON_DROP.store(true, std::sync::atomic::Ordering::SeqCst);
+        }
+    }
+
+    let items: [_; 2] = core::array::from_fn(|_| IncrementOnDrop::default());
+    let serialized = nanoserde::SerJson::serialize_json(&items);
+    let corrupted_serialized = &serialized[..serialized.len() - 1];
+
+    if let Ok(_) =
+        <[IncrementOnDrop; 2] as nanoserde::DeJson>::deserialize_json(corrupted_serialized)
+    {
+        panic!("Unexpected success")
+    }
+
+    assert!(TOGGLED_ON_DROP.load(std::sync::atomic::Ordering::SeqCst))
+}

tests/ron.rs

@@ -1,6 +1,9 @@
 use nanoserde::{DeRon, SerRon};
 
-use std::collections::{BTreeSet, HashMap, HashSet, LinkedList};
+use std::{
+    collections::{BTreeSet, HashMap, HashSet, LinkedList},
+    sync::atomic::AtomicBool,
+};
 
 #[test]
 fn ron_de() {
@@ -380,6 +383,7 @@ fn test_various_escapes() {
 
 // there are only 1024*1024 surrogate pairs, so we can do an exhautive test.
 #[test]
+#[cfg_attr(miri, ignore)]
 fn test_surrogate_pairs_exhaustively() {
     for lead in 0xd800..0xdc00 {
         for trail in 0xdc00..0xe000 {
@@ -420,3 +424,30 @@ fn tuple_struct() {
 
     assert!(test == test_deserialized);
 }
+
+#[test]
+fn array_leak_test() {
+    static TOGGLED_ON_DROP: AtomicBool = AtomicBool::new(false);
+
+    #[derive(Default, Clone, DeRon, SerRon)]
+    struct IncrementOnDrop {
+        inner: u64,
+    }
+
+    impl Drop for IncrementOnDrop {
+        fn drop(&mut self) {
+            TOGGLED_ON_DROP.store(true, std::sync::atomic::Ordering::SeqCst);
+        }
+    }
+
+    let items: [_; 2] = core::array::from_fn(|_| IncrementOnDrop::default());
+    let serialized = nanoserde::SerRon::serialize_ron(&items);
+    let corrupted_serialized = &serialized[..serialized.len() - 1];
+
+    if let Ok(_) = <[IncrementOnDrop; 2] as nanoserde::DeRon>::deserialize_ron(corrupted_serialized)
+    {
+        panic!("Unexpected success")
+    }
+
+    assert!(TOGGLED_ON_DROP.load(std::sync::atomic::Ordering::SeqCst))
+}