Bun.plugin: transpile a copy of typed array onLoad contents

[robobun]

Problem

• Bun.plugin: when onLoad or build.module returns contents as a typed array, the transpiler gets a pointer into that array (src/jsc/bindings/ModuleLoader.cpp:308). It reads them until the transpile ends.
• A worker that writes a SharedArrayBuffer behind contents aborts the process: panic: index out of bounds: the len is 16 but the index is 16. The lexer counts the _ separators of a numeric literal, allocates length - count bytes, then reads the literal again (src/js_parser/lexer.rs:3280). Canary 1.4.3 fails 18 of 18 runs.
• A macro runs JS during the transpile. If it overwrites or detaches an unshared contents buffer, the printer emits garbage: SyntaxError: Unexpected token '='.

Fix

• handleOnLoadResultNotPromise copies the bytes into a WTF::Vector that the OnLoadResult owns. The copy lives until handleVirtualModuleResult returns, after the transpile. An allocation failure rejects the import with an OutOfMemoryError.
• The copy is unconditional for typed arrays. A shared-or-resizable check does not cover a macro that writes or detaches a fixed-length buffer. A string is immutable and stays borrowed.
• Verified: two new tests in test/js/bun/plugin/plugins.test.ts. Both fail on the unfixed debug build and on canary. Also test/js/bun/plugin/ and mock-module.test.ts.
• Self-reviewed: 9 concerns raised, 7 addressed. Notes list the 2 not taken.

Background

• OnLoadResult carries the parsed { contents, loader } result to Bun__transpileVirtualModule. It runs the parser and the printer on contents in place of a file.
• The AST keeps slices of the source for identifier names and string literals. The printer reads them after the macros ran.
• Bun.build plugins already copy contents (JSBundlerPlugin__onLoadAsync).

Notes

Repro for the abort (the fixture in this PR is a tighter version of it). Run bun plugin-sab.ts:

import { isMainThread, Worker, workerData } from "node:worker_threads";
import { plugin } from "bun";

const base = new TextEncoder().encode("export const a = 1_000_000_000_000_000;\n");
if (isMainThread) {
  const bytes = new SharedArrayBuffer(base.length);
  const ready = new SharedArrayBuffer(4);
  new Worker(new URL(import.meta.url), { workerData: { bytes, ready } }).unref();
  const input = new Uint8Array(bytes);
  input.set(base);
  Atomics.wait(new Int32Array(ready), 0, 0, 10_000);
  plugin({
    name: "sab",
    setup(build) {
      build.onResolve({ filter: /.*/, namespace: "sab" }, ({ path }) => ({ path, namespace: "sab" }));
      build.onLoad({ filter: /.*/, namespace: "sab" }, () => ({ contents: input, loader: "ts" }));
    },
  });
  for (let i = 0; i < 2000; i++) await import("sab:m" + i);
  console.log("ok");
  process.exit(0);
} else {
  const { bytes, ready } = workerData;
  const input = new Uint8Array(bytes);
  const at = [];
  for (let i = 0; i < base.length; i++) if (base[i] === 0x5f) at.push(i);
  Atomics.store(new Int32Array(ready), 0, 1);
  Atomics.notify(new Int32Array(ready), 0);
  for (;;) {
    for (const i of at) Atomics.store(input, i, 0x30);
    for (const i of at) Atomics.store(input, i, 0x5f);
  }
}

What the unfixed builds do with the new tests.

• SharedArrayBuffer test, canary 1.4.3 (4ff9193), 18 runs on several cores: 15 abort with the panic above. 3 stop because a literal has a wrong value (1, 570000000000000000000). In those a digit turned into a _ between the two reads, or the lexer parsed a _ as a digit. All 18 fail within 10 imports.
• SharedArrayBuffer test, unfixed debug build, 3 runs: the panic on import 0, 1 and 3. The debug crash handler needs about 5 s to print the trace, so a local bun bd test reports this one as a timeout.
• Macro test, both unfixed builds, for sync onLoad, async onLoad and build.module: SyntaxError: Unexpected token '='. Expected a parameter pattern or a ')' in parameter list. The printer wrote spaces where the names were.

Why the copy does not depend on isShared() or isResizableOrGrowableShared(). Those checks are correct for a native call that does not run JS. This call runs JS: the visit pass calls macros (src/js_parser/visit/visit_expr.rs, src/js_parser_jsc/Macro.rs) on the same thread and the same global object, and the printer runs after them. JS can do these things to an unshared, fixed-length view:

• write it (fill)
• detach it (transfer(), structuredClone with a transfer list, postMessage with a transfer list)
• move its storage: a read of .buffer on a small typed array moves the bytes out of the GC cell, and the old block goes away at the next collection

The macro test in this PR uses a plain new ArrayBuffer(n). It fails with a shared-or-resizable check.

Why the test counts overlapped imports. A release build finishes an import in about 50 µs. On a busy machine the worker's thread may not run during many of them. The fixture counts an import only when the worker completed a pass while it ran, and caps the total so that a starved worker cannot hang the run.

Cost. One malloc and one memcpy of the module source for each plugin-loaded module whose contents is a typed array. The lexer, the visitor and the printer then walk the same bytes, and the printed output is copied once more. String contents take the same path as before.

Behavior that does not change. I compared the fixed debug build with canary on these contents values, through sync onLoad, async onLoad, build.module and require(): empty view, detached view, Buffer.subarray, DataView, Uint16Array, a view on a SharedArrayBuffer, a view on a resizable buffer, an out-of-bounds length-tracking view, a 4 MB module, a string. The results are the same. The fixture and the macro probe also pass with BUN_JSC_validateExceptionChecks=1.

Self-review, the 2 concerns not taken.

• A view of 2 GiB or more now fails with OutOfMemoryError (the WTF::Vector limit). Before, it failed with "File is too large to parse (2 GiB maximum)". Neither build can load such a module.
• The worker test can pass on a broken build if the worker never gets CPU time. It can not fail on a fixed build. The macro test covers the same line with no timing.

Related work.

• Parse a private copy of a shared or resizable buffer in Bun.Transpiler, Bun.markdown, Bun.YAML and Bun.TOML #42310 copies shared or resizable input in Bun.Transpiler, Bun.markdown, Bun.YAML.parse and Bun.TOML.parse. It names this onLoad case as a follow-up. transformSync also runs macros, so a fixed-length buffer has the same macro hazard there.
• Decode a private copy of a JS buffer in the UTF-8 decoders #41574 adds Bun::stableBytes (copy when shared) for the UTF-8 decoders. It takes the same Vector<uint8_t>& storage, so this site can call it later with shared = true.
• Bytes above 0x7F in a typed array are still read as Latin-1. plugin: preserve raw bytes when onLoad/build.module returns an ArrayBufferView #35729 fixes that.
• A bare ArrayBuffer or SharedArrayBuffer is still rejected. plugin: accept ArrayBuffer/SharedArrayBuffer for onLoad contents #33798 accepts them. It adds a second branch next to this one that borrows impl()->data(). That branch needs the same copy.
• Never alias borrowed string bytes on the C++ side of BunString #40404 rewrites the same line (Zig::utf8Slice(view->span())), also as a borrow.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{On-demand reviews are free for the next 9 days. After that, they cost $0.25 per reviewed file.}

Or wait 11 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 07ec876c-3923-49e1-9f37-94ca5873fffd

📥 Commits

Reviewing files that changed from the base of the PR and between 471b586 and ccecc47.

📒 Files selected for processing (4)

• src/jsc/bindings/ModuleLoader.cpp
• src/jsc/bindings/ModuleLoader.h
• test/js/bun/plugin/plugin-shared-contents-fixture.ts
• test/js/bun/plugin/plugins.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status

Reproduced on canary 1.4.3 (4ff9193) with the fixture from this PR. bun test/js/bun/plugin/plugin-shared-contents-fixture.ts aborts with panic: index out of bounds: the len is 16 but the index is 16, or stops on a literal with a wrong value, in 18 of 18 runs. The macro case fails with SyntaxError: Unexpected token '=' on the same build. With this branch, bun bd test test/js/bun/plugin/plugins.test.ts passes (49 tests).

CI (build 114496, finished): 180 of 181 jobs passed. The one red test is test/js/bun/http/serve-pending-promise-abort-leak.test.ts on the x64-asan lane. It fails on main too, and this diff does not touch it. plugins.test.ts passes on every lane, ASAN and Windows included. The diff is ready for a maintainer.

PR: #42405

[claude]

I reviewed this PR and didn't find any bugs. Because it changes memory ownership in the JSC module-loader bindings, a maintainer look at the OnLoadResult lifetime reasoning would still be worthwhile.

What was reviewed:

• OOM path: result.type is still OnLoadResultTypeError at the early return, so the caller's switch takes the reject(onLoadResult.value.error) arm; the throw/stash/tryClearException sequence matches the neighboring error paths.
• Lifetime: onLoadResult is a stack local in handleVirtualModuleResult that outlives Bun__transpileVirtualModule; WTF::Vector<uint8_t> has no inline capacity so its heap buffer pointer survives the two return-by-value moves that the stashed EncodedSlice depends on.
• Tests follow harness conventions (concurrent drain, combined-object assert, tempDir/bunEnv, Atomics.wait deadline + bounded overlap loop, Buffer.alloc over .repeat) and cover sync/async onLoad plus build.module.

Extended reasoning...

Overview

The PR fixes a TOCTOU/memory-safety bug in src/jsc/bindings/ModuleLoader.cpp: when a Bun.plugin onLoad/build.module callback returns typed-array contents, the transpiler previously read the live view->vector() bytes for the duration of the transpile, so a worker writing a SharedArrayBuffer or a macro that mutates/detaches the buffer mid-transpile could abort the process or produce garbage output. The fix copies view->span() into a new WTF::Vector<uint8_t> sourceTextCopy on OnLoadResult and points the EncodedSlice at that owned copy, throwing OutOfMemoryError on tryAppend failure. The header gains the field with a comment stating the no-inline-capacity invariant. Two tests are added to test/js/bun/plugin/plugins.test.ts (a SharedArrayBuffer race fixture and a deterministic macro-overwrite/detach test covering all three entry paths).

Security risks

None introduced. The change strictly narrows the attack surface by replacing a borrowed pointer into user-controlled memory with an owned copy, which is the shape REVIEW.md prescribes ("network/parser callback buffers stored without cloning" is a rejected pattern; "read mutable typed-array vectors once after all observable side effects or copy them"). The OOM branch fails closed via a rejected import. No auth/crypto/permissions code is touched.

Level of scrutiny

Moderate-to-high. The C++ change is only ~10 lines and mirrors neighboring error-path shape exactly, but it lives in JSC bindings and its correctness rests on two lifetime facts I verified: (1) result.type is initialized to OnLoadResultTypeError at function entry and is not changed before the OOM early return, so handleVirtualModuleResult's switch takes the reject arm; (2) the onLoadResult local in handleVirtualModuleResult lives on the stack through the Bun__transpileVirtualModule call, and WTF::Vector<uint8_t>'s default (zero) inline capacity means its move constructor transfers the heap buffer pointer rather than relocating bytes, so the EncodedSlice stashed before the return-by-value chain stays valid. Both hold, and the header comment records the second one. Still, native memory-ownership changes in the module loader are exactly the kind of thing a maintainer familiar with this file should sign off on.

Other factors

Test quality is high and matches repo conventions: added to the existing plugins.test.ts under describe.concurrent, subprocess pipes drained via Promise.all, assertions on a combined { stdout, stderr, exitCode } object, tempDir/bunExe/bunEnv from harness, Buffer.alloc(n, fill).toString() instead of .repeat, no sleep-for-condition (worker readiness via Atomics.wait with a 30s deadline; overlap counted via Atomics.load with a hard total-iteration cap so a starved worker cannot hang the run). The deterministic macro test covers the same code line without any timing dependency and exercises sync onLoad, async onLoad, and build.module, satisfying the "fix the whole class" and "cover the variant matrix" rules. No CODEOWNERS entries cover the changed paths, and there are no outstanding third-party review objections in the timeline.

[robobun]

^{Updated 6:33 PM PT - Sep 11th, 2026}

❌ @robobun, your commit ccecc47 has 1 failures in Build #114496 (All Failures):

• test/js/bun/http/serve-pending-promise-abort-leak.test.ts - code 1 on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 42405

That installs a local version of the PR into your bun-42405 executable, so you can run:

bun-42405 --bun