node:vm: reject invalid cachedData instead of crashing - #32839
Open
robobun wants to merge 13 commits into
Open
Claude / Claude Code Review
completed
Jul 28, 2026 in 16m 32s
Code review found 1 potential issue
Found 1 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/jsc/bindings/NodeVMScript.cpp:176-181 |
NodeVMScript retains a redundant copy of cachedData after unwrapCachedData() copies it |
Annotations
Check warning on line 181 in src/jsc/bindings/NodeVMScript.cpp
claude / Claude Code Review
NodeVMScript retains a redundant copy of cachedData after unwrapCachedData() copies it
Now that `unwrapCachedData()` copies the payload into its own `MallocSpan`, `m_options.cachedData` on `NodeVMScript` is dead storage after `constructScript` returns — its only reader is line 167 — but it's still retained for the script's lifetime, so an accepted-`cachedData` `vm.Script` now holds ~2× the payload bytes (Vector + MallocSpan copy) where before it held 1×. Consider `cachedData = {};` at the end of the `if (!cachedData.isEmpty())` block, or drop the field from `ScriptOptions` entirel
Loading