Ecma262: detached ArrayBuffers are specced to throw often, but implementations do not

[domenic]

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.

[bterlson]

Possibly relevant thread: https://github.com/heycam/webidl/issues/151

[anba]

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]

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

[anba]

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]

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]

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]

"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]

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

[anba]

@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]

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

[annevk]

@evilpie has since abandoned that and I've also pointed out in that bug now that this would affect a lot of web platform APIs that do something with views and buffers. Browsers basically don't deal with the detached buffer situation for those APIs in their C++ as detached buffer means an empty buffer which is safe and harmless.

I think we've reached the point where ECMAScript needs to address this technical debt, with the possible exception of Chrome/V8 promising they'll fix it soon.

[littledan]

Could someone summarize the state of this across browsers, and give a little more background on the interaction with web APIs ? If this is long-term out of sync with all rendering engines that are being maintained going forward, that's a serious issue.

[annevk]

Indeed, it's roughly five years since my first report. For basically any IDL-defined API (https://searchfox.org/mozilla-central/search?q=ArrayBufferView&path=webidl lists a subset) taking a buffer or a view, a detached buffer or view pointing to a detached buffer will be treated as "empty", meaning the empty byte sequence on read and no room to write. (E.g., XMLHttpRequest's send() will transmit the empty byte sequence to the server and not throw.)

As for an example within ECMAScript, https://tc39.github.io/ecma262/#sec-get-arraybuffer.prototype.bytelength suggests implementations need to throw, but Chrome, Firefox, and Safari all return 0.

[annevk]

I'm planning on fixing this at least on the web platform side, by returning the empty byte sequence whenever there's a detached buffer: https://github.com/heycam/webidl/pull/605. That matches implementations and makes it easier for any new features and tests that need to be written around this. (The reason I ran into this again is because we were working on a new API (TextEncoder's encodeInto()) and could not figure out why the testcase that expected throwing was not throwing.)

[anba]

Could someone summarize the state of this across browsers, [...]

At least when testing in the shell, there's still some variation across the different engines depending on how the assignment is executed (plain assignment, Object.defineProperty, Reflect.defineProperty, or Reflect.set):

SM:

non-strict: undefined
strict: undefined
Reflect.set: false
Object.defineProperty: false
Reflect.defineProperty: false

JSC:

non-strict: TypeError: Underlying ArrayBuffer has been detached from the view
strict: TypeError: Underlying ArrayBuffer has been detached from the view
Reflect.set: TypeError: Underlying ArrayBuffer has been detached from the view
Object.defineProperty: TypeError: Attempting to store non-enumerable or non-writable property on a typed array at index: 0
Reflect.defineProperty: false

V8:

non-strict: undefined
strict: undefined
Reflect.set: true
Object.defineProperty: TypeError: Invalid typed array index
Reflect.defineProperty: false

Chakra

non-strict: TypeError: The ArrayBuffer is detached.
strict: TypeError: The ArrayBuffer is detached.
Reflect.set: false
Object.defineProperty: TypeError: Access index is out of range
Reflect.defineProperty: false

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

function test(name, fn) {
  var ta = new Int32Array(10);
  detach(ta.buffer);
  try {
    print(`${name}: ${fn(ta)}`); } catch (e) { print(`${name}: ${e}`); }
}

test("non-strict", function(ta) { ta[0] = 0; });
test("strict", function(ta) { "use strict"; ta[0] = 0; });
test("Reflect.set", function(ta) { return Reflect.set(ta, 0, 0); });
test("Object.defineProperty", function(ta) { return Object.defineProperty(ta, 0, {value: 0}) });
test("Reflect.defineProperty", function(ta) { return Reflect.defineProperty(ta, 0, {value: 0}) });

And that only covers direct attempts to write to a TypedArray with a detached buffer, there are probably more differences when built-ins are involved. For example the spec may use a bog standard Set call in some TypedArray built-ins, but that doesn't necessarily correspond to a [[Set]] (as performed in a plain assignment) in an actual implementation.

[bathos]

As for an example within ECMAScript, https://tc39.github.io/ecma262/#sec-get-arraybuffer.prototype.bytelength suggests implementations need to throw, but Chrome, Firefox, and Safari all return 0.

We’re relying on the as-implemented behavior for byteLength in our implementation of buffer-related Web IDL type conversions and associated behaviors. Is there a reason not to amend the spec to reflect web reality in this regard? Or is there another reliable test possible which I haven’t spotted?

I’d rather not be relying on a behavior that’s officially “wrong,” but I’m not aware of any alternatives.

[rkirsling]

I was about to fix JSC's ArrayBuffer#byteLength this week before realizing the background here.

(_Specifically, I realized this change might be dangerous upon noticing that no engines currently throw when checking byteLength === 0 on the underlying ArrayBuffer of a WebAssembly.Memory instance after calling memory.grow(1). This was in test code, but it wouldn't surprise me if users are already writing code like this._)

Given the above discussion, the years of cross-browser consistency, the fact that WPT expects this behavior, etc., I'd like to create a normative PR for the next meeting to have the spec match web reality here.

[syg]

@rkirsling That sounds like a great idea and I support it. Might you be interested in expanding the scope of ambition for the PR and also include indexed access, which returns undefined in practice instead of throwing?

Edit: We should probably do an audit of all the differences between implementations and spec right now, and check the behaviors across the engines. I'd be happy to help out here.

[rwaldron]

@rkirsling can you cc me when you open that PR? Thanks!

@syg I'll make sure that Test262 has whatever supporting changes/additions that are necessary here.

[rkirsling]

@syg Sure! Judging from the test results under TypedArrayConstructors/internals, it looks like that should amount to adjusting the IsDetachedBuffer checks in 9.4.5.2, 9.4.5.9, and 9.4.5.10.

[littledan]

Thanks for taking this up, @rkirsling . I am so excited to see this resolved!