841904 - WebWorker/xferables: ArrayBuffer of a typed array cannot be transfered.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Janne Kontkanen]

Attachment: Simple example reproducing the issue.

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_2) AppleWebKit/537.17 (KHTML, like Gecko) Chrome/24.0.1312.57 Safari/537.17

Steps to reproduce:

I sent a Uint8Array from a WebWorker to the main thread using postMessage() and transferables. The postMessage() call in the WebWorker looked like this:

self.postMessage(typedArray, [typedArray.buffer]);




Actual results:

The main thread received undefined values.


Expected results:

The main thread should receive the typedArray and the ownership of the underlying ArrayBuffer should be transfered.

It seems that in Firefox you can only transfer typed arrays if you both send them as ArrayBuffers and transfer them as ArrayBuffers, i.e.:

self.postMessage(typedArray.buffer, [typedArray.buffer]);

As far as I understand, HTML5 specification allows both methods, so I think this is a bug in Firefox. Both methods work in Chrome 24.

Comment 1

[Kenneth Russell]

bjacob, jgilbert: there is definitely a bug here in the postMessaging of typed array views when the underlying ArrayBuffer is being transferred. Could you please help categorize and assign this bug? Thanks.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Typed Arrays bugs most likely fall into Core:JavaScript Engine. Also CC'ing a few people.

Comment 3

[Steve Fink [:sfink] [:s:]]

Sorry, I should have filed this bug long ago. A handful of people have hit it, and each time I've asked how important it is to fix, and so far everyone has said it's no big deal. It's really not that hard to fix, though.

It isn't exactly a dupe of bug 789593, but fixing that bug should fix it.

By the way, the behavior is even worse than described. If you serialize typedarray while transferring typedarray.buffer, it will end up neutering the source typedarray as well, and both the sender and receiver will receive an empty array. The original data is completely lost.

Comment 4

[Kenneth Russell]

Please give this bug priority. Transferring of arbitrary object graphs is broken, and real world applications have tripped across this bug. A WebGL conformance test has just been added for this: http://www.khronos.org/registry/webgl/sdk/tests/conformance/typedarrays/typed-arrays-in-workers.html -- and Firefox won't be able to pass these conformance tests until this bug is fixed.

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

Steve: just chatted with Vlad and we were surprised that this would have been considered a low priority; we weren't aware that it had been; please prioritize. How much time do you think it would take to fix this? The 1.0.2 WebGL conformance tests will be ratified in a month from now, we would really like at least Nightly to pass them then.

Comment 6

[Steve Fink [:sfink] [:s:]]

baku has a fix in bug 842081, but it narrowly fixes this issue; typed array cloning still has bugs with respect to the spec after that patch. I'm working on the full fix right now. I hope to finish today.

I believe people have considered this (relatively) low priority because ArrayBuffer cloning works, and proper typed array cloning doesn't introduce any new functionality, so it's not hard to workaround the problem (ie, transfer the buffer and construct whatever typed arrays you want on the recipient side.) If it's only for spec compliance, we have at least one bigger problem (ArrayBufferView doesn't exist at all.)

Comment 7

[Janne Kontkanen]

While the workaround is certainly always possible, it can get somewhat tedious if the code has not been designed with the workaround in mind. This is the case especially if the existing code sends larger nested objects with typed arrays as members from workers to the main thread and these objects are directly used all over the place in the main thread.

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

Steve: if you think that there is a large risk that this would not be fixed in roughly 5-6 weeks from now, then we really need to know, so we can ask for more time to ratify WebGL 1.0.2 tests. But we would really like this to get fixed instead.

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

All sounds great!  One question though..

(In reply to Steve Fink [:sfink] from comment #6)
> If it's only for spec compliance, we have at least one
> bigger problem (ArrayBufferView doesn't exist at all.)

Just as a type/for instanceof purposes?  I agree it would be handy to have this (especially for instanceof).  We should file a separate bug for that.

Comment 10

[Janne Kontkanen]

Out of curiosity -- is this issue being worked on?

Comment 11

[Till Schneidereit [:till]]

I just hit this in Shumway. While I can work around it, re-creating the correct typed array types after the transfer is pretty annoying, as I have to store that information in the message. While Shumway is somewhat unique in its requirements, I can't imagine that this issue falls into that area.

Also, it works most, but not all of the time, or so it seems. I can only reproduce the issue in our testing framework, not when running content in our other embeddings.

Steve, was fixing bug 789593 assumed to fix this completely?

Comment 12

[Steve Fink [:sfink] [:s:]]

(In reply to Till Schneidereit [:till] from comment #11)
> I just hit this in Shumway. While I can work around it, re-creating the
> correct typed array types after the transfer is pretty annoying, as I have
> to store that information in the message. While Shumway is somewhat unique
> in its requirements, I can't imagine that this issue falls into that area.
> 
> Also, it works most, but not all of the time, or so it seems. I can only
> reproduce the issue in our testing framework, not when running content in
> our other embeddings.
> 
> Steve, was fixing bug 789593 assumed to fix this completely?

No, but bug 861925 was. Is this still broken?

Comment 13

[Till Schneidereit [:till]]

(In reply to Steve Fink [:sfink] from comment #12)
> No, but bug 861925 was. Is this still broken?

Urgh, no. What really happened is that the test harness ran in Nightly, whereas I'm using the UX build. The harness' runtime isn't long enough to for an update to ever be downloaded successfully, I guess. All in all, it ran in a late-September build. Sorry for the noise.