feat: support SharedArrayBuffer in napi_create_dataview (#183)

fix

Author: toyobayashi

packages/emnapi/src/value/create.ts

@@ -550,46 +550,46 @@ export function napi_create_dataview (
   byte_offset: size_t,
   result: Pointer<napi_value>
 ): napi_status {
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  let value: number
-
   return $PREAMBLE!(env, (envObject) => {
     $CHECK_ARG!(envObject, arraybuffer)
     $CHECK_ARG!(envObject, result)
     from64('byte_length')
     from64('byte_offset')
     byte_length = byte_length >>> 0
     byte_offset = byte_offset >>> 0
-    const handle = emnapiCtx.handleStore.get(arraybuffer)!
-    if (!handle.isArrayBuffer()) {
-      return envObject.setLastError(napi_status.napi_invalid_arg)
-    }
-    const buffer = handle.value
+    const value = emnapiCtx.handleStore.get(arraybuffer)!.value
 
-    if ((byte_length + byte_offset) > buffer.byteLength) {
-      const err: RangeError & { code?: string } = new RangeError('byte_offset + byte_length should be less than or equal to the size in bytes of the array passed in')
-      err.code = 'ERR_NAPI_INVALID_DATAVIEW_ARGS'
-      throw err
-    }
-
-    const dataview = new DataView(buffer, byte_offset, byte_length)
-    if (buffer === wasmMemory.buffer) {
-      if (!emnapiExternalMemory.wasmMemoryViewTable.has(dataview)) {
-        emnapiExternalMemory.wasmMemoryViewTable.set(dataview, {
-          Ctor: DataView,
-          address: byte_offset,
-          length: byte_length,
-          ownership: ReferenceOwnership.kUserland,
-          runtimeAllocated: 0
-        })
+    const createDataview = (buffer: ArrayBuffer | SharedArrayBuffer): napi_status => {
+      if ((byte_length + byte_offset) > buffer.byteLength) {
+        const err: RangeError & { code?: string } = new RangeError('byte_offset + byte_length should be less than or equal to the size in bytes of the array passed in')
+        err.code = 'ERR_NAPI_INVALID_DATAVIEW_ARGS'
+        throw err
       }
+      const dataview = new DataView(buffer, byte_offset, byte_length)
+      if (buffer === wasmMemory.buffer) {
+        if (!emnapiExternalMemory.wasmMemoryViewTable.has(dataview)) {
+          emnapiExternalMemory.wasmMemoryViewTable.set(dataview, {
+            Ctor: DataView,
+            address: byte_offset,
+            length: byte_length,
+            ownership: ReferenceOwnership.kUserland,
+            runtimeAllocated: 0
+          })
+        }
+      }
+
+      from64('result')
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      const v = emnapiCtx.addToCurrentScope(dataview).id
+      makeSetValue('result', 0, 'v', '*')
+      return envObject.getReturnStatus()
     }
-    from64('result')
 
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
-    value = emnapiCtx.addToCurrentScope(dataview).id
-    makeSetValue('result', 0, 'value', '*')
-    return envObject.getReturnStatus()
+    if (value instanceof ArrayBuffer || emnapiExternalMemory.isSharedArrayBuffer(value)) {
+      return createDataview(value)
+    } else {
+      return envObject.setLastError(napi_status.napi_invalid_arg)
+    }
   })
 }
 

packages/test/CMakeLists.txt

@@ -285,6 +285,7 @@ add_test("array" "./array/binding.c" OFF)
 add_test("constructor" "./constructor/binding.c;./constructor/test_null.c" OFF)
 add_test("conversion" "./conversion/test_conversions.c;./conversion/test_null.c" OFF)
 add_test("dataview" "./dataview/binding.c" OFF)
+target_compile_definitions("dataview" PRIVATE "NAPI_EXPERIMENTAL" "NODE_API_EXPERIMENTAL_NO_WARNING")
 add_test("date" "./date/binding.c" OFF)
 add_test("error" "./error/binding.c" OFF)
 add_test("exception" "./exception/binding.c" OFF)

packages/test/dataview/binding.c

@@ -20,9 +20,18 @@ static napi_value CreateDataView(napi_env env, napi_callback_info info) {
 
   bool is_arraybuffer;
   NODE_API_CALL(env, napi_is_arraybuffer(env, arraybuffer, &is_arraybuffer));
-  NODE_API_ASSERT(env, is_arraybuffer,
-              "Wrong type of arguments. Expects a ArrayBuffer as the first "
-              "argument.");
+
+  if (!is_arraybuffer) {
+    bool is_sharedarraybuffer;
+    NODE_API_CALL(
+        env,
+        node_api_is_sharedarraybuffer(env, arraybuffer, &is_sharedarraybuffer));
+    NODE_API_ASSERT(env,
+                    is_sharedarraybuffer,
+                    "Wrong type of arguments. Expects a SharedArrayBuffer or "
+                    "ArrayBuffer as the first "
+                    "argument.");
+  }
 
   napi_valuetype valuetype1;
   NODE_API_CALL(env, napi_typeof(env, args[1], &valuetype1));

packages/test/dataview/dataview.test.js

@@ -6,7 +6,7 @@ const assert = require('assert')
 const { load } = require('../util')
 
 module.exports = load('dataview').then(test_dataview => {
-// Test for creating dataview
+  // Test for creating dataview with ArrayBuffer
   {
     const buffer = new ArrayBuffer(128)
     const template = Reflect.construct(DataView, [buffer])
@@ -16,11 +16,31 @@ module.exports = load('dataview').then(test_dataview => {
             `Expect ${theDataview} to be a DataView`)
   }
 
+  // Test for creating dataview with SharedArrayBuffer
+  {
+    const buffer = new SharedArrayBuffer(128)
+    const template = new DataView(buffer)
+
+    const theDataview = test_dataview.CreateDataViewFromJSDataView(template)
+    assert.ok(theDataview instanceof DataView,
+              `Expect ${theDataview} to be a DataView`)
+
+    assert.strictEqual(template.buffer, theDataview.buffer)
+  }
+
   // Test for creating dataview with invalid range
   {
     const buffer = new ArrayBuffer(128)
     assert.throws(() => {
       test_dataview.CreateDataView(buffer, 10, 200)
     }, RangeError)
   }
+
+  // Test for creating dataview with SharedArrayBuffer and invalid range
+  {
+    const buffer = new SharedArrayBuffer(128)
+    assert.throws(() => {
+      test_dataview.CreateDataView(buffer, 10, 200)
+    }, RangeError)
+  }
 })