feat(ext/web): use ArrayBuffer.was_detached()

cli/tests/unit/structured_clone_test.ts

@@ -27,4 +27,24 @@ Deno.test("correct DataCloneError message", () => {
     DOMException,
     "Value not transferable",
   );
+
+  const ab = new ArrayBuffer(1);
+  // detach ArrayBuffer
+  structuredClone(ab, { transfer: [ab] });
+  assertThrows(
+    () => {
+      structuredClone(ab, { transfer: [ab] });
+    },
+    DOMException,
+    "ArrayBuffer at index 0 is already detached",
+  );
+
+  const ab2 = new ArrayBuffer(0);
+  assertThrows(
+    () => {
+      structuredClone([ab2, ab], { transfer: [ab2, ab] });
+    },
+    DOMException,
+    "ArrayBuffer at index 1 is already detached",
+  );
 });

core/ops_builtin_v8.rs

@@ -1,5 +1,6 @@
 // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
 use crate::bindings::script_origin;
+use crate::error::custom_error;
 use crate::error::is_instance_of_error;
 use crate::error::range_error;
 use crate::error::type_error;
@@ -443,22 +444,27 @@ fn op_serialize(
   if let Some(transferred_array_buffers) = transferred_array_buffers {
     let state_rc = JsRuntime::state(scope);
     let state = state_rc.borrow_mut();
-    for i in 0..transferred_array_buffers.length() {
-      let i = v8::Number::new(scope, i as f64).into();
+    for index in 0..transferred_array_buffers.length() {
+      let i = v8::Number::new(scope, index as f64).into();
       let buf = transferred_array_buffers.get(scope, i).unwrap();
       let buf = v8::Local::<v8::ArrayBuffer>::try_from(buf).map_err(|_| {
         type_error("item in transferredArrayBuffers not an ArrayBuffer")
       })?;
       if let Some(shared_array_buffer_store) = &state.shared_array_buffer_store
       {
-        // TODO(lucacasonato): we need to check here that the buffer is not
-        // already detached. We can not do that because V8 does not provide
-        // a way to check if a buffer is already detached.
         if !buf.is_detachable() {
           return Err(type_error(
             "item in transferredArrayBuffers is not transferable",
           ));
         }
+
+        if buf.was_detached() {
+          return Err(custom_error(
+            "DOMExceptionOperationError",
+            format!("ArrayBuffer at index {} is already detached", index),
+          ));
+        }
+
         let backing_store = buf.get_backing_store();
         buf.detach();
         let id = shared_array_buffer_store.insert(backing_store);

ext/web/06_streams.js

@@ -193,7 +193,13 @@
    * @returns {boolean}
    */
   function isDetachedBuffer(O) {
-    return ReflectHas(O, isFakeDetached);
+    if (O.byteLength !== 0) {
+      return false;
+    }
+    // TODO(marcosc90) remove isFakeDetached once transferArrayBuffer
+    // actually detaches the buffer
+    return ReflectHas(O, isFakeDetached) ||
+      core.ops.op_arraybuffer_was_detached(O);
   }
 
   /**

ext/web/13_message_port.js

@@ -26,9 +26,6 @@
     SymbolFor,
     SymbolIterator,
     TypeError,
-    WeakSet,
-    WeakSetPrototypeAdd,
-    WeakSetPrototypeHas,
   } = window.__bootstrap.primordials;
 
   class MessageChannel {
@@ -239,8 +236,6 @@
     return [data, transferables];
   }
 
-  const detachedArrayBuffers = new WeakSet();
-
   /**
    * @param {any} data
    * @param {object[]} transferables
@@ -252,19 +247,6 @@
       (a) => ObjectPrototypeIsPrototypeOf(ArrayBufferPrototype, a),
     );
 
-    for (const arrayBuffer of transferredArrayBuffers) {
-      // This is hacky with both false positives and false negatives for
-      // detecting detached array buffers. V8  needs to add a way to tell if a
-      // buffer is detached or not.
-      if (WeakSetPrototypeHas(detachedArrayBuffers, arrayBuffer)) {
-        throw new DOMException(
-          "Can not transfer detached ArrayBuffer",
-          "DataCloneError",
-        );
-      }
-      WeakSetPrototypeAdd(detachedArrayBuffers, arrayBuffer);
-    }
-
     const serializedData = core.serialize(data, {
       hostObjects: ArrayPrototypeFilter(
         transferables,

ext/web/lib.rs

@@ -111,6 +111,7 @@ pub fn init<P: TimersPermission + 'static>(
       op_timer_handle::decl(),
       op_cancel_handle::decl(),
       op_sleep::decl(),
+      op_arraybuffer_was_detached::decl(),
     ])
     .state(move |state| {
       state.put(blob_store.clone());
@@ -337,6 +338,15 @@ fn op_encoding_encode_into(
   Ok(())
 }
 
+#[op(v8)]
+fn op_arraybuffer_was_detached<'a>(
+  _scope: &mut v8::HandleScope<'a>,
+  input: serde_v8::Value<'a>,
+) -> Result<bool, AnyError> {
+  let ab = v8::Local::<v8::ArrayBuffer>::try_from(input.v8_value)?;
+  Ok(ab.was_detached())
+}
+
 /// Creates a [`CancelHandle`] resource that can be used to cancel invocations of certain ops.
 #[op]
 pub fn op_cancel_handle(state: &mut OpState) -> ResourceId {