[pull] main from Jarred-Sumner:main - #1
Merged
Merged
Conversation
pull Bot
pushed a commit
that referenced
this pull request
Jul 25, 2025
…ck traces upon crash in CI (oven-sh#21143) ### What does this PR do? Closes oven-sh#13012 On Linux, when any Bun process spawned by `runner.node.mjs` crashes, we run GDB in batch mode to print a backtrace from the core file. And on all platforms, we run a mini `bun.report` server which collects crashes reported by any Bun process executed during the tests, and after each test `runner.node.mjs` fetches and prints any new crashes from the server. <details> <summary>example 1</summary> ``` #0 crash_handler.crash () at crash_handler.zig:1513 #1 0x0000000002cf4020 in crash_handler.crashHandler (reason=..., error_return_trace=0x0, begin_addr=...) at crash_handler.zig:479 #2 0x0000000002cefe25 in crash_handler.handleSegfaultPosix (sig=<optimized out>, info=<optimized out>) at crash_handler.zig:800 #3 0x00000000045a1124 in WTF::jscSignalHandler (sig=11, info=0x7ffe044e30b0, ucontext=0x0) at vendor/WebKit/Source/WTF/wtf/threads/Signals.cpp:548 #4 <signal handler called> #5 JSC::JSCell::type (this=0x0) at vendor/WebKit/Source/JavaScriptCore/runtime/JSCellInlines.h:137 #6 JSC::JSObject::getOwnNonIndexPropertySlot (this=0x150bc914fe18, vm=..., structure=0x150a0102de50, propertyName=..., slot=...) at vendor/WebKit/Source/JavaScriptCore/runtime/JSObject.h:1348 #7 JSC::JSObject::getPropertySlot<false> (this=0x150bc914fe18, globalObject=0x150b864e0088, propertyName=..., slot=...) at vendor/WebKit/Source/JavaScriptCore/runtime/JSObject.h:1433 #8 JSC::JSValue::getPropertySlot (this=0x7ffe044e4880, globalObject=0x150b864e0088, propertyName=..., slot=...) at vendor/WebKit/Source/JavaScriptCore/runtime/JSCJSValueInlines.h:1108 #9 JSC::JSValue::get (this=0x7ffe044e4880, globalObject=0x150b864e0088, propertyName=..., slot=...) at vendor/WebKit/Source/JavaScriptCore/runtime/JSCJSValueInlines.h:1065 #10 JSC::LLInt::performLLIntGetByID (bytecodeIndex=..., codeBlock=0x150b861e7740, globalObject=0x150b864e0088, baseValue=..., ident=..., metadata=...) at vendor/WebKit/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:878 #11 0x0000000004d7b055 in llint_slow_path_get_by_id (callFrame=0x7ffe044e4ab0, pc=0x150bc92ea0e7) at vendor/WebKit/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:946 #12 0x0000000003dd6042 in llint_op_get_by_id () #13 0x0000000000000000 in ?? () ``` </details> <details> <summary>example 2</summary> ``` #0 crash_handler.crash () at crash_handler.zig:1513 #1 0x0000000002c5db80 in crash_handler.crashHandler (reason=..., error_return_trace=0x0, begin_addr=...) at crash_handler.zig:479 #2 0x0000000002c59f60 in crash_handler.handleSegfaultPosix (sig=<optimized out>, info=<optimized out>) at crash_handler.zig:800 #3 0x00000000042ecc88 in WTF::jscSignalHandler (sig=11, info=0xfffff60141b0, ucontext=0xfffff6014230) at vendor/WebKit/Source/WTF/wtf/threads/Signals.cpp:548 #4 <signal handler called> #5 bun.js.api.FFIObject.Reader.u8 (globalObject=0x4000554e0088) at /var/lib/buildkite-agent/builds/ip-172-31-75-92/bun/bun/src/bun.js/api/FFIObject.zig:65 #6 bun.js.jsc.host_fn.toJSHostCall__anon_1711576 (globalThis=0x4000554e0088, args=...) at /var/lib/buildkite-agent/builds/ip-172-31-75-92/bun/bun/src/bun.js/jsc/host_fn.zig:97 #7 bun.js.jsc.host_fn.DOMCall("Reader"[0..6],bun.js.api.FFIObject.Reader,"u8"[0..2],.{ .reads = .{ ... }, .writes = .{ ... } }).slowpath (globalObject=0x4000554e0088, thisValue=70370172175040, arguments_ptr=0xfffff6015460, arguments_len=1) at /var/lib/buildkite-agent/builds/ip-172-31-75-92/bun/bun/src/bun.js/jsc/host_fn.zig:490 #8 0x000040003419003c in ?? () #9 0x0000400055173440 in ?? () ``` </details> I used GDB instead of LLDB (as the branch name suggests) because it seems to produce more useful stack traces with musl libc. - [x] on linux, use gdb to print from core dump of main bun process crashed - [x] on linux, use gdb to print from all new core dumps (so including bun subprocesses spawned by the test that crashed) - [x] on all platforms, use a mini bun.report server to print a self-reported trace (depends on oven-sh/bun.report#15; for now our package.json points to a commit on the branch of that repo) - [x] fix trying to fetch stack traces too early on windows - [x] use output groups so the traces show up alongside the log for the specific test instead of having to find it in the logs from the entire run - [x] get oven-sh/bun.report#15 merged, and point to a bun.report commit on the main branch instead of the PR branch in package.json ### How did you verify your code works? Manually, and in CI with a crashing test. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
Aug 8, 2025
<details> <summary> observed in https://buildkite.com/bun/bun/builds/22442#annotation-test/js/node/zlib/leak.test.ts </summary> ``` ==5045==ERROR: AddressSanitizer: heap-use-after-free on address 0x5220000243c0 at pc 0x00000dad671b bp 0x14f22d4a4990 sp 0x14f22d4a4988 READ of size 8 at 0x5220000243c0 thread T5 (HeapHelper) ======== Stack trace from GDB for HeapHelper-5045.core: ======== Program terminated with signal SIGABRT, Aborted. #0 0x000014f2c3672eec in ?? () from /lib/x86_64-linux-gnu/libc.so.6 [Current thread is 1 (Thread 0x14f22d4f46c0 (LWP 5050))] #0 0x000014f2c3672eec in ?? () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x000014f2c3623fb2 in raise () from /lib/x86_64-linux-gnu/libc.so.6 #2 0x000014f2c360e472 in abort () from /lib/x86_64-linux-gnu/libc.so.6 #3 0x000000000e3b2ae2 in uw_init_context_1[cold] () #4 0x000000000e3b29fc in _Unwind_Backtrace () #5 0x00000000046a6bab in __sanitizer::BufferedStackTrace::UnwindSlow(unsigned long, unsigned int) () #6 0x00000000046a181d in __sanitizer::BufferedStackTrace::Unwind(unsigned int, unsigned long, unsigned long, void*, unsigned long, unsigned long, bool) () #7 0x00000000046885bd in __sanitizer::BufferedStackTrace::UnwindImpl(unsigned long, unsigned long, void*, bool, unsigned int) () #8 0x0000000004601127 in __asan::ErrorGeneric::Print() () #9 0x0000000004683180 in __asan::ScopedInErrorReport::~ScopedInErrorReport() () #10 0x0000000004686567 in __asan::ReportGenericError(unsigned long, unsigned long, unsigned long, unsigned long, bool, unsigned long, unsigned int, bool) () #11 0x0000000004686d46 in __asan_report_load8 () #12 0x000000000dad671b in ZSTD_sizeof_CCtx (cctx=<optimized out>) at ./build/release-asan/zstd/vendor/zstd/lib/compress/zstd_compress.c:210 #13 0x0000000006d2284d in bun.js.node.zlib.NativeZstd.estimatedSize () at /var/lib/buildkite-agent/builds/ip-172-31-72-121/bun/bun/src/bun.js/node/zlib/NativeZstd.zig:57 #14 ZigGeneratedClasses.JSNativeZstd.JavaScriptCoreBindings.NativeZstd__estimatedSize (thisValue=<optimized out>) at /var/lib/buildkite-agent/builds/ip-172-31-72-121/bun/bun/build/release-asan/codegen/ZigGeneratedClasses.zig:11122 #15 0x000000000852803b in WebCore::JSNativeZstd::visitChildrenImpl<JSC::SlotVisitor> (cell=0x14f22e190840, visitor=...) at ./build/release-asan/./build/release-asan/codegen/ZigGeneratedClasses.cpp:30728 #16 WebCore::JSNativeZstd::visitChildren (cell=0x14f22e190840, visitor=...) at ./build/release-asan/./build/release-asan/codegen/ZigGeneratedClasses.cpp:30734 #17 0x000000000aa99d6c in JSC::MethodTable::visitChildren (this=<optimized out>, cell=<optimized out>, visitor=...) at vendor/WebKit/Source/JavaScriptCore/runtime/ClassInfo.h:115 #18 0x000000000aa99d6c in JSC::SlotVisitor::visitChildren (this=0x14f277028300, cell=0x14f22e190840) #19 JSC::SlotVisitor::drain(WTF::MonotonicTime)::$_0::operator()(JSC::MarkStackArray&) const (this=<optimized out>, stack=...) at vendor/WebKit/Source/JavaScriptCore/heap/SlotVisitor.cpp:509 #20 0x000000000aa8f130 in JSC::SlotVisitor::forEachMarkStack<JSC::SlotVisitor::drain(WTF::MonotonicTime)::$_0>(JSC::SlotVisitor::drain(WTF::MonotonicTime)::$_0 const&) (this=0x14f277028300, func=...) at vendor/WebKit/Source/JavaScriptCore/heap/SlotVisitorInlines.h:193 #21 JSC::SlotVisitor::drain (this=this@entry=0x14f277028300, timeout=<error reading variable: That operation is not available on integers of more than 8 bytes.>, timeout@entry=...) at vendor/WebKit/Source/JavaScriptCore/heap/SlotVisitor.cpp:499 #22 0x000000000aa90590 in JSC::SlotVisitor::drainFromShared (this=0x14f277028300, sharedDrainMode=JSC::SlotVisitor::HelperDrain, timeout=<error reading variable: That operation is not available on integers of more than 8 bytes.>) at vendor/WebKit/Source/JavaScriptCore/heap/SlotVisitor.cpp:699 #23 0x000000000aa08726 in JSC::Heap::runBeginPhase(JSC::GCConductor)::$_1::operator()() const (this=<optimized out>) at vendor/WebKit/Source/JavaScriptCore/heap/Heap.cpp:1508 #24 WTF::SharedTaskFunctor<void (), JSC::Heap::runBeginPhase(JSC::GCConductor)::$_1>::run() (this=<optimized out>) at .WTF/Headers/wtf/SharedTask.h:91 #25 0x000000000aa3b596 in WTF::ParallelHelperClient::runTask(WTF::RefPtr<WTF::SharedTask<void ()>, WTF::RawPtrTraits<WTF::SharedTask<void ()> >, WTF::DefaultRefDerefTraits<WTF::SharedTask<void ()> > > const&) (this=0x14f22e000428, task=...) at vendor/WebKit/Source/WTF/wtf/ParallelHelperPool.cpp:110 #26 0x000000000aa3d976 in WTF::ParallelHelperPool::Thread::work (this=<optimized out>) at vendor/WebKit/Source/WTF/wtf/ParallelHelperPool.cpp:201 #27 0x000000000aa4210d in WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0::operator()() const (this=<optimized out>) at vendor/WebKit/Source/WTF/wtf/AutomaticThread.cpp:225 #28 WTF::Detail::CallableWrapper<WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0, void>::call() (this=<optimized out>) at vendor/WebKit/Source/WTF/wtf/Function.h:53 #29 0x0000000008958ada in WTF::Function<void ()>::operator()() const (this=<optimized out>) at vendor/WebKit/Source/WTF/wtf/Function.h:82 #30 WTF::Thread::entryPoint (newThreadContext=<optimized out>) at vendor/WebKit/Source/WTF/wtf/Threading.cpp:272 #31 0x0000000008a65689 in WTF::wtfThreadEntryPoint (context=0x13b5) at vendor/WebKit/Source/WTF/wtf/posix/ThreadingPOSIX.cpp:255 #32 0x000000000467d347 in asan_thread_start(void*) () #33 0x000014f2c36711f5 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 #34 0x000014f2c36f189c in ?? () from /lib/x86_64-linux-gnu/libc.so.6 ``` </details> `ZSTD_sizeof_CCtx` and `ZSTD_sizeof_DCtx` can not be relied upon to be thread-safe and estimatedSize may be called from any thread
pull Bot
pushed a commit
that referenced
this pull request
Aug 20, 2025
…Worker" (oven-sh#21994) Reverts oven-sh#21962 `vm.ensureTerminationException` allocates a JSString, which is not safe to do from a thread that doesn't own the API lock. ```ts Bun Canary v1.2.21-canary.1 (f706382a) Linux x64 (baseline) Linux Kernel v6.12.38 | musl CPU: sse42 popcnt avx avx2 avx512 Args: "/var/lib/buildkite-agent/builds/ip-172-31-38-185/bun/bun/release/bun-linux-x64-musl-baseline-profile/bun-profile" "/var/lib/buildkite-agent/builds/ip-172-31-38-185/bun/bun/test/js/node/worker_threads"... Features: bunfig http_server jsc tsconfig(3) tsconfig_paths workers_spawned(40) workers_terminated(34) Builtins: "bun:main" "node:worker_threads" Elapsed: 362ms | User: 518ms | Sys: 63ms RSS: 0.34GB | Peak: 100.36MB | Commit: 0.34GB | Faults: 0 | Machine: 8.17GB panic(main thread): Segmentation fault at address 0x0 oh no: Bun has crashed. This indicates a bug in Bun, not your code. To send a redacted crash report to Bun's team, please file a GitHub issue using the link below: http://localhost:38809/1.2.21/Ba2f706382wNgkgUu11luEm6yX+lwy+Dgtt+oEurthoD8214mE___07+09DA2AA 6 | describe("Worker destruction", () => { 7 | const method = ["Bun.connect", "Bun.listen", "fetch"]; 8 | describe.each(method)("bun when %s is used in a Worker that is terminating", method => { 9 | // fetch: ASAN failure 10 | test.skipIf(isBroken && method == "fetch")("exits cleanly", () => { 11 | expect([join(import.meta.dir, "worker_thread_check.ts"), method]).toRun(); ^ error: Command /var/lib/buildkite-agent/builds/ip-172-31-38-185/bun/bun/test/js/node/worker_threads/worker_thread_check.ts Bun.connect failed: Spawned 10 workers RSS 79 MB Spawned 10 workers RSS 87 MB Spawned 10 workers RSS 90 MB at <anonymous> (/var/lib/buildkite-agent/builds/ip-172-31-38-185/bun/bun/test/js/node/worker_threads/worker_destruction.test.ts:11:73) ✗ Worker destruction > bun when Bun.connect is used in a Worker that is terminating > exits cleanly [597.56ms] ✓ Worker destruction > bun when Bun.listen is used in a Worker that is terminating > exits cleanly [503.47ms] » Worker destruction > bun when fetch is used in a Worker that is terminating > exits cleanly 1 pass 1 skip 1 fail 2 expect() calls Ran 3 tests across 1 file. [1125.00ms] ======== Stack trace from GDB for bun-profile-28234.core: ======== Program terminated with signal SIGILL, Illegal instruction. #0 crash_handler.crash () at crash_handler.zig:1523 [Current thread is 1 (LWP 28234)] #0 crash_handler.crash () at crash_handler.zig:1523 #1 0x0000000002db77aa in crash_handler.crashHandler (reason=..., error_return_trace=0x0, begin_addr=...) at crash_handler.zig:471 #2 0x0000000002db2b55 in crash_handler.handleSegfaultPosix (sig=<optimized out>, info=<optimized out>) at crash_handler.zig:792 #3 0x0000000004716b58 in WTF::jscSignalHandler (sig=11, info=0x7ffe54051e90, ucontext=0x0) at vendor/WebKit/Source/WTF/wtf/threads/Signals.cpp:548 #4 <signal handler called> #5 JSC::VM::currentThreadIsHoldingAPILock (this=0x148296c30000) at vendor/WebKit/Source/JavaScriptCore/runtime/VM.h:840 #6 JSC::sanitizeStackForVM (vm=...) at vendor/WebKit/Source/JavaScriptCore/runtime/VM.cpp:1369 #7 0x0000000003f4a060 in JSC::LocalAllocator::allocate(JSC::Heap&, unsigned long, JSC::GCDeferralContext*, JSC::AllocationFailureMode)::{lambda()#1}::operator()() const (this=<optimized out>) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/LocalAllocatorInlines.h:46 #8 JSC::FreeList::allocateWithCellSize<JSC::LocalAllocator::allocate(JSC::Heap&, unsigned long, JSC::GCDeferralContext*, JSC::AllocationFailureMode)::{lambda()#1}>(JSC::LocalAllocator::allocate(JSC::Heap&, unsigned long, JSC::GCDeferralContext*, JSC::AllocationFailureMode)::{lambda()#1} const&, unsigned long) (this=0x148296c38e48, cellSize=16, slowPath=...) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/FreeListInlines.h:46 #9 JSC::LocalAllocator::allocate (this=0x148296c38e30, heap=..., cellSize=16, deferralContext=0x0, failureMode=JSC::AllocationFailureMode::Assert) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/LocalAllocatorInlines.h:44 #10 JSC::GCClient::IsoSubspace::allocate (this=0x148296c38e30, vm=..., cellSize=16, deferralContext=0x0, failureMode=JSC::AllocationFailureMode::Assert) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/IsoSubspaceInlines.h:34 #11 JSC::tryAllocateCellHelper<JSC::JSString, (JSC::AllocationFailureMode)0> (vm=..., size=16, deferralContext=0x0) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/JSCellInlines.h:192 #12 JSC::allocateCell<JSC::JSString> (vm=..., size=16) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/JSCellInlines.h:212 #13 JSC::JSString::create (vm=..., value=...) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/JSString.h:204 #14 0x0000000004479ad1 in JSC::jsNontrivialString (vm=..., s=...) at vendor/WebKit/Source/JavaScriptCore/runtime/JSString.h:846 #15 JSC::VM::ensureTerminationException (this=0x148296c30000) at vendor/WebKit/Source/JavaScriptCore/runtime/VM.cpp:627 #16 JSGlobalObject__requestTermination (globalObject=<optimized out>) at ./build/release/./src/bun.js/bindings/ZigGlobalObject.cpp:3979 #17 0x0000000003405ab8 in bun.js.web_worker.notifyNeedTermination (this=0x542904f0d80) at /var/lib/buildkite-agent/builds/ip-172-31-16-28/bun/bun/src/bun.js/web_worker.zig:558 #18 0x0000000004362b6f in WebCore::Worker::terminate (this=0x984c900000000000) at ./src/bun.js/bindings/webcore/Worker.cpp:266 #19 WebCore::jsWorkerPrototypeFunction_terminateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSWorker*)::{lambda()#1}::operator()() const (this=<optimized out>) at ./build/release/./src/bun.js/bindings/webcore/JSWorker.cpp:549 #20 WebCore::toJS<WebCore::IDLUndefined, WebCore::jsWorkerPrototypeFunction_terminateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSWorker*)::{lambda()#1}>(JSC::JSGlobalObject&, JSC::ThrowScope&, WebCore::jsWorkerPrototypeFunction_terminateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSWorker*)::{lambda()#1}&&) (lexicalGlobalObject=..., throwScope=..., valueOrFunctor=...) at ./src/bun.js/bindings/webcore/JSDOMConvertBase.h:174 #21 WebCore::jsWorkerPrototypeFunction_terminateBody (lexicalGlobalObject=<optimized out>, callFrame=<optimized out>, castedThis=<optimized out>) at ./build/release/./src/bun.js/bindings/webcore/JSWorker.cpp:549 #22 WebCore::IDLOperation<WebCore::JSWorker>::call<&WebCore::jsWorkerPrototypeFunction_terminateBody, (WebCore::CastedThisErrorBehavior)0> (lexicalGlobalObject=..., operationName=..., callFrame=...) at ./src/bun.js/bindings/webcore/JSDOMOperation.h:63 #23 WebCore::jsWorkerPrototypeFunction_terminate (lexicalGlobalObject=<optimized out>, callFrame=0x7ffe540536b8) at ./build/release/./src/bun.js/bindings/webcore/JSWorker.cpp:554 #24 0x000014825580c038 in ?? () #25 0x00007ffe540537b0 in ?? () #26 0x0000148255a626cb in ?? () #27 0x0000000000000000 in ?? () 1 crashes reported during this test ```
pull Bot
pushed a commit
that referenced
this pull request
Oct 7, 2025
…23244) ## Summary Fixes oven-sh#23206 When using `test.each` with object syntax and `$variable` interpolation, string values were being quoted (e.g., `"apple"` instead of `apple`). This didn't match the behavior of `%s` formatting or Jest's behavior. ## Changes - Modified `formatLabel` in `src/bun.js/test/jest.zig` to check if the value is a primitive string and use `toString()` instead of the formatter with `quote_strings=true` - Added regression test in `test/regression/issue/23206.test.ts` ## Example **Before:** ``` test.each([ { name: "apple" }, { name: "banana" } ])("fruit #%# is $name", fruit => { // Test names were: // "fruit #0 is "apple"" // "fruit #1 is "banana"" }); ``` **After:** ``` test.each([ { name: "apple" }, { name: "banana" } ])("fruit #%# is $name", fruit => { // Test names are now: // "fruit #0 is apple" // "fruit #1 is banana" }); ``` ## Test plan - [x] Added regression test that verifies both `%s` and `$name` syntax produce consistent output - [x] Tested with `AGENT=0` - all tests pass - [x] Verified other primitive types (numbers, booleans) still format correctly - [x] Verified complex objects still use proper formatting This matches Jest's behavior after their fix: jestjs/jest#7689 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: pfg <pfg@pfg.pw>
pull Bot
pushed a commit
that referenced
this pull request
Apr 15, 2026
…-sh#29330) ## What Adds an early-return at the top of `ResumableSink.cancel()` when `status == .done`, so `onEnd` fires at most once. Fixes oven-sh#20740 Fixes oven-sh#21463 ## Why When a `fetch()` with a `ReadableStream` request body is aborted, `ResumableSink.cancel()` is called from `FetchTasklet.abortListener()` (FetchTasklet.zig:1203). The HTTP thread then completes with failure and `onProgressUpdate`'s reject path calls `sink.cancel()` a second time at FetchTasklet.zig:576 (and `onBodyReceived` at :342) — `this.sink` is only nulled in `clearSink←clearData←deinit`. `cancel()` (ResumableSink.zig:228) guarded against re-entry only for `status == .piped`. For the JS-route sink it relied on `#js_this.tryGet()` returning null after `detachJS()`, but `JSRef.downgrade()` (JSRef.zig:153-160) preserves the wrapper value as `.weak = <wrapper>`, and `tryGet()` (JSRef.zig:111) returns non-null for any non-empty weak. So the second `cancel()` re-enters the block and re-invokes `onEnd` → `FetchTasklet.writeEndRequest` → unconditional `defer this.deref()` (FetchTasklet.zig:1286). That second deref releases the single ref taken in `startRequestStream()` (FetchTasklet.zig:300) twice. Ref-count math: init(1) + queue(1) + startRequestStream(1) = 3 → cancel#1 deref → 2 → derefFromThread → 1 → cancel#2 deref → 0 → `deinit()`/`destroy()` runs *inside* `onProgressUpdate`, then its defer at :471-477 does `this.mutex.unlock()` + `this.deref()` on freed memory. `jsEnd()` already has an `isDetached()` guard for the same reason; `cancel()` was missing the equivalent. Using `status == .done` (rather than `isDetached()`) keeps the `.piped` branch reachable since piped sinks never set `#js_this` to `.strong`. ## Test `test/js/web/fetch/fetch-abort-stream-body.test.ts` reproduces the use-after-free in a debug/ASAN build: ``` [fetchtasklet] abortListener [fetchtasklet] writeEndRequest hasError? true <- cancel #1 [fetchtasklet] callback success=false ... [fetchtasklet] onProgressUpdate [fetchtasklet] onReject [fetchtasklet] writeEndRequest hasError? true <- cancel #2 (over-deref) [fetchtasklet] deinit ==40720==ERROR: AddressSanitizer: use-after-poison ... in onProgressUpdate ```
pull Bot
pushed a commit
that referenced
this pull request
Apr 17, 2026
) ## Problem Fuzzilli hit a flaky SIGSEGV (fingerprint `2519cad1804eace1`) from: ```js const v13 = Bun.jest().vi; try { v13.mock("function f2() {\n const v6 = new ArrayBuffer();\n ...\n}"); } catch (e) {} Bun.gc(true); ``` `JSMock__jsModuleMock` calls `Bun__resolveSyncWithSource` on the specifier before validating the callback, which sends the garbage string through the resolver. The resolver's auto-install gate at `loadNodeModules` only checks `esm_ != null`; `ESModule.Package.parse` accepts anything that doesn't start with `.` or contain `\` / `%`, so the whole function source is treated as a package name. `enqueueDependencyToRoot` then calls `PackageManager.sleepUntil`, which re-enters `EventLoop.tick()` from inside a call that is itself running inside an event-loop tick: ``` #0 ConcurrentTask.PackedNextPtr.atomicLoadPtr #1 UnboundedQueue(ConcurrentTask).popBatch #3 event_loop.tickConcurrentWithCount #7 AnyEventLoop.tick #8 PackageManager.sleepUntil #9 PackageManager.enqueueDependencyToRoot #10 Resolver.resolveAndAutoInstall #16 Bun__resolveSyncWithSource #17 JSMock__jsModuleMock ``` The same path is reachable from `Bun.resolveSync`, `import()`, and `require.resolve` with any user-provided string. ## Fix Gate the auto-install branch on `strings.isNPMPackageName(esm_.?.name)`. That validator already exists and is used by `bun link`, `bun pm view`, and the bundler; it rejects newlines, spaces, braces, and anything else that could never be a registry package. Specifiers failing the check fall straight through to `.not_found` — the same result the registry fetch would eventually produce — without initializing the package manager or ticking the event loop. This is a resolver-level fix, so it covers every entry point (not just `mock.module`). It also avoids spurious network requests for garbage specifiers; on this container a single resolve of a multi-line specifier dropped from ~275ms to ~16ms. ## Tests - `test/js/bun/resolve/resolve-autoinstall-invalid-name.test.ts` stands up a local registry and verifies zero manifest requests for a set of invalid names with `--install=force`, plus a positive control that a valid name still hits the registry. - `test/js/bun/test/mock/mock-module-non-string.test.ts` gains a case for `mock.module` with newline / whitespace / bracket specifiers (with and without a callback). - Existing `test/cli/run/run-autoinstall.test.ts` (11 tests) and `test/js/bun/test/mock/mock-module.test.ts` all pass. Related: oven-sh#28945, oven-sh#28956, oven-sh#28500, oven-sh#28511. Fingerprint: `2519cad1804eace1`
pull Bot
pushed a commit
that referenced
this pull request
Apr 22, 2026
…uild-cpp) (oven-sh#29545) ## What Adds WebKit-style unified-source bundling to the C++ build and expands the precompiled header. At configure time, `scripts/build/unified.ts` writes `UnifiedSource-<dir>-<n>.cpp` wrappers that `#include` 16 sibling `.cpp` files each, then compiles those instead of the originals. Combined with adding `ZigGlobalObject.h` + `BunClientData.h` to the PCH and dropping a pair of bogus header-level explicit template instantiations, this collapses **547 → 82** translation units. ## Why `-ftime-trace` + ClangBuildAnalyzer on a release `cpp-only` build showed **83 % of compile time is frontend parsing** — every tiny `.cpp` re-parses `ZigGlobalObject.h`, `BunClientData.h`, and the JSDOM converter headers. JSC builds in ~3 min in CI with far more C++ because it bundles 8 files per TU; we were compiling each file standalone. ## Numbers Release `cpp-only`, deps cached, 64-core Linux: | | TUs | CPU time | wall | | --------------------- | --- | -------- | ---- | | main | 547 | 3613 s | — | | `--unifiedSources=off`| 547 | 3464 s | 1:24 | | **this PR** | **82** | **866 s** | **0:40** | ClangBuildAnalyzer: frontend 3004 s → 1260 s → ~550 s; backend 609 s → 407 s. The `JSValueInWrappedObject::visit` template (previously the #1 instantiation at 234 s) no longer appears. ## Knobs - `--unifiedSources=false` restores per-file compilation (useful when iterating on a single `.cpp` and you don't want its 15 bundle-mates recompiling). - `--timeTrace=true` adds `-ftime-trace` so you can re-profile with ClangBuildAnalyzer. - `compile_commands.json` still has an entry per original `.cpp`, so clangd keeps working on individual files. ## Source changes exposed by bundling - `JSValueInWrappedObject.h` — drop `template void ...visit(...)` explicit instantiations from the header (re-instantiated by every includer at ~3.2 s each; upstream WebKit doesn't have them). - `root.h` — add `BunClientData.h` + `ZigGlobalObject.h` to the PCH (parsed once instead of ~130×). - `v8_compatibility_assertions.h` — `__LINE__` → `__COUNTER__` so the namespace-rebinding macro generates unique names per TU. - `V8Context.h` — fix bogus `namespace shim { class Isolate; }` forward decl that was creating a phantom `v8::shim::Isolate` shadowing `v8::Isolate` (real bug, only became visible when shim/ files shared a TU). - Two `#pragma std::once_flag` typos → `#pragma once`; one missing `#pragma once`. - Qualify a handful of `Exception`/`SourceProvider`/`call` references with `JSC::` so they don't become ambiguous when bundled with files that pull in `WebCore::Exception`. - `JSStringDecoder.cpp` — include `JSBufferEncodingType.h` directly instead of relying on a sibling. - `ProcessBindingBuffer.cpp` — `#undef PROCESS_BINDING_NOT_IMPLEMENTED` at EOF so it doesn't leak into the next file in its bundle. ## Excluded from bundling - 16 `webcrypto/CryptoAlgorithm*.cpp` files that share file-static helper names (`aesAlgorithm`, `cryptEncrypt`, `ALG128`, …) — upstream WebKit also compiles these standalone. - `webcore/JSWasmStreamingCompiler.cpp`, `JSDOMPromiseDeferred.cpp`, `JSMessageEventCustom.cpp`, `JSMIMEType.cpp` — wrap types whose `toJS`/`wrapperKey` overloads aren't ADL-reachable, so they rely on ordinary lookup at template-def time. - A handful of large TUs (`ZigGlobalObject.cpp`, `bindings.cpp`, …) that already saturate a core and shouldn't be serialized with siblings. No runtime behaviour change — same code, fewer redundant header parses. ## Follow-ups (not in this PR) - Windows PCH (`/Yc`/`/Yu` plumbing in `compile.ts` is TODO'd). - `ErrorCode.h` pulls in all of `ZigGlobalObject.h` for ~170 TUs that only need forward decls — only matters for `--unifiedSources=false` now. - `BunClientData.h`'s `unique_ptr<ExtendedDOMIsoSubspaces>` instantiates a heavy destructor in every includer; an out-of-line dtor would cut another ~30 s. --------- Co-authored-by: root <root@ip-10-0-2-234.us-west-2.compute.internal> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
Apr 22, 2026
…yToRoot (oven-sh#29483) Fuzzilli found a use-after-poison in the runtime auto-install path. `enqueueDependencyToRoot` passed `&lockfile.buffers.dependencies.items[dep_id]` into `enqueueDependencyWithMainAndSuccessFn`. When the manifest for the requested package is already cached (on disk or in memory) but the extracted tarball is not, control reaches `getOrPutResolvedPackageWithFindResult`, which calls `Lockfile.Package.fromNPM`. That grows `buffers.dependencies` via `ensureUnusedCapacity` to make room for the package's own dependencies, reallocating the backing storage. The subsequent `.extract` branch then read `dependency.behavior.isRequired()` from the freed buffer. ``` #0 getOrPutResolvedPackageWithFindResult PackageManagerEnqueue.zig:1520 dependency.behavior.isRequired() #1 getOrPutResolvedPackage PackageManagerEnqueue.zig:1778 #2 enqueueDependencyWithMainAndSuccessFn PackageManagerEnqueue.zig:523 #3 enqueueDependencyToRoot PackageManagerEnqueue.zig:321 #4 Resolver.enqueueDependencyToResolve resolver.zig:2356 ... #14 Bun__resolveSync #15 functionImportMeta__resolveSyncPrivate (runtime require() path) ``` Two changes: - `enqueueDependencyToRoot` now copies the `Dependency` to the stack before taking its address, matching every other caller of `enqueueDependencyWithMainAndSuccessFn` (`processDependencyListItem`, `processPeerDependencyList`, etc.). - The one read that ran after `fromNPM` now uses the `behavior` parameter that was already passed by value, instead of re-dereferencing `dependency`. Repro (debug/ASAN only): auto-install a package with a warm on-disk manifest but no extracted tarball — `fromNPM` appending even a single dependency forces a realloc of the one-entry buffer. The new test warms the cache, removes the extracted tarballs, and runs `require()` via `-e` so it goes through `Bun__resolveSync` → `enqueueDependencyToRoot`.
pull Bot
pushed a commit
that referenced
this pull request
Apr 23, 2026
`ResolveMessage.create` stored the `referrer` path via `Fs.Path.init`
without cloning. Every caller passes a temporary buffer — the `toUTF8()`
of a `bun.String` that is `deinit()`'d on return — so reading
`.referrer` after the creating frame unwound was a use-after-free.
Found by Fuzzilli as a flaky `use-after-poison` via `vi.mock()` →
`Bun__resolveSyncWithSource` → `resolveMaybeNeedsTrailingSlash`, but it
reproduces deterministically under ASAN with any non-ASCII source path:
```js
let err;
try {
Bun.resolveSync("./does-not-exist", "/tmp/café-🎉/file.js");
} catch (e) { err = e; }
Bun.gc(true);
err.referrer; // use-after-poison
```
```
==3080==ERROR: AddressSanitizer: use-after-poison on address 0x77cca9db0000 ...
READ of size 44 at 0x77cca9db0000 thread T0
#0 in __asan_memcpy
#1 in Zig::toStringCopy(ZigString) helpers.h:217
#2 in ZigString__toValueGC bindings.cpp:3402
#3 in ZigString.toJS ZigString.zig:57
#4 in ResolveMessage.getReferrer ResolveMessage.zig:221
```
In release builds the first 8 bytes of the returned referrer are
overwritten by mimalloc's free-list pointer instead of crashing.
Clone the referrer in `create()` and free it in `finalize()`. Also
`deinit()` the `toUTF8()` temporaries in `processFetchLog` now that
`create()` copies.
Co-authored-by: robobun <robobun@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
Apr 26, 2026
…ven-sh#29718) ## What does this PR do? Fixes the `glob-on-fuse.test.ts` flake on Alpine CI (79 occurrences across 44 of the last 70 builds, e.g. [build 47922](https://buildkite.com/bun/bun/builds/47922)). ### Root cause The test mounts a FUSE filesystem via `python3 fuse-fs.py` once **per test** (4×), polling up to `250 × 5ms = 1.25s` for the mount to appear. On Alpine, this file's deterministic shard slot happens to run **while `docker compose` is still extracting Redis/MinIO images** in the background. With disk I/O saturated, the first python3/libfuse cold-start exceeds the 1.25s budget and the assertion at line 41 fails. Tests 2-4 in the same file then pass (warm page cache, ~170ms per mount), and the retry passes (docker has finished). `run-file-on-fuse.test.ts` has the identical pattern but never flakes because it lands in a different shard whose tests #1-12 are slower, so it runs ~50s after docker finishes. | Shard | Test #13 starts | Docker compose finishes | Result | |---|---|---|---| | glob-on-fuse | t+136s | t+143s (7s **after**) | flake | | run-file-on-fuse | t+195s | t+143s (52s **before**) | pass | ### Fix - Mount once in `beforeAll` / unmount in `afterAll` instead of per-test (4× → 1× mount cycles). - Raise the poll budget from 1.25s to 8s; still exits early if the python process crashes. - `afterAll` runs even if `beforeAll` throws, so cleanup is guaranteed. - Applied the same change to `run-file-on-fuse.test.ts` since it has the same latent issue. ## How did you verify your code works? - `bun bd test test/cli/run/glob-on-fuse.test.ts test/cli/run/run-file-on-fuse.test.ts` → 6 pass, 0 fail - 20 consecutive runs of `glob-on-fuse.test.ts` and 10 of both files together → all pass, no leaked mounts - Passes under simulated cold-cache + I/O contention locally - Verified `afterAll` runs when `beforeAll` throws in Bun's test runner
pull Bot
pushed a commit
that referenced
this pull request
Apr 29, 2026
…en-sh#29910) ## What `Blob.dupeWithContentType` guarded its content_type handling on `duped.isHeapAllocated()` immediately *after* calling `duped.setNotHeapAllocated()`, so both branches were dead. When the source Blob's `content_type` is heap-allocated, the bitwise-copied dupe aliased the same allocation while both sides had `content_type_allocated == true`. This is a regression from oven-sh#23015: the pre-refactor code checked `duped.allocator != null` *before* clearing it at the end of the function; the refactor moved the clear to the top but left the (renamed) guard in place. ## Repro ```js const file = Bun.file(path, { type: "application/x-custom-type-not-in-registry-abcdefghijklm" }); const response = new Response(file); // body holds a dupe that aliases file.content_type await file.write("hello", { type: "application/x-..." }); // frees file.content_type response.headers.get("content-type"); // reads freed memory ``` On ASAN builds: ``` ==716==ERROR: AddressSanitizer: use-after-poison on address 0x71df454301c0 #1 in Zig::toStringCopy(ZigString) helpers.h:217 #2 in WebCore__FetchHeaders__put bindings.cpp:2082 #5 in bun.js.webcore.Response.getOrCreateHeaders Response.zig:358 ``` On release builds the freed slot gets reused and the read produces garbage: ``` TypeError: Header '25' has invalid value: 'ion/x-custom-type-not-in-registry-abcdefghijklm' ``` ## Fix Drop the `isHeapAllocated()` guard and always deep-copy an allocated `content_type` in `dupeWithContentType`. The old `!include_content_type` branch's "resolve to static mime or fall back to empty" is gone — it would have dropped FormData's `multipart/form-data; boundary=...` (and any non-registry type) on `Response.clone()`, and the branch itself was marked `// TODO: fix this / this is a bug`. The `include_content_type` parameter is now a no-op. Since every dupe now owns its `content_type` copy, `Blob.deinit()` frees it. That in turn required closing a few places that held a bitwise-copied Blob alongside the live owner: - `fromJSWithoutDeferGC` `move=true`: deep-copy `name`/`content_type` into the moved-out value so the source JS Blob keeps sole ownership; the BuildArtifact arm now `dupe()`s (its "move" only nulled the store on a local copy). - `getSliceFrom()`: free the dupe's copy before overwriting it with the slice's own type. - `doWrite`/`getWriter`: clear `content_type_allocated` after the in-place free so a registry-resolved static string isn't later freed by `deinit()`. - `BlobOrStringOrBuffer.deinitAndUnprotect`: only deref the store (matching its `deinit()`) since `.blob` is a raw view of a live JS Blob. ## Verified - `bun bd test test/js/web/fetch/blob.test.ts` — 16/16 pass - New UAF test fails on both debug/ASAN (use-after-poison) and system bun (garbage header) without the fix, passes with it - New clone test guards against dropping FormData's boundary on `Response.clone()` - ASAN stress: 1k× `Response.clone`/`blob.slice`/`createObjectURL`+revoke/`new Response([blob])`/`write({type})` — no double-free - RSS is flat across 50k × 1KB-type `Response.clone()` and `blob.slice()` --------- Co-authored-by: robobun <robobun@users.noreply.github.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
pull Bot
pushed a commit
that referenced
this pull request
Apr 30, 2026
Closes oven-sh#29925 Closes oven-sh#22808 Closes oven-sh#24019 ## What does this PR do? Fixes a bug where `Bun.RedisClient` would permanently reject every command with `Connection has failed` after the client entered the failed state (via reconnect exhaustion, manual `close()`, or a fatal socket error). Calling `client.connect()` did not recover the client — the process had to be restarted. ## Root cause Pre-refactor (before oven-sh#23141) `.failed` was a connection status and `doConnect` handled it explicitly: ```zig .failed => { this.client.flags.is_reconnecting = true; this.client.retry_attempts = 0; this.reconnect(); }, ``` The refactor folded `.failed` into `.disconnected` and a new `flags.failed` boolean but never wired up the reset path. Two things stayed sticky: 1. **`flags.failed`** — `send()` short-circuits on this and immediately rejects with `Connection has failed`. Once set in `failWithJSValue`, nothing ever cleared it. 2. **`flags.is_authenticated`** — kept `true` from the prior successful session, so when the new socket's HELLO response arrived, `handleResponse` skipped `handleHelloResponse` (which is guarded by `if (!this.flags.is_authenticated)`) and silently discarded the response. The client never transitioned back to `.connected` and `connect()` would hang until the connection timeout fired. ## Fix Two small resets: - `doConnect` (src/valkey/js_valkey.zig) clears `flags.failed` alongside `is_manually_closed` so an explicit `connect()` hands the client a clean slate. - `onOpen` (src/valkey/valkey.zig) clears `flags.failed`, `is_authenticated`, and `is_selecting_db_internal` so a fresh socket properly replays the HELLO handshake — matching what `onClose`'s auto-reconnect branch already does at L502–504. ## Verification New regression test at `test/regression/issue/29925.test.ts` spawns a local `redis-server` on a random port, drives the client into the failed state via `close()` (same terminal state as max-retries exhaustion), then asserts `connect()` recovers the client and subsequent commands complete round-trip. Gate check confirms the test times out without the fix. Also manually verified: - oven-sh#22808: tight `close()` + `connect()` + `send("FLUSHALL", ["SYNC"])` loop that previously locked up on iter 1 now runs cleanly across many iterations. - oven-sh#24019: after max-retries exhaustion during a redis restart, `client.connect()` recovers the client instead of returning `connected: true` while the next command still rejects. Reproduction from oven-sh#29925: ``` $ bun /tmp/repro.ts first set: Max reconnection attempts reached subsequent #0: Connection has failed ← forever subsequent #1: Connection has failed subsequent #2: Connection has failed ``` After the fix, `await client.connect()` brings the client back online and the next `set`/`get` pair succeeds. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: robobun <robobun@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
May 1, 2026
…double-free in deinit (oven-sh#29988) ## Repro Dev server with a directory watch that has two pending resolution-failure dependencies (`./sub/a` at index 0, `./sub/b` at index 1). Create `sub/a.ts` so dep 0 resolves; because it is not the tail slot, `freeDependencyIndex(0)` pushes index 0 onto `dependencies_free_list`. Shut the server down. ``` ==ERROR: AddressSanitizer: negative-size-param: (size=-6148914691236517206) #1 mem.Allocator.free #2 bake.DevServer.deinit /workspace/bun/src/bake/DevServer.zig:686 #3 bun.js.api.server.NewServer(.http,.debug).deinitIfWeCan Address 0xaaaaaaaaaaaaaaaa is a wild pointer ``` ## Cause `DirectoryWatchStore.freeDependencyIndex` frees `dep.specifier` and (in debug) sets the whole slot to `undefined`, then pushes the index onto `dependencies_free_list`. The slot stays in `dependencies.items`. `DevServer.deinit` iterates every `dependencies.items` slot and calls `alloc.free(watcher.specifier)` without consulting the free list, so free-list slots are freed a second time. In debug builds the `undefined` (0xAA…) slice trips ASAN's negative-size check; in release it is a straight double-free. `memoryCost` has the same blind iteration and would read `.len` from freed memory. ## Fix After freeing, write an empty slice back into `specifier` so the slot is safe to revisit: `alloc.free(&.{})` is a no-op and `.len == 0`. ## Verification New test `deinit with a free-list slot in DirectoryWatchStore.dependencies` in `test/bake/dev/bundle.test.ts` arranges the free-list slot and lets the harness's graceful-exit call `deinit`. - `git stash -- src/ && bun bd test … -t 'deinit with a free-list slot'` → 3/3 **fail** (ASAN abort at DevServer.zig:686) - with fix → 3/3 **pass** - adjacent `removing 'use client' from a component with a pending resolution failure` test still passes --------- Co-authored-by: robobun <robobun@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
May 1, 2026
…ven-sh#29971) ## What `FileSystemRouter`'s constructor (and `reload()`) initialize the error log with the arena allocator: ```zig const allocator = arena.allocator(); ... var log = Log.Log.init(allocator); ``` When route loading produces errors, the error paths did: ```zig arena.deinit(); globalThis.allocator().destroy(arena); return globalThis.throwValue(try log.toJS(...)); // reads arena-backed msgs.items ``` `log.msgs.items` is backed by the arena, so `log.toJS()` reads freed memory. ASAN reports `use-after-poison` in `logger.Log.toJS`. ## Repro ```js // pages/[foo.tsx — missing closing bracket new Bun.FileSystemRouter({ style: "nextjs", dir: "./pages", fileExtensions: [".tsx"] }); ``` Debug (ASAN) build: ``` AddressSanitizer: use-after-poison ... #1 in logger.Log.toJS (src/logger.zig:733) #2 in FileSystemRouter.constructor (src/bun.js/api/filesystem_router.zig:149) ``` ## Fix Build the JS error value first (while the arena is still live — `BuildMessage.create` / `ResolveMessage.create` clone the msg into `globalThis.allocator()`), then free the arena, then throw. Applied to all four `log.toJS()` call sites across `constructor()` and `reload()`. ## Verification - `git stash -- src/ && bun bd test filesystem_router.test.ts -t 'invalid route'` → **fail** (ASAN crash in subprocess) - `git stash pop && bun bd test filesystem_router.test.ts -t 'invalid route'` → **pass**, error message is `Route is missing a closing bracket]` - All 19 existing `filesystem_router.test.ts` tests pass. --------- Co-authored-by: robobun <robobun@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
May 2, 2026
…es (oven-sh#30077) ## What When a chunked (or HTTP/3) request body exceeds `maxRequestBodySize`, `onBufferedBodyChunk` writes the 413 directly on the raw uWS response: ```zig resp.writeStatus("413 Payload Too Large"); resp.endWithoutBody(comptime !http3); ``` `internalEnd` → `markDone()` nulls `onAborted`, so when the socket closes no abort ever fires to detach `ctx.resp` or release the base ref. `this.resp` is left pointing at a completed response whose socket is about to be freed by `us_internal_free_closed_sockets`. If the fetch handler returned a pending Promise: - **resolve**: `handleResolve` → `isAbortedOrEnded()` is false (`this.resp != null`) → `render()` → `runCorkedWithType` corks the freed socket → **heap-use-after-free** (ASAN trace below). - **reject**: `handleReject` reads `resp.hasResponded()` off freed memory, sees `true`, skips the error handler, and returns without ever releasing the base ref → **RequestContext leaks** (`server.pendingRequests` never returns to 0). ## Fix Route through `this.endWithoutBody()` (the `RequestContext` wrapper) instead of the raw `resp.endWithoutBody()`. That path does `detachResponse()` (nulls `this.resp`, clears `onData`/`onAborted`/`onTimeout`) and `deref()` (releases the base ref), matching every other end path in this file. The body promise is rejected with the specific `"Request body exceeded maxRequestBodySize"` error *before* `endWithoutBody()` so `endRequestStreaming()` doesn't overwrite it with a generic `ConnectionClosed`. `has_written_status` is set so any later `renderMissing`/`renderMetadata` knows the status line is already committed. ## Repro ``` ==ERROR: AddressSanitizer: heap-use-after-free #0 us_socket_group socket.c:77 #1 uWS::AsyncSocket<false>::getLoopData() AsyncSocket.h:69 #2 uWS::AsyncSocket<false>::isCorked() AsyncSocket.h:141 #3 uWS::HttpResponse<false>::cork(...) HttpResponse.h:647 #4 uws_res_cork libuwsockets.cpp:1740 #5 ...runCorkedWithType Response.zig:299 #6 ...doRenderBlob RequestContext.zig:1942 ... #11 ...handleResolve RequestContext.zig:220 #12 ...onResolve RequestContext.zig:154 freed by: #1 us_poll_free epoll_kqueue.c:73 #2 us_internal_free_closed_sockets loop.c:305 ``` ## Test `test/js/bun/http/serve-pending-promise-abort-leak.test.ts` — new case sends a raw `Transfer-Encoding: chunked` POST exceeding `maxRequestBodySize` with a handler that holds its resolve/reject, waits for the socket to be reclaimed, then settles the Promise. Asserts `pendingRequests` returns to 0 for both paths, the body was rejected with the right message, and a follow-up request still works. Without the fix: ASAN heap-use-after-free on the resolve path; on release builds the reject path shows `pendingAfterReject: 1` (leak). Co-authored-by: robobun <robobun@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
May 3, 2026
…ven-sh#30136) ## Repro ```js const client = await Bun.connect({ hostname, port, tls, socket: { ... } }); // after handshake: client.end("x"); client.flush(); // ← second markInactive frees *Handlers // peer replies close_notify → onClose derefs freed Handlers ``` ASAN on debug build: ``` ==4075==ERROR: AddressSanitizer: use-after-poison on address 0x7aff355e0469 READ of size 1 at 0x7aff355e0469 thread T0 #0 bun.js.api.bun.socket.NewSocket(true).onClose src/bun.js/api/bun/socket.zig:661:46 #1 deps.uws.handlers.PtrHandler(...).onClose src/deps/uws/handlers.zig:49:61 ... #5 us_internal_ssl_on_close packages/bun-usockets/src/crypto/openssl.c:940:29 ``` ## Cause `end()` → `internalFlush` → `canEndAfterFlush()` → `markInactive()` → `closeAndDetach(.normal)` detaches `this.socket` and calls `us_socket_close(code=0)`. For TLS with `code==0`, `us_internal_ssl_close` sends close_notify and **defers** the raw close until the peer replies (so the loop stays alive to receive it). `markInactive` returns early without clearing `is_active`, relying on the eventual `onClose` → `markInactive` to run `handlers.markInactive()` and free the client-mode `*Handlers`. `flush()` was the only `internalFlush()` caller without an `isDetached()` guard. Calling it in that window re-enters `canEndAfterFlush()` (still `is_active && end_after_flush`) → `markInactive()`, which now sees the detached socket as closed and runs the **full** teardown: `handlers.markInactive()` → `active_connections == 0` → `vm.allocator.destroy(handlers)`. When the peer's close_notify later arrives, `onClose` calls `this.getHandlers()` on freed memory. ## Fix Add the same `isDetached()` early-return to `flush()` that `end()`, `endBuffered()`, `onWritable`, and every other `internalFlush()` caller already have. ## Verification New test in `test/js/bun/net/socket.test.ts` spawns a TLS client that does `end("x"); flush(); flush();` after handshake and awaits `close`. - Without fix (`git stash -- src/`): subprocess aborts with the ASAN trace above; test fails. - With fix: subprocess prints `OK` and exits 0; test passes. Co-authored-by: robobun <robobun@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
May 3, 2026
…0148) ## Repro ```js const server = Bun.listen({ hostname: '127.0.0.1', port: 0, socket: { open(s){s.end()}, data(){} } }); const client = await Bun.connect({ hostname: '127.0.0.1', port: server.port, socket: { data(){}, close(){} } }); // ... after close fires and the native onClose unwinds: client.listener; // ← reads handlers.mode through freed pointer ``` ASAN on debug build: ``` ==4232==ERROR: AddressSanitizer: use-after-poison on address 0x79f744320469 READ of size 1 at 0x79f744320469 thread T0 #0 bun.js.api.bun.socket.NewSocket(false).getListener src/bun.js/api/bun/socket.zig:760:25 #1 TCPSocketPrototype__listenerGetterWrap ZigGeneratedClasses.cpp:66452:34 ``` ## Cause Client-mode `Handlers` are heap-allocated per `Bun.connect` (Listener.zig:795). When the socket closes, the socket's `markInactive` calls `handlers.markInactive()`, which — for non-`.server` modes — drops `active_connections` to zero, `deinit`s and `destroy`s the allocation. `this.handlers` is never cleared, so it's left pointing at freed memory. The `.listener` getter then does: ```zig const handlers = this.handlers orelse return .js_undefined; // non-null, dangling if (handlers.mode != .server or this.socket.isDetached()) { // ← UAF read ``` Same in `setServername` → `isServer()` → `getHandlers().mode`. The reconnection path in `Listener.connect` (line 813-816) also checks `if (prev.handlers) |h| { h.deinit(); destroy(h); }` — a double-free when the previous connection's `markInactive` already freed it. ## Fix In the socket's `markInactive`, capture `handlers.mode == .server` before calling `handlers.markInactive()`, then null `this.handlers` for non-listener-owned modes. `isServer()` now returns `false` on null instead of panicking via `getHandlers()`. `.server`-mode handlers are embedded in the Listener struct (not freed here), so those pointers stay intact. ## Verification New test in `test/js/bun/net/socket.test.ts` spawns a client, waits for close + one `setImmediate` hop (so the deferred `markInactive` has run), then reads `.listener`. - Without fix (`git stash -- src/`): subprocess aborts with the ASAN trace above; test fails on `expect(stdout).toBe("listener:undefined\\n")`. - With fix: subprocess prints `listener:undefined` and exits 0; test passes. Co-authored-by: robobun <robobun@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
May 3, 2026
…sh (oven-sh#30162) ## What Fixes a use-after-free in `fs.promises.cp(src, dest, { recursive: true })` when one file copy fails while sibling `SingleTask`s are still running on the thread pool. ## Repro Recursive `fs.promises.cp` of a directory where copying one file fails (e.g. its destination path is already a directory → `EISDIR`) while ~100 sibling file copies are in flight. Under ASAN: ``` ==4475==ERROR: AddressSanitizer: use-after-poison on address 0x79676fca08b0 WRITE of size 8 at 0x79676fca08b0 thread T22 (Bun Pool 11) #0 atomic.Value(usize).fetchSub #1 NewAsyncCpTask(false).SingleTask.workPoolCallback src/bun.js/node/node_fs.zig:528 ``` ## Cause `finishConcurrently()` used the `has_result` cmpxchg only to ensure the **result** was set once, then immediately enqueued `runFromJSThread` → `deinit()` → `bun.destroy(this)`. It did not wait for `subtask_count` to reach zero, so: - A `SingleTask` that errored called `finishConcurrently(err)` and returned **without** decrementing `subtask_count`. The JS thread then freed the parent while other `SingleTask`s were still dereferencing `cp_task->args` / `cp_task->subtask_count`. - `cpAsync` decremented `subtask_count` after `_cpAsyncDirectory` returned an error, by which time `runFromJSThread` could already have freed `this`. ## Fix - `finishConcurrently(result)` now only records the result (first caller wins). - New `onSubtaskDone()` decrements `subtask_count` with `.acq_rel` ordering; only the caller that drops it to zero enqueues `runFromJSThread`. If no one recorded a result, it defaults to `.success`. - `cpAsync` drops its initial reference via `defer this.onSubtaskDone()`, covering every early return (Windows, non-directory, EISDIR, and the recursive path). - `SingleTask.workPoolCallback` always ends with `this.deinit(); parent.onSubtaskDone();` on both success and error paths. This matches the pattern already used by `AsyncReaddirRecursiveTask`. ## Verification New test in `test/js/node/fs/cp.test.ts` creates a source dir with 128 files plus one whose destination is a pre-existing directory, and runs `fs.promises.cp` 50× in a subprocess. - **Without fix** (`git stash -- src/ && bun bd test`): subprocess aborts with the ASAN `use-after-poison` shown above → test fails. - **With fix** (`bun bd test`): subprocess rejects with `EISDIR` every iteration, exits 0 → test passes. - Full `cp.test.ts` suite: 38 pass, 3 skip (Windows-only), 0 fail. - `zig:check-all` passes on all targets. --------- Co-authored-by: robobun <robobun@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
May 3, 2026
## Problem
`MarkedArrayBuffer.destroy()` did two things:
```zig
allocator.free(content.buffer.slice()); // free the bytes
allocator.destroy(this); // free *this
```
Every constructor that is actually used (`fromString`, `fromBytes`,
`fromJS`, `fromTypedArray`, `fromArrayBuffer`) returns
`MarkedArrayBuffer` **by value**, so `this` is never an individually
heap-allocated struct — it's a stack local, an embedded field, or an
ArrayList slot. The `allocator.destroy(this)` call passes that interior
pointer to mimalloc.
In the readdir Buffer error-cleanup path (`readdirWithEntries` /
`readdirInner`), entries are appended by value via
`Buffer.fromString()`:
- `allocator.destroy(&entries.items[0])` frees `entries.items.ptr`
- the next loop iteration reads `this.*` from poisoned memory
- `entries.deinit()` frees the same pointer again
## Repro
```js
const fs = require('fs');
// dir contains regular files + a self-referential symlink 'loop -> loop'
fs.readdirSync(dir, { encoding: 'buffer', recursive: true });
```
The recursive walk collects Buffer entries for the root, then fails with
`ELOOP` opening the symlink (not in the swallowed `NOENT/NOTDIR/PERM`
set), and enters the cleanup loop. Under ASAN:
```
==3593==ERROR: AddressSanitizer: use-after-poison on address 0x737ec6e50040
READ of size 64 at 0x737ec6e50040 thread T0
#1 MarkedArrayBuffer.destroy array_buffer.zig:591
#2 NodeFS.readdirInner node_fs.zig:5013
#3 NodeFS.readdir node_fs.zig:4518
```
## Fix
- Drop `allocator.destroy(this)` from `MarkedArrayBuffer.destroy()`. The
struct is passed/stored by value; callers own its storage.
- Remove the unused `MarkedArrayBuffer.init()` (the only function that
heap-allocated the struct, zero callers) so there's no pairing that
would leak.
- The readdir call sites keep calling `.destroy()`, which still checks
`this.allocator` before freeing bytes — JS-owned buffers remain
untouched.
Also fixed the adjacent `Dirent` arm of the recursive-sync error
cleanup: `result.name.deref()` → `result.deref()` so `Dirent.path` is
released too (matching the non-recursive and async cleanup sites).
## Verification
New test in `test/js/node/fs/fs.test.ts` creates a temp dir with files +
a self-referential symlink, spawns a subprocess that calls
`readdirSync({encoding:'buffer', recursive:true})`, and asserts it
throws `ELOOP` and exits 0.
```
# without fix
(fail) readdirSync({encoding: 'buffer', recursive: true}) frees entries safely ...
{ exitCode: 134, stdout: "" } # SIGABRT from ASAN
# with fix
(pass) readdirSync({encoding: 'buffer', recursive: true}) frees entries safely ... [1.5s]
{ exitCode: 0, stdout: "ELOOP" }
```
`zig:check-all` passes on all targets.
---------
Co-authored-by: robobun <robobun@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
May 19, 2026
… stack pointer (oven-sh#31020) ## What `resolveMaybeNeedsTrailingSlash` swaps `vm.log` / `resolver.log` to a stack-local `Log` for the duration of `_resolve`, then restores them via a drop guard. The Zig original also swaps and restores `transpiler.linker.log` and `resolver.package_manager.log`; the Rust port had those behind a `TODO(b2-cycle)` and only handled `vm.log` + `resolver.log`. When auto-install is enabled and the resolver lazily creates the `PackageManager` during `_resolve`, `Resolver::get_package_manager` seeds `pm.log` from `resolver.log` — which at that point is the **stack-local** `Log`. Because the restore guard never touched `pm.log`, it was left pointing into a dead stack frame after the function returned. The next resolve at a different stack depth that routes through the auto-install task runner dereferenced that stale pointer in `Log::add_error_fmt`, tripping ASAN's `stack-use-after-scope` (or segfaulting / executing garbage in release builds). Stack at the fault: ``` #0 bun_ast::Log::add_formatted_msg #1 bun_ast::Log::add_error_fmt #2 bun_install::…::run_tasks #7 bun_install::…::enqueue_dependency_to_root #9 bun_resolver::Resolver::enqueue_dependency_to_resolve #14 bun_resolver::Resolver::resolve_and_auto_install #15 bun_jsc::VirtualMachine::_resolve #16 bun_jsc::VirtualMachine::resolve_maybe_needs_trailing_slash::<true> ``` ## Fix Swap and restore `linker.log` and (when present) `package_manager.log` in both copies of the resolve log guard (`VirtualMachine::resolve_maybe_needs_trailing_slash` and `jsc_hooks::resolve_hook`), matching `VirtualMachine.zig`. The restore re-checks `resolver.package_manager` at drop time so a PM that was lazily created during `_resolve` is also pointed back at the VM log. Also adds the missing `<cassert>` include in `wtf-bindings.cpp`, which stopped being pulled in transitively. ## Repro ```js // run from an empty dir with // BUN_CONFIG_INSTALL=fallback BUN_CONFIG_REGISTRY=http://127.0.0.1:1 const realm = new ShadowRealm(); const variants = [ () => realm.importValue("pkg-not-found-a", "x"), () => (() => realm.importValue("pkg-not-found-b", "x"))(), () => (() => (() => realm.importValue("pkg-not-found-c", "x"))())(), () => import("pkg-not-found-f"), ]; for (let i = 0; i < 100; i++) for (const v of variants) try { v()?.catch?.(() => {}); } catch {} ``` Segfaults on `main`, clean after this change. Fixes oven-sh#14432 Fixes oven-sh#22407 --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
May 23, 2026
…s longer than the comparand (oven-sh#31264) ### What does this PR do? Fixes an ASAN `global-buffer-overflow` found by fuzzing the CSS parser: ``` asan:global-buffer-overflow:strncasecmp|eql_case_insensitive_ascii|eql_case_insensitive_ascii|bun_core::string::immutable::eql_case_insensitive_ascii_ignore_length ``` **Repro** ```sh BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING=1 bun -e 'require("bun:internal-for-testing").cssInternals.minifyTest(":nth-child(Nn", "")' ``` ``` ==ERROR: AddressSanitizer: global-buffer-overflow READ of size 2 ... #0 strncasecmp #1 bun_core::strings_impl::eql_case_insensitive_ascii src/bun_core/lib.rs #2 bun_core::string::immutable::eql_case_insensitive_ascii_ignore_length src/bun_core/string/immutable.rs #3 bun_css::css_parser::nth::parse_nth src/css/css_parser.rs #4 bun_css::selectors::parser::parse_nth_pseudo_class src/css/selectors/parser.rs ``` **Cause** `strings_impl::eql_case_insensitive_ascii(a, b, check_len)` defers to `strncasecmp(a, b, a.len())`, which reads up to `a.len()` bytes from *both* buffers. The Zig original (`strings.eqlCaseInsensitiveASCII`) compared against NUL-terminated comptime literals, so `strncasecmp` stopped at the sentinel and reported a mismatch whenever `a` was longer than `b`. Rust byte-string literals carry no terminator, so the An+B parser's ident branch (`parse_nth`), which compares an arbitrary user ident against the keywords `"even" / "odd" / "n" / "-n" / "n-" / "-n-"` with the ignore-length variant, reads past the end of the keyword literal as soon as the ident is longer than the keyword and shares its prefix (`Nn` vs `n`, `n-3` vs `n`, …). Besides the OOB read, the comparison result depended on whatever byte happens to follow the literal in rodata. **Fix** Reject `b.len() < a.len()` up front in `eql_case_insensitive_ascii` before calling `strncasecmp` — the same result the NUL sentinel produced in Zig, so observable behavior is unchanged for every in-bounds input (all other callers of the ignore-length variant already pass equal-length slices). `strncasecmp` now only ever reads within both slices. **Verification** - `bun bd test test/js/bun/css/nth-anplusb-ident.test.ts` without the fix (src/ stashed): aborts with the ASAN global-buffer-overflow above. - With the fix: passes. The new test covers valid `n-<digits>` idents that are longer than the `n`/`n-` keywords (`:nth-child(n-3)`, `:nth-child(N-3)`, `:nth-last-child(n- 42)`), keyword case-insensitivity (`:nth-child(N)`), an invalid ident (`:nth-child(NN)` → parse error), and the exact fuzzer-minimized input run in a subprocess. - `bun bd test test/js/bun/css/css.test.ts`: 1032 pass, 0 fail (no behavior change for the existing suite). - A second fuzz report hits the same overflow through `Bun.build` with a CSS entrypoint containing `:nth-child(Nn`; that path goes through the same `parse_nth` comparison and is covered by this fix (`Bun.build` now reports a parse error instead of aborting). - The `build-rust` CI failures on this PR (unused label / unnecessary `unsafe` warnings in `src/spawn`, `src/install`, `src/crash_handler`, `src/runtime/ffi`, `src/runtime/dns_jsc`) are present on current `main` commits that don't include this change and come from files this PR doesn't touch.
pull Bot
pushed a commit
that referenced
this pull request
Jun 24, 2026
…AF through tunnel (oven-sh#32635) ## What Comprehensive stress testing of the HTTP client proxy code paths (fetch and WebSocket, both `ProxyTunnel` and `WebSocketProxyTunnel`): **661 tests** across 5 new files plus a shared adversarial proxy/origin helper and a subprocess memory-probe fixture. | File | Tests | Covers | | --- | --- | --- | | `proxy-stress-helpers.ts` | n/a | Adversarial CONNECT + absolute-form proxy (http/https outer) with per-stage RST/kill/trickle/split hooks; adversarial origin with content-length / chunked / close-delimited × identity/gzip/deflate/br/zstd, truncation at arbitrary byte offset, redirect, echo. | | `proxy-stress-matrix.test.ts` | 335 | `{http,https}` proxy × `{http,https}` origin × framing × encoding × body-size × keepalive response matrix; upload matrix across string/Uint8Array/Blob/FormData/ReadableStream/async-iterator; streamed response via `getReader()`; trickled 1-byte-per-tick downstream; split CONNECT envelope; RFC 9110 §9.3.6 ignored-header handling; hop-by-hop stripping; method matrix; cross-scheme redirects. | | `proxy-stress-lifecycle.test.ts` | 93 | Proxy RSTs client at every tunnel stage (request-received / upstream-connected / connect-replied / first-client-byte / first-upstream-byte) × both origins × both keepalive; same during upload (string + stream body); proxy drops upstream at every stage; origin RST at 0/10/60/200 response bytes; close-delimited × compression; abort at every stage; abort after headers; abort churn (200× under ASAN); ASAN-only TLS-alert-during-handshake UAF repro. | | `proxy-stress-errors.test.ts` | 51 | CONNECT 400/403/407/500/502/503/504/301/302; proxy unreachable; upstream unreachable (502 via CONNECT and absolute-form); proxy auth (missing/wrong/correct, URL and header) for all 4 combos; inner-TLS verify (CA match, no-CA fail, checkServerIdentity reject); unsupported scheme (ftp/socks4/socks5/socks5h/ws); h2-capable origin through proxy stays HTTP/1.1. | | `proxy-stress-concurrent.test.ts` | 31 | 32× parallel per combo × keepalive; tunnel reuse (sequential + auth-keyed); 12 origins through one proxy; `reject_unauthorized` pool gate (lax → strict forces fresh CONNECT); 4× concurrent 4MB echo; idle pooled tunnel receiving stray data is evicted; 12 subprocess memory probes (6 modes × 2 proxy schemes, 300-1200 iterations each, RSS-growth bounded). | | `proxy-stress-adversarial.test.ts` | 151 | Stacked split-CONNECT × chunked × compression × keepalive; origin status 200-503 × all combos; origin-facing Host/header shape; `checkServerIdentity` approve/reject/GC-loop; path/query edges; `verbose:true`; `AbortSignal.timeout`; interleaved proxy/direct to same origin; CONNECT target shape; **WebSocket proxy matrix** (ws/wss × http/https proxy) with echo, 256KB binary, RST at CONNECT, 407 without auth, rapid close, wss-via-https-proxy open/close churn under GC. | Full suite runs in ~5 min under debug+ASAN. ## Bugs found and fixed Two pre-existing bugs surfaced by the suite (both reproduce on main without any test-helper changes). Fixed here in `src/http/lib.rs`. ### 1. Streamed request body through a CONNECT tunnel never sent `fetch()` to an `https://` origin through any proxy with a `ReadableStream` / async-iterator body hangs forever: CONNECT succeeds, inner TLS handshake completes, the `Transfer-Encoding: chunked` request head is written, then nothing. String/Uint8Array/Blob/FormData bodies are fine; `http://` origins (absolute-form, no tunnel) are fine. Cause (two layers): - The `RequestStage::ProxyHeaders` arm of `on_writable` computes `has_sent_body = self.request_body().is_empty()`. For `HTTPRequestBody::Stream` the bytes buffer is always empty here, so the request jumps straight to `RequestStage::Done` and the `is_streaming_request_body` signal path a few lines below never runs. The non-proxy `send_initial_request_payload` already gates this on `matches!(original_request_body, HTTPRequestBody::Bytes(_))`. - Even once `ProxyBody` is reached, its `Stream` case calls `flush_stream` → `write_to_stream_using_buffer` → `write_to_socket(socket, …)`, which writes plaintext chunked bytes to the outer proxy socket instead of through the inner TLS session. The `Bytes` case right next to it correctly routes through `ProxyTunnel::write`. Fix: mirror the non-proxy path's `Bytes`-only `has_sent_body` check (and matching `debug_assert`) in the `ProxyHeaders` arm; in `write_to_stream_using_buffer`, route through `ProxyTunnel::write` when `self.proxy_tunnel.is_some()`, treating `WantRead`/`WantWrite` from the inner SSL as backpressure. Encrypted output reaches the outer socket via the existing `write_encrypted` callback, same as the `Bytes` path. This also covers the `HTTPThread` drain loop which calls the same `flush_stream`. ### 2. `heap-use-after-free` in `HTTPClient::on_writable` when a TLS alert is buffered with the inner handshake flight ``` READ of size 1 thread T12 (HTTP Client) #0 HTTPClient::on_writable::<true, false> src/http/lib.rs:~2856 #1 bun_http::proxy_tunnel::on_handshake src/http/ProxyTunnel.rs:450 freed by: AsyncHTTP::on_async_http_callback_raw src/http/AsyncHTTP.rs:813 HTTPClient::close_and_fail::<false> bun_http::proxy_tunnel::on_close src/http/ProxyTunnel.rs:577 SSLWrapper::handle_reading src/uws/lib.rs:1053 SSLWrapper::flush src/uws/lib.rs:669 ProxyTunnel::on_writable::<false> src/http/ProxyTunnel.rs:740 HTTPClient::on_writable::<true, false> src/http/lib.rs:~2850 bun_http::proxy_tunnel::on_handshake src/http/ProxyTunnel.rs:450 ``` If the origin closes or sends a TLS alert in the same buffer as its ServerHello flight (origin rejecting client cert, corrupted stream via misbehaving proxy, origin crash), the client's `on_handshake → on_writable → proxy.on_writable → SSLWrapper::flush → handle_reading` chain processes the alert, fires `on_close → close_and_fail`, which runs the result callback and frees the `ThreadlocalAsyncHTTP` embedding `*self`. Control returns to `on_writable`, which immediately reads `self.state.flags.is_waiting_for_cert_check` on freed memory. Same bug class already documented in `start_proxy_handshake`'s comment. Fix: `close_and_fail` → `terminate_socket` synchronously marks the outer socket closed, and the socket handle is owned by the event loop (outlives the client). Check `socket.is_closed()` immediately after `proxy.on_writable()` and return before touching `self`. Deterministic ASAN repro in `proxy-stress-lifecycle.test.ts` ("TLS alert in same buffer as inner handshake"): a CONNECT proxy that double-writes every client→upstream byte, making the origin's TLS stack abort with an alert. Looped 20× with `ASAN_OPTIONS=…:abort_on_error=1` so the HTTP-thread UAF aborts the subprocess before the main thread's clean exit wins the race. ## Verification ``` bun bd test test/js/bun/http/proxy-stress-*.test.ts # 661 pass, 0 fail (debug+ASAN, ~5min) ``` Fail-before (src/ stashed, `bun bd`): - `proxy-stress-matrix.test.ts -t "https-origin POST ReadableStream|https-origin POST async-iterator"` → 8 fail (hang/timeout) - `proxy-stress-lifecycle.test.ts -t "TLS alert in same buffer"` → 1 fail (subprocess aborts with `heap-use-after-free`) Existing `test/js/bun/http/proxy.test.ts` (48 tests) still passes. ## Related - oven-sh#31959 fixed a sibling UAF on the shutdown side of the same `SSLWrapper` callback chain. - oven-sh#30606 touches the same `ProxyTunnel` close path but only in the `.zig` reference files. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
Jun 26, 2026
…oven-sh#32742) ### What does this PR do? Fixes a use-after-free in the HTTP client's CONNECT proxy tunnel, caught by ASAN: ``` READ of size 8 at 0x61e00001fe80 thread T6 #0 Option<RefPtr<ProxyTunnel>>::as_ref #1 proxy_tunnel::on_close ProxyTunnel.rs:525 #2 SSLWrapper::trigger_close_callback uws/lib.rs:833 #3 SSLWrapper::handle_reading uws/lib.rs:1053 ... freed by thread T6 here (same stack, same `handle_reading` call): #5 AsyncHTTP::on_async_http_callback_raw AsyncHTTP.rs:819 #7 HTTPClient::send_progress_update_without_stage_check #9 proxy_tunnel::on_data ProxyTunnel.rs:350 #11 SSLWrapper::trigger_data_callback uws/lib.rs:824 #12 SSLWrapper::handle_reading uws/lib.rs:1046 ``` `SSLWrapper::handle_reading` flushes pending decrypted bytes to the data callback, then runs the close callback, guarded only by `closed_notified`: 1. The flushed data callback completes a keep-alive response through the tunnel. A fatal TLS record error sets only `fatal_error` — none of the shutdown flags — so the wrapper passed `tunnel_poolable`'s `!is_shutdown()` check and the tunnel was handed to the keep-alive pool. Nothing called `wrapper.shutdown()`, so `closed_notified` was never latched. Dispatching the final result then freed the `ThreadlocalAsyncHTTP` that embeds the `HTTPClient`. 2. The guard (`ssl.is_none() || closed_notified()`) passes. 3. `trigger_close_callback()` invokes `on_close(handlers.ctx)` with `ctx` pointing at the freed client. The pooling branch is the only terminal path that doesn't go through `close_proxy_tunnel(true)` → `wrapper.shutdown()` → `closed_notified`, which is the latch the read loop relies on. `SSLWrapper::shutdown` already special-cases the *close_notify* flavor of this for exactly that reason; the fatal-error flavor never reaches `shutdown()`. The fix is one predicate: a tunnel whose wrapper has a fatal error or pending unconsumed input/output is not poolable. That routes it through the orderly teardown that latches `closed_notified`, and the pending-I/O half closes the same hole for a tunnel pooled from a mid-loop data callback while more decrypted bytes or queued output remain. Both are also required for the pool to be correct on its own terms — a poisoned or dirty TLS session must not be handed to the next request. ### How did you verify your code works? New regression test in `test/js/bun/http/proxy.test.ts` (next to the existing close_notify sibling): an HTTPS keep-alive response through a CONNECT proxy with a corrupt TLS record appended to the same TCP burst, followed by a second request that can only complete if the HTTP client thread survived the first. Against an unfixed ASAN debug build the fixture aborts every run: ``` ==20981==ERROR: AddressSanitizer: heap-use-after-free on address 0x61e00001fe80 READ of size 8 at 0x61e00001fe80 thread T6 ... exit=134 ``` With this change it prints `4096 200 200` and exits 0 with no ASAN report. `test/js/bun/http/proxy.test.ts` (49/49), `fetch-proxy-connect-tunnel-split-envelope.test.ts`, `fetch-proxy-tls-intern-race.test.ts`, and `fetch-keepalive.test.ts` all pass.
pull Bot
pushed a commit
that referenced
this pull request
Jun 29, 2026
…ll-driven read (oven-sh#32986) ## Problem Heap use-after-free in Bun Shell when `epoll_ctl` fails while re-registering a pipe's `FilePoll` from a poll-driven read. Found by syscall-fault-injection fuzzing against `origin/main`. Follow-up to oven-sh#32754, which fixed the same failure on the eager spawn-time read path. ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 1, thread T0 #0 <bun_io::pipe_reader::BufferedReaderVTable>::link io/PipeReader.rs:105 #1 <bun_io::pipe_reader::BufferedReaderVTable>::on_read_chunk io/PipeReader.rs:125 #2 <bun_io::pipe_reader::PosixBufferedReader>::read_with_fn io/PipeReader.rs:890 #3 <bun_io::pipe_reader::PosixBufferedReader>::read_socket io/PipeReader.rs:576 #4 <bun_io::pipe_reader::PosixBufferedReader>::on_poll io/PipeReader.rs:529 #5 __bun_run_file_poll runtime/dispatch.rs:677 freed by: <alloc::sync::Arc<bun_runtime::shell::subproc::PipeReader>>::drop ``` ## Repro 1. `PipeReader::start` registers the poll and the eager spawn-time `read_all()` hits `EAGAIN`, so `read_with_fn`'s `EAGAIN` arm re-registers the poll and the spawn returns. 2. The child writes to stdout and the poll fires. `__bun_run_file_poll`'s `BUFFERED_READER` arm dispatches straight into `PosixBufferedReader::on_poll` with a bare `&mut *h` and no keepalive. 3. `read_with_fn` drains the chunk, `recv()` returns a real `EAGAIN`, and `register_poll()` issues another `epoll_ctl`, which fails (`ENOMEM` in the repro). 4. `register_poll` dispatches `on_reader_error`. The shell `PipeReader::on_reader_error` signals the `Cmd`, the `Readable::Pipe` `Arc` is dropped, and the callback's own `guard_from_raw` keepalive becomes the last reference. The code already documents this: "Dropping `guard` is the matching `deref()`; may free `this`." 5. Back in `read_with_fn`, the `EAGAIN` arm still delivers the drained head: `parent.vtable.on_read_chunk(.., ReadState::Drained)` reads the freed vtable. Traced with the test's `LD_PRELOAD` shim: ``` [shim] epoll_ctl(ADD fd=13) unix call#1 -> ok PipeReader::start [shim] recv(fd=13) unix call#1 -> EAGAIN eager read, inside spawn [shim] epoll_ctl(MOD fd=13) unix call#2 -> ok re-register; spawn returns [shim] recv(fd=13) unix call#2 poll fired: the child's bytes [shim] recv(fd=13) unix call#3 real EAGAIN [shim] epoll_ctl(MOD fd=13) unix call#3 -> ENOMEM register_poll fails [shell_subproc] PipeReader(0x..250) onReaderError errno: 12 [shell_subproc] PipeReader(0x..250, stdout) detach() [shell_subproc] PipeReader(0x..250, stdout) deinit() ==ERROR: AddressSanitizer: heap-use-after-free ``` ## Cause `register_poll()`'s failure path dispatches `on_reader_error`, which the `BufferedReaderParent` contract explicitly allows to free the parent, but `register_poll` gave the caller no way to know that happened. `read_with_fn`'s `EAGAIN` arm is the only call site that touches the reader afterwards; every other `register_poll()` is in tail position. The `SAFETY` comment above the `parent` rebind claimed the parent is "never freed mid-call", which holds for `on_read_chunk` re-entry but not for `on_reader_error`. oven-sh#32754 covered this exact sequence on the eager spawn-time entry by holding an `Arc<PipeReader>` across `start()` and `read_all()` in `Readable::start_pipe_reader`. The epoll dispatch has no equivalent keepalive, so the poll-driven entry was still exposed. ## Fix `PosixBufferedReader::register_poll()` now returns whether registration succeeded. `false` means `on_reader_error` was dispatched and `self` must not be touched again, so `read_with_fn`'s `EAGAIN` arm returns there instead of delivering the drained head to a possibly freed parent. The stream has already been completed with the registration error at that point, so nothing is lost. All other `register_poll()` call sites are tail calls and discard the result. ## Test Two new modes in `test/js/bun/shell/shell-pipe-read-fault.test.ts`'s `LD_PRELOAD` fault shim: - `SHELL_RECV_EAGAIN_FIRST=1`: the first `recv()` on each `AF_UNIX` socket returns `EAGAIN`, pushing the first successful read off the eager spawn-time `read_all()` and onto the epoll dispatch. - `SHELL_FAIL_EPOLL_FROM=N`: the Nth and later `epoll_ctl` `ADD`/`MOD` on each `AF_UNIX` socket fail with `ENOMEM`. `N=3` lets the initial registration and the eager read's re-registration succeed, then fails the first poll-driven one. The new test is `skipIf(!isASAN)` because the use-after-free is only reliably observable under ASAN. With `src/io/PipeReader.rs` reverted to `main` it fails in ~1.1s with the `heap-use-after-free` above; with the fix all 6 tests in the file pass. ## Out of scope Shell `PipeReader::on_read_chunk` also calls `self.reader.register_poll()` from a `&mut self` method whose stated contract is that it never frees `self`. If that inner registration fails, the same free can happen under `read_with_fn`'s mid-loop flush instead of its `EAGAIN` arm. Reaching it needs a large (>32 KB) burst in one poll wake; I have not reproduced it, so it is not changed here.
pull Bot
pushed a commit
that referenced
this pull request
Jun 29, 2026
…low-priority queue (oven-sh#33006) ### Symptom AddressSanitizer reports a heap-use-after-free (READ of size 8 and WRITE of size 8 variants) in uSockets' listener bookkeeping while a TLS server accepts connections under load: ``` ==ERROR: AddressSanitizer: heap-use-after-free (WRITE of size 8) #0 us_internal_socket_group_unlink_socket bun-usockets/src/context.c:223 #1 us_internal_socket_close_raw bun-usockets/src/socket.c:291 #2 us_internal_ssl_close bun-usockets/src/crypto/openssl.c #3 close<true> src/uws_sys/socket.rs ``` A second manifestation site is the low-priority queue walker, `us_internal_handle_low_priority_sockets`. The trigger is an ordinary `Bun.serve({tls})` / `node:tls` server whose clients connect, handshake, and disconnect at inopportune times. No unusual client behavior is required. ### Cause uSockets throttles concurrent TLS handshakes. When the 5-per-tick budget runs out, the readable dispatch parks the socket in the loop-wide low-priority queue (`loop->data.low_prio_head`): it is unlinked from `group->head_sockets` and READABLE is removed from its poll. The two lists share the same `prev`/`next` fields, so a socket lives in exactly one at a time. A parked socket can still get a WRITABLE dispatch. When its handshake flight is backpressured (`send` returned short or 0), `us_internal_ssl_on_writable` retries the BIO write, and `us_socket_raw_write` unconditionally runs `us_poll_change(READABLE | WRITABLE)`. READABLE is now re-enabled on a socket that is still in the low-priority queue. The next readable dispatch on that socket, with the budget exhausted, parked it a second time. That path ran `us_internal_socket_group_unlink_socket(g, s)` on a socket whose `prev`/`next` are low-priority-queue links, not group links: - If the socket was the queue head, `group->head_sockets` gets pointed at the next low-priority socket. When that socket is later closed through `us_internal_socket_close_raw`'s low-priority branch, nothing repairs `head_sockets`, and the group list reaches freed memory. `us_internal_socket_group_unlink_socket`'s `next->prev = prev` for a neighbor is the WRITE of size 8. - `loop->data.low_prio_head` can be left pointing at the re-prepended socket as a self-cycle; the queue walker then reads through entries the close path has already freed. That is the READ of size 8 in `us_internal_handle_low_priority_sockets`. - `group->low_prio_count` is incremented a second time for a socket that was already counted. It never returns to zero, which is also what `us_socket_group_deinit`'s `low_prio_count == 0` assertion catches in debug/ASan builds. ### Fix In the parking branch, if `low_prio_state == 1` the socket is already in `loop->data.low_prio_head` and not in `group->head_sockets`. Re-disable READABLE (done just above) and leave it where it is instead of group-unlinking and re-counting it. `us_connecting_socket_close` also calls `us_internal_socket_group_unlink_socket` without checking `low_prio_state`, but it only runs before any candidate leg has opened, when every socket in `connecting_head` is still a `SEMI_SOCKET` and cannot have been parked, so it is not affected. ### Test `test/js/bun/net/socket-syscall-fault.test.ts` drives the exact sequence with the in-tree socket fault injection: a `Bun.listen({tls})` server whose every `send` returns 0, and bursts of raw TLS 1.2 clients from a child process. Without the fix the fixture aborts: ``` bun-debug: packages/bun-usockets/src/context.c:68: void us_socket_group_deinit(struct us_socket_group_t *): Assertion `group->low_prio_count == 0' failed. ``` <details> <summary>Verification runs</summary> - Without the fix: 2/2 runs fail with `exitCode: 134`, `signalCode: "SIGABRT"`, and the assertion above. - With the fix: 3/3 runs pass. - `test/js/bun/util/socket-fault-injection.test.ts`, `test/js/node/tls/tls-syscall-fault.test.ts`, `test/js/node/tls/node-tls-server.test.ts`, and `test/js/bun/net/socket.test.ts` produce identical results before and after the change. The two pre-existing environment failures in the last two (plain TCP `ECONNREFUSED` to the just-bound port) reproduce identically on unmodified `main`. </details> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
Jun 30, 2026
…used (oven-sh#33118) ### What does this PR do? Fixes a silent failure in `Bun.serve`: when the `fetch` handler returns a `Response` whose body has already been used (most commonly the same `Response` object returned for more than one request), the request is answered with a `200` and `Content-Length: 0`. Nothing is thrown, nothing is logged, and `error()` is never invoked. ```js const cached = new Response("cached-route-body"); Bun.serve({ fetch() { return cached; } }); // request #1: 200, body "cached-route-body" // request #2+: 200, Content-Length: 0, empty body, error() never fires ``` Per the fetch model a disturbed body is an error, and Deno and workerd both reject it loudly. Bun's static routes (`routes: { "/x": new Response(...) }`) already throw "Response body has already been used" for a used body at registration time; the dynamic path was the only place a disturbed body got silently dropped. Cause: `RequestContext::do_render_with_body` has explicit arms for `Error` bodies, in-memory bodies, and `Locked` streams, but `Body::Value::Used` fell into the catch-all `_ => {}` arm, which renders the never-assigned context blob: a 200 with `Content-Length: 0`. This affected string, `Uint8Array`, stream, and file bodied Responses alike, and also a Response whose body the handler consumed (`await response.text()`) before returning it. Fix: add a `Body::Value::Used` arm that builds a `TypeError` (`code: "ERR_BODY_ALREADY_USED"`, message `Response body already used. A Response body can only be sent once; create a new Response for each request.`) and routes it through `run_error_handler`, exactly like the existing body-error and locked-stream arms. With an `error()` handler, the handler receives the TypeError and its Response is sent. Without one, the error is logged and the request gets the standard 500, the same as any other error thrown from `fetch`. The existing `has_called_error_handler` guard keeps an `error()` handler that itself returns a used Response from recursing. Static routes, fresh Responses per request, bodiless Responses, and HEAD rendering are unchanged; only the `Used` (disturbed) body state is affected. ### How did you verify your code works? `test/js/bun/http/serve-reused-response.test.ts`: - the same string, `Uint8Array`, and stream bodied Response returned twice: the first request gets the body, later ones invoke `error()` with the TypeError (`code: "ERR_BODY_ALREADY_USED"`) and receive its response - a Response consumed with `.text()` before being returned from an async handler: same error - no `error()` handler (`development: false`): the client gets 500 `"Something went wrong!"`, the error is printed to stderr, and the process exit code matches other unhandled fetch errors - fresh Responses and a static-route Response reused across requests keep working and never call `error()` The already-used cases fail on current `bun test` (they observe the empty 200s and zero `error()` calls) and pass with this change; the two no-reuse tests pass both ways by design.
pull Bot
pushed a commit
that referenced
this pull request
Jul 2, 2026
…en-sh#33242) ### What After a 3xx redirect, `handle_response_metadata` rewrites per-hop request state on the HTTP-thread clone of the `AsyncHTTP`: - `client.url` (and `connected_url`) become a self-borrow into `client.redirect`, a `Vec<u8>` the clone owns and frees in the final-callback teardown (`AsyncHTTP::on_async_http_callback_raw`). - On a cross-origin hop, `Authorization`/`Proxy-Authorization`/`Cookie`/`Host` are removed from `client.header_entries` in place. - The method may be downgraded to GET. `NetworkTask::notify`'s bitwise copy-back (`ptr::write(real, ptr::read(async_http))`) carries all of that into the JS-thread `AsyncHTTP`. When `bun install` retries the task after a retryable failure (5xx or a connection reset on the redirect target), the re-scheduled request therefore: 1. connects through the freed redirect buffer (use after free), and 2. if the redirect was cross-origin, goes out without `Authorization`, so an authorized registry answers 401. ASAN (debug build), deterministic on the first try: ``` ERROR: AddressSanitizer: heap-use-after-free ... thread T1 (HTTP Client) READ of size 1 #0 bun_core::fmt::parse_int::<u16> src/bun_core/fmt.rs:929 #1 <bun_url::URL>::get_port src/url/lib.rs:470 #2 <bun_url::URL>::get_port_auto src/url/lib.rs:474 #3 <bun_http::http_thread::HttpThread>::connect src/http/HTTPThread.rs:602 #4 <bun_http::HTTPClient>::start_ src/http/lib.rs:2635 #6 <bun_http::async_http::AsyncHTTP>::on_start src/http/AsyncHTTP.rs:893 freed by thread T1 (HTTP Client): <bun_http::async_http::AsyncHTTP>::on_async_http_callback_raw src/http/AsyncHTTP.rs:774 previously allocated by thread T1 (HTTP Client): <bun_http::HTTPClient>::handle_response_metadata src/http/lib.rs:5038 ``` On a release build the same sequence does not crash, but the retries never reach the server (each one connects through freed memory) and the install fails. ### Repro A scripted registry where the manifest URL 302-redirects and the redirect target answers a 500 once, then the real packument: ``` GET /BaR -> 302 Location: /redirected/BaR GET /redirected/BaR -> 500 on the first hit, then the packument GET /BaR-0.0.2.tgz -> tarball ``` `bun install` against it aborts under ASAN and fails on release. Any 301/302/307/308 and 1- or 2-hop chains hit the same path. With an authorized registry that redirects cross-origin (the common Artifactory / CodeArtifact / GitHub Packages shape), the retry also loses `Authorization`; that variant fails with `GET <registry>/BaR - 401` even once the URL is fixed. ### Fix `src/http/AsyncHTTP.rs`: the `!has_more` teardown block already releases every clone-owned allocation. Before freeing `client.redirect`, restore the per-hop state that a re-scheduled attempt must not inherit: - `client.url` back to the caller-owned pre-redirect URL (`AsyncHTTP.url`, which borrows memory valid for the original's whole lifetime), and `client.connected_url` (which `connect` derives from it) to default. - `client.header_entries` back to the untouched `AsyncHTTP.request_headers`. The list is bitwise-shared with the JS-thread original, so it must not be dropped or reallocated on the HTTP thread; it was cloned from `request_headers` at init and only ever shrinks, so `clear_retaining_capacity()` + `append_list_assume_capacity()` restores it in place. - `client.method` back to `AsyncHTTP.method`. Nothing that crosses back to the JS thread references clone-freed memory anymore, and a retried request restarts from the original URL with the original headers instead of the last redirect hop's, which is what the install-level retry is meant to do. ### Tests `test/cli/install/bun-install-retry.test.ts`: - `retries a manifest whose redirect target 500s once` - `retries a tarball whose redirect target 500s once` (the sibling retry site in `runTasks`) - `retries an authorized manifest whose cross-origin redirect target 500s once` (also asserts the cross-origin hop itself still does NOT carry `Authorization`, so the spec-mandated strip is unchanged) All three fail on the unfixed build (ASAN abort under `bun bd`, install error with `USE_SYSTEM_BUN=1`). The third additionally fails with a 401 if only the URL is restored and not the headers, so each restore is load-bearing. `test/js/web/fetch/fetch-redirect.test.ts` and `fetch-url-after-redirect.test.ts` still pass, so `response.url` after a redirect is unaffected (it comes from the owned `metadata.url` copy, not from `client.url`). --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
Jul 5, 2026
…buffer cannot be allocated (oven-sh#33326) Fixes a `Segmentation fault at address 0x00000040` (sometimes `0x00000030`) reported from Windows x64 builds, crashing inside boringssl's record copy from uSockets' TLS read loop: ``` memcpy src/vctools/crt/vcruntime/src/string/amd64/memcpy.asm bssl::OPENSSL_memcpy vendor/boringssl/crypto/internal.h:868 SSL_peek vendor/boringssl/ssl/ssl_lib.cc:947 SSL_read vendor/boringssl/ssl/ssl_lib.cc:918 us_internal_ssl_on_data packages/bun-usockets/src/crypto/openssl.c:1797 us_internal_dispatch_ready_poll packages/bun-usockets/src/loop.c:600 uv__fast_poll_process_poll_req vendor/libuv/src/win/poll.c:208 uv_run vendor/libuv/src/win/core.c:737 ``` ## Cause `us_internal_init_loop_ssl_data` (`openssl.c:677`) allocates one 512 KiB plaintext buffer per event loop, lazily, on the loop's first TLS socket, and never checked the result: ```c loop_ssl_data->ssl_read_output = us_malloc(LIBUS_RECV_BUFFER_LENGTH + LIBUS_RECV_BUFFER_PADDING * 2); ``` With `ssl_read_output == NULL`, every later `SSL_read` hands boringssl ```c loop_ssl_data->ssl_read_output + LIBUS_RECV_BUFFER_PADDING + read ``` as its plaintext destination, so the first record of application data memcpy's to `NULL + 32`. `SSL_peek`'s `OPENSSL_memcpy(buf, ...)` at `ssl_lib.cc:947` is the write, and the access violation confirms it is a write fault. `0x30`/`0x40` rather than `0x20` is memcpy's destination-alignment preamble (`dst += VEC_SIZE; dst &= ~(VEC_SIZE - 1)`), which moves the first faulting store for copies larger than eight vector registers. Measured on the copy sizes a real TLS record produces: | memcpy variant | first faulting store for `dst = NULL + 32` | | --- | --- | | 32-byte vectors (AVX) | `0x40` | | 16-byte vectors (SSE) | `0x30` | So the two strikingly stable fault addresses are just CPU dispatch across the affected machines, and `read` is always `0`: the crash is always the connection's first record of application data. Only Windows reports it because Linux and macOS overcommit, so a 512 KiB `malloc` there effectively never returns NULL. Windows fails the commit cleanly, and the loop's much smaller `us_calloc` still succeeds out of an already-committed page, leaving exactly the observed shape: a valid `loop_ssl_data` whose `ssl_read_output` is NULL. ## Fix - Null-check the buffer allocation, the `us_calloc` of `loop_ssl_data`, and the `BIO_meth_new`/`BIO_new` calls beside them, and route the failure through Bun's out-of-memory crash path (`Bun__outOfMemory`, new C entry point next to `Bun__panic`). The process now dies with `Bun ran out of memory` and a stack trace that names the allocation, instead of faulting on the first TLS byte. - Apply the same check to the sibling site: `recv_buf`/`send_buf` in `us_internal_loop_data_init` are the same unchecked `malloc(LIBUS_RECV_BUFFER_LENGTH + LIBUS_RECV_BUFFER_PADDING * 2)`. A NULL `recv_buf` does not fault, it makes every read on the loop fail with `EFAULT` for the life of the process, which is worse to diagnose. - `us_internal_free_loop_ssl_data` left `loop->data.ssl_data` dangling, which defeats the `if (!loop->data.ssl_data)` guard the init function relies on. It now clears the field. A 512 KiB `malloc` effectively never returns NULL on an overcommitting kernel, so the failure path needs the existing socket fault injector to be reachable from a test. This adds an `ssl_loop_buffer` rule to it, which like the rest of the injector is compiled out of release builds. ## Verification The new test spawns a child that arms `ssl_loop_buffer` before its first TLS socket and asserts it reports out of memory rather than reaching a read loop. Reverting only `if (!loop_ssl_data->ssl_read_output) Bun__outOfMemory();` reproduces the reported crash exactly, on Linux, from that same fixture: same fault address, same frames, same boringssl source lines. <details> <summary>Reproduction on the unfixed build (<code>bun bd</code>, ASAN)</summary> ``` ==19592==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000040 ==19592==The signal is caused by a WRITE memory access. ==19592==Hint: address points to the zero page. #0 __memcpy_evex_unaligned_erms #1 bssl::OPENSSL_memcpy(void*, void const*, unsigned long) vendor/boringssl/crypto/internal.h:868:10 #2 SSL_peek vendor/boringssl/ssl/ssl_lib.cc:947:3 #3 SSL_read vendor/boringssl/ssl/ssl_lib.cc:918:13 #4 us_internal_ssl_on_data packages/bun-usockets/src/crypto/openssl.c:1847:21 #5 us_internal_dispatch_ready_poll packages/bun-usockets/src/loop.c:625:38 ``` With the fix: ``` panic(main thread): Bun ran out of memory Bun__outOfMemory src/bun_bin/phase_c_exports.rs:81:5 us_internal_init_loop_ssl_data packages/bun-usockets/src/crypto/openssl.c:696:42 us_internal_ssl_attach packages/bun-usockets/src/crypto/openssl.c:1275:3 ``` </details> `test/js/node/tls/tls-syscall-fault.test.ts` (11 pass), `test/js/bun/util/socket-fault-injection.test.ts` (15 pass), plus `socket-syscall-fault`, `serve-syscall-fault` and `fetch-syscall-fault` (19 pass) are green. The three failures in `test/js/node/tls/` on this machine are pre-existing: two also fail on an unmodified 1.4.0, and `tls.connect should ignore invalid NODE_EXTRA_CA_CERTS` takes 5.75s, just over the 5s local default (CI triples the per-test timeout for ASAN builds). ## Teardown audit The report also asked whether a socket can reach `us_internal_ssl_on_data` after its loop's SSL data has been freed. `us_internal_free_loop_ssl_data` is only reachable from `us_loop_free`, and the only loop Bun frees today is `SpawnSyncEventLoop`'s, which never has a TLS socket attached (its `loop->data.ssl_data` is always NULL, so the free is a no-op). So that is not the cause here. It is worth noting separately that the libuv `us_loop_free` (`eventing/libuv.c:201-206`) calls `us_internal_loop_data_free(loop)` and *then* runs `uv_run(loop->uv_loop, UV_RUN_NOWAIT)`, which is a full libuv iteration that can dispatch socket poll callbacks into the just-freed `recv_buf` and `ssl_data`. The POSIX `us_loop_free` (`eventing/epoll_kqueue.c:56-60`) has no such window. It is unreachable today for the reason above, so it is left out of this PR rather than changing loop teardown ordering without a test that can exercise it.
pull Bot
pushed a commit
that referenced
this pull request
Jul 11, 2026
…ven-sh#33931) ## Symptom Intermittent `EBADF: bad file descriptor, fstat` in unrelated `Bun.file(path).text()` calls on Windows. The most visible CI victim is `test/cli/install/bun-install.test.ts` (15 flaky hits across the last 40 PR builds, spread across many different test cases), because its dummy registry serves every tarball via `new Response(Bun.file(path))`. ``` EBADF: bad file descriptor, fstat syscall: "fstat", errno: -9, code: "EBADF" at async <anonymous> (test/cli/install/bun-install.test.ts:6461) ``` ## Cause `FileResponseStream::start` clears `ReaderFlags::CLOSE_HANDLE` on its `BufferedReader` so it can close the fd itself in `Drop` (src/runtime/server/FileResponseStream.rs:182). `PosixBufferedReader` checks that flag before closing (src/io/PipeReader.rs:283/356/374/385). `WindowsBufferedReader` defines the flag (:1143) and sets it in `Default` (:1155) but never reads it, so `close_impl`'s `Source::File` arm unconditionally calls `File::detach()` which queues `uv_fs_close` on the same CRT fd that `FileResponseStream::Drop` already queued a `Closer::close` for (:547). Both closes are async on the libuv threadpool. Between close #1 freeing the CRT slot and close #2 running, an unrelated `uv_fs_open` (from another `Response(Bun.file)` open, or a `Bun.file().text()`) can be handed the recycled slot; close #2 then closes the wrong fd, and its next `fstat` or `read` sees EBADF. ## Fix Honor `WindowsFlags::CLOSE_HANDLE` in `WindowsBufferedReader::close_impl` via a new `File::detach_borrowed_fd()` that mirrors `detach()` but leaves `close_after_operation` unset, so no `uv_fs_close` is scheduled for a parent-owned fd: - `close_impl` with `CLOSE_HANDLE` set: unchanged (`detach()` schedules `uv_fs_close` now or after the pending read). - `close_impl` with `CLOSE_HANDLE` cleared: `detach_borrowed_fd()`. If idle, drop the `Box<File>` there; if a read is in flight, null `fs.data` and let `on_file_read`'s detached branch reclaim the Box after `complete()`. - `on_file_read`'s `parent_ptr.is_null()` path now handles both shapes (state `Closing`: `on_close_complete` frees; otherwise: free here). `BaseWindowsPipeWriter::close`'s `!owns_fd()` branch already open-coded the same sequence and now routes through the shared `detach_borrowed_fd()`, so the reader and writer close paths share one contract. ## Verification The double-close only exists on Windows (POSIX honors the flag), so the Linux gate cannot observe fail-before. Windows x64-baseline at 91675d0: - fail-before: 5/15 runs of the new test fail under the system bun with `EBADF: bad file descriptor, fstat 'served.bin'` - pass-after: 8/8 under the debug build with this patch The new test is gated behind `isWindows`. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/bun-serve-file.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
Jul 13, 2026
…wn (oven-sh#34035) Fixes `test/bake/dev/request-cookies.test.ts` going red on the `debian 13 x64-asan` lane (seen in [build 72183](https://buildkite.com/bun/bun/builds/72183#019f55d5-6839-41cf-b0f5-3c56ada43ef9) and [build 71964](https://buildkite.com/bun/bun/builds/71964)): ``` dev| ==1715==ERROR: AddressSanitizer: SEGV on unknown address 0x000000007490 error: DevServer panicked at gracefulExit (test/bake/bake-harness.ts:614:17) ✗ DEV:request-cookies-1: request.cookies.get() basic functionality ``` ### Cause `~DevServerSourceProvider` held a raw `Zig::GlobalObject*` and called `m_globalObject->bunVM()` to reach `Bun__removeDevServerSourceProvider`. Under `BUN_DESTRUCT_VM_ON_EXIT=1` (set by the CI runner for the asan lane), the harness's `process.exit(0)` runs `Zig__GlobalObject__destructOnExit`, which does `gcUnprotect(globalObject)` then `collectNow(Sync, Full)` then two `vm.derefSuppressingSaferCPPChecking()`. The global object cell is swept during `collectNow`, but the provider's last `Ref` is only released later from `~CodeCache` inside `~JSC::VM`, so the destructor read `m_bunVM` out of a freed cell. With bmalloc the freed cell usually still holds the old value and the read happens to work, which is why this was ~0.5% in CI and never reproduced locally. When the memory is reused with a zero at that offset the Rust side receives a null `VirtualMachine*` and the next access is `(null)->source_mappings.mutex`, which lands at exactly 0x7490. Deterministic ASAN backtrace with `Malloc=1`: ``` ==79839==ERROR: AddressSanitizer: heap-use-after-free ... #0 Zig::GlobalObject::bunVM() const ZigGlobalObject.h:353 #1 Bake::DevServerSourceProvider::~DevServerSourceProvider() DevServerSourceProvider.h:65 ... #7 JSC::SourceCodeKey::~SourceCodeKey() #12 JSC::CodeCacheMap::~CodeCacheMap() #16 JSC::VM::~VM() #18 Zig__GlobalObject__destructOnExit ZigGlobalObject.cpp:4049 #19 VirtualMachine::global_exit VirtualMachine.rs:1603 #20 Bun__Process__exit ``` ### Fix Store the Rust `VirtualMachine*` directly (`void* m_bunVM`), captured in `create()`, so the destructor no longer indirects through a GC cell. This mirrors `Zig::SourceProvider`, which already stores `m_bunVM` for the same reason. The Rust `VirtualMachine` outlives every GC cell (step 10 of `global_exit` is `self.destroy()`, after `destructOnExit` has finished). ### Verification New ASAN-only case in `test/bake/dev/server-sourcemap.test.ts` runs the dev server with `Malloc=1` + `BUN_DESTRUCT_VM_ON_EXIT=1` so ASAN poisons the swept global-object cell, making the UAF deterministic. Added an `env` option to the `devTest` harness so the test can set those for the spawned dev server. ``` # fail-before (src/ stashed) SUMMARY: AddressSanitizer: heap-use-after-free ZigGlobalObject.h:353:48 in Zig::GlobalObject::bunVM() const (fail) DEV:server-sourcemap-5: DevServerSourceProvider destructor does not touch the swept global object on process exit # pass-after (pass) DEV:server-sourcemap-5: DevServerSourceProvider destructor does not touch the swept global object on process exit ``` `test/bake/dev/server-sourcemap.test.ts` (5 tests) and `test/bake/dev/request-cookies.test.ts` (2 tests) are green. `request-cookies.test.ts` now also passes under the full CI LeakSan config (`BUN_DESTRUCT_VM_ON_EXIT=1` + `detect_leaks=1`). The bug is from a89e61f (oven-sh#22138), which introduced `DevServerSourceProvider` with the raw global-object pointer. <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 1 · 3 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bake/dev/request-cookies.test.ts test/bake/dev/server-sourcemap.test.ts info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05) info: component rust-src is up to date info: checking for self-update (current version: 1.29.0) bun test v1.4.0 (722d6f0) test/bake/dev/server-sourcemap.test.ts: Dev server testing directory: /tmp/bun-dev-test-tI2Y82 bun add v1.4.0 (722d6f0) Resolving dependencies Resolved, downloaded and extracted [2] Saved lockfile installed react@0.0.0-experimental-603e6108-20241029 installed react-dom@0.0.0-experimental-603e6108-20241029 installed react-server-dom-bun@0.0.0-experimental-603e6108-20241029 installed react-refresh@0.0.0-experimental-603e6108-20241029 6 packages installed [462.00ms] bun install v1.4.0 (722d6f0) Checked 6 installs across 7 packages (no changes) [167.00ms] �[0;30mdev|�[0m Started development server: http://localhost:37377 �[0;30mdev|�[0m �[32mBundled page in 2125ms�[0m�[2m:�[0 ... (truncated) release without fix: all passed bun test v1.4.0-canary.1 (1498d7b) test/bake/dev/server-sourcemap.test.ts: Dev server testing directory: /tmp/bun-dev-test-7LPeWv bun add v1.4.0-canary.1 (1498d7b) Resolving dependencies Resolved, downloaded and extracted [0] Saved lockfile installed react@0.0.0-experimental-603e6108-20241029 installed react-dom@0.0.0-experimental-603e6108-20241029 installed react-server-dom-bun@0.0.0-experimental-603e6108-20241029 installed react-refresh@0.0.0-experimental-603e6108-20241029 6 packages installed [9.00ms] bun install v1.4.0-canary.1 (1498d7b) Checked 6 installs across 7 packages (no changes) [0.00ms] �[0;30mdev|�[0m Started development server: http://localhost:43275 �[0;30mdev|�[0m �[32mBundled page in 47ms�[0m�[2m:�[0m pages/[...slug].tsx �[2m+ 2 more�[0m �[0;30mdev|�[0m �[0m�[1m1 |�[0m �[0m�[35mexport�[0m �[0m�[35mdefault�[0m �[0m�[35masync�[0m �[0m�[35mfunction�[0m MyPage(params) { �[0;30mdev|�[0m �[0m�[1m2 |�[0m myFunc()�[0m�[2m;�[0m �[0;30mdev|�[0m �[0m�[1m3 |�[0m �[0m�[35mreturn�[0m �[0m<�[0mh1>{JSON�[0m�[3m�[1m.stringify�[0m(params)}�[0m<�[0m/h1>�[0m�[2m;�[0m �[0;30mdev|�[0m �[0m�[1m4 |�[0m } �[0;30mdev|�[0m �[0m�[1m5 |�[0m �[0;30mdev|�[0m �[0m� ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bake/dev/request-cookies.test.ts test/bake/dev/server-sourcemap.test.ts info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05) info: component rust-src is up to date info: checking for self-update (current version: 1.29.0) bun test v1.4.0 (722d6f0) test/bake/dev/server-sourcemap.test.ts: Dev server testing directory: /tmp/bun-dev-test-dkR1se bun add v1.4.0 (722d6f0) Resolving dependencies Resolved, downloaded and extracted [0] Saved lockfile installed react@0.0.0-experimental-603e6108-20241029 installed react-dom@0.0.0-experimental-603e6108-20241029 installed react-server-dom-bun@0.0.0-experimental-603e6108-20241029 installed react-refresh@0.0.0-experimental-603e6108-20241029 6 packages installed [119.00ms] bun install v1.4.0 (722d6f0) Checked 6 installs across 7 packages (no changes) [97.00ms] �[0;30mdev|�[0m Started development server: http://localhost:44249 �[0;30mdev|�[0m �[32mBundled page in 2351ms�[0m�[2m:�[0m ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05) info: component rust-src is up to date info: checking for self-update (current version: 1.29.0) [configured] bun-profile → bun (stripped) in 690ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [0/6] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05) info: component rust-src is up to date info: component rust-std is up to date nightly-2026-05-06-x86_64-unknown-linux-gnu unchanged - rustc 1.97.0-nightly (e95e73209 2026-05-05) info: checking for self-update (current version: 1.29.0) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/runtime/bake/DevServerSourceProvider.h | 13 +++++++----- test/bake/bake-harness.ts | 5 +++++ test/bake/dev/server-sourcemap.test.ts | 34 ++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/runtime/bake/DevServerSourceProvider.h 1 2 0 test/bake/bake-harness.ts 8 2 0 test/bake/dev/server-sourcemap.test.ts 1 4 0 ``` </details> <!-- robobun:evidence:end -->
pull Bot
pushed a commit
that referenced
this pull request
Jul 16, 2026
…ose_slave_fd (oven-sh#34225) ### What does this PR do? Fixes a regression from oven-sh#33882 where a script that spawns a subprocess with an inline `terminal:` option and awaits `proc.exited` hangs forever if it never calls `terminal.close()`. ### Repro ```js const proc = Bun.spawn([process.execPath, "-e", "console.log('hi')"], { terminal: {}, }); await proc.exited; // process hangs here ``` ### Root cause oven-sh#33882 deferred closing the parent's pty slave fd to `Subprocess::on_process_exit` and added a synchronous drain of the master before the close. That drain calls `reader.read()`, which hits `EAGAIN` (our slave is still open) and re-arms the reader poll via `register_poll()`. Only then do we `close_slave_fd()`, so the reader's EOF (macOS) / `EIO` (Linux) arrives on the *next* epoll tick instead of in the same batch as the pidfd. When that next tick runs after the script's top-level await has already resolved, `on_reader_error` downgrades the Terminal's `JsRef`, but the reader and writer `FilePoll`s are still counted in `loop.active`. Nothing triggers a GC between that downgrade and the next `epoll_wait`, so `Terminal::finalize` (the only path that unregisters those polls) never runs and the process blocks in `epoll_wait` forever. Before oven-sh#33882 the parent's slave fd was closed right after `spawn`, so when the child exited the last slave was already gone and the reader observed `EIO` in the *same* epoll batch as the pidfd. `on_reader_error` downgraded the `JsRef` before the script ended, and the next `onBeforeWait` safepoint finalized the Terminal. Kernel probe on Linux (nonblocking read on master after the last slave closes returns `-1/EIO`, not `0`): ``` === parent keeps slave until EAGAIN then closes === read #0: n=18 data=[hello from child\n] read #1: n=-1 errno=11 (EAGAIN) (closing parent slave now) read #2: n=-1 errno=5 (EIO) ``` ### Fix In `drain_and_close_slave_fd`, after draining and closing `slave_fd`: - call `reader.read()` again so the exit callback fires now (EOF on macOS, `EIO` on Linux) instead of on a later tick when nothing may wake the loop. A grandchild holding the slave keeps this at `EAGAIN` and re-arms, matching the stdout/stderr policy in the same `on_process_exit` body. - `update_ref(false)` on both polls so the event loop can exit regardless; the polls stay registered so grandchild output still arrives while anything else keeps the loop running. - bracket the body with a `ref_()`/`deref_()` scopeguard since both reader callbacks re-enter user JS. `on_reader_done` / `on_reader_error` now gate the exit-callback branch on `READER_DONE` as well as `FINALIZED`, so the second `EIO` dispatch (the still-armed one-shot from the first drain) is a no-op and the exit callback fires exactly once. `#[cfg(unix)]` only; Windows already delivers EOF via `close_pseudoconsole` off-thread. `BufferedReader` / `FileReader` are untouched. ### How did you verify your code works? New test `process exits after subprocess with inline terminal (no terminal.close)` spawns a `bun -e` that runs the repro, sleeps after `child.exited` to give the stale poll a chance to fire, and asserts `{gotOutput: true, exitedSync: true, exitCount: 1, exitCode: 0}`. Fail-before (origin/main `src/` + this test): ``` (fail) ... process exits after subprocess with inline terminal (no terminal.close) [5004.58ms] ^ this test timed out after 5000ms. ``` With the fix: 91 pass / 1 todo / 0 fail. `bun bd test test/js/bun/spawn/spawn.test.ts`: 368 pass, 0 fail. `cargo check -p bun_runtime` clean for `aarch64-apple-darwin` and `x86_64-pc-windows-msvc`. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/terminal/terminal.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
pull Bot
pushed a commit
that referenced
this pull request
Aug 2, 2026
) ## What `JSSink::assign_to_stream` now detaches the freshly created `JSReadable*SinkController` (nulling its `m_sinkPtr`) when the C++ stream-pump setup returns an error, before returning to the caller. ## Why The generated `${name}__assignToStream` functions create the controller with `m_sinkPtr = sinkPtr` and then call into `GlobalObject::assignToStream` → `readDirectStream` / `readStreamIntoSink`. If that setup throws (for example a direct `ReadableStream` whose `pull` getter throws), the controller is never started, so nothing ever calls `end()`/`close()` to null `m_sinkPtr`. The caller's error path (`Writable::init` for `Bun.spawn`) then releases and frees the native sink. When the controller is later swept, its destructor runs `${name}__controllerDetached` / `${name}__finalize` on freed memory. ASAN report: ``` heap-use-after-free on address 0x799feed81c78 READ of size 1 #0 JSSink<FileSink>::js_controller_detached Sink.rs:567 #1 FileSink__controllerDetached generated_jssink.rs:179 #2 JSReadableFileSinkController::~JSReadableFileSinkController() freed by: #12 FileSink::deinit FileSink.rs:1142 #16 Writable::pipe_release Writable.rs:70 #17 Writable::init Writable.rs:339 #18 spawn_maybe_sync js_bun_spawn_bindings.rs:1379 ``` The fix is at the generic `JSSink::assign_to_stream` layer so it covers every sink type (`FileSink`, `NetworkSink`, `FetchRequestBodySink`, ...), not just the spawn path. ## Repro ```js const { openSync, closeSync } = require("node:fs"); const fd = openSync("/tmp/out.txt", "w"); let armed = false; const stream = new ReadableStream({ type: "direct", get pull() { if (armed) throw new Error("pull unavailable"); return () => {}; }, }); armed = true; try { Bun.spawn({ cmd: [process.execPath, "-e", "0"], stdio: [stream, fd, "ignore"] }); } catch {} closeSync(fd); Bun.gc(true); // sweep -> controller dtor -> UAF ``` ## Tests The two existing `spawn.test.ts` cases that cover the stdin-stream-setup-throws path now force a full GC in the child fixture so the controller destructor runs deterministically under debug+ASAN as well. Previously they were only failing on the release-asan lane (where the whole file has been quarantined as `[ASAN] [TIMEOUT]`), which is why this went unnoticed. ``` bun bd test test/js/bun/spawn/spawn.test.ts -t "stdin stream setup fails" ``` fails on `main` (ASAN heap-use-after-free in the child's stderr) and passes with this change. `spawn-stdin-readable-stream-edge-cases.test.ts` and `body-stream.test.ts` continue to pass.
pull Bot
pushed a commit
that referenced
this pull request
Aug 6, 2026
…e cache (oven-sh#37034) ### Problem On the `13 x64-asan` lane, a test that exercises non-ISO Temporal calendars from a test callback can abort after a fully green run with a LeakSanitizer report. Seen in build 89504 on oven-sh#37024, whose `test/js/bun/bun-object/deep-equals-temporal.test.ts` uses `[u-ca=hebrew]`: ``` Direct leak of 624 byte(s) in 1 object(s) allocated from: #1 icu_75::HebrewCalendar::clone() const #2 icu_75::Calendar::createInstance(icu_75::TimeZone*, icu_75::Locale const&, UErrorCode&) #3 ucal_open_75 #4 JSC::TemporalCore::buildCalendarTemplate(WTF::AbstractLocker const&, unsigned int) #5 JSC::TemporalCore::withCalendar<JSC::TemporalCore::calendarYear(...)::$_0>(...) ``` The CI annotation titles this `direct leak of 624b in {closure#0} (src/jsc/JSValue.rs:1664:22)` because that is the first in-repo frame (the test-runner's `JSValue::call`); everything below it is WebKit/ICU. ### Cause `TemporalCore::withCalendar` (`vendor/WebKit/.../temporal/core/CalendarICUBridge.cpp`) keeps up to 8 open `UCalendar` templates in a process-lifetime `LazyNeverDestroyed` `TinyLRUCache`, one per calendar ID (non-ISO arithmetic, plus pure-ISO `PlainDateTime.prototype.with`, which reaches the same path unguarded); LRU eviction `ucal_close`s them, so the set is bounded. The `CalendarCacheEntry` that owns each `UCalendar` is `WTF_MAKE_TZONE_ALLOCATED` (bmalloc), which LSan does not scan, so the libc-allocated `UCalendar` (and the ICU `TimeZone` inside it) is reported as a direct leak even though it is reachable. Whether a given run aborts depends on whether some stale stack or register value still points at the ICU object when LSan scans at exit, hence the intermittence. This is the calendar twin of the already-suppressed `TemporalCore::withTimeZone` entry (same cache design, same TZone-allocated owner). ### Fix - Add a `leak:TemporalCore::buildCalendarTemplate` suppression to `test/leaksan.supp`, mirroring the `withTimeZone` entry. The pattern anchors on the template builder rather than `withCalendar` itself so that a future real leak inside one of the many op lambdas `withCalendar` runs would still be reported; every cached-template allocation carries the builder frame. (`withTimeZone` has no such builder frame, its `ucal_open` is inline, so that entry keeps its existing pattern.) - Drop the `test/no-validate-leaksan.txt` escape hatch oven-sh#37024 added for `deep-equals-temporal.test.ts`, re-enabling leak validation for it; that file exercises the suppressed path on the asan lane. ### Verification On a debug ASAN build, running `bun test test/js/bun/bun-object/deep-equals-temporal.test.ts` under the CI leak-validation env (`BUN_DESTRUCT_VM_ON_EXIT=1`, `detect_leaks=1:abort_on_error=1`, repo suppression file): - with the new entry: clean exit, 5/5 runs - without it: LSan abort with the calendar-template stacks above, 3/3 runs A standalone probe exercising 8 non-ISO calendars plus pure-ISO `PlainDateTime.with` from a timer callback shows the same split (10/10 aborts without, 10/10 clean with; `print_suppressions=1` attributes exactly the ICU template allocations to the new entry). Top-level module code cannot reproduce this: its allocation stacks carry `JSC::JSModuleLoader::evaluateNonVirtual`, which the suppression file already covers wholesale. An ASAN-gated test pinning the entry was part of an earlier revision and was dropped per review; the re-enabled `deep-equals-temporal.test.ts` covers the path in CI instead. The Expect-wrapper shutdown leak mentioned in the dropped no-validate comment is a separate issue tracked in oven-sh#32180: that is `bun test`'s own finalizer-owned memory, while this cache deliberately survives VM teardown, so oven-sh#32180 would not prevent this report. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · docs-only change; test-proof not applicable <!-- robobun:evidence:end --> --------- Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
pull Bot
pushed a commit
that referenced
this pull request
Aug 8, 2026
…-comparison (oven-sh#37168) ### Problem `Bun__deepEquals` has heap-use-after-free when a getter on a nested object mutates one of the objects being compared. All entry points are affected: `Bun.deepEquals`, `expect().toEqual` / `toStrictEqual`, `assert.deepStrictEqual` / `deepEqual`, and `util.isDeepStrictEqual`. ```js // Malloc=1 <bun-asan> repro.mjs const p1 = {}, p2 = {}; for (let i = 0; i < 8; i++) { p1['k'+i] = i; p2['k'+i] = i; } let f = 0; p1.a = { get x() { if (!f++) for (let i = 0; i < 2000; i++) p1['n'+i] = i; return 1; } }; p2.a = { get x() { return 1; } }; p1.z = 1; p2.z = 1; Bun.deepEquals(p1, p2, true); ``` ASAN (with `Malloc=1` so JSC's bmalloc routes through the system allocator): ``` heap-use-after-free READ of size 8 #0 CompactPropertyTableEntry::key() Structure.h #1 PropertyTable::forEachProperty #2 Structure::forEachProperty #3 Bun__deepEquals<...> bindings.cpp freed by: PropertyTable::destroyIndexVector <- PropertyTable::rehash <- PropertyTable::add <- Structure::addNewPropertyTransition <- JSObject::putDirectInternal ``` ### Cause The object fast path walks the structure's `PropertyTable` with `Structure::forEachProperty` and recurses into `Bun__deepEquals` from inside the lambda. Comparing a nested value can run a user getter; if that getter adds (or deletes) properties on the parent object, JSC takes the shared table off the old structure and rehashes it, freeing the index vector the outer walk is iterating. Every remaining sibling property is then read from freed memory and its stale offset fed to `getDirect()`. In release builds this shows up as a SEGV at a forged address or a wrong verdict. ### Fix Collect the (left, right) value pairs into a `MarkedArgumentBuffer` under `forEachProperty` with no side effects, then run `sameValue` and the recursive comparisons after the walk finishes. This is the same shape as `Object.assign`'s fast path (snapshot under `forEachProperty`, side-effectful work after). The buffer keeps the snapshotted values visible to GC, so allocation churn in a getter cannot collect them either. The reverse `o2` walk already did only direct structure reads and now also completes before any user code can run. Verdicts are unchanged for non-mutating comparisons (existing suites pass); a comparison whose getter mutates the object now deterministically compares the snapshot, which matches Node's behavior for the repro above (`true`). ### Verification - New test in `test/js/bun/bun-object/deep-equals.test.ts` (renamed from `deep-equals.spec.ts` to match the test naming convention): spawns an ASAN child with `Malloc=1` (bmalloc routed through the system allocator so ASAN can see the freed table) covering same-structure, mixed-structure, delete, right-side mutation, and GC-churn variants across all entry points. Fails before the fix (ASAN heap-use-after-free abort), passes after. - `test/js/bun/bun-object/`, `test/js/node/assert/deep-equal.test.ts`, `assert-typedarray-deepequal.test.ts`: 542 pass. - `test/js/bun/test/expect.test.js`: 415 pass. - `test/js/node/test/parallel/test-assert-deep-with-error.js`: 2 pass. ### Scope The same pattern exists in `JSC__JSValue__forEachPropertyImpl` in this file (the `Bun.inspect` / `console.log` property walk), where the formatter callback can run a nested value's `inspect.custom` mid-walk. Verified with ASAN to hit the same free/read pair. That is a pre-existing bug in the console/inspect subsystem and is intentionally excluded here; a follow-up fix for that site is in progress. The other `forEachProperty` sites (CommonJS export enumeration, HTTP header writing, the ordered/non-indexed iteration variants) run no user code inside the walk. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 3 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/bun-object/deep-equals.test.ts <!-- robobun:evidence:end -->
pull Bot
pushed a commit
that referenced
this pull request
Aug 8, 2026
…id-format (oven-sh#37169) ### Problem `Bun.inspect` and `console.log` have a heap-use-after-free when formatting a value runs user code that mutates the object being formatted. The default-enabled `Symbol.for("nodejs.util.inspect.custom")` hook on a nested value is enough to trigger it: ```js // Malloc=1 <bun-asan> repro.mjs const p = {}; for (let i = 0; i < 8; i++) p['k'+i] = i; let f = 0; p.a = { [Symbol.for('nodejs.util.inspect.custom')]() { if (!f++) for (let i = 0; i < 256; i++) p['n'+i] = i; return 'a'; } }; p.z = 1; console.log(Bun.inspect(p).length); ``` ASAN (with `Malloc=1` so JSC's bmalloc routes through the system allocator): ``` heap-use-after-free READ of size 8 #0 CompactPropertyTableEntry::key() Structure.h #1 PropertyTable::forEachProperty #2 Structure::forEachProperty #3 JSC__JSValue__forEachPropertyImpl bindings.cpp freed by: PropertyTable::destroyIndexVector <- PropertyTable::rehash <- PropertyTable::add <- Structure::addNewPropertyTransition <- JSObject::putDirectInternal ``` ### Cause The fast path of `JSC__JSValue__forEachPropertyImpl` walks the structure's `PropertyTable` with `Structure::forEachProperty` and invokes the formatter callback from inside the walk. The callback recursively formats the property value, which can run user code: a nested value's `inspect.custom`, or a getter on a built-in subclass (for example an overridden `Map.prototype.size`). If that code adds or deletes properties on the parent object, JSC rehashes the shared table, freeing the index vector the outer walk is iterating, and every remaining entry is read from freed memory. The fast-path guard only inspects the parent's structure, and a parent with plain data properties passes it; the hostile hook lives on a nested value. Same bug class as the deepEquals fix in oven-sh#37168, which deliberately excluded this site. ### Fix Collect the entries (key, attributes, direct value) under `forEachProperty` with no side effects, then resolve remaining values and invoke the callback on the snapshot after the walk finishes. The values go in a `MarkedArgumentBuffer` so GC in a callback cannot collect them; keys are retained as `Identifier`s. The snapshot is per structure walk, so the prototype-chain restart loop still re-reads each prototype's live structure. Properties added to the object while it is being formatted are no longer printed: the walk now reflects the object as it was when formatting started. That matches Node, which collects the key list before formatting values. The other `forEachProperty` sites are unaffected: the non-indexed and ordered variants never take this fast path, and the remaining callers run no user code in the callback. ### Verification - New test in `test/js/bun/util/inspect.test.js` (ASAN-only, child spawned with `Malloc=1`) covering: `inspect.custom` adding properties via `Bun.inspect` and `console.log`, deleting properties, a `Map` subclass `size` getter, the prototype fast-walk of an own-property-less object, and GC churn inside the hook with object-valued siblings formatted afterwards. Fails before the fix (ASAN abort, empty stdout), passes after. - `test/js/bun/util/inspect.test.js`: 74 pass. `test/js/bun/console/`: 85 pass, 1 skip. - `inspect-error.test.js` minified-file snapshots and `inspect-error-leak.test.js` fail identically with and without this diff locally (pre-existing, unrelated to property enumeration).
pull Bot
pushed a commit
that referenced
this pull request
Aug 22, 2026
oven-sh#39947) ### Problem - A worker whose entry point goes through a package.json `imports` or `exports` map leaks 12 KiB (3 `PathBuffer`s, more on Windows) when its thread exits. On an ASAN build LeakSanitizer reports `Direct leak of 12288 byte(s)` allocated in `module_bufs` (`src/resolver/package_json.rs`), reached from `resolve_entry_point_specifier` on the worker thread. - Cause: `MODULE_BUFS` is a thread local `Cell<*mut ModuleBufs>` with nothing that frees the box. The resolver's other per thread buffers (`BufsSlot` in `resolver.rs`, `LazyPathBuf` in `bun_paths`) got a destructor in oven-sh#30875. This one did not. ### Fix - Wrap the pointer in `ModuleBufsSlot`, whose `Drop` destroys the box when the thread exits. Same shape as `BufsSlot`. Access is unchanged, so the recursion notes on the thread local still hold, and the static TLS template is still one pointer. - Correct because the destructor runs when the thread's TLS is torn down, after every resolver frame on that thread has returned. The main thread's box lives for the process, as before. - Verified: `test/js/web/workers/worker-entry-point.test.ts` (new file) runs a worker through an `imports` alias in a child with `detect_leaks=1`. It fails on main with the report above and passes with this change (checked both ways with a debug build). `test/js/bun/binary/tls-segment-size.test.ts` still passes. ### Background - The resolver keeps a few large scratch buffers per thread instead of on the stack. They are boxed on first use and only a pointer sits in TLS, so the TLS segment stays small on every platform. - A worker thread resolves its own entry point and preloads, so it is the common short lived thread that touches these buffers. The bundler's pool threads live as long as the pool. - The ASAN CI lanes run test children with `detect_leaks=1`. The test sets that itself (plus the repo's `test/leaksan.supp`) so that a local ASAN build checks it too. A build without ASAN ignores the options and checks the behaviour only. <details><summary>Notes</summary> Found through oven-sh#39811, whose worker test resolves an `imports` alias and failed on the ASAN lanes because of this leak. oven-sh#39811 carries this change until this lands and is otherwise independent of it. oven-sh#35060 (overflow bundle threads, open) includes the same change as one of its hunks, because its threads are short lived too. The case is in its own file, for the worker entry point resolution cases, rather than in `worker.test.ts`: three of that file's stress cases go over their budget on a debug build on a slow machine, which would hide whether this case itself flips. oven-sh#39811 adds its worker case to the same file. Without `print_suppressions=0` LeakSanitizer prints a "Suppressions used" table to stderr on exit when an unrelated, suppressed allocation exists in the process, so the test passes that along with the suppressions file when the environment does not already set `LSAN_OPTIONS`. </details> <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 1 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/workers/worker-entry-point.test.ts bun test v1.4.0 (4199361) test/js/web/workers/worker-entry-point.test.ts: 41 | LSAN_OPTIONS: 42 | bunEnv.LSAN_OPTIONS ?? 43 | `print_suppressions=0:suppressions=${path.join(import.meta.dir, "..", "..", "..", "leaksan.supp")}`, 44 | }, 45 | ); 46 | expect(stderr).toBe(""); ^ error: expect(received).toBe(expected) - "" + " + ================================================================= + ==385090==ERROR: LeakSanitizer: detected memory leaks + + Direct leak of 12288 byte(s) in 1 object(s) allocated from: + #0 0x000007dd95c8 in malloc crtstuff.c + #1 0x00000be19934 in std::sys::alloc::unix::alloc /root/.rustup/toolchains/nightly-2026-07-20-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/alloc/unix.rs:31:18 + #2 0x00000be184b9 in <std::alloc::System>::alloc_impl /root/.rustup/toolchains/nightly-2026-07-20-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/alloc.rs:149:78 + #3 ... (truncated) release without fix: all passed bun test v1.4.0-canary.1 (2e16ac4) test/js/web/workers/worker-entry-point.test.ts: (pass) package.json imports alias as the entry point > the worker runs and its thread exits without leaking [11.86ms] 1 pass 0 fail 3 expect() calls Ran 1 test across 1 file. [218.00ms] __F:0:S:0 ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/workers/worker-entry-point.test.ts bun test v1.4.0 (4199361) test/js/web/workers/worker-entry-point.test.ts: (pass) package.json imports alias as the entry point > the worker runs and its thread exits without leaking [3743.31ms] 1 pass 0 fail 3 expect() calls Ran 1 test across 1 file. [6.05s] __F:0:S:0 release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 667ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [0/5] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_base64 v0.0.0 (/workspace/bun/src/base64) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/brotli) �[1m�[92m Compiling�[0m bun_outpu ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/resolver/package_json.rs | 23 +++++++++--- test/js/web/workers/worker-entry-point.test.ts | 50 ++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) ``` </details> **gate history** · 1 passed · 1 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/resolver/package_json.rs 2 2 0 test/js/web/workers/worker-entry-point.test.ts 1 2 0 ``` </details> **root cause** · written by the author bot With --target bun or node, the resolver short-circuits node:, bun: and hardcoded builtin specifiers into an external result whose primary path is the bare specifier rather than an absolute file path, and entry-point resolution passed that through, so enqueue_entry_item either tripped the absolute-path assert, reported a misleading "File not found", or for bun:wrap collided with the runtime's pre-registered source and left the build with no entry points. The fix marks entry-point resolutions with their own ImportKind so the resolver no longer applies externalization rules to them, and resolv… <!-- robobun:evidence:end -->
pull Bot
pushed a commit
that referenced
this pull request
Sep 1, 2026
…safe functions (oven-sh#39848) ### Problem - An addon's `napi_finalize` segfaults under `napi_body::Finalizer::run`: Sentry BUN-4MXA and BUN-4NJZ on 1.4.0, and oven-sh#41055 (libsql under `bun test --parallel`). - `NapiRef::callFinalizer` (`napi.h`) copied the finalizer into the task that runs after the GC, so `napi_delete_reference` in between did not cancel it and it ran on freed memory. Node dequeues it. - `ThreadSafeFunction::destroy` (`napi_body.rs`) freed the function before its finalizer ran. After `napi_tsfn_abort` it also waited for every other thread's reference (oven-sh#40671), so a holder that never releases pinned the process. ### Fix - `NapiEnv::m_pendingRefFinalizers` (Node's `pending_finalizers`): the GC adds the ref and queues a drain if the set was empty. Each drain takes the first ref, requeues if any remain, then runs its finalizer. `~NapiRef` removes itself, so a deleted reference never finalizes. - `ThreadSafeFunction::finalize` (was `destroy`) runs from the closing dispatch whatever `thread_count` is: the finalizer first, then the JS-thread release under the lock. It frees the allocation if no thread reference is left, else the last release does (Node's `Finalize` + `MaybeDelete`). - Verified: four new tests in `test/napi/napi.test.ts`, compared with Node 26. ### Background - A `NapiRef` backs a `napi_ref`. `napi_wrap` creates one that holds the JS object weakly, and JSC calls it when it sweeps the object. - Outside `NAPI_EXPERIMENTAL` a finalizer may not run inside the GC, so it is queued on the event loop. Node never finalizes a reference deleted before then. - Node finalizes a threadsafe function on the JS thread when it closes and deletes it after the callback, or at the last `thread_count` release. <details><summary>Notes</summary> Branch history. The first version kept a `HashSet` of refs with a queued task each; the maintainer push on this branch replaced it with the `ListHashSet` drain (`NapiEnv::drainOneRefFinalizer`, one ref per event-loop task, the next drain queued before the finalizer runs so a finalizer that deletes or enqueues other refs is plain) and merged oven-sh#40671 (threadsafe function: finalize on abort without waiting for the other threads, `finalize` replaces `destroy`, `env_teardown_done` renamed `resources_released`). That PR is closed in favour of this one. A `napi_wrap` without a result keeps the copying `callFinalizer()` path: its runtime-owned reference (`NapiRefSelfDeletingWeakHandleOwner`) is deleted right after, so nothing can delete it while the copy is queued. The four tests: `napi_wrap` and `napi_add_finalizer` references deleted in the same turn as the GC, a parent finalizer that deletes children collected by the same GC (in both creation orders), a tsfn finalizer that uses its handle, and (from oven-sh#40671) a tsfn aborted while another thread still holds a reference. The last one waits for the holder thread by deadline (3 s, the holder gives up after 2 s) so it stays under the default test timeout. Fail before, on release 1.4.0 and on an unfixed ASAN debug build. The fixture's native objects are static and record a finalizer that runs after the delete instead of reading freed memory, so the failure is a clean output mismatch with Node: ``` - napi_wrap: collected before delete: true, finalized after delete: 0 - napi_add_finalizer: collected before delete: true, finalized after delete: 0 + napi_wrap: collected before delete: true, finalized after delete: 1 + napi_add_finalizer: collected before delete: true, finalized after delete: 2 ``` Parent and children (the shape from the report): with the parent created after the children, JSC sweeps the parent's weak handle first, so the parent's queued finalizer ran first and then all 8 child finalizers ran on deleted children (`children finalized after delete: 8`). Created before the children, the children's finalizers ran first and the parent's deletes were plain. The fixture runs both orders. `Bun.gc(true)` is `collectNow(Sync)`, which sweeps synchronously, so the task is queued before `Bun.gc` returns and the same-turn delete is deterministic. A conservative scan can keep an object alive, so each attempt uses a fresh object and the output does not depend on the attempt count. BUN-4NJZ (Windows x64, `bun test`): eight frames inside `index.node`, then `napi_body::Finalizer::run` (`napi_body.rs:2405`, the return address after the callback, which symbolizes as the inlined `napi_internal_remove_finalizer` / `NapiEnv::removeFinalizer` / `BoundFinalizer::BoundFinalizer`), `NapiFinalizerTask::run_on_js_thread`, `dispatch::run_task`. Same shape as BUN-4MXA: the addon's finalizer is what is executing. Drains queued during VM shutdown become cleanup hooks (`NapiFinalizerTask::schedule`); `VirtualMachine::run_cleanup_hooks` repeats while hooks push more, so a chain of drains still runs at exit. The tsfn test on an unfixed ASAN build: ``` ERROR: AddressSanitizer: heap-use-after-free #0 napi_get_threadsafe_function_context src/runtime/napi/napi_body.rs:3290 #1 napitests::tsfn_finalizer_uses_handle standalone_tests.cpp #2 <napi_body::Finalizer>::run napi_body.rs:2403 #3 <NapiFinalizerTask>::run_on_js_thread #4 bun_runtime::dispatch::run_task freed by: ThreadSafeFunction::destroy ``` On a release build the stale read still returns the old context, so that test only fails under ASAN. The two reference tests fail on every build. Experimental modules are unchanged: `callFinalizerFromGC` runs the finalizer during the GC for them, as before. The runtime-owned reference is `NapiRefSelfDeletingWeakHandleOwner` in `napi.cpp`. Other paths checked: - Finalizer deletes its own reference (node-addon-api `ObjectWrap`): `runQueuedFinalizer` takes the ref out of the set before calling, so the delete inside the callback is plain. `test/napi/napi-finalizer-delete-ref.test.ts` covers the experimental variant. - Env cleanup while a finalizer is queued: `wrap_cleanup` runs it at once and clears it. The task later finds the cleared callback and does nothing, so it still runs once. - Task dropped at VM teardown (`has_run_cleanup_hooks`): the entry stays in the set until `~NapiRef` or the env goes away. The set holds the pointer as a key only. - A reused address: set membership means a finalizer is owed, so a drain that reaches a new ref at the same address runs a finalizer that is owed anyway, and the set holds each ref once. - `~NapiRef` does one `ListHashSet::remove`, which returns at once while the env never queued anything. `napi_create_reference` refs never enter the set. - Re-entry from the tsfn finalizer into the function it belongs to: `destroy` holds no borrow and no lock while the callback runs. `napi_release_threadsafe_function` returns `napi_invalid_arg` (`thread_count` is 0) and `napi_unref_threadsafe_function` returns `napi_ok` in both runtimes (asserted by the test). `napi_acquire_threadsafe_function` returns `napi_closing` (`closing` is `Closed`), `napi_call_threadsafe_function` returns `napi_invalid_arg` for the same reason, and `napi_ref_threadsafe_function` is a no-op because `maybe_queue_finalizer` disabled the keepalive. None of them frees the function or schedules it, and `finalizer_fun` was taken, so nothing runs the finalizer twice. Suites run with the debug build: `test/napi/napi.test.ts`, `napi-finalizer-delete-ref.test.ts`, `napi-value-ffi.test.ts`, and the node-napi-tests suites `6_object_wrap`, `7_factory_wrap`, `8_passing_wrapped`, `test_finalizer`, `test_reference`, `test_reference_double_free`, `test_general` (both), `test_instance_data` (both), `test_threadsafe_function`, `test_reference_by_node_api_version`, `test_env_teardown_gc`, `test_worker_terminate_finalization`, `test_buffer`. All pass (a CI-like `--timeout` is needed locally, the default 5 s is too short for the ASAN build). oven-sh#38506 changes `NapiEnv::inGC()`. `callFinalizerFromGC` calls it, so the two compose. Closes oven-sh#41055. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/napi/napi.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )