New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 676063 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit 15 days ago
Closed: Jan 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Disallow SharedArrayBuffer in postMessage transfer list

Project Member Reported by binji@chromium.org, Dec 20 2016

Issue description

Posting a SharedArrayBuffer to a Worker is currently required to be in the transfer list, but it should not be: see https://github.com/tc39/ecmascript_sharedmem/issues/98

i.e.

This should throw a DataCloneError:

var sab = new SharedArrayBuffer(4);
worker.postMessage(sab, [sab]);

But this is OK;

worker.postMessage(sab);
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/717025d02aaeaf0d21a674b190eed30c91f11400

commit 717025d02aaeaf0d21a674b190eed30c91f11400
Author: binji <binji@chromium.org>
Date: Wed Dec 21 20:28:23 2016

Temporarily disable sharedarraybuffer worker test to land V8 API change

BUG= 676063 

Review-Url: https://codereview.chromium.org/2590003002
Cr-Commit-Position: refs/heads/master@{#440203}

[modify] https://crrev.com/717025d02aaeaf0d21a674b190eed30c91f11400/third_party/WebKit/LayoutTests/TestExpectations

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/534ddf645be293800ab0bd7107b307f668ac8d5b

commit 534ddf645be293800ab0bd7107b307f668ac8d5b
Author: binji <binji@chromium.org>
Date: Tue Jan 03 22:52:17 2017

Disallow passing a SharedArrayBuffer in the transfer list.

This behavior changed recently. SharedArrayBuffers should not be put in the
transfer list, because they are not detached, and that is the meaning of being
in the transfer list.

This is the V8 side of the change, the Blink side will come next.

Reland of https://codereview.chromium.org/2570433005, it was reverted because
of a Blink-side test failure which has been temporarily disabled; see
https://codereview.chromium.org/2590003002.

BUG=https://bugs.chromium.org/p/chromium/issues/detail?id=676063

Review-Url: https://codereview.chromium.org/2594793005
Cr-Commit-Position: refs/heads/master@{#42054}

[modify] https://crrev.com/534ddf645be293800ab0bd7107b307f668ac8d5b/include/v8.h
[modify] https://crrev.com/534ddf645be293800ab0bd7107b307f668ac8d5b/src/api.cc
[modify] https://crrev.com/534ddf645be293800ab0bd7107b307f668ac8d5b/src/messages.h
[modify] https://crrev.com/534ddf645be293800ab0bd7107b307f668ac8d5b/src/value-serializer.cc
[modify] https://crrev.com/534ddf645be293800ab0bd7107b307f668ac8d5b/src/value-serializer.h
[modify] https://crrev.com/534ddf645be293800ab0bd7107b307f668ac8d5b/test/unittests/value-serializer-unittest.cc

Comment 3 by mr.a...@gmail.com, Jan 4 2017

I'm not sure if this is related or a separate bug, but Chromium 57 throws on both:

worker.postMessage(sab, [sab]) // throws
worker.postMessage(sab) // throws the same error

The error that is thrown is: "#<SharedArrayBuffer> could not be cloned."

LMK if I should open a separate bug or how/if I can help with this
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4631f77040207227d3d732b60f4ece9752a014cf

commit 4631f77040207227d3d732b60f4ece9752a014cf
Author: mfomitchev <mfomitchev@chromium.org>
Date: Wed Jan 04 18:23:18 2017

Marking worker-sharedarraybuffer-transfer.html as crashing.

TBR=binji
BUG= 676063 

Review-Url: https://codereview.chromium.org/2612803002
Cr-Commit-Position: refs/heads/master@{#441412}

[modify] https://crrev.com/4631f77040207227d3d732b60f4ece9752a014cf/third_party/WebKit/LayoutTests/TestExpectations

Comment 5 by binji@chromium.org, Jan 4 2017

#3: yes, this is temporarily broken as I need to land the Blink change; I figured it would be OK, considering SAB is still behind a flag. :)

The Blink change should be along soon (no link yet, however).
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 7 2017

Comment 7 by mr.a...@gmail.com, Jan 12 2017

Ah thanks! I agree that the break isn't too bad behind a flag :)

Looking forward to the fix.

Cheers!

Comment 8 by binji@chromium.org, Jan 13 2017

Status: Fixed (was: Assigned)

Sign in to add a comment