fix: search marker indexing (toeverything#454)

* feat: rename `get_item` to `get_node`

* feat: add fuzzing for search marker

* feat: switch to renamed apis

* chore: add comment

* fix: update marker changes

* test: enable multiple thread test

* fix: lint

Author: darkskygit

Cargo.lock

@@ -31,6 +31,7 @@ ordered-float = { version = "3.6.0", features = ["proptest"] }
 proptest = "1.1.0"
 proptest-derive = "0.3.0"
 rand = "0.8.5"
+rand_chacha = "0.3.1"
 serde = { version = "1.0.155", features = ["derive"] }
 y-sync = { git = "https://github.com/toeverything/y-sync", rev = "aeb0010" }
 yrs = "0.16.5"

libs/jwst-codec/Cargo.toml

@@ -8,8 +8,9 @@ edition = "2021"
 cargo-fuzz = true
 
 [dependencies]
-rand = "0.8"
-libfuzzer-sys = "0.4"
+rand = "0.8.5"
+rand_chacha = "0.3.1"
+libfuzzer-sys = "0.4.6"
 lib0 = "0.16.5"
 yrs = "0.16.5"
 
@@ -43,6 +44,12 @@ path = "fuzz_targets/decode_bytes.rs"
 test = false
 doc = false
 
+[[bin]]
+name = "parallel_insert_text"
+path = "fuzz_targets/parallel_insert_text.rs"
+test = false
+doc = false
+
 [[bin]]
 name = "sync_message"
 path = "fuzz_targets/sync_message.rs"

libs/jwst-codec/fuzz/Cargo.lock

@@ -0,0 +1,65 @@
+#![no_main]
+
+use jwst_codec::*;
+use libfuzzer_sys::fuzz_target;
+use rand::{Rng, SeedableRng};
+use rand_chacha::ChaCha20Rng;
+use std::sync::{
+    atomic::{AtomicUsize, Ordering},
+    Arc,
+};
+
+fuzz_target!(|seed: u64| {
+    println!("seed: {}", seed);
+    let iteration = 10;
+    let rand = ChaCha20Rng::seed_from_u64(seed);
+    let added_len = Arc::new(AtomicUsize::new(0));
+    let mut handles = Vec::new();
+
+    let doc = Doc::default();
+    let mut text = doc.get_or_crate_text("test").unwrap();
+    text.insert(0, "This is a string with length 32.").unwrap();
+
+    // parallel editing text
+    {
+        for i in 0..iteration {
+            let mut text = text.clone();
+            let mut rand = rand.clone();
+            let len = added_len.clone();
+            handles.push(std::thread::spawn(move || {
+                let pos = rand.gen_range(0..text.len());
+                let string = format!("hello {i}");
+
+                text.insert(pos, &string).unwrap();
+                len.fetch_add(string.len(), Ordering::SeqCst);
+            }));
+        }
+    }
+
+    // parallel editing doc
+    {
+        for i in 0..iteration {
+            let doc = doc.clone();
+            let mut rand = rand.clone();
+            let len = added_len.clone();
+            handles.push(std::thread::spawn(move || {
+                let mut text = doc.get_or_crate_text("test").unwrap();
+                let pos = rand.gen_range(0..text.len());
+                let string = format!("hello doc{i}");
+
+                text.insert(pos, &string).unwrap();
+                len.fetch_add(string.len(), Ordering::SeqCst);
+            }));
+        }
+    }
+
+    for handle in handles {
+        handle.join().unwrap();
+    }
+
+    assert_eq!(
+        text.to_string().len(),
+        32 /* raw length */
+        + added_len.load(Ordering::SeqCst) /* parallel text editing: insert(pos, string) */
+    );
+});

libs/jwst-codec/fuzz/Cargo.toml

@@ -179,6 +179,14 @@ impl Default for Item {
 }
 
 impl Item {
+    pub fn left(&self, store: &DocStore) -> Option<Arc<Item>> {
+        self.left_id.and_then(|id| store.get_item(id))
+    }
+
+    pub fn right(&self, store: &DocStore) -> Option<Arc<Item>> {
+        self.right_id.and_then(|id| store.get_item(id))
+    }
+
     pub fn is_empty(&self) -> bool {
         self.len() == 0
     }
@@ -211,7 +219,7 @@ impl Item {
 
     pub fn get_parent(&self, store: &DocStore) -> JwstCodecResult<Option<Arc<Item>>> {
         let parent = match &self.parent {
-            Some(Parent::Id(id)) => store.get_item(*id).and_then(|i| i.as_item()),
+            Some(Parent::Id(id)) => store.get_node(*id).and_then(|i| i.as_item()),
             Some(Parent::String(_)) => None,
             Some(Parent::Type(ty)) => ty.read().unwrap().start.as_ref().and_then(|i| i.as_item()),
             None => None,

libs/jwst-codec/fuzz/fuzz_targets/parallel_insert_text.rs

@@ -136,11 +136,16 @@ impl DocStore {
         }
     }
 
-    pub fn get_item<I: Into<Id>>(&self, id: I) -> Option<StructInfo> {
-        self.get_item_with_idx(id).map(|(item, _)| item)
+    pub fn get_item<I: Into<Id>>(&self, id: I) -> Option<Arc<Item>> {
+        self.get_node_with_idx(id)
+            .and_then(|(item, _)| item.as_item())
     }
 
-    pub fn get_item_with_idx<I: Into<Id>>(&self, id: I) -> Option<(StructInfo, usize)> {
+    pub fn get_node<I: Into<Id>>(&self, id: I) -> Option<StructInfo> {
+        self.get_node_with_idx(id).map(|(item, _)| item)
+    }
+
+    pub fn get_node_with_idx<I: Into<Id>>(&self, id: I) -> Option<(StructInfo, usize)> {
         let id = id.into();
         if let Some(items) = self.items.read().unwrap().get(&id.client) {
             if let Some(index) = Self::get_item_index(items, id.clock) {
@@ -151,7 +156,7 @@ impl DocStore {
         None
     }
 
-    pub fn split_item<I: Into<Id>>(
+    pub fn split_node<I: Into<Id>>(
         &mut self,
         id: I,
         diff: u64,
@@ -295,7 +300,7 @@ impl DocStore {
         let mut o = s.clone();
 
         while let Some(left_id) = o.left_id() {
-            if let Some(left_struct) = self.get_item(left_id) {
+            if let Some(left_struct) = self.get_node(left_id) {
                 if left_struct.is_item() {
                     o = left_struct;
                 } else {
@@ -334,7 +339,7 @@ impl DocStore {
             // Item { id: (1, 0), content: Content::Type(YText) }
             //        ^^ Parent::Id((1, 0))
             Some(Parent::Id(parent_id)) => {
-                match self.get_item(*parent_id) {
+                match self.get_node(*parent_id) {
                     Some(StructInfo::Item(_)) => match &item.content.as_ref() {
                         Content::Type(ty) => {
                             item.parent.replace(Parent::Type(ty.clone()));
@@ -355,11 +360,11 @@ impl DocStore {
             }
             // no item.parent, borrow left.parent or right.parent
             None => {
-                if let Some(left) = item.left_id.and_then(|left_id| self.get_item(left_id)) {
+                if let Some(left) = item.left_id.and_then(|left_id| self.get_node(left_id)) {
                     item.parent = left.parent().cloned();
                     item.parent_sub = left.parent_sub().cloned();
                 } else if let Some(right) =
-                    item.right_id.and_then(|right_id| self.get_item(right_id))
+                    item.right_id.and_then(|right_id| self.get_node(right_id))
                 {
                     item.parent = right.parent().cloned();
                     item.parent_sub = right.parent_sub().cloned();
@@ -405,8 +410,8 @@ impl DocStore {
                         parent_lock.as_deref_mut().unwrap()
                     };
 
-                    let mut left = item.left_id.and_then(|left_id| self.get_item(left_id));
-                    let mut right = item.right_id.and_then(|right_id| self.get_item(right_id));
+                    let mut left = item.left_id.and_then(|left_id| self.get_node(left_id));
+                    let mut right = item.right_id.and_then(|right_id| self.get_node(right_id));
 
                     let right_is_null_or_has_left = match &right {
                         None => true,
@@ -423,7 +428,7 @@ impl DocStore {
                     {
                         // set the first conflicting item
                         let mut o = if let Some(left) = left.clone() {
-                            left.right_id().and_then(|right_id| self.get_item(right_id))
+                            left.right_id().and_then(|right_id| self.get_node(right_id))
                         } else if let Some(parent_sub) = &item.parent_sub {
                             let o = parent.map.as_ref().and_then(|m| m.get(parent_sub).cloned());
                             o.as_ref().map(|o| self.get_start_item(o))
@@ -466,7 +471,7 @@ impl DocStore {
                                         break;
                                     }
                                     o = match c.right_id {
-                                        Some(right_id) => self.get_item(right_id),
+                                        Some(right_id) => self.get_node(right_id),
                                         None => None,
                                     };
                                 }
@@ -894,7 +899,7 @@ mod tests {
             };
             doc_store.add_item(struct_info.clone()).unwrap();
 
-            assert_eq!(doc_store.get_item(Id::new(1, 9)), Some(struct_info));
+            assert_eq!(doc_store.get_node(Id::new(1, 9)), Some(struct_info));
         }
 
         {
@@ -910,13 +915,13 @@ mod tests {
             doc_store.add_item(struct_info1).unwrap();
             doc_store.add_item(struct_info2.clone()).unwrap();
 
-            assert_eq!(doc_store.get_item(Id::new(1, 25)), Some(struct_info2));
+            assert_eq!(doc_store.get_node(Id::new(1, 25)), Some(struct_info2));
         }
 
         {
             let doc_store = DocStore::new();
 
-            assert_eq!(doc_store.get_item(Id::new(1, 0)), None);
+            assert_eq!(doc_store.get_node(Id::new(1, 0)), None);
         }
 
         {
@@ -932,7 +937,7 @@ mod tests {
             doc_store.add_item(struct_info1).unwrap();
             doc_store.add_item(struct_info2).unwrap();
 
-            assert_eq!(doc_store.get_item(Id::new(1, 35)), None);
+            assert_eq!(doc_store.get_node(Id::new(1, 35)), None);
         }
     }
 
@@ -956,7 +961,7 @@ mod tests {
         doc_store.add_item(struct_info1.clone()).unwrap();
         doc_store.add_item(struct_info2).unwrap();
 
-        let s1 = doc_store.get_item(Id::new(1, 0)).unwrap();
+        let s1 = doc_store.get_node(Id::new(1, 0)).unwrap();
         assert_eq!(s1, struct_info1);
         let left = doc_store.split_at_and_get_left((1, 1)).unwrap();
         assert_eq!(left.len(), 2); // octo => oc_to

libs/jwst-codec/src/doc/codec/item.rs

@@ -13,7 +13,7 @@ impl Iterator for ListIterator<'_> {
         while let Some(n) = self.next.take() {
             self.next = n
                 .right_id()
-                .and_then(|right_id| self.store.get_item(right_id));
+                .and_then(|right_id| self.store.get_node(right_id));
 
             if n.deleted() {
                 continue;

libs/jwst-codec/src/doc/store.rs

@@ -66,10 +66,7 @@ pub(crate) trait ListType: AsInner<Inner = YTypeRef> {
                     }
                 }
 
-                pos.right = item
-                    .right_id
-                    .and_then(|right_id| store.get_item(right_id))
-                    .and_then(|right| right.as_item());
+                pos.right = item.right(store);
                 pos.left = Some(item);
             } else {
                 break;
@@ -84,9 +81,8 @@ pub(crate) trait ListType: AsInner<Inner = YTypeRef> {
             return Err(JwstCodecError::IndexOutOfBound(index));
         }
 
-        let inner = self.as_inner().write().unwrap();
-        if let Some(store) = inner.store.upgrade() {
-            let mut store = store.write().unwrap();
+        let mut inner = self.as_inner().write().unwrap();
+        if let Some(mut store) = inner.store_mut() {
             let pos = self.find_pos(&inner, &store, index);
             Self::insert_after(inner, &mut store, pos, contents)?;
         }
@@ -103,7 +99,7 @@ pub(crate) trait ListType: AsInner<Inner = YTypeRef> {
         // insert in between an splitable item.
         if pos.offset > 0 {
             debug_assert!(pos.left.is_some());
-            let (_, right) = store.split_item(pos.left.as_ref().unwrap().id, pos.offset)?;
+            let (_, right) = store.split_node(pos.left.as_ref().unwrap().id, pos.offset)?;
             pos.right = right.as_item();
         }
 
@@ -138,10 +134,7 @@ pub(crate) trait ListType: AsInner<Inner = YTypeRef> {
         }
 
         let inner = self.as_inner().read().unwrap();
-
-        if let Some(store) = inner.store.upgrade() {
-            let store = store.read().unwrap();
-
+        if let Some(store) = inner.store() {
             let pos = self.find_pos(&inner, &store, index);
 
             if pos.offset == 0 {
@@ -183,7 +176,7 @@ pub(crate) trait ListType: AsInner<Inner = YTypeRef> {
         // delete in between an splitable item.
         if pos.offset > 0 {
             debug_assert!(pos.left.is_some());
-            let (_, right) = store.split_item(pos.left.as_ref().unwrap().id, pos.offset)?;
+            let (_, right) = store.split_node(pos.left.as_ref().unwrap().id, pos.offset)?;
             pos.right = right.as_item();
         }
 
@@ -192,15 +185,15 @@ pub(crate) trait ListType: AsInner<Inner = YTypeRef> {
                 if !item.deleted() {
                     let content_len = item.len() as i64;
                     if remaining < content_len {
-                        store.split_item(item.id, remaining as u64)?;
+                        store.split_node(item.id, remaining as u64)?;
                     }
                     remaining -= content_len;
                     store.delete_item(&item, Some(&mut lock));
                 }
 
                 pos.right = item
                     .right_id
-                    .and_then(|right_id| store.get_item(right_id))
+                    .and_then(|right_id| store.get_node(right_id))
                     .and_then(|right| right.as_item());
                 pos.left = Some(item);
             } else {