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.

Sign up for GitHub

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

[DON'T MERGE THIS] Normative: Allow transferring SharedArrayBuffer objects #336

Closed
wants to merge 5 commits into from
Closed

[DON'T MERGE THIS] Normative: Allow transferring SharedArrayBuffer objects #336

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
77ca2d9
Normative: Allow transferring SharedArrayBuffer objects
leobalter Oct 28, 2021
4c40d69
Editorial: Rename GetWrappedValue's callerRealm to receiverRealm
leobalter Oct 29, 2021
01a2614
Editorial: Rename GetWrappedValue to GetTransferrableValue
leobalter Oct 29, 2021
b82d6b2
Shortcut the SAB cloning steps
leobalter Oct 29, 2021
b9d7628
Simplify GetTransferrableValue
leobalter Oct 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions spec.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,12 @@ <h1>[[Call]] ( _thisArgument_, _argumentsList_ )</h1>
1. NOTE: Any exception objects produced after this point are associated with _callerRealm_.
1. Let _wrappedArgs_ be a new empty List.
1. For each element _arg_ of _argumentsList_, do
1. Let _wrappedValue_ be ? GetWrappedValue(_targetRealm_, _arg_).
1. Let _wrappedValue_ be ? GetTransferrableValue(_targetRealm_, _arg_).
1. Append _wrappedValue_ to _wrappedArgs_.
1. Let _wrappedThisArgument_ to ? GetWrappedValue(_targetRealm_, _thisArgument_).
1. Let _wrappedThisArgument_ to ? GetTransferrableValue(_targetRealm_, _thisArgument_).
1. Let _result_ be the Completion Record of Call(_target_, _wrappedThisArgument_, _wrappedArgs_).
1. If _result_.[[Type]] is ~normal~ or _result_.[[Type]] is ~return~, then
1. Return ? GetWrappedValue(_callerRealm_, _result_.[[Value]]).
1. Return ? GetTransferrableValue(_callerRealm_, _result_.[[Value]]).
1. Else,
1. Throw a *TypeError* exception.
</emu-alg>
Expand Down Expand Up @@ -179,7 +179,7 @@ <h1>PerformShadowRealmEval ( _sourceText_, _callerRealm_, _evalRealm_ )</h1>
1. Suspend _evalContext_ and remove it from the execution context stack.
1. Resume the context that is now on the top of the execution context stack as the running execution context.
1. If _result_.[[Type]] is not ~normal~, throw a *TypeError* exception.
1. Return ? GetWrappedValue(_callerRealm_, _result_.[[Value]]).
1. Return ? GetTransferrableValue(_callerRealm_, _result_.[[Value]]).
</emu-alg>
<emu-note type=editor>
In the case of an abrupt ~throw~ completion, the type of error to be created should match the type of the abrupt throw completion record. This could be revisited when merging into the main specification. Additionally, in the case of a ~break~ or ~continue~ completion, since those are not supported, a TypeError is expected. There should be no ~return~ completion because this is a top level script evaluation, in which case a return |Statement| must result in a parsing error.
Expand Down Expand Up @@ -224,21 +224,36 @@ <h1>ShadowRealmImportValue ( _specifierString_, _exportNameString_, _callerRealm
1. If _hasOwn_ is *false*, throw a *TypeError* exception.
1. Let _value_ be ? Get(_exports_, _string_).
1. Let _realm_ be _f_.[[Realm]].
1. Return ? GetWrappedValue(_realm_, _value_).
1. Return ? GetTransferrableValue(_realm_, _value_).
</emu-alg>
</emu-clause>

<emu-clause id="sec-getwrappedvalue" aoid="GetWrappedValue">
<h1>GetWrappedValue ( _callerRealm_, _value_ )</h1>
<emu-clause id="sec-gettransferrablevalue" aoid="GetTransferrableValue">
<h1>GetTransferrableValue ( _receiverRealm_, _value_ )</h1>
<emu-alg>
1. Assert: _callerRealm_ is a Realm Record.
1. Assert: _receiverRealm_ is a Realm Record.
1. If Type(_value_) is Object, then
1. If IsCallable(_value_) is *false*, throw a TypeError exception.
1. Return ! WrappedFunctionCreate(_callerRealm_, _value_).
1. If IsCallable(_value_) is *true*,
1. Return ! WrappedFunctionCreate(_receiverRealm_, _value_).
1. If _value_ has an [[ArrayBufferData]] internal slot, then
1. If IsSharedArrayBuffer(_value_) if *true*, return ? CloneSharedArrayBuffer(_value_, _receiverRealm_.[[Intrinsics]].%SharedArrayBuffer%).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pass the receiver realm instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I mentioned above:

I created a new abstraction that can be reused to clone sharedarraybuffers in general. In the future, constructor can be optional where it defaults to the existing %SharedArrayBuffer% whenever the abstraction is used. We don't need it for the ShadowRealms usage, targeting the specific one from the receiverRealm.

1. throw a TypeError exception.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. throw a TypeError exception.
1. Throw a TypeError exception.

1. Return _value_.
</emu-alg>
</emu-clause>

<emu-clause id="sec-clonesharedarraybuffer" aoid="CloneSharedArrayBuffer">
<h1>CloneSharedArrayBuffer ( _buffer_, _constructor_ )</h1>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should expect a realm argument to avoid calling this with arbitrary constructors.

<emu-alg>
1. Assert: _buffer_ is an Object with a [[SharedArrayBuffer]] internal.
1. Let _clone_ be ? OrdinaryCreateFromConstructor(_constructor_, "%SharedArrayBuffer.prototype%", « [[ArrayBufferData]], [[ArrayBufferByteLength]] »).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the receiver realm's intrinsic here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the second argument is meant to be a string with the default proto. This proto will be fetched from the given constructor.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also mentioned this in a comment above.

usage of "%SharedArrayBuffer.prototype%" that doesn't need to be prefixed by the realm. The constructor argument for OrdinaryCreateFromConstructor is the important piece there and SharedArrayPrototype.prototype is not configurable, it should be safe.

In fact, it's meant to be just an string, regardless realms. This is string is gonna be used to access the matching intrinsic property of the given constructor.

Also, this name is just symbolic, as the %SharedArrayBuffer%.prototype will always be available:

OrdinaryCreateFromConstructor ( constructor, intrinsicDefaultProto [ , internalSlotsList ] )

1. Assert: intrinsicDefaultProto is this specification's name of an intrinsic object. The corresponding object must be an intrinsic that is intended to be used as the [[Prototype]] value of an object.
2. Let proto be ? GetPrototypeFromConstructor(constructor, intrinsicDefaultProto).
3. Return ! OrdinaryObjectCreate(proto, internalSlotsList).

GetPrototypeFromConstructor ( constructor, intrinsicDefaultProto )

1. Assert: intrinsicDefaultProto is this specification's name of an intrinsic object. The corresponding object must be an intrinsic that is intended to be used as the [[Prototype]] value of an object.
2. Let proto be ? Get(constructor, "prototype").
3. If Type(proto) is not Object, then
  a. Let realm be ? GetFunctionRealm(constructor).
  b. Set proto to realm's intrinsic object named intrinsicDefaultProto.
4. Return proto.

The name would only be used in step 3.b for the abstraction above, but %SharedArrayBuffer%.prototype is non configurable, non writable and the constructor is always the intrinsic one.

That means OrdinaryCreateFromConstructor results in OrdinaryObjectCreate(_receiverRealm_.[[Intrinsics]].%SharedArrayBuffer.prototype%, « [[ArrayBufferData]], [[ArrayBufferByteLength]] »).

I can actually shift usage from OrdinaryCreateFromConstructor to OrdinaryObjectCreate. The current draft uses it to match the slice method in SharedArrayBuffer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other reason for using OrdinaryCreateFromConstructor is only if the new abstraction CloneArrayBuffer becomes reusable within ECMAScript, but apparently I need to avoid this new abstraction as it became noise here.

1. Assert: _value_ and _clone_ are instances of SharedArrayBuffer of different Realms each.
1. Set _clone_.[[ArrayBufferData]] to _value_.[[ArrayBufferData]].
1. Set _clone_.[[ArrayBufferByteLength]] to _value_.[[ArrayBufferByteLength]].
1. Return _clone_.
</emu-alg>>
</emu-clause>

<emu-clause id="sec-validateshadowrealmobject" aoid="ValidateShadowRealmObject">
<h1>ValidateShadowRealmObject ( _O_ )</h1>
<p>The abstract operation ValidateShadowRealmObject takes argument _O_. It performs the following steps when called:</p>
Expand Down