fix: memory leak in to_value (cloudwego#105)

* fix: memory leak in to_value

* bump 0.3.12

.github/workflows/ci.yml

@@ -2,8 +2,6 @@ name: 'CI'
 on:
   pull_request:
   push:
-    branches:
-      - main
 
 env:
   RUST_BACKTRACE: 1

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "sonic-rs"
-version = "0.3.11"
+version = "0.3.12"
 authors = ["Volo Team <[email protected]>"]
 edition = "2021"
 description = "Sonic-rs is a fast Rust JSON library based on SIMD"
@@ -18,7 +18,7 @@ cfg-if = "1.0"
 serde = { version = "1.0", features = ["rc", "derive"] }
 itoa = "1.0"
 ryu = "1.0"
-faststr = "0.2"
+faststr = { version = "0.2", features = ["serde"] }
 smallvec = "1.11"
 bumpalo = "3.13"
 bytes = "1.4"

src/index.rs

@@ -66,7 +66,7 @@ impl<I: Index> std::ops::IndexMut<I> for Value {
     ///
     /// ```
     /// # use sonic_rs::json;
-    /// #
+    /// # use sonic_rs::object;
     /// let mut data = json!({ "x": 0, "z": null });
     ///
     /// // replace an existing key
@@ -84,11 +84,14 @@ impl<I: Index> std::ops::IndexMut<I> for Value {
     /// //insert an key in a null value
     /// data["z"]["zz"] = json!("insert in null");
     ///
+    /// data["z"]["zz1"] = object!{}.into();
+    ///
+    ///
     /// assert_eq!(data, json!({
     ///   "x": 1,
     ///   "y": [true, 2, 3],
     ///   "a": { "b": {"c": {"d": true}}},
-    ///    "z": {"zz": "insert in null"}
+    ///    "z": {"zz": "insert in null", "zz1": {}}
     /// }));
     /// ```
     #[inline]
@@ -182,6 +185,7 @@ macro_rules! impl_str_index {
                         if shared.is_null() {
                             shared = Shared::new_ptr();
                             *v = Value::new_object(shared, 8);
+                            v.mark_root();
                         } else {
                             unsafe { std::ptr::write(v, Value::new_object(shared, DEFAULT_OBJ_CAP)) };
                         }
@@ -282,3 +286,4 @@ where
         (**self).as_key()
     }
 }
+

src/serde/de.rs

@@ -308,7 +308,7 @@ impl<'de, R: Reader<'de>> Deserializer<R> {
     {
         let shared = self.parser.get_shared_inc_count();
         let mut val = Value::new_null(shared.data_ptr());
-        let val = if self.parser.read.index() == 0 {
+        let mut val = if self.parser.read.index() == 0 {
             // get n to check trailing characters in later
             let n = val.parse_with_padding(self.parser.read.as_u8_slice())?;
             self.parser.read.eat(n);
@@ -321,10 +321,10 @@ impl<'de, R: Reader<'de>> Deserializer<R> {
         };
 
         // deserialize `Value` must be root node
-        debug_assert!(val.is_static() || val.is_root());
-        if !val.shared_parts().is_null() {
+        if !val.is_scalar() {
             std::mem::forget(shared);
         }
+        val.mark_root();
 
         let val = ManuallyDrop::new(val);
         // #Safety

src/value/node.rs

@@ -1194,6 +1194,7 @@ impl Value {
 
     #[inline]
     pub(crate) fn grow<T>(&mut self, capacity: usize) {
+        assert!(self.is_array() || self.is_object());
         if self.is_static() {
             self.mark_shared(Shared::new_ptr());
             self.mark_root();
@@ -1367,7 +1368,6 @@ impl Value {
         parser.parse_value(&mut visitor)?;
         self.data = visitor.nodes()[0].data;
         self.meta = visitor.nodes()[0].meta;
-        self.mark_root();
         Ok(parser.read.index())
     }
 
@@ -1388,7 +1388,6 @@ impl Value {
         parser.parse_value_without_padding(&mut visitor)?;
         self.data = visitor.nodes()[0].data;
         self.meta = visitor.nodes()[0].meta;
-        self.mark_root();
         Ok(())
     }
 
@@ -1416,6 +1415,10 @@ impl Value {
         unsafe { &*(self.data.sval) }
     }
 
+    pub(crate) fn is_scalar(&self) -> bool {
+        self.meta.tag() < STRING
+    }
+
     pub(crate) fn str_len(&self) -> usize {
         debug_assert!(self.is_str());
         let s = unsafe { &*self.data.sval };

src/value/ser.rs

@@ -12,7 +12,6 @@ use crate::{
     serde::ser::key_must_be_str_or_num,
     util::arc::Arc,
     value::node::Value,
-    JsonValueTrait,
 };
 
 /// Convert a `T` into `sonic_rs::Value` which can represent any valid JSON data.
@@ -64,16 +63,13 @@ pub fn to_value<T>(value: &T) -> Result<Value>
 where
     T: ?Sized + Serialize,
 {
-    let shared = Arc::new(Shared::new());
-    let mut value = to_value_in(
-        unsafe { NonNull::new_unchecked(shared.data_ptr() as *mut _) },
-        value,
-    )?;
-    if value.is_number() {
+    let shared = Shared::new_ptr();
+    let mut value = to_value_in(unsafe { NonNull::new_unchecked(shared as *mut _) }, value)?;
+    if value.is_scalar() {
         value.mark_shared(std::ptr::null());
+        std::mem::drop(unsafe { Arc::from_raw(shared) });
     } else {
         value.mark_root();
-        std::mem::forget(shared);
     }
     Ok(value)
 }
@@ -783,7 +779,9 @@ where
 mod test {
     use std::collections::HashMap;
 
-    use super::*;
+    use serde::{Deserialize, Serialize};
+
+    use crate::{to_value, Value};
 
     #[derive(Debug, serde::Serialize, Hash, Default, Eq, PartialEq)]
     struct User {
@@ -825,4 +823,38 @@ mod test {
         println!("{:?}", got);
         assert!(got.is_err());
     }
+
+    #[derive(Default, Clone, Serialize, Deserialize, Debug)]
+    pub struct CommonArgs {
+        pub app_name: Option<String>,
+    }
+
+    #[derive(Default, Clone, Serialize, Deserialize, Debug)]
+    struct Foo {
+        a: i64,
+        b: Vec<Value>,
+    }
+
+    #[test]
+    #[cfg(not(feature = "arbitrary_precision"))]
+    fn test_to_value2() {
+        use crate::prelude::*;
+
+        let mut value = Value::default();
+
+        let args = CommonArgs {
+            app_name: Some("test".to_string()),
+        };
+        let foo: Foo =
+            crate::from_str(r#"{"a": 1, "b":[123, "a", {}, [], {"a":null}, ["b"], 1.23]}"#)
+                .unwrap();
+
+        value["arg"] = to_value(&args).unwrap_or_default();
+        value["bool"] = to_value(&true).unwrap_or_default();
+        value["foo"] = to_value(&foo).unwrap_or_default();
+        value["arr"] = to_value(&[1, 2, 3]).unwrap_or_default();
+        value["arr"][2] = to_value(&args).unwrap_or_default();
+
+        assert_eq!(value["arr"][2]["app_name"].as_str(), Some("test"));
+    }
 }

src/value/shared.rs

@@ -116,28 +116,3 @@ impl Drop for SharedCtxGuard {
         set_shared(self.old);
     }
 }
-
-pub(crate) struct CheckCtxGuard {
-    is_root: bool,
-}
-
-impl CheckCtxGuard {
-    /// assign `new_shared` into SharedCtx
-    pub(crate) fn new() -> Self {
-        let old = get_shared();
-        if old.is_null() {
-            set_shared(Shared::new_ptr());
-            Self { is_root: true }
-        } else {
-            Self { is_root: false }
-        }
-    }
-}
-
-impl Drop for CheckCtxGuard {
-    fn drop(&mut self) {
-        if self.is_root {
-            set_shared(std::ptr::null());
-        }
-    }
-}