Repository navigation
deps: update lolhtml to v3.0.1 - #1
Closed
github-actions[bot] wants to merge 1 commit into
Closed
github-actions[bot] wants to merge 1 commit into
github-actions[bot] wants to merge 1 commit into
Conversation
github-actions
Bot
force-pushed
the
deps/update-lolhtml
branch
from
May 3, 2026 01:33
402ebc9 to
ebb4306
Compare
github-actions
Bot
force-pushed
the
deps/update-lolhtml
branch
from
May 10, 2026 01:33
ebb4306 to
04036b5
Compare
github-actions
Bot
force-pushed
the
deps/update-lolhtml
branch
from
June 7, 2026 01:42
04036b5 to
c39e154
Compare
github-actions
Bot
force-pushed
the
deps/update-lolhtml
branch
from
August 2, 2026 01:32
c39e154 to
d123255
Compare
Seantheprogrammer93
pushed a commit
that referenced
this pull request
Sep 27, 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 -->
Seantheprogrammer93
pushed a commit
that referenced
this pull request
Sep 27, 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>
Seantheprogrammer93
pushed a commit
that referenced
this pull request
Sep 27, 2026
…p_until (oven-sh#41149) Fuzzilli found an ASAN stack-buffer-overflow in `run_tasks` -> `Log::add_msg` -> `Vec::push` during runtime auto-install. ## What happened `enqueue_dependency_to_root` blocks in `sleep_until`. `sleep_until` ticks the JS event loop between `is_done` polls. Each poll calls `run_tasks`, which reads `manager.log` to log manifest 4xx errors. The event loop tick can run module transpilation or `AsyncModule::resume_loading_module`. Both swap the VM's log pointers (`jsc_vm.log`, `transpiler.log`, `resolver.log`, `linker.log`, `pm.log`) and restore them on exit. They do not save from the same source: - `resolve_maybe_needs_trailing_slash` saves `jsc_vm.log` and swaps every pointer except `transpiler.log`. - `transpile_source_code_inner` saves `transpiler.log` and restores `pm.log` to it. - `AsyncModule::resume_loading_module` saves `jsc_vm.log` but restores `transpiler.log` to it. When a module transpile runs during a resolve's `sleep_until` tick, its restore sets `pm.log` to the old `transpiler.log`, not the resolve's scoped log. The 404 diagnostics from `run_tasks` then land in the wrong log. With more interleaving across calls `pm.log` ends up at a dead stack `Log`, and the next `run_tasks` poll reads it. ``` READ of size 8 at 0x7ffd98a149c0 thread T0 #0 RawVecInner::capacity #1 Log::add_msg (lib.rs:2369) #3 Log::add_error_fmt (lib.rs:2032) #4 run_tasks (runTasks.rs:494) #5 Closure::is_done (PackageManagerEnqueue.rs:534) #7 AnyEventLoop::tick_raw (AnyEventLoop.rs:148) #8 PackageManager::sleep_until (PackageManager.rs:1063) #9 enqueue_dependency_to_root (PackageManagerEnqueue.rs:571) ... oven-sh#18 resolve_maybe_needs_trailing_slash (VirtualMachine.rs:4259) Address is located in stack of thread T0 in frame #0 to_js_host_call (host_fn.rs:677) [144, 152) 'scope' <== Memory access at offset 160 overflows this variable [176, 240) 'scope_storage' ``` ## Fix - Swap and restore `transpiler.log` with the other log pointers in `resolve_maybe_needs_trailing_slash`. It can no longer drift from `jsc_vm.log` across a resolve. - Snapshot `pm.log` on entry to `enqueue_dependency_to_root`'s `sleep_until`. Re-assert it before each `run_tasks` poll. `run_tasks` always sees the caller's log, whatever the event loop tick did to it. ## Test The test runs a 404 registry in the parent process on `port: 0`. The child queues `require()` calls with `setImmediate`, so they run during `sleep_until`'s event-loop tick and trigger `transpile_source_code_inner`'s log swap. Without the fix, the 404 errors land in the VM log and print to stderr at exit. With the fix, stderr is empty. Verified on current `main` (6f27257): the test fails without the source change and passes with it. Supersedes oven-sh#31120, which the stale bot closed after 90 days. Same change, rebased onto current main; moved to this branch so the fix can be tracked. <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 6 · 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/pr_gate.xml" test/js/bun/resolve/resolve-autoinstall-log-dangling.test.ts bun test v1.4.1 (a6c4cc2) test/js/bun/resolve/resolve-autoinstall-log-dangling.test.ts: (pass) repeated failing auto-install resolves at varying stack depth don't read a dangling pm.log [1075.94ms] 114 | stderr: "pipe", 115 | }); 116 | 117 | const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); 118 | 119 | expect(stderr).toBe(""); ^ error: expect(received).toBe(expected) - "" + "error: GET http://localhost:37987/autoinstall-missing-pkg-0 - 404 + + error: GET http://localhost:37987/autoinstall-missing-pkg-1 - 404 + + error: GET http://localhost:37987/autoinstall-missing-pkg-2 - 404 + + error: GET http://localhost:37987/autoinstall-missing-pkg-3 - 404 + + error: GET http://localhost:37987/autoinstall-missing-pkg-4 - 404 + + error: GET http://localhost:37987/autoinstall-missing-pkg-5 - 404 + + error: GET http://localhost:37987/autoinstall-missing-pkg-6 - 404 + + error: GET http:// ... (truncated) release without fix: 1 FAILED bun test v1.4.1-canary.1 (a6c4cc2) test/js/bun/resolve/resolve-autoinstall-log-dangling.test.ts: (pass) repeated failing auto-install resolves at varying stack depth don't read a dangling pm.log [22.63ms] 114 | stderr: "pipe", 115 | }); 116 | 117 | const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); 118 | 119 | expect(stderr).toBe(""); ^ error: expect(received).toBe(expected) - "" + "error: GET http://localhost:35697/autoinstall-missing-pkg-0 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-1 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-2 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-3 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-4 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-5 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-6 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-7 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-8 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-9 - ... (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/pr_gate.xml" test/js/bun/resolve/resolve-autoinstall-log-dangling.test.ts bun test v1.4.1 (a6c4cc2) test/js/bun/resolve/resolve-autoinstall-log-dangling.test.ts: (pass) repeated failing auto-install resolves at varying stack depth don't read a dangling pm.log [1104.72ms] (pass) module transpile during auto-install's event-loop tick doesn't desync pm.log [689.71ms] 2 pass 0 fail 8 expect() calls Ran 2 tests across 1 file. [3.73s] __F:0:S:0 release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 590ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/141] gen generated_host_exports.rs generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 244 extern-C blocks audited [2/141] gen JS modules (bundle-modules) Preprocess modules (8046ms) Bundle modules (666ms) Postprocesss modules (969ms) Bundle Functions (629ms) Generate Code (40ms) [10.36s] Bundled "src/js" for production 2594 kb 197 internal modules 13 native modules 50 internal functions across 16 files [2/141] cargo bun_runtime → libbun_runtime.a �[1m�[92m Compiling�[0m bun_output_tags v0.0.0 (/workspace/bun/src/bun_output_tags) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_parsers v0.0.0 (/workspace/bun/src/parsers) �[1m�[92m Compiling�[0m bun_install v0.0.0 (/workspace/bun/src/install) �[1m�[92m Compiling�[0m bun_jsc v0.0.0 (/workspace/bun/src/jsc) �[1m�[92m Compiling�[0m bun_dispatch v0.0.0 (/workspace/bun/src/dispatch) �[1m�[92m Compiling�[0m bun_jsc_macros v0.0.0 (/workspace/bun/src/jsc_macros) ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` .../PackageManager/PackageManagerEnqueue.rs | 6 ++ src/jsc/VirtualMachine.rs | 5 ++ .../resolve-autoinstall-log-dangling.test.ts | 65 ++++++++++++++++++++++ 3 files changed, 76 insertions(+) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 6 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/install/PackageManager/PackageManagerEnqueue.rs 1 2 19 src/jsc/VirtualMachine.rs 5 3 19 …js/bun/resolve/resolve-autoinstall-log-dangling.test.ts 3 9 18 ``` </details> <!-- robobun:evidence:end -->
Seantheprogrammer93
pushed a commit
that referenced
this pull request
Sep 27, 2026
…ide its own socket callback (oven-sh#42693) ### Problem - A `fetch()` with its own TLS context (for example `tls: { serverName }`) holds the last ref to that `HTTPContext` once the 60-entry context cache evicts it. When the server closes the connection, a debug ASAN build reports `AddressSanitizer: heap-use-after-free` in `us_internal_ssl_detach` (`packages/bun-usockets/src/crypto/openssl.c:1827`). A release build shows nothing. - The request drops its ref in the result callback (`src/http/AsyncHTTP.rs:745`), inside the close callback of the context's own socket. That frees the context and the socket group embedded in it. `us_internal_ssl_on_close` then reads `s->group->loop` (`openssl.c:2303`). - Found by fuzzing, no user report. It needs over 60 `tls` configs in one process, or a request older than the 30-minute cache TTL. ### Fix - The last deref of an `HTTPContext` no longer frees it in place. `#[ref_count(destroy = Self::destroy_between_ticks)]` queues it on the HTTP thread, and `process_events` frees it between `drain_events()` and the next `tick()`. - Correct because all work of the HTTP thread runs inside those two calls. No socket callback is on the stack between them, whoever dropped the last ref. - Verified: `test/js/web/fetch/fetch-http2-client.test.ts`. Two new ASAN-only tests (h2 and HTTP/1.1) fail with `src/` at main. A third checks that the context is still freed. ### Background - An `HTTPContext` embeds one uSockets socket group, and each socket points into it (`s->group`). - Custom TLS contexts are refcounted (oven-sh#29334). The cache holds one ref and each request holds one. Eviction drops only the cache ref. - `HttpThread::process_events` is the HTTP thread's loop. `drain_events()` starts and aborts requests. `tick()` runs the socket callbacks. - `WindowsNamedPipeContext` defers its free with the same hook. <details><summary>Notes</summary> ASAN report on main (3f7f046), HTTP/1.1 run, trimmed: ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 8, 72 bytes inside of 208-byte region, thread T4 (HTTP Client) #0 us_internal_ssl_detach packages/bun-usockets/src/crypto/openssl.c:1827 #1 us_internal_ssl_on_close packages/bun-usockets/src/crypto/openssl.c:2303 #2 us_internal_socket_close_raw packages/bun-usockets/src/socket.c:328 #3 us_internal_ssl_on_end packages/bun-usockets/src/crypto/openssl.c:2393 freed by thread T4 (HTTP Client): oven-sh#12 HTTPContext<true>::destroy oven-sh#15 RefPtr<HTTPContext<true>>::drop oven-sh#19 AsyncHTTP::on_async_http_callback_raw src/http/AsyncHTTP.rs:745 oven-sh#21 HTTPClient::dispatch_result_and_reset src/http/lib.rs:1618 oven-sh#22 HTTPClient::fail src/http/lib.rs:3970 oven-sh#23 HTTPClient::on_close::<true> src/http/lib.rs:2163 oven-sh#24 Handler<true>::on_close src/http/HTTPContext.rs:1341 oven-sh#28 us_internal_ssl_on_close packages/bun-usockets/src/crypto/openssl.c:2301 ``` The h2 run frees through `fail_from_h2`, called by `ClientSession::fail_streams`, called by `ClientSession::on_close`. oven-sh#42647 fixed a different defect with the same trigger (a stream freed twice in the h2 deliver loop). With this change that trigger is gone too: the context's `Drop`, which closes the session's socket, no longer runs inside the deliver loop. One more path fails on main with the same report: a keep-alive socket that the server closes before it answers. The request dials again from inside the close callback, and the new context displaces the ref on the old one. A redirect, an idle timeout, a DNS failure, an abort and a normal end of the response on an evicted context show no report on main. Why the fix is in `HTTPContext` and not in uSockets. `HTTPContext` is the only socket group owner in bun that freed its group from inside a socket callback. `Listener` frees its group from a finalizer, and the other TLS users share per-VM groups. The safety contract on `SocketGroup::destroy` (`src/uws_sys/SocketGroup.rs`) already says that it must not run while the loop walks the group. A C-side change that passes `loop` into `us_internal_ssl_detach` also stops this report. It leaves the context free to die under Rust frames that still hold its raw pointer, and the next read of `s->group` after a dispatch brings the report back (that read came with oven-sh#31584, and `ssl_release_batch` after it). Two comments in uSockets said the opposite of the Rust contract: `us_socket_group_deinit` called a deinit from `on_close` fine. They now say what `SocketGroup::destroy` says. No C code changes. An earlier draft of this branch routed each holder (`AsyncHTTP::on_async_http_callback_raw`, `HttpThread::connect`) through a queue of refs. The hook replaces that: no holder can get it wrong, and only dead contexts are queued, not one ref per request. No wakeup is needed. A context that dies during `tick()` is freed as soon as that tick returns. One that dies during `drain_events()` (a request that fails at start, a cache eviction) is freed right after it. At process exit the queue is not drained, like the cache. Related history: oven-sh#31660 (closed) is a crash of the same class in the Zig implementation, where the last ref went away inside `onLongTimeout`. Tests: - The ASAN-only block from oven-sh#42647 now takes a protocol. The child holds a response open on a context made by `serverName`, runs 61 more TLS configs to evict it, and writes `evicted` to stderr. The two new tests then destroy the server side of the held TLS socket, over h2 and over HTTP/1.1. With `src/` at main both fail with the report above. - `an evicted custom TLS context is freed when its last request ends` runs on each build. It does not fail on main. It exists because the deferral adds a way to leak: if the queue is never drained, a dead context lives forever. The test keeps one request busy on a context, leaves a second connection idle in that context's keep-alive pool, evicts the context, ends the busy request, and waits for the server to see the idle connection close. With the `heap::destroy` call removed the test times out. Suites run on the debug ASAN build: `fetch-http2-client.test.ts` (70 pass), `fetch-http2-leak.test.ts` (7), `fetch-http2-adversarial.test.ts` (20), `fetch.tls.test.ts` (34), `fetch-redirect.test.ts` (30), `fetch-tls-abortsignal-timeout.test.ts` (6), `fetch-proxy-tls-intern-race.test.ts` (1), `tls-keepalive.test.ts` (4 pass, 2 skip), `fetch-abort-ssl-context-eviction.test.ts` (1), `regression/issue/27358.test.ts` (2). On a release build of this branch: `tls-keepalive.test.ts` 6 pass (same config: 20000 requests, 0 MB growth. 200 distinct configs: 1 MB growth), `fetch-http2-client.test.ts` 66 pass and 4 skip, `fetch-abort-ssl-context-eviction.test.ts` 1 pass. </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 0 · 5 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 2 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/web/fetch/fetch-http2-client.test.ts" bun test v1.4.3 (b993710) test/js/web/fetch/fetch-http2-client.test.ts: (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > GET: status, headers and body round-trip [1172.89ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response body larger than one DATA frame [989.87ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent requests multiplex on one h2 session [1222.39ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body streams as raw DATA frames [467.33ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST: request body is delivered as DATA frames [1562.88ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > gzip content-encoding is decompressed [1655.77ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body larger than initial send window [960.24m ... (truncated) release without fix: 4 skipped bun test v1.4.3-canary.1 (722de17) test/js/web/fetch/fetch-http2-client.test.ts: (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > GET: status, headers and body round-trip [73.34ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response body larger than one DATA frame [65.19ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > gzip content-encoding is decompressed [89.92ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST: request body is delivered as DATA frames [102.83ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > connection-specific request headers are stripped before HPACK [74.16ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > multiple Set-Cookie response headers survive HPACK decode [74.43ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent requests multiplex on one h2 session [101.86ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > abort sends RST_STREAM; siblings on the session survive [107.88ms] (pass) fetch() over HTTP/2 (BUN_FE ... (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/pr_gate.xml" "test/js/web/fetch/fetch-http2-client.test.ts" bun test v1.4.3 (b993710) test/js/web/fetch/fetch-http2-client.test.ts: (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > GET: status, headers and body round-trip [1119.35ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response body larger than one DATA frame [988.06ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent requests multiplex on one h2 session [1222.20ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body streams as raw DATA frames [531.27ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST: request body is delivered as DATA frames [1925.52ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > gzip content-encoding is decompressed [2051.61ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body larger than initial send window [837.85m ... (truncated) release with fix: 4 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 846ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/128] gen generated_host_exports.rs generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 243 extern-C blocks audited [2/128] gen cpp.rs (cppbind) [3/128] gen JS modules (bundle-modules) Preprocess modules (9358ms) Bundle modules (81ms) Postprocesss modules (248ms) Bundle Functions (727ms) Generate Code (35ms) [10.46s] Bundled "src/js" for production 2600 kb 197 internal modules 13 native modules 50 internal functions across 16 files [3/127] cargo bun_runtime → libbun_runtime.a �[1m�[92m Compiling�[0m bun_threading v0.0.0 (/workspace/bun/src/threading) �[1m�[92m Compiling�[0m bun_react_compiler v0.0.0 (/workspace/bun/src/react_compiler) �[1m�[92m Compiling�[0m bun_io v0.0.0 (/workspace/bun/src/io) �[1m�[92m Compiling�[0m bun_watcher v0.0.0 (/workspace/bun/src/watcher) �[1m�[92m Compiling�[0m bun_crash_handler v0.0.0 (/workspace/bun/src/crash_handler) �[1m�[92m Compiling�[0m bun_event_loop v0.0.0 (/workspace/bun/src/event_loop) �[1m�[92m Compiling�[0m bun_ ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` packages/bun-usockets/src/context.c | 10 ++- packages/bun-usockets/src/loop.c | 10 +-- src/http/HTTPContext.rs | 17 +++- src/http/HTTPThread.rs | 18 ++++ test/js/web/fetch/fetch-http2-client.test.ts | 125 ++++++++++++++++++++++----- 5 files changed, 151 insertions(+), 29 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests packages/bun-usockets/src/context.c 1 1 28 packages/bun-usockets/src/loop.c 1 1 28 src/http/HTTPContext.rs 4 5 28 src/http/HTTPThread.rs 5 15 28 test/js/web/fetch/fetch-http2-client.test.ts 4 11 28 ``` </details> <!-- robobun:evidence:end -->
Seantheprogrammer93
pushed a commit
that referenced
this pull request
Sep 27, 2026
…text (oven-sh#42520) ### Problem - `class Upload extends File {}` inside a `node:vm` context crashes on `new Upload(...)`: `panic(main thread): Segmentation fault at address 0x28`. `class S extends fs.Stats {}`, `BigIntStats`, and `Reflect.construct(File, args, fnFromContext)` crash the same way. Node v26.3.0 constructs the object. - These constructors do `static_cast<Zig::GlobalObject*>(getFunctionRealm(lexicalGlobalObject, newTarget))` (`JSDOMFile.cpp:75`, `NodeFSStatBinding.cpp:843`, `NodeFSStatFSBinding.cpp:372`). For a function from a context the realm is a `NodeVMGlobalObject`, which is not a `Zig::GlobalObject`. `JSBlobStructure()` and `getStructure<isBigInt>()` then read a `LazyClassStructure` past the end of the cell. ### Fix - The three sites call `defaultGlobalObject(getFunctionRealm(...))`. It returns the realm when it is a Bun global, and the default Bun global when it is not. - The other native constructors already do this (`generate-classes.ts:2387`, `JSDOMWrapperCache.cpp:42`, `NodeDirent.cpp:177`). - The `StatFs` constructor is not reachable from JS, so that site has no test. - Verified: `test/js/node/vm/vm.test.ts`, four new cases. All four exit with SIGSEGV on 1.4.3-canary.1. Also ran the `fs.Stats`, `Blob`, and globals suites. ### Background - `newTarget` is the constructor that `new` was applied to. In `new Upload()`, `File` runs with `newTarget === Upload`. - `getFunctionRealm(newTarget)` returns the global object the function was created in. A native constructor takes its base `Structure` from that global. `createSubclassStructure` then applies `newTarget.prototype`. - A `node:vm` context has its own global, `NodeVMGlobalObject`. It and `Zig::GlobalObject` (the Bun global) are sibling subclasses of `Bun::GlobalScope`. Only `Zig::GlobalObject` holds the structures of Bun classes. - A ShadowRealm global is a `Zig::GlobalObject`. The comment at these sites names that case, and the cast was valid for it. <details><summary>Notes</summary> Repro (`bun f.js file`, `bun f.js stats`, `bun f.js bigint`): ```js const vm = require("vm"), fs = require("fs"); const ctx = vm.createContext({ File, Stats: fs.Stats }); const which = process.argv[2] || "file"; try { if (which === "file") console.log("ok", vm.runInContext(`class Upload extends File {}; new Upload(["a"], "a.txt")`, ctx).name); if (which === "stats") console.log("ok", typeof vm.runInContext(`class S extends Stats {}; new S()`, ctx).isFile); if (which === "bigint") console.log("ok", typeof Reflect.construct(fs.statSync(".", { bigint: true }).constructor, [], vm.runInContext("(function(){})", ctx))); } catch (e) { console.log("threw", e.message); } console.log("survived"); ``` | | `file` | `stats` | `bigint` | | --- | --- | --- | --- | | 1.4.3-canary.1 | segfault at 0x28 | segfault at 0x28 | segfault at 0x28 | | this branch | `ok a.txt` | `ok function` | `threw Invalid argument type in ToBigInt operation` | | node v26.3.0 | `ok a.txt` | `ok function` | `threw Cannot mix BigInt and other types` | ASAN on main with `Malloc=1` in the environment. `heap-buffer-overflow`, `READ of size 8`, "located 3776 bytes after 4184-byte region" (`File`), 896 bytes after (`Stats`), 912 bytes after (`BigIntStats`). The region is the `NodeVMGlobalObject` cell. ``` #0 JSC::LazyProperty<JSC::JSGlobalObject, JSC::Structure>::getInitializedOnMainThread LazyProperty.h:95 #1 JSC::LazyClassStructure::getInitializedOnMainThread LazyClassStructure.h:104 #2 Zig::GlobalObject::JSBlobStructure() ZigGeneratedClasses+lazyStructureHeader.h:7 #3 JSDOMFile::construct src/jsc/bindings/JSDOMFile.cpp:79 #5 llint_op_super_construct_varargs ``` ``` #1 JSC::LazyClassStructure::getInitializedOnMainThread LazyClassStructure.h:104 #2 Bun::getStructure<false>(Zig::GlobalObject*) src/jsc/bindings/NodeFSStatBinding.cpp:167 (<true>: line 164) #3 Bun::constructJSStatsObject<false>(JSGlobalObject*, CallFrame*) src/jsc/bindings/NodeFSStatBinding.cpp:847 #4 Bun::constructStats src/jsc/bindings/NodeFSStatBinding.cpp:915 ``` Without `Malloc=1` the debug build of main reads zero there and stops at `Structure.h:415: runtime error: member call on null pointer of type 'const JSC::Structure *'`. Fail-before and pass-after, same test file: - `USE_SYSTEM_BUN=1 bun test test/js/node/vm/vm.test.ts -t "belongs to a context"`: 0 pass, 4 fail (`exitCode: 139`, `signalCode: "SIGSEGV"`). - Debug ASAN build of main with only the test added: 0 pass, 4 fail (`exitCode: 1`). - Debug ASAN build of this branch: 4 pass. The whole file: 285 pass, 0 fail. Other cases checked by hand on this branch: - A newTarget that is a plain function, a bound function, or a Proxy from the context. The new test covers these. A bound function has no `prototype`, so the object gets the prototype of the constructor. - A revoked Proxy from the context throws `TypeError: Cannot get function realm from revoked Proxy`. - A newTarget whose `prototype` is not an object falls back to `File.prototype` or `fs.Stats.prototype`. - `class X extends File {}` inside a `ShadowRealm` still works. `File` and `X` both belong to the ShadowRealm global there. - An instance of the context subclass works with `FormData`, `.text()`, and a forced GC. Suites run on the debug build: `test/js/node/vm/vm.test.ts`, `test/js/node/fs/fs-stats-constructor.test.ts`, `test/js/node/fs/fs-stats-truncate.test.ts`, `test/js/node/fs/fs.test.ts -t Stats`, `test/js/bun/globals.test.js`, `test/js/web/fetch/blob.test.ts`, and the Node tests `test-fs-stat.js`, `test-fs-stat-bigint.js`, `test-fs-statfs.js`, `test-fs-watchfile.js`. Related: - oven-sh#32434 changes the `File` cast to `defaultGlobalObject` as one line of a larger `File.prototype` change. It does not touch `fs.Stats`. - oven-sh#42514 fixes the same kind of cast at another site (`UtilInspect.cpp`). - `new BigIntStats(...)` stores `atimeMs` as a Number where Node stores a BigInt, so `.atime` throws. That is a separate bug and is not part of this PR. </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/js/node/vm/vm.test.ts <!-- robobun:evidence:end -->
Seantheprogrammer93
pushed a commit
that referenced
this pull request
Sep 27, 2026
…peline is uploaded as JSON (oven-sh#43639) ## What Two changes to CI's machine images and the pipeline that bakes them. ### The image's record is lines the bake script writes The record a bake writes about the machine (what arrived on the day of the bake, for keying caches of build outputs) becomes `bun-image.txt`, in place of `bun-image.json`. It was JSON, assembled by a JavaScript program kept as text in `scripts/build/ci-images/spec.ts` and run under the image's Node.js. JSON is what needed a program: neither `sh` nor Windows PowerShell 5.1 can write it safely without one. The only reader hashes the file's bytes, so the layout is free, and lines can be written by ordinary steps in either shell: ``` name: linux-aarch64-debian-8e6e635fb2cbd2fd image: {"os":"linux","arch":"aarch64","distro":"debian",…} tool nodejs: pinned 26.3.0 tool llvm: packageDatabase tool glibc-sysroot: observed tool prefetch: notRecorded decided by the commit being built, not by this file observed glibc-sysroot: b8f58b00c18b… package adduser 3.134 package libc6:amd64 2.41-12+deb13u4 ``` - The generator knows the facts and every tool's identity, so the `image:` and `tool` lines are literal in the script. The `observed` lines and the `package` lines are read on the machine. - Nothing is best-effort. The package query is a statement of its own into a scratch file, because `sh` has no pipefail and a query piped straight into `sort` could fail unnoticed; on Windows `scoop export` runs through `run`, so its exit code is checked. On every path a query that listed nothing, or an observed tool that left nothing, fails the bake. - `recordImage` is a tool made of steps like every other tool; `record-image.mjs` is no longer written into the bake directories, and `image.json` no longer carries the list of tools that only that program read. `recordedTool` is the one place that turns a tool's identity into its line. - dpkg's packages are listed as `${binary:Package}`. On the arm64 build image, which also has amd64 packages, `libc6` and `libc6:amd64` used to be the same line twice. - The separate sha256 of the package list is gone: the file is what gets hashed. ### The pipeline is uploaded as JSON `.buildkite/ci.ts` wrote its pipeline with a hand-written YAML serializer, `toYaml()`, which quoted a string only when it held certain punctuation. A string of digits was written bare and YAML read it as a number: Canonical's AWS account id, `"099720109477"`, became the agent tag `base-image-owner=99720109477`, and both Ubuntu bakes were cancelled with "Failed to create agent". A release such as `"3.20"` would have become `3.2` the same way. `buildkite-agent pipeline upload` parses every file it is given with a YAML parser, whatever its name, and JSON is YAML. So the pipeline is now `JSON.stringify(pipeline)`, written to `.buildkite/ci.json`, and `toYaml()` is deleted. In JSON every string is quoted, so a string cannot lose its type. Read by a YAML parser, the generated pipeline is the same tree as before, except for the values that were being misread and an empty `depends_on`, which is now an empty list where it used to be written as nothing and read as null. ## What it does to the images All eight baked images get new names, so this PR's build bakes them all. Every Linux bake script is otherwise unchanged; the Windows scripts differ in the record section only. ## Verified - The generated record section run for real in Debian 13 and Alpine 3.23 containers: name, facts, tool lines, observed values, sorted packages, no duplicate lines; a `dpkg-query` that exits 3 stops the script with 3; an empty or missing observation stops it. - The Windows record lines run under PowerShell against a `scoop` that fails (the script throws on its exit code), one that lists no apps (it throws), and one that prints its JSON over several lines (recorded). - The pipeline generated as YAML by the old serializer and as JSON, for a PR build and a `main` build with the release step: the JSON file reads the same through a YAML parser as through `JSON.parse`, and the trees are equal except where the old serializer misread a string or dropped an empty list. On this PR's build the agent reports `Successfully parsed and uploaded pipeline #1 from "ci.json"`, and both Ubuntu bakes got their machines. - Tests: `test/internal/source-lints/ci-images.test.ts` asserts the exact record section for Debian and for Windows, the Alpine package query, and a literal record line for each kind of identity. - shellcheck on the generated sh, PowerShell's parser on both Windows scripts, `tsc` for both script projects.
Seantheprogrammer93
pushed a commit
that referenced
this pull request
Sep 27, 2026
…ed settles (oven-sh#43752) ### Problem - On a `type: "direct"` stream, `reader.read()` settles two microtasks after the chunk is delivered. If the source ends the stream right after the chunk, `reader.closed` settles first. `Duplex.fromWeb` destroys on that rejection and drops the chunk: it emits only `["error:source failed"]`. - The cause: `readableStreamDefaultReaderRead` (`src/jsc/bindings/webcore/streams/JSReadableStreamDefaultReader.cpp:139`) resolved the read promise with the promise that `onPull` allocated. That adoption costs two microtasks. `handleError` rejects `reader.closed` directly. ### Fix - `JSDirectStreamController::onPull` takes the caller's read promise and settles it on every path. `m_pendingRead` holds that promise. `readMany()` passes its own inner promise and keeps its timing. - Correct because the JS implementation before oven-sh#33193 did the same: `read()` returned the pump's promise. - Verified: `test/js/web/streams/streams.test.js`. The 10 new tests fail without the fix. All 624 pass with it. - Self-reviewed: 17 concerns raised, 3 survived, all 3 addressed with tests. ### Background - A direct stream has no queue. `pull(controller)` writes bytes and hands them to the reader with `flush()`, `end()` or `close()`. - The controller keeps the head-of-line `read()` in `m_pendingRead`, not in the reader's `[[readRequests]]` list. `pipeTo`, `tee` and `for await` use that list and do not change. ### Downsides - A read loop calls `pull()` again two microtasks sooner. A sync `pull()` that keeps the controller and is not idempotent can run twice when its `close()` lands within two microtasks of its last `flush()`. With chunks paced by the event loop it already ran twice. - A read loop sees one chunk for each `flush()`. Before, two flushes could arrive as one chunk. The bytes are the same. <details><summary>Notes</summary> **Repro (order).** Bun 1.4.3-canary (367d939) and main a2b69f7 print the direct line reversed. ```js async function order(rs) { const log = []; const reader = rs.getReader(); reader.closed.catch(() => log.push("closed rejected")); await reader.read().then(() => log.push("read #1 fulfilled")); await new Promise(r => setImmediate(r)); return log; } const def = new ReadableStream({ async pull(c) { c.enqueue(new TextEncoder().encode("a")); await undefined; c.error(new Error("source failed")); } }, { highWaterMark: 0 }); const direct = new ReadableStream({ type: "direct", async pull(c) { c.write("a"); await c.flush(); c.close(new Error("source failed")); } }); console.log("default:", JSON.stringify(await order(def))); // ["read #1 fulfilled","closed rejected"] console.log("direct: ", JSON.stringify(await order(direct))); // before: ["closed rejected","read #1 fulfilled"] ``` The order is also reversed when the chunk arrives later and `close(error)` follows `flush()` in the same tick, with `error()` in place of `close(error)`, when `pull()` rejects after the flush, and when `reader.closed` fulfils (`flush()` then `close()`, or `end()` with a pending read). The `test.each` covers these six producers. **Overlapping reads.** The delay also let a later read pass an earlier one. With `pull(c) { c.write("a"); c.close(); }` and two `read()` calls, read 2 reported `done` before read 1 delivered `a`. The same happened when `end()` armed a final chunk with nobody reading: the read after it reported `done` first. A consumer that stops at the first `done` lost the chunk. Three tests under "overlapping read()s are observed in the order they were issued" cover this, and the path where read 2 waits in `[[readRequests]]` until `onFlush` makes it the pending read. **What a reader observes differently.** I ran one probe script of 22 reader scenarios on the build before and after. Seven lines differ. No scenario loses or gains data. | scenario | before | after | | --- | --- | --- | | microtasks from `read()` to its reaction, chunk ready | 2 | 0 | | flush, then `close()` in the same tick | `closed`, `a`, done | `a`, `closed`, done | | async pull: flush `a`, `b`, `c`, then `close()` | `a`, `closed`, `bc`, done | `a`, `b`, `c`, `closed`, done | | 4 concurrent reads, 3 chunks, then `close()` | ..., `closed`, read3 done | ..., read3 done, `closed` | | 3 concurrent reads, `close(error)` after chunk 1 | `closed`, read0 `a`, read1 rejects, read2 rejects | read0 `a`, read1 rejects, `closed`, read2 rejects | | second `pull()` throws with read0 pending | `closed`, read0 rejects, read1 rejects | read0 rejects, `closed`, read1 rejects | | `cancel()` with a pending read | `closed`, cancel resolves, read done | `closed`, read done, cancel resolves | The last row now matches the order of the spec (ReadableStreamCancel closes the stream and runs the close steps before the cancel promise resolves). Unhandled-rejection reports, `for await`, `pipeTo`, `Response.text()` and the `readMany()` result shapes are the same on both builds. **The first Downsides bullet, measured.** Source: `pull(c) { pulls++; queueMicrotask(() => { c.write("a"); c.flush(); /* close() N microtasks later */ }); }`, read with a `read()` loop. | `close()` lands after | before | after | | --- | --- | --- | | 0 microtasks | `a`, 1 pull | `a`, 1 pull | | 1 | `a`, 1 pull | `a`, 2 pulls | | 2 | `a`, 1 pull | `aa`, 2 pulls | | 3 | `a`, 2 pulls | `aa`, 3 pulls | | 4 | `aa`, 2 pulls | `aaa`, 3 pulls | A reader calls `pull()` again for each `read()` once a sync `pull()` has returned. That contract does not change. Only the moment of the next `read()` does. An async `pull()` is not affected: the controller does not call it again while its promise is pending. An idempotent source that keeps the controller gets extra calls that do nothing. A sync `pull()` that starts an async loop over `x`, `y` and does not return its promise gives `xyx` on both builds when `setImmediate` paces the chunks. With microtask pacing it gave `xy` before and gives `xyx` now. **`readMany()`.** It maps the pump's `{value, done}` through one internal reaction, for every controller kind. A `reader.closed` handler still runs before a `readMany()` result, on a started default stream too. A `read()` issued behind a pending `readMany()` can now settle first, as on a default stream. This PR leaves `readMany()` as it is. **No `then` lookup in the pump.** The pump settles the read with `JSPromise::fulfill`, as it did for its own promise. Before, the last adoption step resolved the read promise with the `{value, done}` object, so a patched `Object.prototype.then` ran for it. Now it does not. I kept `fulfill`: a `then` getter would run user code in the middle of `finishClose` and `cancelSteps`. A patched `Promise.prototype.then` also no longer receives the pump's internal promise. **Removed branch.** `onPull` returned `m_pendingRead` when `pull()` left the stream not readable. `handleError`, `finishClose` and `cancelSteps` all clear `m_pendingRead` before the stream leaves the readable state, so that branch could not run. The new read settles from the stream state. **Review concerns I did not act on.** - If a source hook throws after the read was registered, `read()` returns a promise rejected with that error, and the registered promise is dropped. Before, the pump's promise was dropped the same way. - A review proposed a timing window that delays the next sync `pull()`, to hide the first Downsides bullet. That window would put back the delay that this PR removes. **Found outside this PR.** Both builds do these. No open PR covers them. - A sync `pull()` that calls `write("a")`, `flush()` and then fails before it returns (`close(error)`, `error()` or a throw) loses `a` for `read()`, `Readable.fromWeb` and `Duplex.fromWeb`. `for await` and `pipeTo` get `a` and then the error. Their read request is queued before `pull()` runs, so `flush()` delivers at once. A `read()` promise is registered after `pull()` returns, and the failure frees the buffer first. The same producer with an `await` before the failure delivers `a` to every consumer. - If `pull()` calls `reader.releaseLock()` before it returns, the `read()` that started it stays registered on the controller. It then takes the first chunk of the next reader, and that reader's `read()` stays pending. Related to oven-sh#43748, which makes `Readable.fromWeb` watch `reader.closed`. **Local runs (debug + ASAN build).** `streams.test.js` 624 pass, also with `BUN_JSC_validateExceptionChecks=1`. The 10 new tests fail on 1.4.3-canary and on a debug build with `src/` from main. They pass 100 of 100 with `--rerun-each 10`. `serve-direct-readable-stream` 169, `async-iterator-stream` 95, `body.test.ts` 774, `body-async-iterator` 6, `body-stream-excess` 4, `node-stream` 104, `web-stream-state` 46, `spawn-stdin-readable-stream` 37, `html-rewriter` 186, `sync-pull-fast-path` 7, `streams-string-limit` 8, and `test-stream-duplex-from.js`, `test-whatwg-webstreams-adapters-to-streamduplex.js`, `test-whatwg-webstreams-adapters-to-readablewritablepair.js`, `test-webstreams-finished.js`, `test-webstreams-pipeline.js`, `test-whatwg-readablestream.mjs`, `test-readable-from-web-enqueue-then-close.js`, `test-stream-readable-from-web-termination.js`. Three tests time out at 5 s on this debug build with and without the change: `fetch.stream.test.ts` "Content-Length response works (multiple parts)", `AsyncLocalStorage.test.ts` "re-entering a storage inside run() does not grow the context", `bun-write.test.js` "on large files". None of them reads a direct stream through a reader. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/web/streams/streams.test.js <!-- robobun:evidence:end -->
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.
What does this PR do?
Updates lolhtml to version v3.0.1
Compare: cloudflare/lol-html@77127cd...608cc4a
Auto-updated by this workflow