-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
postMessage, WebAssembly.Memory, and undetachable ArrayBuffers #4601
Comments
Thanks for the detailed issue report! I'm guessing some of the folks CCed are the ones who would be involved in determining for Chrome whether or not to adopt the Firefox behavior. Do you personally believe the Firefox behavior is better? Also, do you know who to ping from Safari?
If the fix is indeed this simple, I think we should also add a note explaining why it might throw, as this is pretty subtle :). |
I prefer the Firefox behavior, but Chrome has been shipping its copy-instead-of-transfer behavior for long enough that I'd want to do some investigation before changing it in a stable release. @kmiller68 is the main Safari person who I've been interacting with around wasm stuff. |
Yeah, the current Chrome behavior is possible to implement (by copying yourself), and is also surprising. But it also seems like the kind of thing that websites could be relying on. Do we have any UMA stats for when this happens? If not, can we add some? |
After some digging in the code, I think this behavior goes all the way back to 2014 (before WebAssembly, this behavior was present for asm.js), and isn't currently UMA-counted. We can add a counter, but given how long we've had this behavior in the wild it may be hard to change. |
How many ways can you create a undetachable ArrayBuffer? (just wasm and asm.js?) |
I'm actually pretty fuzzy on the asm.js case: it seems from code inspection that at that time, in V8, it was true of asm.js memories, but those are no longer special. |
Thanks for filing this issue. The Firefox semantics sound ideal to me, if they are web-compatible. This was an unintentional omission of mine when formalizing the WebAssembly/JS interface; I was misunderstanding that the serialize algorithm would handle this. |
Safari also throws a TypeError if you try to transfer a WebAssembly.Memory generated ArrayBuffer. I agree with others that throwing seems like the ideal semantics. |
In particular, DetachArrayBuffer() can throw for the ArrayBuffer used as the backing memory in the WebAssembly JS API. Fixes #4601.
In particular, DetachArrayBuffer() can throw for the ArrayBuffer used as the backing memory in the WebAssembly JS API. This matches the current behavior in Firefox and Safari. Fixes #4601.
Note, I added this throwing behavior in tc39/ecma262#1112 + WebAssembly/spec#712 . We discussed it in the March 2018 TC39 meeting. The only real oversight seems to be |
In particular, DetachArrayBuffer() can throw for the ArrayBuffer used as the backing memory in the WebAssembly JS API. This matches the current behavior in Firefox and Safari. Fixes #4601.
In particular, DetachArrayBuffer() can throw for the ArrayBuffer used as the backing memory in the WebAssembly JS API. This matches the current behavior in Firefox and Safari. Fixes #4601.
I filed whatwg/webidl#724 and WebAudio/web-audio-api#1861 as follow-ups. |
Also whatwg/streams#1001 |
There's a difference in behavior between Firefox and Chrome for the following code:
Firefox throws a TypeError, while Chrome happily "transfers" the buffer. Except it doesn't transfer, but rather is silently copied. This is because the ArrayBuffer objects returned from the
buffer
getter are not allowed to be detached, except when the WebAssembly.Memory is grown (there are more details here, of course, but I don't think they're relevant to HTML).It seems that neither of these behaviors is specified, e.g., there's no check about "non-detachability" or "wasm memory association" in the postMessage transfer algorithm.
The infrastructure for handling it is already available: when WebAssembly [creates a WebAssembly.Memory](https://webassembly.github.io/spec/js-api/index.html#create-a-memory-buffer] object, it sets
[[ArrayBufferDetachKey]]
to"WebAssembly.Memory"
. And ECMAScript allows passing a key to DetachArrayBuffer.So, having looked at all that, I think this may be a trivial fix: since HTML does not pass a
key
argument to DetachArrayBuffer, it defaults toundefined
. And therefore step 4 of DetachArrayBuffer will throw a TypeError, sinceSameValue("WebAssembly.Memory", undefined)
is false.Therefore, I think the fix is to replace
!
with?
preceding the call to DetachArrayBuffer in step 5.4.4 of the transfer algorithm (2.8.7), and DetachArrayBuffer itself will do the right thing.All of this is assuming the Firefox behavior is what we want, of course.
@Ms2ger @littledan @domenic @dtig @binji
The text was updated successfully, but these errors were encountered: