Don't panic when retrieving slices from detached typed arrays

mozjs/Cargo.toml

@@ -2,7 +2,7 @@
 name = "mozjs"
 description = "Rust bindings to the Mozilla SpiderMonkey JavaScript engine."
 repository.workspace = true
-version = "0.19.0"
+version = "0.20.0"
 authors = ["The Servo Project Developers"]
 license.workspace = true
 edition.workspace = true

mozjs/src/typedarray.rs

@@ -6,6 +6,8 @@
 //! typed arrays or wrapping existing JS reflectors, and prevents reinterpreting
 //! existing buffers as different types except in well-defined cases.
 
+use std::ptr::NonNull;
+
 use crate::context::NoGC;
 use crate::conversions::ConversionResult;
 use crate::conversions::FromJSValConvertible;
@@ -124,10 +126,17 @@ pub enum CreateWith<'a, T: 'a> {
     Slice(&'a [T]),
 }
 
+#[derive(Clone, Copy)]
+enum ArrayData<T> {
+    NotYetComputed,
+    Detached,
+    Computed(NonNull<[T]>),
+}
+
 /// A typed array wrapper.
 pub struct TypedArray<T: TypedArrayElement, S: JSObjectStorage> {
     object: S,
-    computed: Cell<Option<*mut [T::Element]>>,
+    computed: Cell<ArrayData<T::Element>>,
 }
 
 unsafe impl<T> CustomTrace for TypedArray<T, *mut JSObject>
@@ -155,24 +164,28 @@ impl<T: TypedArrayElement, S: JSObjectStorage> TypedArray<T, S> {
 
             Ok(TypedArray {
                 object: S::from_raw(unwrapped),
-                computed: Cell::new(None),
+                computed: Cell::new(ArrayData::NotYetComputed),
             })
         }
     }
 
-    fn data(&self) -> *mut [T::Element] {
-        if let Some(data) = self.computed.get() {
-            return data;
+    fn data(&self) -> Option<NonNull<[T::Element]>> {
+        if let ArrayData::Computed(data) = self.computed.get() {
+            return Some(data);
         }
 
         let data = unsafe { T::length_and_data(self.object.as_raw()) };
-        self.computed.set(Some(data));
+        self.computed.set(if let Some(data) = data {
+            ArrayData::Computed(data)
+        } else {
+            ArrayData::Detached
+        });
         data
     }
 
     /// Returns the number of elements in the underlying typed array.
     pub fn len(&self) -> usize {
-        self.data().len()
+        self.data().map_or(0, |data| data.len())
     }
 
     /// # Unsafety
@@ -188,8 +201,9 @@ impl<T: TypedArrayElement, S: JSObjectStorage> TypedArray<T, S> {
     }
 
     /// Retrieves an owned data that's represented by the typed array.
+    /// Returns None if the underlying buffer is detached.
     #[allow(deprecated)]
-    pub fn to_vec(&self) -> Vec<T::Element>
+    pub fn to_vec(&self) -> Option<Vec<T::Element>>
     where
         T::Element: Clone,
     {
@@ -198,7 +212,7 @@ impl<T: TypedArrayElement, S: JSObjectStorage> TypedArray<T, S> {
         // the underlying buffer can easily invalidated when transferred with
         // postMessage to another thread (To remedy that, we shouldn't
         // execute any JS code between getting the data pointer and using it).
-        unsafe { self.as_slice().to_vec() }
+        unsafe { self.as_slice().map(|slice| slice.to_vec()) }
     }
 
     /// # Unsafety
@@ -210,22 +224,18 @@ impl<T: TypedArrayElement, S: JSObjectStorage> TypedArray<T, S> {
     ///
     /// Panics if the underlying data points to a nullptr.
     #[deprecated = "use as_slice_safe instead"]
-    pub unsafe fn as_slice(&self) -> &[T::Element] {
-        let data = self.data();
-        assert!(!data.is_null());
-        &*data
+    pub unsafe fn as_slice(&self) -> Option<&[T::Element]> {
+        self.data().map(|data| data.as_ref())
     }
 
-    pub fn as_slice_safe<'a>(&self, _no_gc: &'a NoGC) -> &'a [T::Element] {
+    /// Returns None if the underlying array buffer is detached.
+    /// Otherwise, returns Some with the slice data.
+    pub fn as_slice_safe<'a>(&self, _no_gc: &'a NoGC) -> Option<&'a [T::Element]> {
         // SAFETY: The slice can only be invalidated by invoking JS engine
         //         behaviour that detaches the underlying typed array.
         //         The slice's lifetime is bounded by the provided NoGC token,
         //         which prevents any JS engine interaction.
-        unsafe {
-            let data = self.data();
-            assert!(!data.is_null());
-            &*data
-        }
+        self.data().map(|data| unsafe { data.as_ref() })
     }
 
     /// # Unsafety
@@ -240,22 +250,16 @@ impl<T: TypedArrayElement, S: JSObjectStorage> TypedArray<T, S> {
     ///
     /// Panics if the underlying data points to a nullptr.
     #[deprecated = "use as_mut_slice_safe instead"]
-    pub unsafe fn as_mut_slice(&mut self) -> &mut [T::Element] {
-        let data = self.data();
-        assert!(!data.is_null());
-        &mut *self.data()
+    pub unsafe fn as_mut_slice(&mut self) -> Option<&mut [T::Element]> {
+        self.data().map(|mut data| data.as_mut())
     }
 
-    pub fn as_mut_slice_safe<'a>(&mut self, _no_gc: &'a NoGC) -> &'a mut [T::Element] {
+    pub fn as_mut_slice_safe<'a>(&mut self, _no_gc: &'a NoGC) -> Option<&'a mut [T::Element]> {
         // SAFETY: The slice can only be invalidated by invoking JS engine
         //         behaviour that detaches the underlying typed array.
         //         The slice's lifetime is bounded by the provided NoGC token,
         //         which prevents any JS engine interaction.
-        unsafe {
-            let data = self.data();
-            assert!(!data.is_null());
-            &mut *self.data()
-        }
+        self.data().map(|mut data| unsafe { data.as_mut() })
     }
 
     /// Return a boolean flag which denotes whether the underlying buffer
@@ -298,21 +302,27 @@ impl<T: TypedArrayElementCreator + TypedArrayElement, S: JSObjectStorage> TypedA
     }
 
     unsafe fn update_raw(data: &[T::Element], result: *mut JSObject) {
-        let buffer = T::length_and_data(result);
+        let Some(mut buffer) = T::length_and_data(result) else {
+            return;
+        };
         assert!(data.len() <= buffer.len());
-        ptr::copy_nonoverlapping(data.as_ptr(), buffer as *mut T::Element, data.len());
+        ptr::copy_nonoverlapping(
+            data.as_ptr(),
+            buffer.as_mut().as_mut_ptr(), /* as *mut T::Element*/
+            data.len(),
+        );
     }
 }
 
 /// Internal trait used to associate an element type with an underlying representation
 /// and various functions required to manipulate typed arrays of that element type.
 pub trait TypedArrayElement {
     /// Underlying primitive representation of this element type.
-    type Element;
+    type Element: Copy;
     /// Unwrap a typed array JS reflector for this element type.
     unsafe fn unwrap_array(obj: *mut JSObject) -> *mut JSObject;
     /// Retrieve the length and data of a typed array's buffer for this element type.
-    unsafe fn length_and_data(obj: *mut JSObject) -> *mut [Self::Element];
+    unsafe fn length_and_data(obj: *mut JSObject) -> Option<NonNull<[Self::Element]>>;
 }
 
 /// Internal trait for creating new typed arrays.
@@ -337,13 +347,13 @@ macro_rules! typed_array_element {
                 $unwrap(obj)
             }
 
-            unsafe fn length_and_data(obj: *mut JSObject) -> *mut [Self::Element] {
+            unsafe fn length_and_data(obj: *mut JSObject) -> Option<NonNull<[Self::Element]>> {
                 let mut len = 0;
                 let mut shared = false;
                 let mut data = ptr::null_mut();
                 $length_and_data(obj, &mut len, &mut shared, &mut data);
                 assert!(!shared);
-                std::ptr::slice_from_raw_parts_mut(data, len)
+                NonNull::new(data).map(|data| NonNull::slice_from_raw_parts(data, len))
             }
         }
     };

mozjs/tests/typedarray.rs

@@ -8,11 +8,11 @@ use mozjs::jsapi::{JSObject, OnNewGlobalHookOption, Type};
 use mozjs::jsval::UndefinedValue;
 use mozjs::realm::AutoRealm;
 use mozjs::rooted;
-use mozjs::rust::wrappers2::JS_NewGlobalObject;
+use mozjs::rust::wrappers2::{DetachArrayBuffer, JS_NewGlobalObject};
 use mozjs::rust::{evaluate_script, CompileOptionsWrapper};
 use mozjs::rust::{JSEngine, RealmOptions, Runtime, SIMPLE_GLOBAL_CLASS};
 use mozjs::typedarray;
-use mozjs::typedarray::{CreateWith, Uint32Array};
+use mozjs::typedarray::{ArrayBuffer, CreateWith, Uint32Array};
 
 #[test]
 fn typedarray() {
@@ -50,13 +50,10 @@ fn typedarray() {
         assert!(rval.is_object());
 
         typedarray!(&in(context) let array: Uint8Array = rval.to_object());
-        assert_eq!(array.unwrap().as_slice_safe(context), &[0, 2, 4][..]);
-
-        typedarray!(&in(context) let array: Uint8Array = rval.to_object());
-        assert_eq!(array.unwrap().len(), 3);
-
-        typedarray!(&in(context) let array: Uint8Array = rval.to_object());
-        assert_eq!(array.unwrap().to_vec(), vec![0, 2, 4]);
+        let uint8array = array.unwrap();
+        assert_eq!(uint8array.as_slice_safe(context), Some(&[0, 2, 4][..]));
+        assert_eq!(uint8array.len(), 3);
+        assert_eq!(uint8array.to_vec(), Some(vec![0, 2, 4]));
 
         typedarray!(&in(context) let array: Uint16Array = rval.to_object());
         assert!(array.is_err());
@@ -73,14 +70,12 @@ fn typedarray() {
         .is_ok());
 
         typedarray!(&in(context) let array: Uint32Array = rval.get());
-        assert_eq!(array.unwrap().as_slice_safe(context), &[1, 3, 5][..]);
-
-        typedarray!(&in(context) let mut array: Uint32Array = rval.get());
-        array.as_mut().unwrap().update(&[2, 4, 6]);
-        assert_eq!(array.unwrap().as_slice_safe(context), &[2, 4, 6][..]);
+        let mut uint32array = array.unwrap();
+        assert_eq!(uint32array.as_slice_safe(context), Some(&[1, 3, 5][..]));
+        uint32array.update(&[2, 4, 6]);
+        assert_eq!(uint32array.as_slice_safe(context), Some(&[2, 4, 6][..]));
 
-        rooted!(&in(context) let rval = ptr::null_mut::<JSObject>());
-        typedarray!(&in(context) let array: Uint8Array = rval.get());
+        typedarray!(&in(context) let array: Uint8Array = ptr::null_mut());
         assert!(array.is_err());
 
         rooted!(&in(context) let mut rval = ptr::null_mut::<JSObject>());
@@ -89,16 +84,44 @@ fn typedarray() {
         );
 
         typedarray!(&in(context) let array: Uint32Array = rval.get());
-        assert_eq!(array.unwrap().as_slice_safe(context), &[0, 0, 0, 0, 0]);
-
-        typedarray!(&in(context) let mut array: Uint32Array = rval.get());
-        array.as_mut().unwrap().update(&[0, 1, 2, 3]);
-        assert_eq!(array.unwrap().as_slice_safe(context), &[0, 1, 2, 3, 0]);
+        let mut uint32array = array.unwrap();
+        assert_eq!(
+            uint32array.as_slice_safe(context),
+            Some(&[0, 0, 0, 0, 0][..])
+        );
+        uint32array.update(&[0, 1, 2, 3]);
+        assert_eq!(
+            uint32array.as_slice_safe(context),
+            Some(&[0, 1, 2, 3, 0][..])
+        );
 
         typedarray!(&in(context) let view: ArrayBufferView = rval.get());
-        assert_eq!(view.unwrap().get_array_type(), Type::Uint32);
+        let view = view.unwrap();
+        assert_eq!(view.get_array_type(), Type::Uint32);
+        assert_eq!(view.is_shared(), false);
 
-        typedarray!(&in(context) let view: ArrayBufferView = rval.get());
-        assert_eq!(view.unwrap().is_shared(), false);
+        rooted!(&in(context) let mut rval = ptr::null_mut::<JSObject>());
+        assert!(ArrayBuffer::create(
+            context.raw_cx(),
+            CreateWith::Slice(&[1, 2, 3]),
+            rval.handle_mut()
+        )
+        .is_ok());
+        typedarray!(&in(context) let arraybuffer: ArrayBuffer = rval.get());
+        assert_eq!(
+            arraybuffer.as_ref().unwrap().as_slice_safe(context),
+            Some(&[1, 2, 3][..])
+        );
+
+        assert!(DetachArrayBuffer(context, rval.handle()));
+
+        typedarray!(&in(context) let detached_arraybuffer: ArrayBuffer = rval.get());
+        assert_eq!(
+            detached_arraybuffer
+                .as_ref()
+                .unwrap()
+                .as_slice_safe(context),
+            None,
+        );
     }
 }