Skip to content
This repository was archived by the owner on Jan 25, 2022. It is now read-only.

Formalize embedding-specific constraints on worker types and reaction to illegal sharing #39

Closed
lars-t-hansen opened this issue Dec 17, 2015 · 19 comments

Comments

@lars-t-hansen
Copy link
Collaborator

In Firefox we'll likely have to prevent SharedWorkers and ServiceWorkers from sending or receiving shared memory, since we anticipate that that will cause complications when paired with process separation. (See https://bugzil.la/1232973 .)

The shared memory spec already hints at this in section 8.1, but it may be beneficial for interop to give this restriction teeth, somehow, eg, by providing information about what should happen when trying to send a SharedArrayBuffer in an illegal way. Clearly this is more in the purview of WHATWG/W3C than ECMA, but even so. The information would go in what is now section 8.5.

(EDIT: Corrected the bugzilla link.)

@jfbastien
Copy link
Contributor

FYI @slightlyoff

@lars-t-hansen
Copy link
Collaborator Author

The possibility of nested workers complicates these restrictions a little; with nested workers in the mix it's at least plausible that a SharedWorker or ServiceWorker would be allowed to create its own workers, and it might be reasonable for those clusters of workers to share memory. In those settings it may be that the SharedWorker or ServiceWorker main loop should be allowed to reject a futexWait, for the same reasons the browser's main thread should be allowed to reject them.

@lars-t-hansen
Copy link
Collaborator Author

More complications:

MessageChannels can be used to transmit values but the sender may not be aware of who the receiver is and it may be impractical to require an exception to be thrown at the sending end, instead it may be necessary for something to show up through the error channel on the receiving end.

It's currently possible to transmit values window-to-window, but do we want to allow SharedArrayBuffers to be transmitted that way?

I'm inclined to think that for the initial work it will be most desirable (beause most conservative) to allow window-to/from-dedicated-worker SAB transmission only, ie not window-to-window or to/from any other kind of worker or agent, but that may still require dealing with MessageChannels.

@lars-t-hansen
Copy link
Collaborator Author

FYI @annevk

@annevk
Copy link
Member

annevk commented Mar 2, 2016

A MessageChannel consists of two entangled ports, each of which has an owner that's associated with a realm (in some way). So it should be possible to know something, no?

To what kind of realms/globals do we want to restrict SharedArrayBuffer? Only dedicated workers? If we allowed them in windows they could block, no?

@lars-t-hansen
Copy link
Collaborator Author

A MessageChannel consists of two entangled ports, each of which has an owner that's associated with a realm (in some way). So it should be possible to know something, no?

Possibly. :baku seemed a little doubtful, if I remember correctly. That could just be an implementation limitation, I haven't had time to follow that path further down.

To what kind of realms/globals do we want to restrict SharedArrayBuffer? Only dedicated workers? If we allowed them in windows they could block, no?

SAB can be allowed to be present everywhere (and currently it is, and I think it should be); it is futexWait that needs to be restricted if we want to avoid blocking, but that's orthogonal. The issue is really about which pairs of agent types allow a SAB to be (or not to be) transfered from one of the pair to the other, ie shared between the members of the pair.

I think starting conservative is good: transfering from a Window to/from its dedicated workers, and from a dedicated worker to/from its own dedicated workers (assuming nested workers), is probably all we really must support. I really am a little out of my depth here, and I don't think we know what all the usage patterns will be.

@annevk
Copy link
Member

annevk commented Mar 2, 2016

I wonder if I can successfully summon @bakulf to help out. @domenic might also have ideas here.

@annevk
Copy link
Member

annevk commented Mar 2, 2016

I talked to @bakulf. Basically, if SharedArrayBuffer wants to have varying behavior per realm, we need to revise the StructuredCloneWithTransfer algorithm, again, to serialize objects into some kind of intermediate format, and then deserialize them on the other hand.

The reason is MessageChannel. It has a queue and things put into the queue don't come back out straight away. And since you can transfer a MessagePort, cloned or transferred objects put into the queue, need to be created in the correct realm when coming out, not when going in.

Arguably we'd need to revise the algorithm anyway to account for this, but so far you can sort of wave your hands since it doesn't matter much. But with varying-per-realm-behavior, it's impossible to avoid tackling this.

Having just rewritten the algorithm, I might hold of on this a bit until someone can convince me this is not a lot of work.

@domenic
Copy link
Member

domenic commented Mar 2, 2016

If it helps, I think the serialization format could be specced without too much trouble as an ES record. E.g. { [[CloneTag]]: NumberObject, [[NumberData]]: input's [[NumberData]] }, or { [[CloneTag]]: DataView, [[ViewedArrayBuffer]]: ..., [[ByteLength]]: ..., [[ByteOffset]]: ... }.

Seems like a big pain though.

@annevk
Copy link
Member

annevk commented Mar 2, 2016

Yeah, something like that is probably the way to go. That would probably also make it easier to specify the cyclic references without making it seem like we are doing some magical operation. But yeah, I think I might work on a couple of other things first and revisit this later since this isn't done in a day.

@lars-t-hansen
Copy link
Collaborator Author

OK, thanks. It's probably inevitable that we don't want to share memory among certain pairs of agents that can communicate by postMessage, but there are actually two ways to accomplish that. One is to restrict the structured clone mechanism. The other is to realize that the shared memory can only be accessed in the receiving agent if the agent can create a TypedArray on top of the SharedArrayBuffer that it receives, and therefore to allow SharedArrayBuffers to be tagged in such a way that that creation fails. (The distinction may not matter to the implemenation, I'm thinking only about spec work.) The tagging will have something to do with "compatible" agent clusters, maybe. We might need that in order to spec what this bug is really about anyway.

Whether that is uglier to spec than just speccing restrictions on structured clone I don't know. It probably moves the problem from the structured clone spec to the ES spec. The ES spec might say that "if the buffer is a SAB and the SAB's tag is incompatible with the current agent's tag then treat the SAB as length 0 / throw an exception". I've seen worse.

@annevk
Copy link
Member

annevk commented Mar 3, 2016

The problem is that the StructuredCloneWithTransfer directly allocates in a given realm. And this works since everything so far can go to any realm and will not fail to be created there. Once that changes and creation can fail, there are two options specification-wise:

  1. Define serialization and deserialization as discussed above. Maybe with special records or a new specification type. This has the benefit of matching implementations closer, removing some special language around swapping object references, and clarity. It is however a bit of work and needs to be done carefully.
  2. Whenever MessageChannel is involved, first StructuredCloneWithTransfer into a user-agent-defined realm and annotate somehow what realms are acceptable for SharedArrayBuffer and other objects with similar limitations. Then, when the message is about to be delivered, StructuredCloneWithTransfer again on the object in the user-agent-defined realm coupled with the annotations, and do what you want to do there. This is similar to serialization and deserialization, except it reuses much more existing infrastructure and will therefore be easier to define. It might however be less clear to the onlooker what is happening and does not really match implementations well.

@lars-t-hansen
Copy link
Collaborator Author

Note 4 of section 1.4 of the revised Agents spec has some cleaned-up language around this. That note is non-normative but the paragraph it comments on is not. LMK what you think. Fundamentally the constraint belongs in the Agents spec because any forward progress guarantee depends on it, or, if you like, it is implied by the forward progress guarantee and could all be turned into a non-normative note on that section.

@annevk
Copy link
Member

annevk commented Mar 8, 2016

The more I think about it, the more I think we need to go with option 1 above, but I have many things on my plate at the moment so I don't really want to commit to doing that yet.

@annevk
Copy link
Member

annevk commented Mar 8, 2016

And I guess either way we'll need to resolve what failure to allocate will mean. Technically we already have that problem with ArrayBuffer, which is not guaranteed to allocate and may throw a RangeError. I don't know how user agents handle that situation today, but we should do something similar for allocating SharedArrayBuffer in a realm where it cannot be.

@lars-t-hansen
Copy link
Collaborator Author

Memo to self: notify Anne on the whatwg bug linked above when we need to move forward with this.

@lars-t-hansen lars-t-hansen modified the milestone: Stage 4 Feb 9, 2017
@annevk
Copy link
Member

annevk commented Apr 16, 2017

whatwg/html#935 is fixed now. And the remainder of this will be fixed by whatwg/html#2518 most likely.

Our solution thus far has become generic and supports MessageChannel, as well as shared and service workers (though they can only share with descendants, not owners).

@lars-t-hansen
Copy link
Collaborator Author

whatwg/html#935 is fixed now. And the remainder of this will be fixed by whatwg/html#2518 most likely.

I also think that's likely, so let's re-review here once that PR lands.

@annevk
Copy link
Member

annevk commented May 2, 2017

I think this is all done now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants