Skip to content

ffi: refresh cached string buffers on every call - #65051

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:ffi-temp-string-buffer-reuse
Open

ffi: refresh cached string buffers on every call#65051
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:ffi-temp-string-buffer-reuse

Conversation

@trivikr

@trivikr trivikr commented Aug 5, 2026

Copy link
Copy Markdown
Member

Fixes: #65050

Native code can mutate temporary string storage during an FFI call. Rewrite cached buffers on every conversion so a later call with the same JavaScript string receives a fresh copy of its UTF-8 bytes.


Assisted-by: codex: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 Aug 5, 2026
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (bf2f995) to head (8f76513).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65051      +/-   ##
==========================================
- Coverage   90.33%   90.33%   -0.01%     
==========================================
  Files         760      760              
  Lines      248522   248517       -5     
  Branches    46904    46889      -15     
==========================================
- Hits       224513   224486      -27     
- Misses      15444    15453       +9     
- Partials     8565     8578      +13     
Files with missing lines Coverage Δ
lib/internal/ffi/fast-api.js 95.54% <ø> (+0.76%) ⬆️

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

@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 7, 2026
MILLERMARRU

This comment was marked as low quality.

Native code can mutate temporary string storage during an FFI call.
Rewrite cached buffers on every conversion so a later call with the
same JavaScript string receives a fresh copy of its UTF-8 bytes.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex:gpt-5.6-sol
@trivikr
trivikr force-pushed the ffi-temp-string-buffer-reuse branch from a88fe4c to 8f76513 Compare August 9, 2026 23:22

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@trivikr trivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 10, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 10, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikr trivikr added request-ci Add this label to start a Jenkins CI on a PR. needs-ci PRs that need a full CI run. and removed needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR. labels Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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 reuses mutated temporary string buffer without restoring its contents

5 participants