Workers: Transferable Objects from workers have zero length

RESOLVED FIXED in mozilla27

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: d1plo1d, Assigned: sfink)

Tracking

({testcase})

18 Branch
mozilla27
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

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.
Component: Untriaged → DOM: Workers
Product: Firefox → Core
Could you post a minimal testcase, please.
Flags: needinfo?(d1plo1d)
Keywords: testcase-wanted
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.
Flags: needinfo?(d1plo1d)
Assignee: nobody → amarchesini
I can reproduce it. I take this bug.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Posted patch patch (obsolete) — Splinter Review
Attachment #715488 - Flags: review?(sphink)
Posted patch patchSplinter Review
a better approach.
Attachment #715488 - Attachment is obsolete: true
Attachment #715488 - Flags: review?(sphink)
Attachment #715557 - Flags: review?(sphink)
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!
Attachment #715557 - Flags: review?(sphink)
Posted patch mochitestSplinter Review
Here just the mochitest
Attachment #718984 - Flags: review?(sphink)
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.
Attachment #718984 - Flags: review?(sphink) → review+
Assignee: amarchesini → sphink
Depends on: 789593
Whoops, I should have landed this too. Will do.
Actually, I won't yet, because the combination of buffer-preserving structured clone and transferables doesn't work. As this test shows nicely.
(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?
Depends on: 861925
(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.
https://hg.mozilla.org/mozilla-central/rev/b87bbbb0c612
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.