fixup! copy less

packages/ses/NEWS.md

@@ -3,9 +3,10 @@ User-visible changes in SES:
 # Next release
 
 - Adds `ArrayBuffer.p.immutable` and `ArrayBuffer.p.transferToImmutable` as a shim for a future proposal. It makes an ArrayBuffer-like object whose contents cannot be mutated. However, due to limitations of the shim
-  - Unlike `ArrayBuffer` and `SharedArrayBuffer` this ArrayBuffer-like object cannot be transfered or cloned between JS threads.
-  - Unlike `ArrayBuffer` and `SharedArrayBuffer`, this ArrayBuffer-like object cannot be used as the backing store of TypeArrays or DataViews.
-  - On Node 20, we emulate `transferToFixedLength` with a combination of `slice` to copy the original and `structuredClone` to detach it. Node 21 does not have this inefficiency.
+  - Unlike `ArrayBuffer` and `SharedArrayBuffer` this shim's ArrayBuffer-like object cannot be transfered or cloned between JS threads.
+  - Unlike `ArrayBuffer` and `SharedArrayBuffer`, this shim's ArrayBuffer-like object cannot be used as the backing store of TypeArrays or DataViews.
+  - On Node >= 21 we use the builtin `transferToFixed` to transfer exclusive access to the array buffer contents. On Node <= 20, we emulate `transferToFixedLength` with `structuredClone`. On platforms with neither `transferToFixedLength` nor `structuredClone`, we use `slice` to copy the contents, but have no way to detach the original.
+  - Even after the upcoming `transferToImmutable` proposal is implemented by the platform, the current code will still replace it with the shim implementation, in accord with shim best practices. See https://github.com/endojs/endo/pull/2311#discussion_r1632607527 . It will require a later manual step to delete the shim, after manual analysis of the compat implications.
 
 # v1.5.0 (2024-05-06)
 

packages/ses/src/commons.js

@@ -180,28 +180,75 @@ export const arrayBufferSlice = uncurryThis(arrayBufferPrototype.slice);
 const { asUintN: bigIntAsUintN } = BigInt;
 const { structuredClone } = globalThis;
 export const arrayBufferTransferToFixedLength =
-  // @ts-expect-error absent from Node 20, which we still support
+  // @ts-expect-error absent from Node <= 20 and not yet in TS type
+  // eslint-disable-next-line no-nested-ternary
   arrayBufferPrototype.transferToFixedLength
-    ? // @ts-expect-error absent from Node 20, which we still support
+    ? // @ts-expect-error absent from Node <= 20 and not yet in TS type
       uncurryThis(arrayBufferPrototype.transferToFixedLength)
-    : (arrayBuffer, newLength = arrayBuffer.byteLength) => {
-        // There is no `transferToFixedLength` on Node 20, which we still support.
-        // In that case, we use `slice` to get a fixed-length copy and,
-        // if present, the WHATWG HTML `structuredClone` to detach the original.
-
-        // `slice` accepts negative arguments but `transferToFixedLength` does not...
-        // get at the underlying ToIndex operation through `BigInt.asUintN`
-        // (avoiding the redundant allocation of e.g. `ArrayBuffer(newLength)`)
-        // and ToNumber through unary `+` (rather than `Number(newLength)`,
-        // which fails to reject BigInts).
-        newLength = +newLength;
-        bigIntAsUintN(newLength, 0n);
-
-        const copied = arrayBufferSlice(arrayBuffer, 0, newLength);
-        structuredClone &&
-          structuredClone(arrayBuffer, { transfer: [arrayBuffer] });
-        return copied;
-      };
+    : structuredClone
+      ? /**
+         * @param {ArrayBuffer} arrayBuffer
+         * @param {number} newLength
+         * @returns {ArrayBuffer}
+         */
+        (arrayBuffer, newLength = arrayBuffer.byteLength) => {
+          // There is no `transferToFixedLength` on Node <= 20, but there
+          // is web-standard `structuredClone` on Node >= 17, on all modern
+          // browsers, and on many other JS platforms.
+          // In those cases, we first use `structuredClone` to get a fresh
+          // buffer with exclusive access to the underlying data, while
+          // detaching it from the original `arrayBuffer`.
+
+          newLength = +newLength;
+          bigIntAsUintN(newLength, 0n);
+
+          const newBuffer = /** @type {ArrayBuffer} */ (
+            structuredClone(arrayBuffer, {
+              transfer: [arrayBuffer],
+            })
+          );
+          if (newLength >= newBuffer.byteLength) {
+            return newBuffer;
+          }
+          // If the requested length is shorter than the length of `buffer`,
+          // we use `slice` to shorted the returned result, but at the cost
+          // of an extra copy.
+          //
+          // `slice` accepts negative arguments but `transferToFixedLength`
+          // does not...
+          // get at the underlying ToIndex operation through `BigInt.asUintN`
+          // (avoiding the redundant allocation of e.g. `ArrayBuffer(newLength)`)
+          // and ToNumber through unary `+` (rather than `Number(newLength)`,
+          // which fails to reject BigInts).
+          return arrayBufferSlice(newBuffer, 0, newLength);
+        }
+      : /**
+         * @param {ArrayBuffer} arrayBuffer
+         * @param {number} newLength
+         * @returns {ArrayBuffer}
+         */
+        (arrayBuffer, newLength = arrayBuffer.byteLength) => {
+          // There is no `transferToFixedLength` on Node <= 20,
+          // and no `structuredClone` on Node <= 17 and possibly on some
+          // non-browser JavaScript platforms.
+          // In those cases,
+          // and assuming the absence of `transfer`, we cannot detach
+          // the original, but we must still produce a new fresh buffer with
+          // exclusive mutability of its underlying state. We use `slice`
+          // both to make this exclusive copy and size it appropriately.
+
+          // `slice` accepts negative arguments but `transferToFixedLength`
+          // does not...
+          // get at the underlying ToIndex operation through `BigInt.asUintN`
+          // (avoiding the redundant allocation of e.g. `ArrayBuffer(newLength)`)
+          // and ToNumber through unary `+` (rather than `Number(newLength)`,
+          // which fails to reject BigInts).
+          newLength = +newLength;
+          bigIntAsUintN(newLength, 0n);
+
+          const newBuffer = arrayBufferSlice(arrayBuffer, 0, newLength);
+          return newBuffer;
+        };
 
 export const mapSet = uncurryThis(mapPrototype.set);
 export const mapGet = uncurryThis(mapPrototype.get);

packages/ses/src/immutable-array-buffer-shim.js

@@ -8,99 +8,108 @@ import {
   TypeError,
 } from './commons.js';
 
-// If it already exists, don't replace it with the shim.
-if (!('transferToImmutable' in arrayBufferPrototype)) {
-  /**
-   * This class only exists within the shim, as a convience for imperfectly
-   * emulating the proposal, which would not have this class. In the proposal,
-   * `transferToImmutable` makes a new `ArrayBuffer` that inherits from
-   * `ArrayBuffer.prototype` as you'd expect. In the shim, `transferToImmutable`
-   * makes a normal object that inherits from
-   * `ImmutableArrayBufferInternal.prototype`, which has been surgically
-   * altered to inherit from `ArrayBuffer.prototype`. The constructor is
-   * captured for use internal to this module, and is made otherwise inaccessible.
-   * Therefore, `ImmutableArrayBufferInternal.prototype` and all its methods
-   * and accessor functions effectively become hidden intrinsics.
-   *
-   * TODO handle them as hidden intrinsics, so they get hardened when they should.
-   */
-  class ImmutableArrayBufferInternal {
-    /** @type {ArrayBuffer} */
-    #buffer;
-
-    constructor(buffer) {
-      // This also enforces that `buffer` is a genuine `ArrayBuffer`.
-      // This constructor is deleted from the prototype below.
-      this.#buffer = arrayBufferSlice(buffer, 0);
-    }
-
-    get byteLength() {
-      return this.#buffer.byteLength;
-    }
-
-    get detached() {
-      this.#buffer; // shim brand check
-      return false;
-    }
+if ('transferToImmutable' in arrayBufferPrototype) {
+  // If `transferToImmutable` already exists, it may be because the platform
+  // has implemented the proposal this shim emulates.
+  // See https://github.com/endojs/endo/pull/2311#discussion_r1632607527
+  // Conditional shims for non stage 4 features have been seen as problematic by
+  // the community as there is a risk that the program would start relying on a
+  // shim behavior that does not match what the final spec says, resulting in
+  // breakage when the engine starts implementing the feature.
+  // eslint-disable-next-line @endo/no-polymorphic-call
+  console.warn('Overriding existing transferToImmutable with shim');
+}
 
-    get maxByteLength() {
-      // Not underlying maxByteLength, which is irrelevant
-      return this.#buffer.byteLength;
-    }
+/**
+ * This class only exists within the shim, as a convience for imperfectly
+ * emulating the proposal, which would not have this class. In the proposal,
+ * `transferToImmutable` makes a new `ArrayBuffer` that inherits from
+ * `ArrayBuffer.prototype` as you'd expect. In the shim, `transferToImmutable`
+ * makes a normal object that inherits from
+ * `ImmutableArrayBufferInternal.prototype`, which has been surgically
+ * altered to inherit from `ArrayBuffer.prototype`. The constructor is
+ * captured for use internal to this module, and is made otherwise inaccessible.
+ * Therefore, `ImmutableArrayBufferInternal.prototype` and all its methods
+ * and accessor functions effectively become hidden intrinsics.
+ *
+ * TODO handle them as hidden intrinsics, so they get hardened when they should.
+ */
+class ImmutableArrayBufferInternal {
+  /** @type {ArrayBuffer} */
+  #buffer;
 
-    get resizable() {
-      this.#buffer; // shim brand check
-      return false;
-    }
+  constructor(buffer) {
+    // This also enforces that `buffer` is a genuine `ArrayBuffer`.
+    // This constructor is deleted from the prototype below.
+    this.#buffer = arrayBufferSlice(buffer, 0);
+  }
 
-    get immutable() {
-      this.#buffer; // shim brand check
-      return true;
-    }
-
-    slice(begin = 0, end = undefined) {
-      return arrayBufferSlice(this.#buffer, begin, end);
-    }
-
-    resize(_newByteLength = undefined) {
-      this.#buffer; // shim brand check
-      throw TypeError('Cannot resize an immutable ArrayBuffer');
-    }
-
-    transfer(_newLength = undefined) {
-      this.#buffer; // shim brand check
-      throw TypeError('Cannot detach an immutable ArrayBuffer');
-    }
-
-    transferToFixedLength(_newLength = undefined) {
-      this.#buffer; // shim brand check
-      throw TypeError('Cannot detach an immutable ArrayBuffer');
-    }
-
-    transferToImmutable(_newLength = undefined) {
-      this.#buffer; // shim brand check
-      throw TypeError('Cannot detach an immutable ArrayBuffer');
-    }
+  get byteLength() {
+    return this.#buffer.byteLength;
+  }
+
+  get detached() {
+    this.#buffer; // shim brand check
+    return false;
+  }
+
+  get maxByteLength() {
+    // Not underlying maxByteLength, which is irrelevant
+    return this.#buffer.byteLength;
+  }
+
+  get resizable() {
+    this.#buffer; // shim brand check
+    return false;
+  }
+
+  get immutable() {
+    this.#buffer; // shim brand check
+    return true;
+  }
+
+  slice(begin = 0, end = undefined) {
+    return arrayBufferSlice(this.#buffer, begin, end);
+  }
+
+  resize(_newByteLength = undefined) {
+    this.#buffer; // shim brand check
+    throw TypeError('Cannot resize an immutable ArrayBuffer');
+  }
+
+  transfer(_newLength = undefined) {
+    this.#buffer; // shim brand check
+    throw TypeError('Cannot detach an immutable ArrayBuffer');
+  }
+
+  transferToFixedLength(_newLength = undefined) {
+    this.#buffer; // shim brand check
+    throw TypeError('Cannot detach an immutable ArrayBuffer');
   }
 
-  const ImmutableArrayBufferInternalPrototype =
-    ImmutableArrayBufferInternal.prototype;
-  // @ts-expect-error can only delete optionals
-  delete ImmutableArrayBufferInternalPrototype.constructor;
-
-  setPrototypeOf(ImmutableArrayBufferInternalPrototype, arrayBufferPrototype);
-
-  defineProperties(
-    arrayBufferPrototype,
-    getOwnPropertyDescriptors({
-      get immutable() {
-        return false;
-      },
-      transferToImmutable(newLength = undefined) {
-        return new ImmutableArrayBufferInternal(
-          arrayBufferTransferToFixedLength(this, newLength),
-        );
-      },
-    }),
-  );
+  transferToImmutable(_newLength = undefined) {
+    this.#buffer; // shim brand check
+    throw TypeError('Cannot detach an immutable ArrayBuffer');
+  }
 }
+
+const ImmutableArrayBufferInternalPrototype =
+  ImmutableArrayBufferInternal.prototype;
+// @ts-expect-error can only delete optionals
+delete ImmutableArrayBufferInternalPrototype.constructor;
+
+setPrototypeOf(ImmutableArrayBufferInternalPrototype, arrayBufferPrototype);
+
+defineProperties(
+  arrayBufferPrototype,
+  getOwnPropertyDescriptors({
+    get immutable() {
+      return false;
+    },
+    transferToImmutable(newLength = undefined) {
+      return new ImmutableArrayBufferInternal(
+        arrayBufferTransferToFixedLength(this, newLength),
+      );
+    },
+  }),
+);