Skip to content

test: audit ASAN quarantine lists; remove 77 stale entries - #34741

Merged
Jarred-Sumner merged 1 commit into
mainfrom
farm/11cb513f/audit-asan-expectations
Jul 21, 2026
Merged

Jarred-Sumner merged 1 commit into
mainfrom
farm/11cb513f/audit-asan-expectations

Conversation

@robobun

@robobun robobun commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Audited every [ ASAN ] entry in test/expectations.txt and every entry in test/no-validate-exceptions.txt against a release-asan build at 9dc6c37. Each test was run under four configs (bare ASAN / +validateExceptionChecks / +LeakSanitizer / full CI), and every removal candidate was re-verified 3x.

test/expectations.txt (11 [ ASAN ] entries removed)

No test in this file reproduces an AddressSanitizer heap error anymore. Removed entries:

Test Was Now
worker_threads/worker_threads.test.ts CRASH (bad free) bare clean; flaky LSAN leak 1/4 → stays in no-validate-leaksan.txt
worker_threads/worker_destruction.test.ts CRASH (bad free) bare clean; test-body timeout under BUN_DESTRUCT_VM_ON_EXIT → stays in no-validate-leaksan.txt
node/watch/fs.watch.test.ts CRASH (bad free) bare clean; test-body timeout under BUN_DESTRUCT_VM_ON_EXIT → added to no-validate-leaksan.txt
test-worker-unref-from-message-during-exit.js CRASH (use-after-poison) stable clean 3/3 under full CI config
test-fs-watch.js CRASH (use-after-poison) stable clean 5/5
test-fs-watch-recursive-watch-file.js CRASH (use-after-poison) stable clean 5/5
test-fs-promises-watch.js CRASH (use-after-poison) stable clean 5/5
cli/test/parallel.test.ts TIMEOUT stable clean 3/3, ~22s under full config
cli/test/isolation.test.ts TIMEOUT stable clean 3/3, ~6s under full config
bun/io/bun-write-leak.test.ts LEAK stable clean 3/3, ~2s
test-net-error-twice.js SKIP (slow write) stable clean 3/3, ~0.5s

Left in place: the two transfer-terminate entries added earlier today (#34686, known ~1/4000 flake); the next-pages / next-auth / napi / inspect / tls-sql / spawn / type-export entries (still fail under at least one config or could not be verified locally); the four [ LEAK ] entries that hit their 5s test-body timeout under ASAN.

test/no-validate-exceptions.txt (63 lines removed)

60 entries now pass 3x under BUN_JSC_validateExceptionChecks=1 on a release-asan build, plus 2 entries for files that no longer exist (cli/install/bun-repl.test.ts removed in fa3a30f, node/test/system-ca/test-native-root-certs.test.mjs), plus one orphaned section header.

The vendor/elysia/* entries are left as-is (repo is cloned in CI via test/vendor.json, not present locally).

test/no-validate-leaksan.txt

Added test/js/node/watch/fs.watch.test.ts (test-body timeout under BUN_DESTRUCT_VM_ON_EXIT, no sanitizer report). Removed test-fs-watch.js, test-fs-watch-recursive-watch-file.js, and test-fs-promises-watch.js (stable clean 5/5 under LSAN).

Remaining unchecked-exception sites in Bun code

The still-failing entries in no-validate-exceptions.txt cluster around these throw → unchecked pairs in src/jsc/:

Throw Unchecked at Repro
NapiClass.cpp:120 finishCreation JSObject::defineOwnNonIndexProperty 33× napi tests, require-cache.test.ts
napi_create_function napi.cpp:954 napi_set_named_property :598 etc. addon Init() pattern (#32911)
defaultBunSQLObject BunObject.cpp:319 itself BunObject.test.ts, import-meta.test.js, resolve.test.ts
JSObject::putInlineSlow Process_functionDlopen BunProcess.cpp:397 napi 4_object_factory, 5_function_factory
jsString jsFunctionWrap NodeModuleModule.cpp:220 node-module-module.test.js
jsSubstring jsFunctionNodeModuleModuleConstructor NodeModuleModule.cpp:172 module-resolve-filename-paths.test.js
isArraySlowInline determineSpecificType ErrorCode.cpp:348 isArray-proxy-crash.test.ts
importModuleInner NodeVM.cpp:303 moduleLoaderImportModuleInner NodeVM.cpp:1688 test-vm-module-referrer-realm.mjs
JSGenericTypedArrayView::create jsPublicKeyObjectPrototype_export :40 node-crypto.test.js
convertDictionaryToJS JSURLPatternInit.cpp:148 convertURLPatternInputToJS JSURLPatternResult.cpp:89 urlpattern.test.ts
normalizeCryptoAlgorithmParameters SubtleCrypto.cpp:135 JSDOMPromiseDeferred::reject :194/:159 webcrypto tests
evaluateWithScopeExtension JSInjectedScriptHost.cpp:120 ...PrototypeFunctionEvaluateWithScopeExtension :275 inspect.test.ts (WebKit)
JSOrderedHashTable::getImpl executeBoundCall Interpreter.cpp:1223 next-pages tests (WebKit)

Audited every [ASAN] entry in test/expectations.txt and every entry in
test/no-validate-exceptions.txt against a release-asan build at 9dc6c37,
running each under four configs (bare asan / +validateExceptionChecks /
+LSAN / full CI) and verifying clean results 3x.

expectations.txt (11 removed):
  No longer crash under bare ASAN; worker_threads, worker_destruction and
  fs.watch.test.ts fail only under BUN_DESTRUCT_VM_ON_EXIT (already covered
  by no-validate-leaksan.txt, fs.watch.test.ts added there). The fs-watch
  node-ported tests, test-worker-unref-from-message-during-exit, and
  test-net-error-twice are stable-clean under full CI config.
  cli/test/parallel and isolation now finish in <25s (were TIMEOUT).
  bun-write-leak passes its leak assertion.

no-validate-exceptions.txt (63 lines removed):
  60 entries pass 3x under validateExceptionChecks=1, plus two deleted
  files (bun-repl.test.ts, system-ca/test-native-root-certs.test.mjs) and
  one empty section header.

no-validate-leaksan.txt:
  Add fs.watch.test.ts (5s test-body timeout under BUN_DESTRUCT_VM_ON_EXIT,
  no leak report). Remove the three fs-watch node tests (stable-clean 5x
  under LSAN).
@robobun
robobun requested a review from Jarred-Sumner as a code owner July 20, 2026 00:18
@robobun

robobun commented Jul 20, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 6:24 PM PT - Jul 19th, 2026

❌ @robobun, your commit 91e7d29 has 1 failures in Build #75887 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 34741

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

bun-34741 --bun

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 4 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 99db1ca7-b7f9-4b07-8f55-dc094944d414

📥 Commits

Reviewing files that changed from the base of the PR and between 99fc2f8 and 91e7d29.

📒 Files selected for processing (3)
  • test/expectations.txt
  • test/no-validate-exceptions.txt
  • test/no-validate-leaksan.txt

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

@robobun

robobun commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

x64-asan lane is green (all 20 shards passed, no retries) in build 75887, so every unquarantined test held.

The red on the build is 9 tests all tagged [flaky] or [pre-existing] by ci:errors, none on the asan lane and none touched by this diff: complex-workspace (install flake), terminal-platform-gaps (Windows ConPTY), test-tonic (external fetch ECONNRESET), test-net-connect-memleak (pre-existing alpine, already quarantined), test-fs-promises-file-handle-readFile, test-http-client-leaky-with-double-response, test-http-server-connections-checking-leak, bun-jsc, webview-chrome.

Ready for review.

@Jarred-Sumner
Jarred-Sumner merged commit aa21e21 into main Jul 21, 2026
76 of 78 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the farm/11cb513f/audit-asan-expectations branch July 21, 2026 01:29
robobun added a commit that referenced this pull request Jul 24, 2026
Brings in #35002 (remove ~39k lines of dead Rust) and its follow-ups
(#35019, #35052, #35152, #35225, #35293, #35326). The binary-size
check compares against current main; this branch was 137 commits
behind, so it still carried the dead code main dropped and registered
as +630KB..+1.7MB on x64 while aarch64 linux showed -513KB/-601KB
(different dead-code elimination outcomes per target). This stack's
own native contribution is 19 files / +392 -45 lines; src/js is
net -977 lines (domain.ts +692 vs fast-utf8-stream.ts -856 etc).

Also: drop the hoisted pbkdf2 .bind handlers back to closures (review
nit), and take main's expectations.txt since #34741 audited the stale
ASAN entries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants