Skip to content

resolver: add bun:main to HardcodedModule.Alias (fix flaky test)#29719

Merged
Jarred-Sumner merged 2 commits into
mainfrom
claude/flamboyant-mcclintock-94cd25
Apr 26, 2026
Merged

resolver: add bun:main to HardcodedModule.Alias (fix flaky test)#29719
Jarred-Sumner merged 2 commits into
mainfrom
claude/flamboyant-mcclintock-94cd25

Conversation

@dylan-conway

Copy link
Copy Markdown
Member

What

Add bun:main to HardcodedModule.Alias.bun_extra_alias_kvs so the runtime transpiler stops rewriting import("bun:main") into import("main").

Why (the flake)

bun-main-entry-point.test.ts has been flaky since it landed in #29450. The test was never exercising the code path it claimed to:

  • bun:main is in HardcodedModule.map but was missing from the Alias map
  • so RuntimeTranspilerStore.zig:534 stripped the bun: prefix, leaving import("main") in the emitted JS
  • at runtime that fell through to resolveAndAutoInstall, which fetched the npm main package (main@1000.0.1) over the network
  • the test's typeof m !== "object" check passed against the npm package, so it "passed"
  • when the registry fetch was slow or failed on CI, stdout was empty → flake

Confirmed by observing require.cache gain ~/.bun/install/cache/main@1000.0.1@@@1/index.js after the import, and by reproducing the failure deterministically with [install] auto = "disable".

Test changes

  • Add package.json: "{}" to the temp dirs so auto-install is off — any future regression fails loudly with "Cannot find package 'main'" instead of silently passing via npm
  • Assert Object.keys(m).length === 0 (the real wrapper exports nothing; the npm package exports default,length,name,prototype)
  • Collapse assertions into one toEqual({stdout, stderr, exitCode, signalCode}) so a failure shows the child's stderr
  • Update the preload test's expected order — now that import("bun:main") actually evaluates the wrapper, entry.mjs runs before the preload's await resumes (ENTRY_OK\nPRELOAD_OK\n)

How did you verify your code works?

  • bun bd test test/js/bun/resolve/bun-main-entry-point.test.ts — 3 pass, 30 consecutive runs on Windows
  • USE_SYSTEM_BUN=1 bun test ... — 2 fail (correctly catches the bug on unfixed bun)
  • bun bd test test/js/bun/http/bun-serve-html-entry.test.ts -t "bun:main" — pass
  • bun bd test test/js/bun/resolve/import-meta*.test.* — 47 pass
  • bun bd test test/js/node/module/node-module-module.test.js — 28 pass
  • bun run zig:check-all — pass

…alled

`bun:main` was in `HardcodedModule.map` but missing from
`Alias.bun_extra_alias_kvs`, so the runtime transpiler's import-record
pass (RuntimeTranspilerStore.zig:534) didn't recognise it as a builtin
and stripped the `bun:` prefix, leaving a bare `import("main")` in the
emitted JS. At runtime that fell through `_resolve` to
`resolveAndAutoInstall`, which fetched the npm `main` package over the
network. The bun-main-entry-point tests "passed" by loading that npm
package and flaked whenever the registry fetch was slow or failed.

Adding the alias keeps the specifier intact so it round-trips through
the existing `_resolve` / `getHardcodedModule` paths and resolves to the
ServerEntryPoint wrapper as intended.

Also harden the tests: disable auto-install via an empty package.json,
assert the wrapper namespace has no exports (the npm package has four),
collapse the assertions into one toEqual so a future failure surfaces
stderr/exit/signal, and update the preload test's expected output order
now that import("bun:main") actually evaluates the wrapper (entry runs
before the preload's await resumes).
@robobun

robobun commented Apr 25, 2026

Copy link
Copy Markdown
Collaborator
Updated 2:08 PM PT - Apr 25th, 2026

@dylan-conway, your commit 33287f5 has 1 failures in Build #47932 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 29719

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

bun-29719 --bun

@coderabbitai

coderabbitai Bot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: dd9b2d28-fc7e-47c9-b2b4-14197dd26ede

📥 Commits

Reviewing files that changed from the base of the PR and between ed1c48f and cdc39bb.

📒 Files selected for processing (2)
  • src/bun.js/HardcodedModule.zig
  • test/js/bun/resolve/bun-main-entry-point.test.ts

Walkthrough

This pull request adds support for the "bun:main" built-in module specifier by introducing it to the HardcodedModule enum and updating Bun-specific alias tables. Test files are enhanced with stricter validation of the module wrapper and improved isolation measures.

Changes

Cohort / File(s) Summary
Module Resolution
src/bun.js/HardcodedModule.zig
Added enum mapping and alias table entries to support "bun:main" as a built-in module specifier.
Test Suite
test/js/bun/resolve/bun-main-entry-point.test.ts
Enhanced test isolation with minimal package.json files, strengthened wrapper module validation using Symbol.toStringTag instead of generic type checks, and updated expected output ordering to match wrapper evaluation timing.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding bun:main to HardcodedModule.Alias to fix the flaky test caused by runtime transpiler rewriting.
Description check ✅ Passed The description fully covers both required sections: What (the change and its purpose) and How (comprehensive verification across multiple test runs and environments).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM — one-line alias-table addition matching its siblings, plus test hardening; the inline nit about process.getBuiltinModule is non-blocking.

Extended reasoning...

Overview

One production line in src/bun.js/HardcodedModule.zig: adds bun:main to bun_extra_alias_kvs, alongside identically-shaped entries for bun:app, bun:ffi, bun:jsc, bun:sqlite, bun:wrap, and bun:internal-for-testing. The rest is test hardening in test/js/bun/resolve/bun-main-entry-point.test.ts (disable auto-install via package.json, tighten assertions, fix expected output order now that the wrapper actually evaluates).

I confirmed the diagnosis against src/bun.js/RuntimeTranspilerStore.zig:527-538: without this alias, Alias.get("bun:main", ...) misses, falls through to the hasPrefixComptime("bun:") branch that strips the prefix to bare "main", which then resolves/auto-installs the npm main package. Adding the alias makes it hit the continue at line 531 like every other bun:* builtin.

Security risks

None introduced — if anything this closes a minor supply-chain footgun where import("bun:main") could fetch an arbitrary npm package named main over the network. No auth, crypto, permissions, or untrusted-input parsing is touched.

Level of scrutiny

Low. The production change is a single comptime string-map entry following the exact pattern of six adjacent siblings; bun:main was already present in HardcodedModule.map and the enum, so this just brings the Alias table into agreement. Neither file is covered by CODEOWNERS.

Other factors

  • The bug-hunter's nit (a small per-call WTF::StringImpl leak when process.getBuiltinModule("bun:main") is called) is real but negligible: that call returns undefined, so there's no reason to loop on it, and the leak is in pre-existing ModuleLoader.cpp cleanup logic rather than this diff. It's appropriately filed as a nit and doesn't block the flake fix.
  • The updated preload-test expectation (ENTRY_OK\nPRELOAD_OK\n) is a behavior change, but it's the correct behavior now that import("bun:main") actually evaluates the wrapper instead of an npm package — the old order was an artifact of the bug.
  • PR description shows 30 consecutive Windows runs green plus passes on the related import-meta, node-module-module, and bun-serve-html-entry suites.

.{ "bun:app", .{ .path = "bun:app" } },
.{ "bun:ffi", .{ .path = "bun:ffi" } },
.{ "bun:jsc", .{ .path = "bun:jsc" } },
.{ "bun:main", .{ .path = "bun:main" } },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Adding bun:main to bun_aliases makes process.getBuiltinModule("bun:main") reach getHardcodedModule, which clones entry_point.contents into a fresh bun.String and returns it with .tag = .esm; the C++ side (ModuleLoader.cpp, case SyntheticModuleType::ESM: return {};) then drops the ErrorableResolvedSource without deref'ing source_code, leaking one copy of the wrapper source per call. Tiny leak via an obscure API, but it's a new path opened by this PR — easiest fix is to deref res.result.value.source_code before return {} in the ESM case (or have the bun:main branch return null when called from Bun__resolveAndFetchBuiltinModule).

Extended reasoning...

What the bug is

process.getBuiltinModule("bun:main") now allocates a heap-backed WTF::StringImpl (a clone of jsc_vm.entry_point.contents) and discards it without decrementing its refcount. Each call leaks one copy of the entry-point wrapper source.

Before this PR, Bun__resolveAndFetchBuiltinModule returned false for "bun:main" (it wasn't in HardcodedModule.Alias.bun_aliases), so process.getBuiltinModule("bun:main") returned undefined without allocating anything. Adding the alias opens a code path that was never exercised by this caller.

Code path that triggers it

  1. process.getBuiltinModule is bound directly to Process_functionLoadBuiltinModule (BunProcess.cpp:4032) — there is no JS-side cache in front of it.
  2. Process_functionLoadBuiltinModule (BunProcess.cpp:3913) calls Bun::resolveAndFetchBuiltinModule (ModuleLoader.cpp:577), which declares a stack-local ErrorableResolvedSource res and calls Bun__resolveAndFetchBuiltinModule(&res, ...).
  3. In Zig (ModuleLoader.zig:837), HardcodedModule.Alias.bun_aliases.get("bun:main") now succeeds (the line added by this PR). HardcodedModule.map.get("bun:main") (line 839) yields .@"bun:main", and getHardcodedModule is called (line 844).
  4. getHardcodedModule for .@"bun:main" (ModuleLoader.zig:1148-1155), when jsc_vm.entry_point.generated is true (the normal case for bun run script), returns:
    ResolvedSource{
        .source_code = bun.String.cloneUTF8(jsc_vm.entry_point.contents),
        .tag = .esm,
        .source_code_needs_deref = true,
        ...
    }
    cloneUTF8toBunStringComptime heap-allocates a fresh ref-counted WTF::StringImpl.
  5. Back in C++ (ModuleLoader.cpp:605-607), the switch on res.result.value.tag hits:
    case SyntheticModuleType::ESM:
        return {};
    and the function returns jsUndefined() to the caller.

Why existing code doesn't prevent it

ErrorableResolvedSource is a plain typedef struct (headers-handwritten.h:132-135) with no destructor, and BunString is a tagged value type with no RAII cleanup. The SyntheticModuleType::ESM case in resolveAndFetchBuiltinModule simply returns without touching res.result.value.source_code, so the cloned string's refcount stays at 1 forever.

The other non-registry builtin reachable on this path, bun:wrap, uses String.init on a static buffer (no heap allocation) and .tag = .javascript (hits the default: branch, also return {}), so it never tickled this. bun:main is the first alias whose getHardcodedModule branch heap-allocates and is then thrown away by resolveAndFetchBuiltinModule.

Step-by-step proof

Under bun run /tmp/x.mjs:

  1. ServerEntryPoint.generate() sets entry_point.generated = true and entry_point.contents = "import '/tmp/x.mjs';\n..." (~a few hundred bytes).
  2. User code calls process.getBuiltinModule("bun:main").
  3. Step 3 above: alias lookup succeeds (new in this PR).
  4. Step 4 above: cloneUTF8 allocates a WTF::StringImpl of those ~hundreds of bytes, refcount = 1.
  5. Step 5 above: case ESM: return {};res goes out of scope, no destructor runs, the StringImpl is leaked.
  6. Process_functionLoadBuiltinModule returns jsUndefined().
  7. Repeat: for (;;) process.getBuiltinModule("bun:main"); → RSS grows unbounded.

Before this PR, step 3 returned nullreturn false at ModuleLoader.zig:837, so step 4 never ran.

Impact

A small per-call memory leak (one wrapper-source string, typically a few hundred bytes) on every call to process.getBuiltinModule("bun:main"). The call also returns undefined, so users have no reason to invoke it repeatedly — impact in real programs is negligible. It is, however, a genuine regression introduced by this PR rather than a pre-existing issue.

How to fix

Either:

  • In ModuleLoader.cpp's resolveAndFetchBuiltinModule, deref res.result.value.source_code before return {} in the ESM (and default) cases when source_code_needs_deref is set; or
  • In Bun__resolveAndFetchBuiltinModule (ModuleLoader.zig), special-case .@"bun:main" to return false before calling getHardcodedModule, since this caller can't do anything useful with an ESM-tagged source anyway.

@Jarred-Sumner Jarred-Sumner merged commit da74c5d into main Apr 26, 2026
63 of 64 checks passed
@Jarred-Sumner Jarred-Sumner deleted the claude/flamboyant-mcclintock-94cd25 branch April 26, 2026 00:15
dylan-conway added a commit that referenced this pull request Apr 26, 2026
Follow-up to #29719.

`bun:main` statically imports the entry file, so `await
import("bun:main")` at the entry's top level is a TLA self-cycle:
`bun:main` waits for `entry.mjs` (async dep), and `entry.mjs` waits for
`bun:main`'s evaluation promise. Per spec that promise never settles.

The old JSC module loader broke these cycles early, which is why #29719
passed locally (tested against `892042c2`, pre-#29393). After #29393
(WebKit module-loader rewrite) the loader correctly leaves the promise
unsettled, so the test now hangs and times out at 90s — see build 48023
(debian-asan, win x64, win x64-baseline).

Fix: drop the top-level `await` and use `import("bun:main").then(...)`
so `entry.mjs` finishes synchronously, `bun:main` finishes, and the
import resolves on the next microtask. The preload and `--hot` tests are
unaffected (preload isn't in `bun:main`'s dep graph).

Note: this also surfaced that Bun now hangs forever on any unsettled-TLA
cycle instead of exiting 13 like Node — separate PR coming for that.

## How did you verify your code works?

- `bun bd test test/js/bun/resolve/bun-main-entry-point.test.ts` — 3
pass, 20 consecutive runs on Windows
- `USE_SYSTEM_BUN=1 bun test ...` — 2 fail (still catches the alias bug
on unfixed bun)
xhjkl pushed a commit to xhjkl/bun that referenced this pull request May 14, 2026
…n-sh#29719)

## What

Add `bun:main` to `HardcodedModule.Alias.bun_extra_alias_kvs` so the
runtime transpiler stops rewriting `import("bun:main")` into
`import("main")`.

## Why (the flake)

`bun-main-entry-point.test.ts` has been flaky since it landed in oven-sh#29450.
The test was never exercising the code path it claimed to:

- `bun:main` is in `HardcodedModule.map` but was missing from the
**`Alias`** map
- so `RuntimeTranspilerStore.zig:534` stripped the `bun:` prefix,
leaving `import("main")` in the emitted JS
- at runtime that fell through to `resolveAndAutoInstall`, which
**fetched the npm `main` package** (`main@1000.0.1`) over the network
- the test's `typeof m !== "object"` check passed against the npm
package, so it "passed"
- when the registry fetch was slow or failed on CI, stdout was empty →
flake

Confirmed by observing `require.cache` gain
`~/.bun/install/cache/main@1000.0.1@@@1/index.js` after the import, and
by reproducing the failure deterministically with `[install] auto =
"disable"`.

## Test changes

- Add `package.json: "{}"` to the temp dirs so auto-install is off — any
future regression fails loudly with "Cannot find package 'main'" instead
of silently passing via npm
- Assert `Object.keys(m).length === 0` (the real wrapper exports
nothing; the npm package exports `default,length,name,prototype`)
- Collapse assertions into one `toEqual({stdout, stderr, exitCode,
signalCode})` so a failure shows the child's stderr
- Update the preload test's expected order — now that
`import("bun:main")` actually evaluates the wrapper, entry.mjs runs
before the preload's await resumes (`ENTRY_OK\nPRELOAD_OK\n`)

## How did you verify your code works?

- `bun bd test test/js/bun/resolve/bun-main-entry-point.test.ts` — 3
pass, 30 consecutive runs on Windows
- `USE_SYSTEM_BUN=1 bun test ...` — 2 fail (correctly catches the bug on
unfixed bun)
- `bun bd test test/js/bun/http/bun-serve-html-entry.test.ts -t
"bun:main"` — pass
- `bun bd test test/js/bun/resolve/import-meta*.test.*` — 47 pass
- `bun bd test test/js/node/module/node-module-module.test.js` — 28 pass
- `bun run zig:check-all` — pass
xhjkl pushed a commit to xhjkl/bun that referenced this pull request May 14, 2026
…9738)

Follow-up to oven-sh#29719.

`bun:main` statically imports the entry file, so `await
import("bun:main")` at the entry's top level is a TLA self-cycle:
`bun:main` waits for `entry.mjs` (async dep), and `entry.mjs` waits for
`bun:main`'s evaluation promise. Per spec that promise never settles.

The old JSC module loader broke these cycles early, which is why oven-sh#29719
passed locally (tested against `892042c2`, pre-oven-sh#29393). After oven-sh#29393
(WebKit module-loader rewrite) the loader correctly leaves the promise
unsettled, so the test now hangs and times out at 90s — see build 48023
(debian-asan, win x64, win x64-baseline).

Fix: drop the top-level `await` and use `import("bun:main").then(...)`
so `entry.mjs` finishes synchronously, `bun:main` finishes, and the
import resolves on the next microtask. The preload and `--hot` tests are
unaffected (preload isn't in `bun:main`'s dep graph).

Note: this also surfaced that Bun now hangs forever on any unsettled-TLA
cycle instead of exiting 13 like Node — separate PR coming for that.

## How did you verify your code works?

- `bun bd test test/js/bun/resolve/bun-main-entry-point.test.ts` — 3
pass, 20 consecutive runs on Windows
- `USE_SYSTEM_BUN=1 bun test ...` — 2 fail (still catches the alias bug
on unfixed bun)
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.

3 participants