Skip to content

Conversation

@anonrig
Copy link
Member

@anonrig anonrig commented Jul 28, 2025

Here's a repro we saw couple of weeks ago:

Findings

  • Passing a v8 promise to a C++ method that has a non-void return value might trigger GC
  • FastOneByteString's should be addressed before any GC occurs
  • v8 promise triggering GC will corrupt FastOneByteString

Reproduction

  • Force trigger GC while unwrapping jsg::Promise (v8::Function::New can trigger GC)
  • Access v8 fast one byte string value will show corruption

Fix (alternatives)

  • One alternative is to remove v8::FastOneByteString
  • Second alternative is to unwrap FastOneByteString before any other parameters

@jasnell
Copy link
Collaborator

jasnell commented Jul 28, 2025

+1 to removing FastOneByteString

@anonrig anonrig force-pushed the yagiz/repro-v8-fast-api-bug branch from d107bb8 to c97b321 Compare July 28, 2025 17:35
@github-actions
Copy link

The generated output of @cloudflare/workers-types has been changed by this PR. If this is intentional, run just generate-types to update the snapshot. Alternatively, you can download the full generated types:

Full Type Diff

@anonrig anonrig closed this Jul 28, 2025
jasnell added a commit to nodejs/node that referenced this pull request Aug 7, 2025
Minor warning about the use of FastOneByteString.

PR-URL: #59275
Refs: cloudflare/workerd#4625
Reviewed-By: Yagiz Nizipli <[email protected]>
panva pushed a commit to panva/node that referenced this pull request Aug 7, 2025
Minor warning about the use of FastOneByteString.

PR-URL: nodejs#59275
Refs: cloudflare/workerd#4625
Reviewed-By: Yagiz Nizipli <[email protected]>
targos pushed a commit to nodejs/node that referenced this pull request Aug 8, 2025
Minor warning about the use of FastOneByteString.

PR-URL: #59275
Refs: cloudflare/workerd#4625
Reviewed-By: Yagiz Nizipli <[email protected]>
mete0rfish pushed a commit to mete0rfish/node-contribute that referenced this pull request Aug 9, 2025
Minor warning about the use of FastOneByteString.

PR-URL: nodejs#59275
Refs: cloudflare/workerd#4625
Reviewed-By: Yagiz Nizipli <[email protected]>
panva pushed a commit to panva/node that referenced this pull request Aug 9, 2025
Minor warning about the use of FastOneByteString.

PR-URL: nodejs#59275
Refs: cloudflare/workerd#4625
Reviewed-By: Yagiz Nizipli <[email protected]>
RafaelGSS pushed a commit to nodejs/node that referenced this pull request Aug 12, 2025
Minor warning about the use of FastOneByteString.

PR-URL: #59275
Refs: cloudflare/workerd#4625
Reviewed-By: Yagiz Nizipli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants