fix(string): stabilize rope implementation and address edge cases

Author: akash-R-A-J

core/string/src/iter.rs

@@ -392,49 +392,48 @@ impl<'a> Iterator for RopeCodePointsIter<'a> {
                 self.current = None;
             }
 
-            if let Some(slice) = self.stack.pop() {
-                if slice.header.vtable.kind == crate::JsStringKind::Rope {
-                    // SAFETY: The header is guaranteed to be a `RopeString` because the kind is `Rope`.
-                    let r = unsafe {
-                        &*std::ptr::from_ref(slice.header).cast::<crate::vtable::RopeString>()
-                    };
-                    let left_len = r.left.len();
+            let slice = self.stack.pop()?;
 
-                    if slice.start < left_len {
-                        let left_end = std::cmp::min(slice.end, left_len);
-                        if slice.end > left_len {
-                            self.stack.push(RopeSlice {
-                                // SAFETY: The child pointers in a `RopeString` are always valid `JsStringHeader` pointers.
-                                header: unsafe { &*r.right.ptr.as_ptr().cast() },
-                                start: 0,
-                                end: slice.end - left_len,
-                            });
-                        }
-                        self.stack.push(RopeSlice {
-                            // SAFETY: The child pointers in a `RopeString` are always valid `JsStringHeader` pointers.
-                            header: unsafe { &*r.left.ptr.as_ptr().cast() },
-                            start: slice.start,
-                            end: left_end,
-                        });
-                    } else {
+            if slice.header.vtable.kind == crate::JsStringKind::Rope {
+                // SAFETY: The header is guaranteed to be a `RopeString` because the kind is `Rope`.
+                let r = unsafe {
+                    &*std::ptr::from_ref(slice.header).cast::<crate::vtable::RopeString>()
+                };
+                let left_len = r.left.len();
+
+                if slice.start < left_len {
+                    let left_end = std::cmp::min(slice.end, left_len);
+                    if slice.end > left_len {
                         self.stack.push(RopeSlice {
                             // SAFETY: The child pointers in a `RopeString` are always valid `JsStringHeader` pointers.
                             header: unsafe { &*r.right.ptr.as_ptr().cast() },
-                            start: slice.start - left_len,
+                            start: 0,
                             end: slice.end - left_len,
                         });
                     }
+                    self.stack.push(RopeSlice {
+                        // SAFETY: The child pointers in a `RopeString` are always valid `JsStringHeader` pointers.
+                        header: unsafe { &*r.left.ptr.as_ptr().cast() },
+                        start: slice.start,
+                        end: left_end,
+                    });
                 } else {
-                    let s = (slice.header.vtable.as_str)(slice.header)
-                        .get(slice.start..slice.end)
-                        .expect("valid slice");
-                    let iter = s.code_points();
-                    // SAFETY: The lifetime of the code points is tied to the rope string, which is guaranteed to live for 'a.
-                    let iter = unsafe {
-                        std::mem::transmute::<CodePointsIter<'_>, CodePointsIter<'a>>(iter)
-                    };
-                    self.current = Some((None, iter));
+                    self.stack.push(RopeSlice {
+                        // SAFETY: The child pointers in a `RopeString` are always valid `JsStringHeader` pointers.
+                        header: unsafe { &*r.right.ptr.as_ptr().cast() },
+                        start: slice.start - left_len,
+                        end: slice.end - left_len,
+                    });
                 }
+            } else {
+                let s = (slice.header.vtable.as_str)(slice.header)
+                    .get(slice.start..slice.end)
+                    .expect("valid slice");
+                let iter = s.code_points();
+                // SAFETY: The lifetime of the code points is tied to the rope string, which is guaranteed to live for 'a.
+                let iter =
+                    unsafe { std::mem::transmute::<CodePointsIter<'_>, CodePointsIter<'a>>(iter) };
+                self.current = Some((None, iter));
             }
         }
     }

core/string/src/vtable/slice.rs

@@ -54,8 +54,6 @@ impl SliceString {
     }
 }
 
-// Unused slice_clone removed.
-
 #[inline]
 fn slice_as_str(header: &JsStringHeader) -> JsStr<'_> {
     // SAFETY: The header is part of a SliceString and it's aligned.
@@ -65,10 +63,35 @@ fn slice_as_str(header: &JsStringHeader) -> JsStr<'_> {
 
 #[inline]
 fn slice_dealloc(ptr: NonNull<JsStringHeader>) {
-    // SAFETY: This is part of the correct vtable which is validated on construction.
-    // The pointer is guaranteed to be a valid `NonNull<JsStringHeader>` pointing to a `SliceString`.
-    unsafe {
-        drop(Box::from_raw(ptr.cast::<SliceString>().as_ptr()));
+    // Iteratively destroy slice chains to avoid recursive drop stack overflow.
+    let mut current = ptr;
+
+    loop {
+        // SAFETY: the vtable ensures this pointer refers to `SliceString`.
+        unsafe {
+            // Take ownership of the slice node.
+            let slice_ptr = current.cast::<SliceString>();
+            let mut slice_box = Box::from_raw(slice_ptr.as_ptr());
+
+            // Extract the parent string.
+            let parent =
+                std::mem::replace(&mut slice_box.owned, crate::StaticJsStrings::EMPTY_STRING);
+
+            // Drop the slice node itself.
+            drop(slice_box);
+
+            // If the parent is another slice and we are the last reference,
+            // continue iteratively instead of recursing.
+            if parent.kind() == JsStringKind::Slice && parent.refcount() == Some(1) {
+                current = parent.ptr;
+                std::mem::forget(parent);
+                continue;
+            }
+
+            // Otherwise drop normally and finish.
+            drop(parent);
+            break;
+        }
     }
 }