842081 - Workers: Transferable Objects from workers have zero length

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Rob Gilson]

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

Steps to reproduce:

Using Firefox 18.0.2:
1. Create a web worker from a blob containing js which performs steps 2 and 3
2. In the worker create a Float32Array with a non-zero size
3. In the worker post the Float32Array as a transferable object
4. Check the length of the array in the main thread (outside of the worker) using worker.onmessage.


Actual results:

The length of the transfered Float32Array received in the worker.onmessage callback is zero and no error is raised.


Expected results:

The transfered Float32Array should have it's correct size and contents transfered back from the worker. Specifically, and for testing purposes, when it is received in the main thread via worker.onmessage it should have a non-zero length consistent with the length it was initialized with in the web worker.

This is consistent with what happens in Chrome 24.0.1312.57.

Comment 1

[Loic]

Could you post a minimal testcase, please.

Comment 2

[Rob Gilson]

Sure, I've posted a simple test for this bug on jsfiddle: http://jsfiddle.net/WErgh/3/

PS. I'm not sure if this testcase is minimal: I don't know if the blob creation from a closure trick is necessary to recreate the bug or if this will work with any webworker. In any case it demonstrates the inconsistent results across chrome and firefox.

Comment 3

[Andrea Marchesini [:baku]]

I can reproduce it. I take this bug.

Comment 4

[Andrea Marchesini [:baku]]

Attachment: patch

Comment 5

[Andrea Marchesini [:baku]]

Attachment: patch

a better approach.

Comment 6

[Andrea Marchesini [:baku]]

https://tbpl.mozilla.org/?tree=Try&rev=23245101e2a0

Comment 7

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

Comment on attachment 715557 [details] [diff] [review]
patch

Review of attachment 715557 [details] [diff] [review]:
-----------------------------------------------------------------

(Already discussed over IRC, but recording it here.) This is a good spot fix for this problem, but the underlying issue is bug 789593 -- typed arrays are not cloned in the way described in the spec. At the time structured clone was implemented, I don't think the spec was even written, so it cloned each view into a separate view/buffer pair with the same data. This really doesn't make sense in a Transferable world, and isn't what you want even without transferrables.

But this patch motivated me to implement the fix for bug 789593, which is now pending review. I added JS shell tests but not the sort of test you have here. I think we want your test in addition. Can you extract the test and attach it to bug 789593, then dupe this bug to it? Can happen anytime before or after bug 789593 lands. (jorendorff's review queue is pretty long, so it may take a little while.)

Thanks!

Comment 8

[Andrea Marchesini [:baku]]

Attachment: mochitest

Here just the mochitest

Comment 9

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

Comment on attachment 718984 [details] [diff] [review]
mochitest

Review of attachment 718984 [details] [diff] [review]:
-----------------------------------------------------------------

This test twists my brain in a knot, but it seems valid. I don't know what that sync_worker stuff was, but I won't worry about it.

Comment 10

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

Whoops, I should have landed this too. Will do.

Comment 11

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

Actually, I won't yet, because the combination of buffer-preserving structured clone and transferables doesn't work. As this test shows nicely.

Comment 12

[Andrea Marchesini [:baku]]

(In reply to Steve Fink [:sfink] from comment #11)
> Actually, I won't yet, because the combination of buffer-preserving
> structured clone and transferables doesn't work. As this test shows nicely.

Are you working on this? Can you file a bug if it doesn't exist yet?

Comment 13

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

(In reply to Andrea Marchesini (:baku) from comment #12)
> 
> Are you working on this? Can you file a bug if it doesn't exist yet?

Yes, I am. Sorry; bug 861925 is now filed.

Comment 14

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/e7d649603075

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Steve Fink [:sfink] from comment #14)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/e7d649603075

Backed out because bug 861925 was backed out and this then broke tests https://hg.mozilla.org/integration/mozilla-inbound/rev/262daca03163

Comment 16

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/b87bbbb0c612

Comment 17

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/b87bbbb0c612