Review requested:

• @nodejs/ffi

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.31%. Comparing base (a576f1c) to head (8ef6d08).
⚠️ Report is 51 commits behind head on main.

@@           Coverage Diff           @@
##             main   #65051   +/-   ##
=======================================
  Coverage   90.30%   90.31%           
=======================================
  Files         760      760           
  Lines      248526   248521    -5     
  Branches    46886    46896   +10     
=======================================
+ Hits       224439   224445    +6     
+ Misses      15515    15502   -13     
- Partials     8572     8574    +2     

... and 28 files with indirect coverage changes

• ❄️ 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.

Converted to draft as test-ffi-fast-buffer is failing on ci.nodejs.org/job/node-test-commit-aix/64314

---
duration_ms: 1355.201
exitcode: 1
severity: fail
stack: |-
  Test failure: 'fast FFI refreshes cached temporary string buffers'
  Location: test/ffi/test-ffi-fast-buffer.js:99:1
  AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

  '' !== 'yello'

      at TestContext.<anonymous> (/home/iojs/build/workspace/node-test-commit-aix/nodes/aix72-power9/test/ffi/test-ffi-fast-buffer.js:108:12)
      at Test.runInAsyncScope (node:async_hooks:227:14)
      at Test.run (node:internal/test_runner/test:1397:25)
      at Test.processPendingSubtests (node:internal/test_runner/test:969:18)
      at Test.postRun (node:internal/test_runner/test:1537:19)
      at Test.run (node:internal/test_runner/test:1462:12)
      at process.processTicksAndRejections (node:internal/process/task_queues:104:5)
      at async Test.processPendingSubtests (node:internal/test_runner/test:969:7) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: '',
    expected: 'yello',
    operator: 'strictEqual',
    diff: 'simple'
  }

  (node:38469944) internal/test/binding: These APIs are for internal testing only. Do not use them.
  (Use `node --trace-warnings ...` to show where the warning was created)
  (node:38469944) ExperimentalWarning: FFI is an experimental feature and might change at any time
...

There was a bug in test.

AIX does not support Node’s PPC64 fast-call trampoline, so it exposes the test’s invalid assumption: the pointer returned by overwrite_string() points into temporary storage owned by Node’s generic FFI invocation. That storage is only valid during the native call. AIX frees/reuses the storage immediately, yielding ''.

Fast-path platforms retain the cached Buffer, so "yello" remains readable.

It was fixed in 89cc17f by observing the mutation during the native call instead of returning a pointer to temporary storage.

I'll re-request review after the CI is successful.

The ffi contract explicitly says JavaScript strings are copied to temporary NUL-terminated UTF-8 storage “for the duration of the call”

string values are copied to temporary NUL-terminated UTF-8 strings for the duration of the call.

The fast path happens to retain reusable buffers longer, but that is an implementation detail, not a lifetime guarantee.
The test should return the observed byte by value as done in 8ef6d08 and not the pointer.

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

@mcollina @jasnell One of the tests was incorrect, and it surfaced in CI failures in AIX.
It's fixed now in 8ef6d08 and CI is successful.

Can you please provide your approval again?

Landed in 54ac460