From 5d814a4c244d489b4ae51002a0cf1d3c2fe16058 Mon Sep 17 00:00:00 2001 From: Luca Casonato Date: Wed, 25 Aug 2021 13:48:53 +0200 Subject: [PATCH] feat: ArrayBuffer in structured clone transfer (#11840) --- core/bindings.rs | 112 +++++++++++++++++++++++++++++++++++-- ext/console/02_console.js | 21 ++++++- ext/web/13_message_port.js | 73 +++++++++++++++++++++--- ext/web/internal.d.ts | 3 + ext/web/message_port.rs | 16 ++++-- runtime/js/11_workers.js | 6 +- runtime/js/99_main.js | 4 +- tools/wpt/expectation.json | 5 +- 8 files changed, 213 insertions(+), 27 deletions(-) diff --git a/core/bindings.rs b/core/bindings.rs index 9bfe767c3b..58de6a38ac 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -813,8 +813,10 @@ fn serialize( } }; - let options = - options.unwrap_or(SerializeDeserializeOptions { host_objects: None }); + let options = options.unwrap_or(SerializeDeserializeOptions { + host_objects: None, + transfered_array_buffers: None, + }); let host_objects = match options.host_objects { Some(value) => match v8::Local::::try_from(value.v8_value) { @@ -827,10 +829,61 @@ fn serialize( None => None, }; + let transfered_array_buffers = match options.transfered_array_buffers { + Some(value) => match v8::Local::::try_from(value.v8_value) { + Ok(transfered_array_buffers) => Some(transfered_array_buffers), + Err(_) => { + throw_type_error(scope, "transfered_array_buffers not an array"); + return; + } + }, + None => None, + }; + let serialize_deserialize = Box::new(SerializeDeserialize { host_objects }); let mut value_serializer = v8::ValueSerializer::new(scope, serialize_deserialize); + value_serializer.write_header(); + + if let Some(transfered_array_buffers) = transfered_array_buffers { + let state_rc = JsRuntime::state(scope); + let state = state_rc.borrow_mut(); + for i in 0..transfered_array_buffers.length() { + let i = v8::Number::new(scope, i as f64).into(); + let buf = transfered_array_buffers.get(scope, i).unwrap(); + let buf = match v8::Local::::try_from(buf) { + Ok(buf) => buf, + Err(_) => { + throw_type_error( + scope, + "item in transfered_array_buffers not an ArrayBuffer", + ); + return; + } + }; + 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() { + throw_type_error( + scope, + "item in transfered_array_buffers is not transferable", + ); + return; + } + let backing_store = buf.get_backing_store(); + buf.detach(); + let id = shared_array_buffer_store.insert(backing_store); + value_serializer.transfer_array_buffer(id, buf); + let id = v8::Number::new(scope, id as f64).into(); + transfered_array_buffers.set(scope, i, id); + } + } + } + match value_serializer.write_value(scope.get_current_context(), value) { Some(true) => { let vector = value_serializer.release(); @@ -843,10 +896,11 @@ fn serialize( } } -#[derive(Serialize, Deserialize)] +#[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct SerializeDeserializeOptions<'a> { host_objects: Option>, + transfered_array_buffers: Option>, } fn deserialize( @@ -871,8 +925,10 @@ fn deserialize( } }; - let options = - options.unwrap_or(SerializeDeserializeOptions { host_objects: None }); + let options = options.unwrap_or(SerializeDeserializeOptions { + host_objects: None, + transfered_array_buffers: None, + }); let host_objects = match options.host_objects { Some(value) => match v8::Local::::try_from(value.v8_value) { @@ -885,9 +941,21 @@ fn deserialize( None => None, }; + let transfered_array_buffers = match options.transfered_array_buffers { + Some(value) => match v8::Local::::try_from(value.v8_value) { + Ok(transfered_array_buffers) => Some(transfered_array_buffers), + Err(_) => { + throw_type_error(scope, "transfered_array_buffers not an array"); + return; + } + }, + None => None, + }; + let serialize_deserialize = Box::new(SerializeDeserialize { host_objects }); let mut value_deserializer = v8::ValueDeserializer::new(scope, serialize_deserialize, &zero_copy); + let parsed_header = value_deserializer .read_header(scope.get_current_context()) .unwrap_or_default(); @@ -897,6 +965,40 @@ fn deserialize( scope.throw_exception(exception); return; } + + if let Some(transfered_array_buffers) = transfered_array_buffers { + let state_rc = JsRuntime::state(scope); + let state = state_rc.borrow_mut(); + if let Some(shared_array_buffer_store) = &state.shared_array_buffer_store { + for i in 0..transfered_array_buffers.length() { + let i = v8::Number::new(scope, i as f64).into(); + let id_val = transfered_array_buffers.get(scope, i).unwrap(); + let id = match id_val.number_value(scope) { + Some(id) => id as u32, + None => { + throw_type_error( + scope, + "item in transfered_array_buffers not number", + ); + return; + } + }; + if let Some(backing_store) = shared_array_buffer_store.take(id) { + let array_buffer = + v8::ArrayBuffer::with_backing_store(scope, &backing_store); + value_deserializer.transfer_array_buffer(id, array_buffer); + transfered_array_buffers.set(scope, id_val, array_buffer.into()); + } else { + throw_type_error( + scope, + "transfered array buffer not present in shared_array_buffer_store", + ); + return; + } + } + } + } + let value = value_deserializer.read_value(scope.get_current_context()); match value { diff --git a/ext/console/02_console.js b/ext/console/02_console.js index 178b3955bf..455df45707 100644 --- a/ext/console/02_console.js +++ b/ext/console/02_console.js @@ -362,6 +362,7 @@ const entries = []; let iter; + let valueIsTypedArray = false; switch (options.typeName) { case "Map": @@ -376,6 +377,7 @@ default: if (isTypedArray(value)) { iter = ArrayPrototypeEntries(value); + valueIsTypedArray = true; } else { throw new TypeError("unreachable"); } @@ -385,7 +387,24 @@ const next = () => { return iter.next(); }; - for (const el of iter) { + while (true) { + let el; + try { + const res = iter.next(); + if (res.done) { + break; + } + el = res.value; + } catch (err) { + if (valueIsTypedArray) { + // TypedArray.prototype.entries doesn't throw, unless the ArrayBuffer + // is detached. We don't want to show the exception in that case, so + // we catch it here and pretend the ArrayBuffer has no entries (like + // Chrome DevTools does). + break; + } + throw err; + } if (entriesLength < inspectOptions.iterableLimit) { ArrayPrototypePush( entries, diff --git a/ext/web/13_message_port.js b/ext/web/13_message_port.js index d0d9e160e6..2768c0a926 100644 --- a/ext/web/13_message_port.js +++ b/ext/web/13_message_port.js @@ -15,6 +15,8 @@ const { defineEventHandler } = window.__bootstrap.event; const { DOMException } = window.__bootstrap.domException; const { + ArrayBuffer, + ArrayPrototypeFilter, ArrayPrototypeIncludes, ArrayPrototypePush, ObjectSetPrototypeOf, @@ -22,6 +24,9 @@ SymbolFor, SymbolToStringTag, TypeError, + WeakSet, + WeakSetPrototypeAdd, + WeakSetPrototypeHas, } = window.__bootstrap.primordials; class MessageChannel { @@ -139,11 +144,11 @@ this[_id], ); if (data === null) break; - let message, transfer; + let message, transferables; try { const v = deserializeJsMessageData(data); message = v[0]; - transfer = v[1]; + transferables = v[1]; } catch (err) { const event = new MessageEvent("messageerror", { data: err }); this.dispatchEvent(event); @@ -151,7 +156,10 @@ } const event = new MessageEvent("message", { data: message, - ports: transfer, + ports: ArrayPrototypeFilter( + transferables, + (t) => t instanceof MessagePort, + ), }); this.dispatchEvent(event); } @@ -193,12 +201,22 @@ function deserializeJsMessageData(messageData) { /** @type {object[]} */ const transferables = []; + const hostObjects = []; + const arrayBufferIdsInTransferables = []; + const transferedArrayBuffers = []; for (const transferable of messageData.transferables) { switch (transferable.kind) { case "messagePort": { const port = createMessagePort(transferable.data); ArrayPrototypePush(transferables, port); + ArrayPrototypePush(hostObjects, port); + break; + } + case "arrayBuffer": { + ArrayPrototypePush(transferedArrayBuffers, transferable.data); + const i = ArrayPrototypePush(transferables, null); + ArrayPrototypePush(arrayBufferIdsInTransferables, i); break; } default: @@ -207,21 +225,53 @@ } const data = core.deserialize(messageData.data, { - hostObjects: transferables, + hostObjects, + transferedArrayBuffers, }); + for (const i in arrayBufferIdsInTransferables) { + const id = arrayBufferIdsInTransferables[i]; + transferables[id] = transferedArrayBuffers[i]; + } + return [data, transferables]; } + const detachedArrayBuffers = new WeakSet(); + /** * @param {any} data - * @param {object[]} tranferables + * @param {object[]} transferables * @returns {globalThis.__bootstrap.messagePort.MessageData} */ - function serializeJsMessageData(data, tranferables) { + function serializeJsMessageData(data, transferables) { + const transferedArrayBuffers = ArrayPrototypeFilter( + transferables, + (a) => a instanceof ArrayBuffer, + ); + + for (const arrayBuffer of transferedArrayBuffers) { + // 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); + } + let serializedData; try { - serializedData = core.serialize(data, { hostObjects: tranferables }); + serializedData = core.serialize(data, { + hostObjects: ArrayPrototypeFilter( + transferables, + (a) => a instanceof MessagePort, + ), + transferedArrayBuffers, + }); } catch (err) { throw new DOMException(err.message, "DataCloneError"); } @@ -229,7 +279,8 @@ /** @type {globalThis.__bootstrap.messagePort.Transferable[]} */ const serializedTransferables = []; - for (const transferable of tranferables) { + let arrayBufferI = 0; + for (const transferable of transferables) { if (transferable instanceof MessagePort) { webidl.assertBranded(transferable, MessagePort); const id = transferable[_id]; @@ -244,6 +295,12 @@ kind: "messagePort", data: id, }); + } else if (transferable instanceof ArrayBuffer) { + ArrayPrototypePush(serializedTransferables, { + kind: "arrayBuffer", + data: transferedArrayBuffers[arrayBufferI], + }); + arrayBufferI++; } else { throw new DOMException("Value not transferable", "DataCloneError"); } diff --git a/ext/web/internal.d.ts b/ext/web/internal.d.ts index 3a2a0c1be1..6e72c1a80c 100644 --- a/ext/web/internal.d.ts +++ b/ext/web/internal.d.ts @@ -88,6 +88,9 @@ declare namespace globalThis { declare type Transferable = { kind: "messagePort"; data: number; + } | { + kind: "arrayBuffer"; + data: number; }; declare interface MessageData { data: Uint8Array; diff --git a/ext/web/message_port.rs b/ext/web/message_port.rs index 4af99d5014..d8d8e5907d 100644 --- a/ext/web/message_port.rs +++ b/ext/web/message_port.rs @@ -16,6 +16,7 @@ use tokio::sync::mpsc::UnboundedSender; enum Transferable { MessagePort(MessagePort), + ArrayBuffer(u32), } type MessagePortMessage = (Vec, Vec); @@ -127,6 +128,7 @@ pub fn op_message_port_create_entangled( pub enum JsTransferable { #[serde(rename_all = "camelCase")] MessagePort(ResourceId), + ArrayBuffer(u32), } fn deserialize_js_transferables( @@ -146,6 +148,9 @@ fn deserialize_js_transferables( .map_err(|_| type_error("Message port is not ready for transfer"))?; transferables.push(Transferable::MessagePort(resource.port)); } + JsTransferable::ArrayBuffer(id) => { + transferables.push(Transferable::ArrayBuffer(id)); + } } } Ok(transferables) @@ -165,6 +170,9 @@ fn serialize_transferables( }); js_transferables.push(JsTransferable::MessagePort(rid)); } + Transferable::ArrayBuffer(id) => { + js_transferables.push(JsTransferable::ArrayBuffer(id)); + } } } js_transferables @@ -182,11 +190,9 @@ pub fn op_message_port_post_message( data: JsMessageData, ) -> Result<(), AnyError> { for js_transferable in &data.transferables { - match js_transferable { - JsTransferable::MessagePort(id) => { - if *id == rid { - return Err(type_error("Can not transfer self message port")); - } + if let JsTransferable::MessagePort(id) = js_transferable { + if *id == rid { + return Err(type_error("Can not transfer self message port")); } } } diff --git a/runtime/js/11_workers.js b/runtime/js/11_workers.js index 2f94131199..f01dc3d5f7 100644 --- a/runtime/js/11_workers.js +++ b/runtime/js/11_workers.js @@ -284,11 +284,11 @@ while (!this.terminated) { const data = await hostRecvMessage(this.#id); if (data === null) break; - let message, transfer; + let message, transferables; try { const v = deserializeJsMessageData(data); message = v[0]; - transfer = v[1]; + transferables = v[1]; } catch (err) { const event = new MessageEvent("messageerror", { cancelable: false, @@ -300,7 +300,7 @@ const event = new MessageEvent("message", { cancelable: false, data: message, - ports: transfer, + ports: transferables.filter((t) => t instanceof MessagePort), }); this.dispatchEvent(event); } diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js index af6309338c..fc3eeeccae 100644 --- a/runtime/js/99_main.js +++ b/runtime/js/99_main.js @@ -135,12 +135,12 @@ delete Object.prototype.__proto__; if (data === null) break; const v = deserializeJsMessageData(data); const message = v[0]; - const transfer = v[1]; + const transferables = v[1]; const msgEvent = new MessageEvent("message", { cancelable: false, data: message, - ports: transfer, + ports: transferables.filter((t) => t instanceof MessagePort), }); try { diff --git a/tools/wpt/expectation.json b/tools/wpt/expectation.json index 62434935a1..dd607e608f 100644 --- a/tools/wpt/expectation.json +++ b/tools/wpt/expectation.json @@ -16195,8 +16195,7 @@ "Blob paired surrogates (invalid utf-8)", "Blob empty", "Blob NUL", - "File basic", - "ArrayBuffer" + "File basic" ] } } @@ -16252,7 +16251,7 @@ "message-channels": { "basics.any.html": true, "close.any.html": true, - "dictionary-transferrable.any.html": false, + "dictionary-transferrable.any.html": true, "implied-start.any.html": true, "no-start.any.html": true, "user-activation.tentative.any.html": false,