Skip to content

ffi: preserve strings during reentrant calls#64551

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:ffi-reentrant-callbacks
Open

ffi: preserve strings during reentrant calls#64551
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:ffi-reentrant-callbacks

Conversation

@trivikr

@trivikr trivikr commented Jul 17, 2026

Copy link
Copy Markdown
Member

Fixes: #64550

Temporary buffers used to convert JavaScript strings for Fast FFI calls were
cached per library and argument index. A reentrant callback could make another
FFI call using the same argument index, overwriting or replacing a buffer still
in use by the outer native call.

This changes the cache to be scoped by FFI wrapper and active call depth. Each
reentrant invocation therefore receives separate string storage while buffers
remain reusable across non-overlapping calls.


Assisted-by: openai:gpt-5.6-sol

Cache temporary string conversion buffers by wrapper and active call
depth. This prevents nested FFI calls from overwriting or replacing
buffers still in use by an outer native call.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.6-sol
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@nodejs-github-bot nodejs-github-bot added ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run. labels Jul 17, 2026
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 17, 2026
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.93750% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.24%. Comparing base (608112a) to head (fe9f7d0).
⚠️ Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/ffi/fast-api.js 60.93% 25 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64551      +/-   ##
==========================================
+ Coverage   90.21%   90.24%   +0.02%     
==========================================
  Files         739      739              
  Lines      241650   241737      +87     
  Branches    45542    45560      +18     
==========================================
+ Hits       218013   218147     +134     
+ Misses      15155    15119      -36     
+ Partials     8482     8471      -11     
Files with missing lines Coverage Δ
lib/internal/ffi/fast-api.js 81.26% <60.93%> (-5.33%) ⬇️

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

@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 17, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ffi Issues and PRs related to experimental Foreign Function Interface support. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffi: Fast API string buffer is corrupted by reentrant callbacks

3 participants