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

detached ArrayBuffers are specced to throw often, but implementations do not #678

Open
domenic opened this Issue Aug 24, 2016 · 10 comments

Comments

Projects
None yet
6 participants
@domenic
Member

domenic commented Aug 24, 2016

Previously discussed several times. @annevk brought it up 2 years ago in https://esdiscuss.org/topic/arraybuffer-neutering.

My understanding is that TC39 has decided to hope that one day implementations start throwing here. I am not aware of any implementation plans to do so, but maybe my info is out of date.

In any case, it's worth having an open tracking issue under "web reality" so that people trying to implement the ES spec realize that it doesn't match the reality of the web and implementations.

@domenic domenic added the web reality label Aug 24, 2016

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Aug 24, 2016

Member

Possibly relevant thread: heycam/webidl#151

Member

bterlson commented Aug 24, 2016

Possibly relevant thread: heycam/webidl#151

@anba

This comment has been minimized.

Show comment
Hide comment
@anba

anba Feb 6, 2017

Contributor

It's also probably worth mentioning that all current implementations violate one or more of the invariants listed in 6.1.7.3 Invariants of the Essential Internal Methods for operations on typed arrays with detached array buffers. So it's not recommended to simply copy the bevaviour of web browsers.

Contributor

anba commented Feb 6, 2017

It's also probably worth mentioning that all current implementations violate one or more of the invariants listed in 6.1.7.3 Invariants of the Essential Internal Methods for operations on typed arrays with detached array buffers. So it's not recommended to simply copy the bevaviour of web browsers.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Feb 10, 2017

Member

@anba Which invariants do you see violated? Do you have a test case? (Possibly relevant thread: tc39/test262#841)

Member

littledan commented Feb 10, 2017

@anba Which invariants do you see violated? Do you have a test case? (Possibly relevant thread: tc39/test262#841)

@anba

This comment has been minimized.

Show comment
Hide comment
@anba

anba Feb 10, 2017

Contributor

One of the [[OwnPropertyKeys]] invariants is violated in all engines:

function detach(ab) {
  if (ArrayBuffer.transfer) {
    ArrayBuffer.transfer(ab);
  } else if (typeof detachArrayBuffer === "function") {
    detachArrayBuffer(ab);
  } else if (typeof transferArrayBuffer === "function") {
    transferArrayBuffer(ab)
  } else if (typeof Worker === "function") {
    try { eval("%ArrayBufferNeuter(ab)") } catch (e) {
      var w = new Worker("");
      w.postMessage(ab, [ab]);
      w.terminate();
    }
  } else {
    throw new TypeError("cannot detach array buffer");
  }
}

var ta = new Int32Array(1);

// Observe ta[0] as non-configurable.
print(JSON.stringify(Object.getOwnPropertyDescriptor(ta, 0)));

detach(ta.buffer);

// Expected: Throws TypeError
// Actual: Prints empty string
// Violates: [[OwnPropertyKeys]]
// The returned List must contain at least the keys of all non-configurable own properties that have previously been observed. 
print(Object.getOwnPropertyNames(ta));

And related to this an issue with for-in because of:

EnumerateObjectProperties must obtain the own property keys of the target object by calling its [[OwnPropertyKeys]] internal method.

var ta = new Int32Array(1);
print(JSON.stringify(Object.getOwnPropertyDescriptor(ta, 0)));
detach(ta.buffer);
// Must print "0" or throw.
for (var p in ta) print(p);

Violation for [[GetOwnProperty]] in V8 and SM, because non-configurable properties can disappear

If P's attributes other than [[Writable]] may change over time or if the property might disappear, then P's [[Configurable]] attribute must be true.

var ta = new Int32Array(1);
print(JSON.stringify(Object.getOwnPropertyDescriptor(ta, 0)));
detach(ta.buffer);
// Must not print undefined.
print(JSON.stringify(Object.getOwnPropertyDescriptor(ta, 0)));

Similar issue for [[HasProperty]] in V8/SM:

If P was previously observed as a non-configurable data or accessor own property of the target, [[HasProperty]] must return true.

var ta = new Int32Array(1);
print(JSON.stringify(Object.getOwnPropertyDescriptor(ta, 0)));
detach(ta.buffer);
// Must not print false.
print(0 in ta);

And also [[Delete]]:

If P was previously observed to be a non-configurable own data or accessor property of the target, [[Delete]] must return false.

var ta = new Int32Array(1);
print(JSON.stringify(Object.getOwnPropertyDescriptor(ta, 0)));
detach(ta.buffer);
// Must not print true.
print(delete ta[0]);

And strictly speaking typed arrays aren't even modifiable in JSC, because they're reported as non-configurable + non-writable, but that's unrelated to the detachment issue.

Contributor

anba commented Feb 10, 2017

One of the [[OwnPropertyKeys]] invariants is violated in all engines:

function detach(ab) {
  if (ArrayBuffer.transfer) {
    ArrayBuffer.transfer(ab);
  } else if (typeof detachArrayBuffer === "function") {
    detachArrayBuffer(ab);
  } else if (typeof transferArrayBuffer === "function") {
    transferArrayBuffer(ab)
  } else if (typeof Worker === "function") {
    try { eval("%ArrayBufferNeuter(ab)") } catch (e) {
      var w = new Worker("");
      w.postMessage(ab, [ab]);
      w.terminate();
    }
  } else {
    throw new TypeError("cannot detach array buffer");
  }
}

var ta = new Int32Array(1);

// Observe ta[0] as non-configurable.
print(JSON.stringify(Object.getOwnPropertyDescriptor(ta, 0)));

detach(ta.buffer);

// Expected: Throws TypeError
// Actual: Prints empty string
// Violates: [[OwnPropertyKeys]]
// The returned List must contain at least the keys of all non-configurable own properties that have previously been observed. 
print(Object.getOwnPropertyNames(ta));

And related to this an issue with for-in because of:

EnumerateObjectProperties must obtain the own property keys of the target object by calling its [[OwnPropertyKeys]] internal method.

var ta = new Int32Array(1);
print(JSON.stringify(Object.getOwnPropertyDescriptor(ta, 0)));
detach(ta.buffer);
// Must print "0" or throw.
for (var p in ta) print(p);

Violation for [[GetOwnProperty]] in V8 and SM, because non-configurable properties can disappear

If P's attributes other than [[Writable]] may change over time or if the property might disappear, then P's [[Configurable]] attribute must be true.

var ta = new Int32Array(1);
print(JSON.stringify(Object.getOwnPropertyDescriptor(ta, 0)));
detach(ta.buffer);
// Must not print undefined.
print(JSON.stringify(Object.getOwnPropertyDescriptor(ta, 0)));

Similar issue for [[HasProperty]] in V8/SM:

If P was previously observed as a non-configurable data or accessor own property of the target, [[HasProperty]] must return true.

var ta = new Int32Array(1);
print(JSON.stringify(Object.getOwnPropertyDescriptor(ta, 0)));
detach(ta.buffer);
// Must not print false.
print(0 in ta);

And also [[Delete]]:

If P was previously observed to be a non-configurable own data or accessor property of the target, [[Delete]] must return false.

var ta = new Int32Array(1);
print(JSON.stringify(Object.getOwnPropertyDescriptor(ta, 0)));
detach(ta.buffer);
// Must not print true.
print(delete ta[0]);

And strictly speaking typed arrays aren't even modifiable in JSC, because they're reported as non-configurable + non-writable, but that's unrelated to the detachment issue.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Feb 10, 2017

Member

Well, all that sounds like a pretty fundamental barrier to spec'ing something more likely to be web-compatible. It sounds like the violations are all coming from TypedArray elements being non-configurable--that if we make these non-configurable properties visible, the only way to get out of it later is to throw when trying to do other stuff. It wouldn't violate the invariants to make them configurable, but then throw all over the place as if they were non-configurable, right? Just a bit ugly.

Member

littledan commented Feb 10, 2017

Well, all that sounds like a pretty fundamental barrier to spec'ing something more likely to be web-compatible. It sounds like the violations are all coming from TypedArray elements being non-configurable--that if we make these non-configurable properties visible, the only way to get out of it later is to throw when trying to do other stuff. It wouldn't violate the invariants to make them configurable, but then throw all over the place as if they were non-configurable, right? Just a bit ugly.

@anba

This comment has been minimized.

Show comment
Hide comment
@anba

anba Feb 10, 2017

Contributor

It seems like web-reality has shifted a bit, because at least latest Chakra and JSC seem to throw when accessing/setting indexed properties on a detached typed array.

Contributor

anba commented Feb 10, 2017

It seems like web-reality has shifted a bit, because at least latest Chakra and JSC seem to throw when accessing/setting indexed properties on a detached typed array.

@jswalden

This comment has been minimized.

Show comment
Hide comment
@jswalden

jswalden Feb 11, 2017

Contributor

"It wouldn't violate the invariants to make them configurable, but then throw all over the place as if they were non-configurable, right? Just a bit ugly." Sounds right to me on all accounts. Seems like the only out available.

Contributor

jswalden commented Feb 11, 2017

"It wouldn't violate the invariants to make them configurable, but then throw all over the place as if they were non-configurable, right? Just a bit ugly." Sounds right to me on all accounts. Seems like the only out available.

@evilpie

This comment has been minimized.

Show comment
Hide comment
@evilpie

evilpie Feb 13, 2017

Contributor

@anba So detached typed array indexed element access throws in release version of Chakra and JSC? Since when?

Contributor

evilpie commented Feb 13, 2017

@anba So detached typed array indexed element access throws in release version of Chakra and JSC? Since when?

@anba

This comment has been minimized.

Show comment
Hide comment
@anba

anba Feb 13, 2017

Contributor

@evilpie I don't know if the release versions of WebKit/Safari throw an error (I've only tried the latest JSC from source), but at least Edge 38.14393.0.0 throws a TypeError when accessing elements on a detached typed array.

Contributor

anba commented Feb 13, 2017

@evilpie I don't know if the release versions of WebKit/Safari throw an error (I've only tried the latest JSC from source), but at least Edge 38.14393.0.0 throws a TypeError when accessing elements on a detached typed array.

@evilpie

This comment has been minimized.

Show comment
Hide comment
@evilpie

evilpie Jun 23, 2018

Contributor

Considering that Edge has been shipping throwing on detached typed arrays, I plan on changing Firefox to also throw.

Contributor

evilpie commented Jun 23, 2018

Considering that Edge has been shipping throwing on detached typed arrays, I plan on changing Firefox to also throw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment