ffi: add new methods and error codes by ShogunPanda · Pull Request #62762 · nodejs/node

ShogunPanda · 2026-04-15T18:28:25Z

Added null support in DynamicLibrary loading on non-Windows (dlopen(NULL) semantics), plus docs clarifying null is not supported on Windows.
Added a new FFI benchmark benchmark/ffi/getpid.js using ffi.dlopen(null, ...) + uv_os_getpid. Thanks to @bengl for Foreign Function Interface (FFI) implementation #46905
Documented new FFI error codes in doc/api/errors.md as asked from @jasnell in lib,src,test,doc: add node:ffi module #62072 (comment)
Code documented behavior of why JS callback may abort the process. This address what requested by @jasnell in lib,src,test,doc: add node:ffi module #62072 (comment).

As promised to @justjake, also the following followups requested in #62072 (comment):

Added ffi.exportArrayBuffer(...) and ffi.exportArrayBufferView(...).
Added raw pointer helper ffi.getRawPointer(source) (Buffer | ArrayBuffer | ArrayBufferView), with strong “unsafe/dangerous” docs and tests.
Moved byte-export copying to native code (exportBytes) to avoid extra JS Buffer wrapper allocations.

nodejs-github-bot · 2026-04-15T18:28:31Z

Review requested:

@nodejs/config
@nodejs/gyp
@nodejs/performance

justjake · 2026-04-15T18:32:06Z

Great! Thank you. It looks like I won't need my own unsafe-pointer once node:ffi gets stabilized 🎉 😄

codecov · 2026-04-15T19:52:19Z

Codecov Report

❌ Patch coverage is 57.20165% with 104 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.66%. Comparing base (d7ab027) to head (1fee1dd).
⚠️ Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
src/ffi/data.cc	41.17%	45 Missing and 15 partials ⚠️
src/node_ffi.cc	46.80%	24 Missing and 1 partial ⚠️
src/ffi/types.cc	59.57%	19 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #62762      +/-   ##
==========================================
- Coverage   89.69%   89.66%   -0.03%     
==========================================
  Files         706      706              
  Lines      218143   218304     +161     
  Branches    41730    41772      +42     
==========================================
+ Hits       195655   195746      +91     
- Misses      14401    14475      +74     
+ Partials     8087     8083       -4

Files with missing lines	Coverage Δ
lib/ffi.js	`96.56% <100.00%> (+0.71%)`	⬆️
src/node_errors.h	`86.48% <ø> (ø)`
src/node_ffi.h	`72.72% <ø> (ø)`
src/ffi/types.cc	`59.82% <59.57%> (+0.14%)`	⬆️
src/node_ffi.cc	`69.04% <46.80%> (-0.04%)`	⬇️
src/ffi/data.cc	`71.06% <41.17%> (-7.72%)`	⬇️

... and 42 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.

meixg · 2026-04-16T07:41:30Z

LGTM
One small API thought: would it be better to expose this as something like getRawReference() instead of getRawPointer(), returning both the pointer and byteLength? For byte sources, the pointer is usually
meant to be used together with its length.

ShogunPanda · 2026-04-16T09:51:45Z

@meixg I don't think it is acually necessary. When using the function you already own the source (getRawPointer(buffer)) so you can call the byteLength directly, isn't it?

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

addaleax · 2026-04-17T11:53:53Z

+  if (Buffer::HasInstance(args[0])) {
+    ptr = reinterpret_cast<uintptr_t>(Buffer::Data(args[0]));
+  } else if (args[0]->IsArrayBuffer()) {


This is already covered by the IsArrayBufferView branch below

Suggested change

if (Buffer::HasInstance(args[0])) {

ptr = reinterpret_cast<uintptr_t>(Buffer::Data(args[0]));

} else if (args[0]->IsArrayBuffer()) {

if (args[0]->IsArrayBuffer()) {

addaleax · 2026-04-17T11:54:22Z

      std::memchr(*value, '\0', value.length()) != nullptr) {
-    env->ThrowTypeError((label + " must not contain null bytes").c_str());
+    THROW_ERR_INVALID_ARG_VALUE(
+        env, (label + " must not contain null bytes").c_str());


Suggested change

env, (label + " must not contain null bytes").c_str());

env, "%s must not contain null bytes", label);

Ideally, this would be applied to all other formatting operations too

addaleax · 2026-04-17T11:54:28Z

                      " must have either 'returns', 'return' or 'result' "
                      "property";
-    env->ThrowTypeError(msg.c_str());
+    THROW_ERR_INVALID_ARG_VALUE(env, msg.c_str());


Suggested change

THROW_ERR_INVALID_ARG_VALUE(env, msg.c_str());

THROW_ERR_INVALID_ARG_VALUE(env, msg);

addaleax · 2026-04-17T11:54:35Z

                      " must have either 'parameters' or 'arguments' "
                      "property";
-    env->ThrowTypeError(msg.c_str());
+    THROW_ERR_INVALID_ARG_VALUE(env, msg.c_str());


Suggested change

THROW_ERR_INVALID_ARG_VALUE(env, msg.c_str());

THROW_ERR_INVALID_ARG_VALUE(env, msg);

addaleax · 2026-04-17T11:54:41Z

      std::string msg =
          "Return value type of function " + name + " must be a string";
-      env->ThrowTypeError(msg.c_str());
+      THROW_ERR_INVALID_ARG_VALUE(env, msg.c_str());


Suggested change

THROW_ERR_INVALID_ARG_VALUE(env, msg.c_str());

THROW_ERR_INVALID_ARG_VALUE(env, msg);

addaleax · 2026-04-17T11:58:37Z

          ("Argument " + std::to_string(index) +
           " must be a buffer, an ArrayBuffer, a string, or a bigint")
              .c_str());


Suggested change

("Argument " + std::to_string(index) +

" must be a buffer, an ArrayBuffer, a string, or a bigint")

.c_str());

"Argument %s"

" must be a buffer, an ArrayBuffer, a string, or a bigint", index);

addaleax · 2026-04-17T11:58:56Z

+                                    ("Argument " + std::to_string(index) +
+                                     " must be a non-negative pointer bigint")
+                                        .c_str());


Suggested change

("Argument " + std::to_string(index) +

" must be a non-negative pointer bigint")

.c_str());

"Argument %s"

" must be a non-negative pointer bigint", index);

addaleax · 2026-04-17T11:59:30Z

+            ("Invalid ArrayBuffer backing store for argument " +
+             std::to_string(index))
+                .c_str());


Suggested change

("Invalid ArrayBuffer backing store for argument " +

std::to_string(index))

.c_str());

"Invalid ArrayBuffer backing store for argument %s",

index);

addaleax · 2026-04-17T11:59:59Z

            ("Invalid ArrayBufferView backing store for argument " +
             std::to_string(index))
                .c_str());


Suggested change

("Invalid ArrayBufferView backing store for argument " +

std::to_string(index))

.c_str());

"Invalid ArrayBufferView backing store for argument %s", index);

addaleax · 2026-04-17T12:00:25Z

+  if (Buffer::HasInstance(args[0])) {
+    source_data = reinterpret_cast<uint8_t*>(Buffer::Data(args[0]));
+    source_len = Buffer::Length(args[0]);
+  } else if (args[0]->IsArrayBuffer()) {


ditto here

Suggested change

if (Buffer::HasInstance(args[0])) {

source_data = reinterpret_cast<uint8_t*>(Buffer::Data(args[0]));

source_len = Buffer::Length(args[0]);

} else if (args[0]->IsArrayBuffer()) {

if (args[0]->IsArrayBuffer()) {

And more broadly than that, you can probably just implement this a lot more easily by using ArrayBufferViewContents

addaleax · 2026-04-17T15:53:53Z

+      return;
+    }
+    library_path = *filename;
+    lib->path_ = std::string(*filename);


Suggested change

lib->path_ = std::string(*filename);

lib->path_ = filename.ToString();

addaleax · 2026-04-17T15:54:13Z

+    if (ThrowIfContainsNullBytes(env, filename, "Library path")) {
+      return;
+    }
+    library_path = *filename;


This is a use-after-free bug – *filename goes out of scope outside of this conditional

addaleax · 2026-04-17T15:55:09Z

-    env->ThrowRangeError(
-        (std::string("The ") + label + " is too large").c_str());
+    THROW_ERR_OUT_OF_RANGE(
+        env, (std::string("The ") + label + " is too large").c_str());


Suggested change

env, (std::string("The ") + label + " is too large").c_str());

"The %s is too large", label);

addaleax · 2026-04-17T15:55:46Z

        (std::string("The ") + label + " exceeds the platform pointer range")
            .c_str());


Suggested change

(std::string("The ") + label + " exceeds the platform pointer range")

.c_str());

"The %s exceeds the platform pointer range", label);

addaleax · 2026-04-17T15:57:16Z

+  if (Buffer::HasInstance(args[0])) {
+    source_data = reinterpret_cast<uint8_t*>(Buffer::Data(args[0]));
+    source_len = Buffer::Length(args[0]);
+  } else if (args[0]->IsArrayBuffer()) {


And more broadly than that, you can probably just implement this a lot more easily by using ArrayBufferViewContents

ShogunPanda requested review from bengl and jasnell April 15, 2026 18:28

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 15, 2026

ShogunPanda mentioned this pull request Apr 15, 2026

lib,src,test,doc: add node:ffi module #62072

Closed

justjake mentioned this pull request Apr 15, 2026

Node.js support anomalyco/opentui#939

Open

meixg approved these changes Apr 16, 2026

View reviewed changes

ShogunPanda changed the title ~~ffi: added new methods and error codes~~ ffi: add new methods and error codes Apr 16, 2026

ShogunPanda force-pushed the ffi-followup branch from 927da3b to 3e3b1da Compare April 16, 2026 15:15

lib: add new methods and error codes

1fee1dd

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

ShogunPanda force-pushed the ffi-followup branch from 3e3b1da to 1fee1dd Compare April 16, 2026 15:17

addaleax reviewed Apr 17, 2026

View reviewed changes

	env, (label + " must not contain null bytes").c_str());
	env, "%s must not contain null bytes", label);

	THROW_ERR_INVALID_ARG_VALUE(env, msg.c_str());
	THROW_ERR_INVALID_ARG_VALUE(env, msg);

	lib->path_ = std::string(*filename);
	lib->path_ = filename.ToString();

	env, (std::string("The ") + label + " is too large").c_str());
	"The %s is too large", label);

		(std::string("The ") + label + " exceeds the platform pointer range")
		.c_str());

	(std::string("The ") + label + " exceeds the platform pointer range")
	.c_str());
	"The %s exceeds the platform pointer range", label);

Uh oh!

Conversation

ShogunPanda commented Apr 15, 2026

Uh oh!

nodejs-github-bot commented Apr 15, 2026

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

meixg commented Apr 16, 2026

Uh oh!

ShogunPanda commented Apr 16, 2026

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!

5 participants

justjake commented Apr 15, 2026 •

edited

Loading

codecov bot commented Apr 15, 2026 •

edited

Loading