node-api: add napi_create_external_sharedarraybuffer by bnoordhuis · Pull Request #62623 · nodejs/node

bnoordhuis · 2026-04-07T09:16:02Z

Creates a SharedArrayBuffer from externally managed memory.

Fixes: #62259

Creates a SharedArrayBuffer from externally managed memory. Fixes: nodejs#62259

nodejs-github-bot · 2026-04-07T09:16:07Z

Review requested:

@nodejs/node-api

codecov · 2026-04-07T11:38:36Z

Codecov Report

❌ Patch coverage is 80.76923% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.78%. Comparing base (f48ac91) to head (5d2dd37).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
src/js_native_api_v8.cc	80.76%	0 Missing and 5 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #62623      +/-   ##
==========================================
+ Coverage   89.77%   89.78%   +0.01%     
==========================================
  Files         697      697              
  Lines      215749   215775      +26     
  Branches    41304    41295       -9     
==========================================
+ Hits       193681   193740      +59     
+ Misses      14161    14121      -40     
- Partials     7907     7914       +7

Files with missing lines	Coverage Δ
src/js_native_api_v8.cc	`76.54% <80.76%> (+0.05%)`	⬆️

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

mcollina

lgtm

nodejs-github-bot · 2026-04-07T12:16:57Z

CI: https://ci.nodejs.org/job/node-test-pull-request/72534/

legendecas · 2026-04-07T19:16:40Z

src/js_native_api.h

                                 node_api_basic_finalize finalize_cb,
                                 void* finalize_hint,
                                 napi_value* result);
+NAPI_EXTERN napi_status NAPI_CDECL napi_create_external_sharedarraybuffer(


This should be an experimental API first.

Suggested change

NAPI_EXTERN napi_status NAPI_CDECL napi_create_external_sharedarraybuffer(

#ifdef NAPI_EXPERIMENTAL

#define NODE_API_EXPERIMENTAL_HAS_CREATE_EXTERNAL_SHAREDARRAYBUFFER

NAPI_EXTERN napi_status NAPI_CDECL napi_create_external_sharedarraybuffer(

Shouldn't the macro be NODE_API_EXPERIMENTAL_HAS_CREATE_EXTERNAL_SHAREDARRAYBUFFER?

legendecas · 2026-04-07T19:18:39Z

test/node-api/test_buffer/test.js

+  global.gc();
+  await tick(10);
+  console.log('gc3');
+  assert.strictEqual(binding.getDeleterCallCount(), 3);


Can we use const { gcUntil } = require('../../common/gc')? It would be more stable in tests on GC..

This is what other parts of the test file already do. You want me to update it in its entirety?

I think it would be helpful. But non-blocking.

legendecas · 2026-04-07T19:19:06Z

doc/api/n-api.md

+
+<!-- YAML
+added: REPLACEME
+napiVersion: 1


This would be an experimental API first.

Suggested change

napiVersion: 1

legendecas · 2026-04-07T19:20:07Z

doc/api/n-api.md

+Create a `SharedArrayBuffer` with externally managed memory.
+
+See the entry on [`napi_create_external_arraybuffer`][] for runtime
+compatibility.


I think a difference worth pointing out here is that the finalizer in napi_create_external_arraybuffer will be invoked on the JS thread. But this finalize_cb could be invoked on an arbitrary thread.

KevinEady · 2026-04-07T20:01:03Z

src/js_native_api.h

                                 node_api_basic_finalize finalize_cb,
                                 void* finalize_hint,
                                 napi_value* result);
+NAPI_EXTERN napi_status NAPI_CDECL napi_create_external_sharedarraybuffer(


New Node-API functions should be prefixed with node_api_ and not napi_.

What is the logic here? napi_create_external_arraybuffer for example is called, well, napi_create_external_arraybuffer.

for context: #38573

KevinEady · 2026-04-07T20:15:43Z

src/js_native_api_v8.cc

+    void (*finalize_cb)(void* external_data, void* finalize_hint),
+    void* finalize_hint,
+    napi_value* result) {
+  return v8impl::NewExternalSharedArrayBuffer(


Should this return napi_no_external_buffers_allowed if external buffers are not allowed, eg. when running in a sandboxed environment? Like napi_create_external_buffer does if V8_SANDBOX macro is defined.

vmoroz · 2026-04-07T20:52:45Z

doc/api/n-api.md

 JavaScript `ArrayBuffer`s are described in
 [Section ArrayBuffer objects][] of the ECMAScript Language Specification.

+#### `napi_create_external_sharedarraybuffer`


Please use the node_api_ prefix for all node Node-API functions instead of napi_.

vmoroz · 2026-04-07T20:56:00Z

src/js_native_api.h

+    napi_env env,
+    void* external_data,
+    size_t byte_length,
+    void (*finalize_cb)(void* external_data, void* finalize_hint),


We typically do not use the inline function pointer types.
Can we use the node_api_basic_finalize here?
I guess it should OK to document there that env is null there.

I did that intentionally, of course. Passing nullptr is just setting up users for segfaults.

I think the semantic of this finalizer is different from node_api_basic_finalize. node_api_basic_finalize is still invoked on the JS thread. But this finalizer could be invoked on non-main-JS thread. I think it is worth a dedicated type.

vmoroz · 2026-04-07T21:00:33Z

src/js_native_api_v8.cc

  return status;
 }

+napi_status NewExternalSharedArrayBuffer(


Can we move this implementation directly into the node_api_create_external_sharedarraybuffer. This is the pattern we use for all other Node-API functions. In practice it avoids having extra frame in the call stack when we debug code and unnecessary redirection.

Sure, but I'm merely copying a pattern that's used elsewhere in this file. Seems a little inconsistent?

vmoroz · 2026-04-07T21:03:41Z

src/js_native_api_v8.cc

+  auto shared_backing_store =
+      std::shared_ptr<v8::BackingStore>(std::move(unique_backing_store));
+  auto shared_array_buffer =
+      v8::SharedArrayBuffer::New(env->isolate, shared_backing_store);


Please add std::move for the shared_backing_store to avoid extra ref counter interlocked increment and decrement.

vmoroz · 2026-04-07T21:05:28Z

src/js_native_api_v8.cc

  return status;
 }

+napi_status NewExternalSharedArrayBuffer(


Please make sure that the node_api_create_external_sharedarraybuffer has the typical set of parameter checks as we do for other Node-API functions.

vmoroz · 2026-04-07T21:10:08Z

src/js_native_api_v8.cc

+  if (finalize_cb != nullptr) {
+    deleter_data = new FinalizerData{finalize_cb, finalize_hint};
+  }
+  auto unique_backing_store = v8::SharedArrayBuffer::NewBackingStore(


It is better to avoid auto for readability when possible.
IMHO, we should only use it for lambdas and iterators.
In other trivial cases like here using the real types can help readability and see the issues sooner like with the missing std::move below.

The counterargument is that repeating oneself at the left- and right-hand side of an assignment often results in unwieldy line noise. auto is also used quite a bit elsewhere in this file, and not just for lambdas.

For the record: I'm pretty much the founder of Node's C++ house style and I've always been a-okay with judicious use of auto.

I think it is okay to use auto when the type is clear: https://github.com/nodejs/node/blob/main/doc/contributing/cpp-style-guide.md#using-auto

I would find this auto good because the right hand side is clear that this is a v8 backing store.

node-api: add napi_create_external_sharedarraybuffer

3b8f808

Creates a SharedArrayBuffer from externally managed memory. Fixes: nodejs#62259

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API. labels Apr 7, 2026

github-project-automation bot added this to Node-API Team Project Apr 7, 2026

github-project-automation bot moved this to Need Triage in Node-API Team Project Apr 7, 2026

bnoordhuis added 2 commits April 7, 2026 11:21

squash! lint

6bc42e2

squash! more lint

32fe154

bnoordhuis force-pushed the fix62259 branch from d461da4 to 32fe154 Compare April 7, 2026 09:40

bnoordhuis added 2 commits April 7, 2026 11:49

squash! more lint

5578c1a

squash! more more lint

5d2dd37

ronag requested review from addaleax, jasnell and joyeecheung April 7, 2026 11:18

mcollina approved these changes Apr 7, 2026

View reviewed changes

ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 7, 2026

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 7, 2026

ronag added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 7, 2026

ronag requested a review from vmoroz April 7, 2026 14:16

legendecas reviewed Apr 7, 2026

View reviewed changes

legendecas removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 7, 2026

legendecas moved this from Need Triage to In Progress in Node-API Team Project Apr 7, 2026

KevinEady reviewed Apr 7, 2026

View reviewed changes

vmoroz reviewed Apr 7, 2026

View reviewed changes

Uh oh!

Conversation

bnoordhuis commented Apr 7, 2026

Uh oh!

nodejs-github-bot commented Apr 7, 2026

Uh oh!

codecov bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Apr 7, 2026

Uh oh!

legendecas Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov bot commented Apr 7, 2026 •

edited

Loading

legendecas Apr 7, 2026 •

edited

Loading