You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
test(plugin): run the GC before each require() instead of at every allocation - #48135
bun bd test test/js/bun/plugin/plugins.test.ts fails on a debug (ASAN) build of main. keeps the object alive across a GC so require() can create the module again needs 45 s and ends with this test timed out after 5000ms.
Its child process gets BUN_JSC_slowPathAllocsBetweenGCs=1: a full GC at each slow-path allocation. A debug build then needs 13.8 s to print one line.
Fix
The child process calls Bun.gc(true) before each require() and runs with BUN_JSC_useZombieMode=1 instead.
Verified: it passes in 10 of 10 runs of that command (main: 0 of 8). Other tests time out on both (Notes).
Background
A plugin's onLoad can return { exports, loader: "object" }. Bun reads exports each time JSC creates the module. bun test: Vitest compatibility #44833 keeps it alive with the module's source.
JSC creates the module again when require() runs one microtask turn after import() created it. The 8 rounds try 8 turns. Round 1 hits it today.
useZombieMode overwrites the cells that a GC frees. Without it, a freed object often reads as before, and about half of the unfixed runs pass.
Considered a longer slowPathAllocsBetweenGCs interval: a GC falls into that turn only for some intervals. The explicit call gives up GCs at other points.
Notes
All numbers are from one Linux x64 machine (12 cores) and main at 7ca0042. The debug build is bun bd. The release build is --profile=release --lto=off.
Time
Before
After
Debug build, the test alone
45.2 s with --timeout 300000. By default: (fail) ... [5009.09ms]
0.66 to 0.98 s (6 runs)
Debug build, the test in a run of the whole file
timeout
0.74 to 1.66 s (10 runs)
Release build, the child process alone
2.5 to 2.9 s
0.03 s
console.log("hi") takes 0.27 s on the debug build, and 13.8 s with BUN_JSC_slowPathAllocsBetweenGCs=1. So fewer rounds cannot bring the old test under 5 s.
On main nothing stops the child process of the timed-out test. It was still at 75 % of one core more than 20 s after the test run ended.
The test without the fix
For these runs only, src/jsc/modules/ObjectModule.cpp got a switch that disables the fix of #44833 in two ways. The switch is not part of this PR.
The code before the fix: gcProtect the object, gcUnprotect it after the first read, keep a raw pointer.
The WeakHandleOwner that the fix added never reports the object as reachable.
Build
Fix
New test body
debug
in place
pass, 70 of 70 runs
debug
disabled, way 1
fail, 20 of 20: runtime error: member call on null pointer of type 'JSC::JSCell' where Bun reads the exports
fail, 20 of 20: panic(main thread): Segmentation fault at address 0x0
release
disabled, way 2
fail, 20 of 20: panic(main thread): abort() called
The test file itself gives the same result in each of the six cases. On the debug build, way 2 shows as a timeout, because that build needs 9.4 s to print the backtrace of the assertion.
Without BUN_JSC_useZombieMode=1, way 1 passes 12 of 20 runs on the debug build and 8 of 20 on the release build. A dead cell often keeps its bytes after the GC.
The old test body fails in both ways too, after 29 to 34 s on the debug build.
test/js/bun/test/jest-each-gc-root.test.ts and test/js/bun/transpiler/transpiler-error-gc-uaf.test.ts already use useZombieMode for the same purpose.
Where JSC creates the module again
A log line in the function that reads the exports shows one read of each served*.ts and data*.json in rounds 0 and 2 to 7. Round 1 has two reads of served1.ts and two of data1.json: one from import() and one from require().
moduleRegistryFetchSettled calls makeModule and queues moduleRegistryModuleSettled. A synchronous load that arrives between the two calls makeModule again (JSModuleLoader::hostLoadImportedModule).
In round 0 the Bun.gc(true) runs before the first read. Way 2 already fails there. Way 1 fails in round 1.
The old test ran a GC at each slow-path allocation of the whole process. The new test runs 8, one before each require(). It has no GC inside require() or inside the function that reads the exports.
A longer interval instead of the explicit call
The old test body on the release build, with the fix disabled in way 1. Failed runs of 10, for each interval:
slowPathAllocsBetweenGCs
1
2
3
4
5
7
10
15
20
30
50
100
alone
10
9
8
9
7
9
9
5
0
0
0
6
with useZombieMode
10
10
10
10
10
10
10
10
10
10
0
10
On the debug build the old body takes 7.7 s at an interval of 5, about 3 s at 10, 1.5 s at 20 and 0.5 s at 50. An interval that is fast enough finds the bug only while the allocation counts line up. At 50 they do not.
Runs of the whole file on the debug build
Runs
This test
Other tests
This PR
10
pass in 10
no failure in 4 runs, 2 or 3 timeouts in 6 runs
main
8
timeout in 8
no failure in 3 runs, 1 to 14 timeouts in 5 runs
The other timeouts are patched Promise methods can't observe module loading > modules from plugins (two tests, 2.9 to 4.9 s when they pass), module mocks of the same block, and twice one of the doesn't leak the global of a test file tests. This PR does not change them.
They depend on the load of the machine. In the last 8 runs, other guests took 30 to 42 % of its CPU time (steal time). In the fastest run (39 s for the file) no test needed more than 3.5 s.
…allocation
The test "keeps the object alive across a GC so require() can create the
module again" ran its child process with BUN_JSC_slowPathAllocsBetweenGCs=1.
With that option a debug build takes over ten seconds to start. The test
needed 45 seconds there and timed out at the default of 5 seconds.
The child process now calls Bun.gc(true) before each require(). It runs
with BUN_JSC_useZombieMode=1, which overwrites the cells that a GC frees.
The test takes under one second on a debug build.
Status: ready for review. The test of this PR passes on every CI lane. CI is red only on tests that also fail on main.
How I reproduced it. On main at 7ca0042, with a debug (ASAN) build on Linux x64:
$ bun bd test test/js/bun/plugin/plugins.test.ts -t "keeps the object alive across a GC"
(fail) a module made from an object > keeps the object alive across a GC so require() can create the module again [5009.09ms]
^ this test timed out after 5000ms.
With --timeout 300000 the same test passes after 45.2 s. With this PR it passes in 0.7 to 1.7 s.
test/js/bun/plugin/plugins.test.ts has 270 pass and 0 fail on Debian 13 x64 ASAN, Debian 13 x64, Ubuntu 25.04 x64, Alpine aarch64, macOS aarch64 and macOS x64. It has 267 pass and 0 fail on Windows x64 and Windows aarch64. No lane reports a failure of this file.
28 test files are red, and each is red on main too: 27 files in test/aot/ and test/bundler/compile-aot-size.test.ts. This PR does not touch them.
No actionable comments were generated in the recent review.
Walkthrough
The object-loader GC test now calls Bun.gc(true) before requiring each generated importer. Its subprocess enables BUN_JSC_useZombieMode=1 instead of BUN_JSC_slowPathAllocsBetweenGCs=1.
Changes
Object-loader GC test
Layer / File(s)
Summary
Configure and run object-loader GC test test/js/bun/plugin/plugins.test.ts
The subprocess enables zombie mode. The test explicitly collects before requiring each generated importer. The comment notes the startup cost of the prior allocation setting.
Suggested reviewers: jarred-sumner
Merge Risk: Minimal · up to 7e625
The updated test collects after import creates the retained object and before require consumes it. No actionable merge-blocking issue was established.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title clearly describes the main test change.
Description check
The description explains the change and verification in detail. It uses different headings from the template but provides the required information.
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Reviewing files that changed from the base of the PR and between 7ca0042 and 7e625cf.
Files selected for processing: 1
test/js/bun/plugin/plugins.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Behaviour change: none
Problem
bun bd test test/js/bun/plugin/plugins.test.tsfails on a debug (ASAN) build of main.keeps the object alive across a GC so require() can create the module againneeds 45 s and ends withthis test timed out after 5000ms.BUN_JSC_slowPathAllocsBetweenGCs=1: a full GC at each slow-path allocation. A debug build then needs 13.8 s to print one line.Fix
Bun.gc(true)before eachrequire()and runs withBUN_JSC_useZombieMode=1instead.Background
onLoadcan return{ exports, loader: "object" }. Bun readsexportseach time JSC creates the module. bun test: Vitest compatibility #44833 keeps it alive with the module's source.require()runs one microtask turn afterimport()created it. The 8 rounds try 8 turns. Round 1 hits it today.useZombieModeoverwrites the cells that a GC frees. Without it, a freed object often reads as before, and about half of the unfixed runs pass.slowPathAllocsBetweenGCsinterval: a GC falls into that turn only for some intervals. The explicit call gives up GCs at other points.Notes
All numbers are from one Linux x64 machine (12 cores) and main at 7ca0042. The debug build is
bun bd. The release build is--profile=release --lto=off.Time
--timeout 300000. By default:(fail) ... [5009.09ms]console.log("hi")takes 0.27 s on the debug build, and 13.8 s withBUN_JSC_slowPathAllocsBetweenGCs=1. So fewer rounds cannot bring the old test under 5 s.On main nothing stops the child process of the timed-out test. It was still at 75 % of one core more than 20 s after the test run ended.
The test without the fix
For these runs only,
src/jsc/modules/ObjectModule.cppgot a switch that disables the fix of #44833 in two ways. The switch is not part of this PR.gcProtectthe object,gcUnprotectit after the first read, keep a raw pointer.WeakHandleOwnerthat the fix added never reports the object as reachable.runtime error: member call on null pointer of type 'JSC::JSCell'where Bun reads the exportsASSERTION FAILED: !source.isCell() || handle->get() == source.asCell()panic(main thread): Segmentation fault at address 0x0panic(main thread): abort() calledBUN_JSC_useZombieMode=1, way 1 passes 12 of 20 runs on the debug build and 8 of 20 on the release build. A dead cell often keeps its bytes after the GC.test/js/bun/test/jest-each-gc-root.test.tsandtest/js/bun/transpiler/transpiler-error-gc-uaf.test.tsalready useuseZombieModefor the same purpose.Where JSC creates the module again
A log line in the function that reads the exports shows one read of each
served*.tsanddata*.jsonin rounds 0 and 2 to 7. Round 1 has two reads ofserved1.tsand two ofdata1.json: one fromimport()and one fromrequire().moduleRegistryFetchSettledcallsmakeModuleand queuesmoduleRegistryModuleSettled. A synchronous load that arrives between the two callsmakeModuleagain (JSModuleLoader::hostLoadImportedModule).In round 0 the
Bun.gc(true)runs before the first read. Way 2 already fails there. Way 1 fails in round 1.The old test ran a GC at each slow-path allocation of the whole process. The new test runs 8, one before each
require(). It has no GC insiderequire()or inside the function that reads the exports.A longer interval instead of the explicit call
The old test body on the release build, with the fix disabled in way 1. Failed runs of 10, for each interval:
slowPathAllocsBetweenGCsuseZombieModeOn the debug build the old body takes 7.7 s at an interval of 5, about 3 s at 10, 1.5 s at 20 and 0.5 s at 50. An interval that is fast enough finds the bug only while the allocation counts line up. At 50 they do not.
Runs of the whole file on the debug build
patched Promise methods can't observe module loading > modules from plugins(two tests, 2.9 to 4.9 s when they pass),module mocksof the same block, and twice one of thedoesn't leak the global of a test filetests. This PR does not change them.jsoncwhen--loader, bunfig[loader]orBun.buildnames it #36648 and resolver: passpathsas an argument and deleteResolver::custom_dir_paths#47752 report the timeout of this test in their own debug runs.