Skip to content
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

SharedArrayBuffer and postMessage() #4732

Closed
annevk opened this issue Jun 25, 2019 · 35 comments · Fixed by #4734
Closed

SharedArrayBuffer and postMessage() #4732

annevk opened this issue Jun 25, 2019 · 35 comments · Fixed by #4734
Labels
topic: cross-origin-embedder-policy Issues and ideas around the new "require CORP for subresource requests and frames and etc" proposal topic: cross-origin-opener-policy Issues and ideas around the new "inverse of rel=noopener" header topic: serialize and transfer

Comments

@annevk
Copy link
Member

annevk commented Jun 25, 2019

As per tc39/ecma262#1435 and WebAssembly/threads#124 the plan is that SharedArrayBuffer is enabled by default, as defined by JavaScript, but its high-resolution timer capabilities are restricted via restricting postMessage(). (Other APIs that can enable high-resolution timer capabilities with SharedArrayBuffer independently of postMessage() would have to be similarly restricted. No such APIs as of yet, hopefully [AllowShared] will allow us to track these.)

To make postMessage() do the right thing, both Cross-Origin-Opener-Policy (COOP; #3740) and Cross-Origin-Embedder-Policy (COEP; #4175) have to be set for the document (or shared/service worker).

I realized we did not have a tracking issue for that specific change, so here it is.

(For clarity, without postMessage() "Shared" in SharedArrayBuffer is meaningless and it's effectively equivalent to ArrayBuffer.)

cc @whatwg/security @rniwa @csreis

@annevk annevk added topic: serialize and transfer topic: cross-origin-opener-policy Issues and ideas around the new "inverse of rel=noopener" header topic: cross-origin-embedder-policy Issues and ideas around the new "require CORP for subresource requests and frames and etc" proposal labels Jun 25, 2019
annevk added a commit that referenced this issue Jun 25, 2019
This depends on the work to add Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy. It specifies how postMessage() is affected when both those headers are set for the agent clusters they impact.

Closes #4732.
annevk added a commit that referenced this issue Aug 14, 2019
This depends on the work to add Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy. It specifies how postMessage() is affected when both those headers are set for the agent clusters they impact.

Closes #4732.
annevk added a commit that referenced this issue Oct 28, 2019
This depends on the work to add Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy. It specifies how postMessage() is affected when both those headers are set for the agent clusters they impact.

Closes #4732.
@annevk
Copy link
Member Author

annevk commented Jan 2, 2020

Folding #4872 into this, we'll expose this COOP+COEP state in JavaScript as self.crossOriginIsolated.

Also, you can now play with this in Firefox Nightly: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer/Planned_changes.

annevk added a commit that referenced this issue Jan 2, 2020
This depends on the work to add Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy. It specifies how postMessage() is affected when both those headers are set for the agent clusters they impact.

Additionally, it exposes this state through self.crossOriginIsolated.

Tests: see links in #4732.

Closes #4732.
@mmis1000
Copy link

mmis1000 commented Jan 8, 2020

It looks COOP+COEP does not inherited to blob: uri script?

It seems now every dynamic created script can't use sharedArrayBuffer to share data on firefox nightly(74.0a1 (2020-01-06)).

The most worst thing, it break feature detection of SharedArrayBuffer.
Because the SharedArrayBuffer do exist, just you can't use it to pass data between worker.
The script detected it may try to use it and result in a crash.

If blocking this is actually desired, maybe we should hide the whole constructor instead so it won't break backward compatibility?

Some script break by current implementation i found:
https://github.com/qiaozi-tech/WXInlinePlayer/blob/1671bf6a50f88bcd935a87b98830319a3608b2b2/src/loader/stream.js#L52-L57

@annevk
Copy link
Member Author

annevk commented Jan 8, 2020

It looks COOP+COEP does not inherited to blob: uri script?

That's something we should fix (and specify).

The most worst thing, it break feature detection of SharedArrayBuffer.

This is by design to not "fragment" JavaScript. self.crossOriginIsolated is to be consulted instead. If this ends up breaking too many sites we'll have to reconsider of course.

@annevk
Copy link
Member Author

annevk commented Jan 14, 2020

@mmis1000 do you have an example of blob: URLs not working in Firefox? I created web-platform-tests/wpt#21146 (and various other tests) and thus far I cannot reproduce, except for blob: URL popups. I was expecting blob: URL dedicated workers to not work, but they do as far as I can tell. (It'd be even better if you could file a bug at https://bugzilla.mozilla.org/enter_bug.cgi?product=Core.)

@annevk
Copy link
Member Author

annevk commented Feb 7, 2020

@mmis1000 I'd really appreciate it if you could look into this one more time.

@mmgeorge
Copy link

mmgeorge commented Feb 19, 2020

This change to SharedArrayBuffer is a pretty major issue for us at Esri as we are currently using feature detection to determine whether or not use use SharedArrayBuffer or a workaround that involves copying over ArrayBuffers. This breaks our JS mapping API on FF nightly (features do not display), and we will have to backport a fix to all previous versions of our API with this issue, as well as to our enterprise deployments.

It seems like this change would essentially break every app that currently uses SharedArrayBuffer, hiding the constructor would have been a bit nicer here as at least it wouldn't be a breaking change.

Do we know for sure that this proposal is final? We ideally want to patch our API before this is live in FF (seems like this is currently in the next version?)

@annevk
Copy link
Member Author

annevk commented Feb 20, 2020

This is still the plan of action, yes. (And yes, Firefox 75 might have SharedArrayBuffer available, but no postMessage().) The reason for not hiding the constructor is that "we" don't want to put conditionals around "language features". Therefore the conditional is in serialization/deserialization.

@dasa
Copy link

dasa commented Feb 20, 2020

And yes, Firefox 75 might have SharedArrayBuffer available, but no postMessage().

This seems to be rolling out with Firefox 74 since it's already broken at: https://developers.arcgis.com/javascript/latest/sample-code/layers-featurelayer/live/index.html

There's no mention of this in either of these:
https://www.fxsitecompat.dev/en-CA/versions/74/
https://www.fxsitecompat.dev/en-CA/versions/75/

@bzbarsky
Copy link
Contributor

At the moment, the SharedArrayBuffer constructor is only enabled in Firefox in "early beta or earlier". See https://searchfox.org/mozilla-central/rev/5a10be606f2d76ef22f1f44565749490de991d35/modules/libpref/init/all.js#1206-1210

74 is still in early beta, so SharedArrayBuffer is enabled there at the moment, to allow testing. As 74 moves to late beta and release, SharedArrayBuffer will be disabled.

@mmgeorge
Copy link

mmgeorge commented Feb 20, 2020

The reason for not hiding the constructor is that "we" don't want to put conditionals around "language features". Therefore the conditional is in serialization/deserialization.

Hmm I guess I'm just kind of confused on this point. Without the serialization/deserialization, there's no use-case for SharedArrayBuffer. Whatever logic you have to workaround not having access to SharedArrayBuffer has to happen at construction time, so you still do need to wrap creating SharedArrayBuffers in a conditional. Effectively this replaces the typical feature detection with a different flag that needs to be checked instead.

This is further complicated by the fact that browsers already do support SharedArrayBuffer. This means that to check for whether or not you can use shared memory, we have to do both feature detection and check this new conditional. Basically:

function hasSharedMemory() {
  const hasSharedArrayBuffer = "SharedArrayBuffer" in global;
  const notCrossOriginIsolated = global.crossOriginIsolated === false; 

  return hasSharedArrayBuffer && !notCrossOriginIsolated;
}

Breaking feature detection in this way is also kind of scary for using new browser features going forward. I suppose one could argue that new JS standards should be treated skeptically until they are fully implemented by all browsers.

@annevk
Copy link
Member Author

annevk commented Feb 21, 2020

SharedArrayBuffer cannot be detached, which is a useful property. And it would allow some code patterns that do not require threaded usage, but can, not have to use different buffers. And yeah, this is a somewhat unusual case that doesn't generally apply to features.

@syg
Copy link
Contributor

syg commented Mar 11, 2020

Apologies for not engaging here earlier.

It's also V8's preference to gate the existence of the SharedArrayBuffer constructor on COOP+COEP, precisely for the reasons of not breaking existing feature detection in the wild.

@annevk I take the "don't make language features conditional" point, but pragmatism here trumps that IMO. I find the argument from @mmgeorge compelling: browsers that don't implement SharedArrayBuffer in practice can't be meaningfully distinguished from sites that don't opt into COOP+COEP, so I see no reason to make web devs write two kinds of feature detection code for SABs.

Could we change this?

Edit: To preempt Atomics not being gated on as well, I have plans to enable Atomics to be usable on TypedArrays backed by regular ArrayBuffers to align with wasm.

@annevk
Copy link
Member Author

annevk commented Mar 12, 2020

We haven't seen any breakage in the Firefox Nightly/Beta landscape thus far from having SharedArrayBuffer's constructor exposed and also have postMessage() throw outside of cross-origin isolated environments. (Emscripten doesn't do any meaningful feature detection either.)

And the reason to have SharedArrayBuffer available is because it can also be used without threads as a non-detachable ArrayBuffer and there are numerous web platform APIs that will work with it regardless of the environment being cross-origin isolated.

For a plan communicated well over a year ago and reiterated at various points this is also an extremely late request for change.

cc @lukewagner

@mmis1000
Copy link

mmis1000 commented Mar 12, 2020

The SharedArrayBuffer don't even work with TextEncoder/TextDecoder.
Lots of api don't work with it either.
I am curious about who actually use SharedArrayBuffer for non cross worker reason and for what reason?

@bzbarsky
Copy link
Contributor

The SharedArrayBuffer don't even work with TextEncoder/TextDecoder.

It does per spec. That part is not implemented in Gecko yet, looks like...

@lukewagner
Copy link

Another potential benefit of having SAB always be available, while keeping only postMessage() gated on COOP+COEP, is that it would enable the creation of ungated Web APIs where the host could write into a SAB from a background host thread. (This assumes the host's racy writes could not be used to construct a high-res timer.) A concrete example would be avoiding the copying inherent in BYOB streams. If SAB's existence was gated, so necessarily would be these Web APIs.

@syg
Copy link
Contributor

syg commented Mar 12, 2020

For a plan communicated well over a year ago and reiterated at various points this is also an extremely late request for change.

For not engaging here until so late, that's on us. You're right that this request for change is very late. This slipped through the cracks in V8 and Chrome for too long. This request isn't lightly made: we don't want to break a non-trivial population of real-world users.

Data

We haven't seen any breakage in the Firefox Nightly/Beta landscape thus far from having SharedArrayBuffer's constructor exposed and also have postMessage() throw outside of cross-origin isolated environments. (Emscripten doesn't do any meaningful feature detection either.)

The Chrome team has gathered data that shows there will be real-world breakage.
To wit, use counters and real-world feature detection code.

SAB use is at just under 1.25% among all pageloads in Chrome and growing. Until we ship COOP+COEP, SharedArrayBuffer is available where sharing it across agents was allowed. E.g., on desktop, where we had site isolation. That's quite a lot of users.

Edit: I said something previously that suggests Chrome turned off the SAB constructor. It has been and remains still available on platforms with site isolation until COOP+COEP ships.

We also collected the top scripts that use SABs. I went through the top 4 to see how they're used. Relevant pretty-printed snippets below:

https://static.adsafeprotected.com/sca.17.4.114.js

function() {
  if (
    "function" == typeof SharedArrayBuffer &&
    util.fnc(SharedArrayBuffer)
  )
    try {
      var a = new SharedArrayBuffer(1024);
      if (
        "[object SharedArrayBuffer]" == a.toString() &&
        a.byteLength &&
        1024 === a.byteLength
      )
        try {
          SharedArrayBuffer(1024);
        } catch (a) {
          return !0;
        }
    } catch (a) {}
  return !1;
}

That function is then used to check if SABs can be used.

https://meetfam.com/static/fam/fam.ad180408bcd0b1f105e5.js
https://meetfam.com/static/fam/fam-discount.7e41fe517671d8b5bfad.js
https://unpkg.com/react@16.12.0/umd/react.development.js

var sharedProfilingBuffer = enableProfiling ? // $FlowFixMe Flow doesn't know about SharedArrayBuffer
typeof SharedArrayBuffer === 'function' ? new SharedArrayBuffer(profilingStateSize * Int32Array.BYTES_PER_ELEMENT) : // $FlowFixMe Flow doesn't know about ArrayBuffer
typeof ArrayBuffer === 'function' ? new ArrayBuffer(profilingStateSize * Int32Array.BYTES_PER_ELEMENT) : null // Don't crash the init path on IE9
: null;

These ultimately all come from React or inlined parts of React.

React's profiling feature uses a SharedArrayBuffer to read profiling info of the Scheduler from a Worker, and this is gated on whether or not the SharedArrayBuffer constructor exists.

Next steps

V8's engagement is indeed late. At the same time, not breaking our existing users is the higher order concern.

Is the data compelling enough to consider the change?

Non-detachable ArrayBuffers

And the reason to have SharedArrayBuffer available is because it can also be used without threads as a non-detachable ArrayBuffer and there are numerous web platform APIs that will work with it regardless of the environment being cross-origin isolated.

For the non-detachable use case, that seems like a useful feature to add to ArrayBuffers themselves. I'd be happy to champion such a thing.

@syg
Copy link
Contributor

syg commented Mar 12, 2020

Another potential benefit of having SAB always be available, while keeping only postMessage() gated on COOP+COEP, is that it would enable the creation of ungated Web APIs where the host could write into a SAB from a background host thread. (This assumes the host's racy writes could not be used to construct a high-res timer.) A concrete example would be avoiding the copying inherent in BYOB streams. If SAB's existence was gated, so necessarily would be these Web APIs.

I don't think that's necessarily true. Hosts can provide under-the-hood shared buffers without giving users the ability to create new ones via an exposed SharedArrayBuffer constructor. The .constructor property can either be nulled out. Or even if it were available, that's not the worst thing. I agree technically there's not much harm in letting users create SABs for intra-thread use. Again, the high-order bit here is not breaking existing feature detection code, which only relies on the existence of the SharedArrayBuffer global constructor.

Edit: Put another way, I see no reason to gate future Web APIs that want to vend SABs on the decision of the visibility of the constructor.

@lukewagner
Copy link

lukewagner commented Mar 13, 2020

Hosts can provide under-the-hood shared buffers without giving users the ability to create new ones via an exposed SharedArrayBuffer constructor.

I'm talking about future Web APIs that, motivated by the general performance theme of "APIs shouldn't perform allocations; they should write into views of buffers/memories allocated by client code", accept a view to a SAB so that they can write into it racily from another thread. So client JS code needs to be able to create the SAB to use the Web API. (Yes, a workaround could be defining some new function for constructing SABs.)

@kmiller68
Copy link

kmiller68 commented Mar 18, 2020

@syg Huh? I was speaking to the fact that Anne was showing there's a way to get an ArrayBuffer with shared enabled in JS. Although, perhaps I misunderstood the purpose of the comment.

Maybe what Anne was saying is: Why disable the SAB constructor if you can just get a SAB via Wasm? Which is a question I'd also like to know the answer to.

@syg
Copy link
Contributor

syg commented Mar 18, 2020

@kmiller68 The reasoning for how we arrived at this point is as follows.

The fact that you can get a SAB out of a shared wasm memory is intentional even when the SharedArrayBuffer is not present due to not having cross-origin isolation. The original plan which was to make SAB and shared wasm memory always available (for the reasons listed in #4732 (comment)), regardless of COOP+COEP, and to have COOP+COEP gate the postMessage-ability of SABs.

I requested on behalf of Chrome that we also gate the SharedArrayBuffer constructor on COOP+COEP to not break and degrade existing sites, because Chrome on desktop has had SharedArrayBuffer available since it ships with site isolation. Since the motivation for gating the constructor is back compat and not objection to the technical arguments, the compromise is that wasm shared memory remains available everywhere.

@annevk
Copy link
Member Author

annevk commented Mar 21, 2020

I started updating existing tests at web-platform-tests/wpt#22358 to account for not exposing the constructor all the time. I'll specify it as part of #4734 I think.

(@syg and I discussed the possibility of exposing the constructor on some globals, but V8 were not in favor and I don't feel particularly strongly about it.)

annevk added a commit that referenced this issue Mar 23, 2020
This depends on the work to add Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy. It specifies how postMessage() is affected when both those headers are set for the agent clusters they impact.

Additionally, it exposes this state through self.crossOriginIsolated.

Tests: see links in #4732.

Closes #4732.
@annevk
Copy link
Member Author

annevk commented Mar 23, 2020

All tests have outstanding PRs now. Deleting the constructor is specified in the aforementioned PR. https://bugzilla.mozilla.org/show_bug.cgi?id=1624266 tracks aligning Firefox with this change. Review of all of this appreciated.

annevk added a commit that referenced this issue Mar 25, 2020
This depends on the work to add Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy. It specifies how postMessage() is affected when both those headers are set for the agent clusters they impact.

Additionally, it exposes this state through self.crossOriginIsolated.

Tests: see links in #4732.

Closes #4732.
annevk added a commit that referenced this issue Apr 7, 2020
This depends on the work to add Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy. It specifies how postMessage() is affected when both those headers are set for the agent clusters they impact.

Additionally, it exposes this state through self.crossOriginIsolated.

Tests: see links in #4732.

Closes #4732.
@yutakahirano yutakahirano mentioned this issue Jun 15, 2020
3 tasks
domenic pushed a commit that referenced this issue Jun 25, 2020
annevk added a commit that referenced this issue Jun 26, 2020
This depends on the work to add Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy. It specifies how postMessage() is affected when both those headers are set for the agent clusters they impact.

Additionally, it exposes this state through self.crossOriginIsolated.

Tests: see links in #4732.

Closes #4732.
annevk added a commit that referenced this issue Jun 29, 2020
A top-level navigation response with Cross-Origin-Opener-Policy set to same-origin and Cross-Origin-Embedder-Policy set to require-corp will create a cross-origin isolated browsing context group. And all agent clusters therein will be cross-origin isolated as well (shared and service workers can still not be, as they sit on the side).

This change also:

* Gates SharedArrayBuffer exposure behind that primitive for web compatibility reasons.
* Gates SharedArrayBuffer sharing behind that primitive.
* Exposes it through self.crossOriginIsolated.
* Makes document.domain return before it mutates the origin.
* Makes agent clusters keyed on origin.

Tests:

* web-platform-tests/wpt#17719
* web-platform-tests/wpt#17760
* web-platform-tests/wpt#17761
* web-platform-tests/wpt#17802
* web-platform-tests/wpt#17909
* web-platform-tests/wpt#18543
* web-platform-tests/wpt#20116
* web-platform-tests/wpt#22358

Closes #4732. Closes #5122. Closes #5444.

Follow-up: #5435.
annevk added a commit that referenced this issue Jul 8, 2020
A top-level navigation response with Cross-Origin-Opener-Policy set to same-origin and Cross-Origin-Embedder-Policy set to require-corp will create a cross-origin isolated browsing context group. And all agent clusters therein will be cross-origin isolated as well (shared and service workers can still not be, as they sit on the side).

This change also:

* Gates SharedArrayBuffer exposure behind that primitive for web compatibility reasons.
* Gates SharedArrayBuffer sharing behind that primitive.
* Exposes it through self.crossOriginIsolated.
* Makes document.domain return before it mutates the origin.
* Makes agent clusters keyed on origin.

Tests:

* web-platform-tests/wpt#17719
* web-platform-tests/wpt#17760
* web-platform-tests/wpt#17761
* web-platform-tests/wpt#17802
* web-platform-tests/wpt#17909
* web-platform-tests/wpt#18543
* web-platform-tests/wpt#20116
* web-platform-tests/wpt#22358

Closes #4732. Closes #5122. Closes #5444.

Follow-up: #5435 (and #5362).
mfreed7 pushed a commit to mfreed7/html that referenced this issue Sep 11, 2020
mfreed7 pushed a commit to mfreed7/html that referenced this issue Sep 11, 2020
A top-level navigation response with Cross-Origin-Opener-Policy set to same-origin and Cross-Origin-Embedder-Policy set to require-corp will create a cross-origin isolated browsing context group. And all agent clusters therein will be cross-origin isolated as well (shared and service workers can still not be, as they sit on the side).

This change also:

* Gates SharedArrayBuffer exposure behind that primitive for web compatibility reasons.
* Gates SharedArrayBuffer sharing behind that primitive.
* Exposes it through self.crossOriginIsolated.
* Makes document.domain return before it mutates the origin.
* Makes agent clusters keyed on origin.

Tests:

* web-platform-tests/wpt#17719
* web-platform-tests/wpt#17760
* web-platform-tests/wpt#17761
* web-platform-tests/wpt#17802
* web-platform-tests/wpt#17909
* web-platform-tests/wpt#18543
* web-platform-tests/wpt#20116
* web-platform-tests/wpt#22358

Closes whatwg#4732. Closes whatwg#5122. Closes whatwg#5444.

Follow-up: whatwg#5435 (and whatwg#5362).
@JSmith01
Copy link

Right now Chrome and Firefox has inconsistent behavior for SharedArrayBuffer disabling. When I try to create an instance - I'd get some kind of error immediately on an attempt. Then I have workaround like:

const SharedArrayBuffer = (new WebAssembly.Memory({ initial: 1, maximum: 1, shared: true })).__proto__.constructor;
const m = new SharedArrayBuffer(1);

And this won't give me an error until I use postMessage. Chrome issues deprecation warning when I try to instantiate shared wasm memory, Firefox silently allows me to have SharedArrayBuffer instance. The only error I'll get is when I pass memory or plain SharedArrayBuffer to postMessage call.

That seems to be inconsistent behavior to me, when the one way fails immediately and the other only when used. As a JS developer I'd prefer if WebAssembly.Memory construction with shared: true fails when crossOriginIsolated is falsy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: cross-origin-embedder-policy Issues and ideas around the new "require CORP for subresource requests and frames and etc" proposal topic: cross-origin-opener-policy Issues and ideas around the new "inverse of rel=noopener" header topic: serialize and transfer
Development

Successfully merging a pull request may close this issue.

9 participants