Skip to content

Check if the memory is backed by a SAB by checking the constructor name #381

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

Merged
merged 2 commits into from
Jul 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 9 additions & 3 deletions Plugins/PackageToJS/Templates/runtime.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,12 @@ class SwiftRuntime {
// Cache the DataView as it's not a cheap operation
let cachedDataView = new DataView(wasmMemory.buffer);
let cachedUint8Array = new Uint8Array(wasmMemory.buffer);
if (typeof SharedArrayBuffer !== "undefined" && wasmMemory.buffer instanceof SharedArrayBuffer) {
// Check the constructor name of the buffer to determine if it's backed by a SharedArrayBuffer.
// We can't reference SharedArrayBuffer directly here because:
// 1. It may not be available in the global scope if the context is not cross-origin isolated.
// 2. The underlying buffer may be still backed by SAB even if the context is not cross-origin
// isolated (e.g. localhost on Chrome on Android).
if (Object.getPrototypeOf(wasmMemory.buffer).constructor.name === "SharedArrayBuffer") {
Copy link
Preview

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

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

Same here: using constructor.name may be brittle under minification or prototype mutation. A stronger check is wasmMemory.buffer.constructor === globalThis.SharedArrayBuffer with an Object.prototype.toString fallback.

Suggested change
if (Object.getPrototypeOf(wasmMemory.buffer).constructor.name === "SharedArrayBuffer") {
if (wasmMemory.buffer.constructor === globalThis.SharedArrayBuffer ||
Object.prototype.toString.call(wasmMemory.buffer) === "[object SharedArrayBuffer]") {

Copilot uses AI. Check for mistakes.

Copy link
Preview

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

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

Wrap prototype access in optional chaining to avoid runtime errors if the prototype is null: Object.getPrototypeOf(wasmMemory.buffer)?.constructor?.name === 'SharedArrayBuffer'.

Suggested change
if (Object.getPrototypeOf(wasmMemory.buffer).constructor.name === "SharedArrayBuffer") {
if (Object.getPrototypeOf(wasmMemory.buffer)?.constructor?.name === "SharedArrayBuffer") {

Copilot uses AI. Check for mistakes.

// When the wasm memory is backed by a SharedArrayBuffer, growing the memory
// doesn't invalidate the data view by setting the byte length to 0. Instead,
// the data view points to an old buffer after growing the memory. So we have
Expand Down Expand Up @@ -796,8 +801,9 @@ class SwiftRuntime {
throw new Error("threadChannel is not set in options given to SwiftRuntime. Please set it to request transferring objects.");
}
const broker = getMessageBroker(this.options.threadChannel);
const sendingObjects = decodeObjectRefs(sending_objects, sending_objects_count, this.getDataView());
const transferringObjects = decodeObjectRefs(transferring_objects, transferring_objects_count, this.getDataView());
const dataView = this.getDataView();
const sendingObjects = decodeObjectRefs(sending_objects, sending_objects_count, dataView);
const transferringObjects = decodeObjectRefs(transferring_objects, transferring_objects_count, dataView);
broker.request({
type: "request",
data: {
Expand Down
8 changes: 7 additions & 1 deletion Runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,13 @@ export class SwiftRuntime {
// Cache the DataView as it's not a cheap operation
let cachedDataView = new DataView(wasmMemory.buffer);
let cachedUint8Array = new Uint8Array(wasmMemory.buffer);
if (typeof SharedArrayBuffer !== "undefined" && wasmMemory.buffer instanceof SharedArrayBuffer) {

// Check the constructor name of the buffer to determine if it's backed by a SharedArrayBuffer.
// We can't reference SharedArrayBuffer directly here because:
// 1. It may not be available in the global scope if the context is not cross-origin isolated.
// 2. The underlying buffer may be still backed by SAB even if the context is not cross-origin
// isolated (e.g. localhost on Chrome on Android).
if (Object.getPrototypeOf(wasmMemory.buffer).constructor.name === "SharedArrayBuffer") {
Copy link
Preview

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

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

Relying on constructor.name can break if code is minified or prototypes are altered. Consider using a direct equality check when globalThis.SharedArrayBuffer is available (wasmMemory.buffer.constructor === globalThis.SharedArrayBuffer) or fall back to Object.prototype.toString.call(wasmMemory.buffer) === '[object SharedArrayBuffer]' for more robust detection.

Suggested change
if (Object.getPrototypeOf(wasmMemory.buffer).constructor.name === "SharedArrayBuffer") {
if (
(globalThis.SharedArrayBuffer && wasmMemory.buffer.constructor === globalThis.SharedArrayBuffer) ||
Object.prototype.toString.call(wasmMemory.buffer) === '[object SharedArrayBuffer]'
) {

Copilot uses AI. Check for mistakes.

Copy link
Preview

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

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

Accessing Object.getPrototypeOf(...).constructor without checking for null/undefined can throw if buffer has a null prototype. Add optional chaining: Object.getPrototypeOf(wasmMemory.buffer)?.constructor?.name === 'SharedArrayBuffer'.

Suggested change
if (Object.getPrototypeOf(wasmMemory.buffer).constructor.name === "SharedArrayBuffer") {
if (Object.getPrototypeOf(wasmMemory.buffer)?.constructor?.name === "SharedArrayBuffer") {

Copilot uses AI. Check for mistakes.

// When the wasm memory is backed by a SharedArrayBuffer, growing the memory
// doesn't invalidate the data view by setting the byte length to 0. Instead,
// the data view points to an old buffer after growing the memory. So we have
Expand Down