Skip to content

src: handle empty MaybeLocal in cjs_lexer::Parse#63885

Open
anonrig wants to merge 1 commit into
nodejs:mainfrom
anonrig:fix-cjs-lexer-tolocalchecked
Open

src: handle empty MaybeLocal in cjs_lexer::Parse#63885
anonrig wants to merge 1 commit into
nodejs:mainfrom
anonrig:fix-cjs-lexer-tolocalchecked

Conversation

@anonrig

@anonrig anonrig commented Jun 13, 2026

Copy link
Copy Markdown
Member

CreateString() and Parse() in src/node_cjs_lexer.cc unconditionally called ToLocalChecked() on the results of String::NewFromOneByte(), String::NewFromUtf8() and Set::Add(). If string or handle allocation fails or an exception is pending on the isolate, these return an empty MaybeLocal and ToLocalChecked() aborts the whole process:

FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal
 1: node::OnFatalError(char const*, char const*)
 3: node::cjs_lexer::Parse(v8::FunctionCallbackInfo<v8::Value> const&)

Parse() is on the hot path of every ESM import of a CJS module (cjsPreparseModuleExports in lib/internal/modules/esm/translators.js), which sits on a normal JS frame and can handle a thrown exception. This change makes CreateString() return MaybeLocal<String> and propagates failure from Parse() as a regular pending JavaScript exception, so an allocation failure surfaces as a recoverable module load error instead of a fatal abort. This matches the failure behavior of the previous WASM-based cjs-module-lexer and the MaybeLocal handling conventions used elsewhere in src/ (e.g. AsArray in src/util-inl.h).

No regression test: the failure requires V8 string/handle allocation to fail inside the binding (export names are bounded by source length, and Set::Add on a fresh Set runs no user code), which cannot be triggered deterministically from JS. Verified locally that test/es-module passes and that the stress reproducer from the issue runs clean.

Fixes: #63323
Refs: #61456

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Jun 13, 2026
@richardlau richardlau added the lts-watch-v24.x PRs that may need to be released in v24.x label Jun 13, 2026
CreateString() and Parse() in node_cjs_lexer.cc unconditionally called
ToLocalChecked() on the results of String::NewFromOneByte(),
String::NewFromUtf8() and Set::Add(). If string or handle allocation
fails or an exception is pending on the isolate, these return an empty
MaybeLocal and ToLocalChecked() aborts the process with
"FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal".

Since Parse() is on the hot path of every ESM import of a CJS module
(cjsPreparseModuleExports), propagate the failure as a regular pending
JavaScript exception instead so callers can recover.

Fixes: nodejs#63323
Refs: nodejs#61456
Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
@anonrig anonrig force-pushed the fix-cjs-lexer-tolocalchecked branch from a2d3572 to 5990609 Compare June 13, 2026 16:48
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 13, 2026
mcollina

This comment was marked as off-topic.

@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

@anonrig anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 13, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 13, 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

c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue Add this label to land a pull request using GitHub Actions. lts-watch-v24.x PRs that may need to be released in v24.x needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash in cjs_lexer::Parse since v24.14.0: FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal

6 participants