feat(ses): ImmutableArrayBuffer

erights commented 3 weeks ago

Closes: #XXXX Refs: #1538 #1331 #2311

Description

A step towards fixing #1331 , likely by restaging #1538 on this one and then fixing it. By making ImmutableArrayBuffer as-if part of the language, in #1538 passStyleOf will be able to recognize one even though it carries its own methods, avoiding the eval-twin problems that otherwise thwart such plans. This is one of the candidates explained at #1331 . That passStyleOf behavior opens the door for marshal to serialize and unserialize these as additional ocapn Passables.

Additionally, by proposing it to tc39 as explained below, we'd enable immutable TypedArrays and DataViews as well, and XS could place all these in ROM cheaply, while conforming to the language spec. When also hardened, XS could judge these to be pure. Attn @phoddie @patrick-soquet

From the initial class comment:

ImmutableArrayBuffer is intended to be a peer of ArrayBuffer and SharedArrayBuffer, but only with the non-mutating methods they have in common. We're adding this to ses as if it was already part of the language, and we consider this implementation to be a shim for an upcoming tc39 proposal.

As a proposal it would take additional steps that would the shim does not:

to have ImmutableArrayBuffer be shared between threads (in spec speak, "agent") in exactly the same way SharedArrayBuffer is shared between agents. Unlike SharedArrayBuffer, sharing an ImmutableArrayBuffer does not introduce any observable concurrency. Unlike ArrayBuffer, sharing an ImmutableArrayBuffer does not detach anything.
when used as a backing store of a TypedArray or DataView, all the query methods would work, but the mutating methods would throw. In this sense, the wrapping TypedArray or DataView would also be immutable.

Note that the class isn't immutable until hardened by lockdown. Even then, the instances are not immutable until hardened. This class does not harden its instances itself to preserve similarity with ArrayBuffer and SharedArrayBuffer.

Security Considerations

The eval-twin problem explained at #1331 is a security problem. This PR is one candidate for solving that problem, unblocking #1538 so it can fix #1331. Further, if accepted into a future version of the language, the immutability it provides will generally help security.

Scaling Considerations

This shim implementation likely does more copying than even a naive native implementation would. A native implementation may even engage in copy-on-write tricks that this shim cannot. Use of the shim should beware of these "extra" copying costs.

Compatibility and Documentation Considerations

Generally we've kept hardened JS close to standard JS, and we've kept the ses-shim close to hardened JS. With this PR, we'd need to explain ImmutableArrayBuffer as part of the hardened JS implemented by the ses-shim, even though it is not yet proposed to tc39.

Testing Considerations

Ideally, we should identify the subset of test262 ArrayBuffer tests that should be applicable to ImmutableArrayBuffer, and duplicate them for that purpose.

Upgrade Considerations

Nothing breaking.

NEWS.md updated

erights commented 3 weeks ago

@phoddie @patrick-soquet I cannot add you as official reviewers. But please consider yourself reviewers anyway. I am eager for your comments. Would this as a proposal, and eventually in the language, help XS in the way I explain above?

erights commented 3 weeks ago

I am very skeptical of "shimming" something that has very little current agreement in committee. I would much rather we explore approaches that do not require us to modify the ses shim for now, as I stated in #1331 (comment)

@gibson042 @FUDCo @phoddie @patrick-soquet , if you'll be in Helsinki and have time, try shopping this around informally. Once I realized that it's a natural sibling of ArrayBuffer and SharedArrayBuffer and wrote out the implications -- that it gives us effectively immutable TypedArrays and DataViews, and it allows communications by sharing immutable data without copying, and it provides a de jure way for XS to put these in ROM -- my sense is that it might have wide appeal in tc39. If the temperature of this room seems warm, I would like to proceed with this approach. Please let us know, thanks!

gibson042 commented 3 weeks ago

I realized that it's a natural sibling of ArrayBuffer and SharedArrayBuffer

It's not clear to me that immutability is best expressed by a sibling class rather than a detail of any particular ArrayBuffer (like resizeability but in some ways its opposite). One could imagine new ArrayBuffer(length, { immutable: true }) and arrayBuffer.transferToImmutable(length) each returning a permanently non-detached immutable instance as described above and for which the transfer methods return distinct immutable instances backed by the same memory (e.g., immutableBuffer.transfer() !== immutableBuffer but their data necessarily matches).

phoddie commented 3 weeks ago

It's not clear to me that immutability is best expressed by a sibling class rather than a detail of any particular ArrayBuffer (like resizeability but in some ways its opposite).

Yes, agreed. We worked to avoid having resizable ArrayBuffers use a separate constructor. Similarly, one of our objections to Records & Tuples is creating new constructors/classes for immutable versions of existing objects.

erights commented 3 weeks ago

... rather than a detail of any particular ArrayBuffer ...

As long as passStyleOf can tell that it is looking at such an immutable ArrayBuffer, that would serve our needs just as well. Although it would make a more awkward shim, because it would be more surprising when the immutable variant of ArrayBuffer (which the shim would still implement the same way) cannot be used as the backing store of a DataView or TypedArray. But since that's only a shim limitation that would be absent from the proposal, it is worth it.

So please shop that around as well. Or instead, if you're so inclined.

erights commented 3 weeks ago

Oh yuck, the shim will need to replace all the non-generic builtins on ArrayBuffer.prototype with wrappers that test what flavor of this they have and dispatch/overload on that. Again, a shim-only cost, but a surprisingly expensive one.

phoddie commented 3 weeks ago

Setting aside the shim for a moment, it seems like small extensions to the ArrayBuffer API are sufficient:

Create immutable ArrayBuffer with .transferToImmutable() method, analogous to .transferToFixedLength(). This is as efficient as the proposed ImmutableArrayBuffer(buffer) constructor as an implementation can either copy immediately or on-write.
Test for immutability with .immutable property, analogous to .resizable and .detached.

erights commented 3 weeks ago

Setting aside the shim for a moment, it seems like small extensions to the ArrayBuffer API are sufficient:

Create immutable ArrayBuffer with .transferToImmutable() method, analogous to .transferToFixedLength(). This is as efficient as the proposed ImmutableArrayBuffer(buffer) constructor as an implementation can either copy immediately or on-write.

Test for immutability with .immutable property, analogous to .resizable and .detached.

I agree, and am currently working on a modified shim to try to emulate this as best as I practically can. I just noticed another problem in shimming this though. Node 20, which we still support, has no way within the language to detach an ArrayBuffer. Therefore, on Node 20, transferToImmutable will copy (using slice) and leave the original undetached. Just another loss of fidelity for the shim that needs to be documented. Sigh. Still worth it to have a cleaner and more minimal proposal.

erights commented 3 weeks ago

Closing in favor of https://github.com/endojs/endo/pull/2311 which implements @phoddie 's suggestion. Please review that one instead. Thanks.

gibson042 commented 3 weeks ago

Node 20, which we still support, has no way within the language to detach an ArrayBuffer. Therefore, on Node 20, transferToImmutable will copy (using slice) and leave the original undetached.

Not within the language, but it does have extensions that are sufficient, specifically structuredClone:

node -e '
  console.log("node", process.version);
  const buf = new ArrayBuffer(10);
  console.log("initial byteLength", buf.byteLength);
  structuredClone(new ArrayBuffer(0), { transfer: [buf] });
  console.log("post-transfer byteLength", buf.byteLength);
  try {
    buf.slice();
    console.error("detach failed");
  } catch (err) {
    console.log("detach worked!", err.message);
  }
'
node v20.10.0
initial byteLength 10
post-transfer byteLength 0
detach worked! Cannot perform ArrayBuffer.prototype.slice on a detached ArrayBuffer

So you could do something like

const detachArrayBuffer = ArrayBuffer.prototype.transfer
  ? buf => void buf.transfer()
  : buf => void structuredClone(buf, { transfer: [buf] });

endojs / endo