Skip to content

node:worker_threads: per-thread --use-system-ca, real eventLoopUtilization, --cpu-prof in workers, node's online timing, error.code / stack-getter / timeOrigin fixes, async_hooks WORKER resource, worker_threads dc channel (+10 upstream tests) - #34424

Open
cirospaciari wants to merge 98 commits into
mainfrom
ciro/worker-threads-node-tests

Conversation

@cirospaciari

@cirospaciari cirospaciari commented Jul 16, 2026 •

Copy link
Copy Markdown
Member

Ports the Node v26.3.0 worker-related upstream tests that were still missing and fixes the node:worker_threads behaviour they expose. Ten upstream files are added, byte-identical to upstream, with no expectations.txt or sanitizer exemptions; each is enabled by one of the fixes below.

upstream test fix that enables it
test-diagnostics-channel-worker-threads.js worker_threads diagnostics channel
test-worker-hasref.js async_hooks WORKER resource
test-worker-error-stack-getter-throws.js error clone survives a throwing stack getter
test-perf-hooks-worker-timeorigin.js process-wide performance.timeOrigin
test-worker-internal-modules.mjs error.code survives non-cloneable errors
test-tls-get-ca-certificates-worker-{,no-}use-system-ca.js per-thread --use-system-ca
sequential/test-cpu-prof-dir-worker.js, sequential/test-cpu-prof-worker-argv.js --cpu-prof in workers
sequential/test-worker-eventlooputil.js real eventLoopUtilization() (this test previously hung, so it could not be vendored)

(A first batch of tests that already passed — http2-stream-terminate, unsupported-eval-on-url, cleanup-handles, dispose, and the -pe alias with test-preload-worker — landed on main separately while this was open and is no longer part of this diff.)

Behaviour fixes

worker_threads diagnostics channel (worker_threads.ts). Node publishes the new Worker on the worker_threads channel at the end of the constructor; Bun never did. The channel is resolved at module load, not lazily: diagnostics_channel uses Map internally (a lazy require picks up user tampering — it broke the "tampered Map prototype" test), and dc's registry is a WeakRefMap whose subscribe() swaps the prototype in place, so a module-load strong ref is what keeps the published channel from being re-created underneath the publisher. Verified against the node binary: identical payload, one publish per Worker, none without subscribers, nested workers publish on their own thread's channel.

async_hooks WORKER resource (worker_threads.ts, async_hooks.ts, async_hooks_tick.ts). init was only ever delivered for TickObject, so hooks looking for WORKER (what why-is-node-running relies on) never fired. The Worker constructor now emits one; hasRef() follows ref()/unref() and reads back undefined once the thread has exited, ref()/unref() no-op after exit, a throwing init hook is fatal as in node. Found on the way: createHook invoked init bare, so this.disable() inside it threw — node calls it as a method on the hook.

Worker error clone survives a throwing Error.prepareStackTrace (Worker.cpp). Cloning reads stack; a throwing getter took the whole clone down and the parent got pretty-printed text in message. Node drops just the unreadable stack; the clone is retried once with an own undefined stack, only on the failure path and via setStackPropertyAlreadyMaterialized() so the getter can't run again.

performance.timeOrigin is process-wide (VirtualMachine.rs). Each worker VM took its own Instant::now(), so a worker's origin drifted by the spawn delay (264 ms measured vs node's 0). Every thread now reports the process origin; Bun.nanoseconds() becomes "since the process started" as documented.

error.code survives a non-cloneable thrown value. A worker throwing something structured clone can't serialize fell back to message text and the parent rebuilt a bare Error, losing code — Bun's own ResolveMessage (ERR_UNKNOWN_BUILTIN_MODULE) is exactly that. code is carried alongside the message; the message text is unchanged byte for byte.

'online' fires at node's timing (WorkerMessagingProxy.cpp, web_worker.rs). Bun posted online only after the entry point's promise settled, so a worker whose top level never returns (while(true)) never reported online and terminate() on it never resolved. dispatchOnline did two separable things — the Pending→Running state flip that also gates message routing, and posting the event — and moving both regresses message-routing, heap-profile and destruction tests (measured; details in the discussion below). Only the event moves: it is posted before the entry loads, the state flip stays where it was. Consequences that fall out: a worker whose entry throws or fails to resolve reports online then error like node; the bun-API Web Worker's 'open' likewise now precedes the entry point (pinned by a test in worker.test.ts).

MessagePort listeners get the port as this — injectFakeEmitter's wrapper dropped the receiver. BroadcastChannel#ref()/unref() return the channel as in node instead of undefined. process.execArgv treats a node whole-token alias such as -pe as value-taking only ahead of the script name, so bun run -pe script.js no longer swallows the script.

--cpu-prof profiles worker threads (web_worker.rs, BunCPUProfiler.rs, jsc_hooks.rs). Node writes one profile per thread and honours a Worker's own execArgv; Bun configured the profiler on the main-thread run path only, so a process with a worker wrote one profile and execArgv: ['--cpu-prof'] did nothing. Inheritance follows node_worker.cc: no execArgv inherits the parent's settings, an explicit list (even []) replaces them, and a worker's own --cpu-prof, --cpu-prof-md, --cpu-prof-dir, --cpu-prof-name and --cpu-prof-interval are honoured, including flags that follow another option's separate value. Default file names carry the real thread id (CPU.<date>.<time>.<pid>.<threadId>.<seq>; the main thread's 0 segment is what it always was), so concurrent threads don't collide; a custom --cpu-prof-name still collides across threads, as node's does. Also fixed on the way: the sampling interval was a thread_local, so workers silently sampled at the 1000 µs default (312 → 1529 samples at --cpu-prof-interval 100), and an interval that fits u32 but not c_int panicked — it clamps.

performance.eventLoopUtilization() (perf_hooks.ts, internal/perf/event_loop_utilization.ts, usockets loop_data.h/epoll_kqueue.c/loop.c, VirtualMachine.rs, web_worker.rs) was hardcoded zeros, and worker.performance.eventLoopUtilization() a stub. The loop already brackets its park, so idle accounting is two clock reads on ticks that were going to sleep anyway; Windows uses libuv's uv_metrics_idle_time (UV_METRICS_IDLE_TIME, which node enables unconditionally). Semantics match node's: the main thread reports zeros until its loop first runs (an entry point's synchronous top level sees {0,0,0}; the time spent in a top-level await counts), a worker counts from before its script starts, and a worker's ELU is readable from the parent. Elapsed and idle come from the same clock (they diverged across sleep on macOS), idle accumulated before the loop is considered started is not charged (bun run holds the stamp back while the entry graph loads, since an import transpiles off-thread and the loop parks for it, and lets it through once the entry root is evaluating, via the existing entry_evaluation_started signal, so a top-level await still counts), and the mid-park counter is published through a seqlock so a reader never sees a torn idle/entry pair — the shared math lives in internal/perf/event_loop_utilization as node's does, including the deliberately unguarded NaN for two identical samples.

Two traps for anyone touching this: us_internal_loop_data_t is us_loop_t's first member and is mirrored in Rust (src/uws_sys/InternalLoopData.rs) — adding a field without the mirror shifts num_polls and the loop silently stops parking; and a counter folded in only when a park ends reads stale mid-park, which libuv solves the same way (publish the park's entry time).

--use-system-ca is a per-thread (per-Environment) option (web_worker.rs, VirtualMachine.rs, Arguments.rs, jsc_hooks.rs, SSLConfig.rs, ssl_config.rs, fetch.rs, usockets openssl.c/root_certs.cpp/quic.c, tls.ts). Previously a Worker's execArgv was ignored for everything but --no-addons, --no-use-system-ca did not exist, and the CA decision was a process-global boolean read by a process-wide cached store. Now:

  • Resolution follows node_worker.cc: a thread starts from its parent's resolved decision, a custom env re-derives it from that env's NODE_USE_SYSTEM_CA, and flags win — its own execArgv's, or the parent's flags when it has none. The main thread's inheritable value is flag-only (the env var is not promoted into a flag). --no-use-system-ca is accepted on the CLI, in execArgv and in NODE_OPTIONS.
  • The decision reaches every context a thread creates: tls/net client and server contexts, the default client context fetch uses, and WebSocket. Default stores are built once per decision and shared; a thread whose decision differs from the process default gets its own default client context (and Bun.serve without TLS options stays plain HTTP in such a thread).
  • The store is main's (tls: embed root CAs as DER and parse them lazily, SIMD base64 for certificate PEM, bump BoringSSL #40862): the bundled roots, OpenSSL's default paths (SSL_CERT_FILE / the hash directory), and NODE_EXTRA_CA_CERTS in every mode, plus the OS store when the thread asked for it (on Linux the OS store is the Rust system loader, which also reads every regular file in SSL_CERT_DIR; on macOS/Windows it is the OS store alone); --use-openssl-ca selects OpenSSL's default lookups instead of the bundled roots and ignores the OS store, as NewRootCertStore does. tls.getCACertificates('default') reports the same rules, per thread.
  • ⚠️ When comparing against node, unset NODE_EXTRA_CA_CERTS — with it set, real node fails the two upstream tests exactly like an unfixed bun.

Known gaps / notes

  • The HTTP/3 client context is a process-lifetime singleton on the HTTP thread and is built with the process default store, so a Worker's own --use-system-ca / --no-use-system-ca does not reach its H3 connections (H1/H2, tls, and WebSocket in the same worker do follow it). Per-thread H3 would need per-connection context selection; same bucket as the SQL and S3 seeds noted in the comments.
  • --use-openssl-ca combined with --use-system-ca is rejected by the CLI, where node accepts the pair (openssl wins); pre-existing.
  • A Worker whose custom env omits NODE_USE_SYSTEM_CA while the process env sets it follows the parent's decision (node re-derives from the custom env → bundled only). Documented corner.
  • hasRef() reads back true after terminate() until the exit is delivered — the ref is held on purpose so 'exit' is observed; node reads back undefined sooner.
  • Terminating a worker while it is still loading its modules can trip a pre-existing ASSERTION FAILED: !scope.exception() || !result (objectConstructorDefineProperty via the worker preload) on debug/asan builds — the same failure the already-vendored test-worker-message-port-transfer-terminate.js shows on x64-asan. test-worker-vm-context-terminate and test-worker-abort-on-uncaught-exception-terminate, which the online fix would otherwise enable, are left out for that reason; the behaviour is covered by a Bun-side test that avoids the window.
  • Native warning output (process.emitWarning, deprecation and unhandled-rejection warnings) inside a Worker still goes to the parent's real fd 2 rather than the worker's piped stderr (node routes it to w.stderr); JS console.* is routed correctly. Rerouting emitWarning through console.error would change main-thread warning formatting, so the fix is routing the native console client at the worker's stdio — left for a separate change. Related: process.listenerCount('warning') is 0 (node 1) and a user 'warning' listener suppresses the default print.
  • Bun's async_hooks implements AsyncLocalStorage/AsyncResource only; MESSAGEPORT resources (test-worker-messageport-hasref, …-inspect-during-init-hook) remain out of scope. Also still missing from the vendored set: execArgv validation (ERR_WORKER_INVALID_EXEC_ARGV), resourceLimits, moveMessagePortToContext, data: URL MIME handling, process.env defineProperty semantics.

Found via the asan shard, not fixed here

  1. terminate() with an in-flight dns.lookup is a use-after-free (Linux only — the lib_c path): GlobalData::drop tears the Resolver down while in-flight lookups still hold an IntrusiveRc. test-worker-dns-terminate.js was removed from this PR rather than suppressed; it needs a teardown fix in dns.rs.
  2. Work landing on a terminating worker after its shutdown drain is a UAF, not just a leak. markTerminating only serializes postTaskTo; the work pool and JSC's DeferredWorkTimer enqueue directly onto concurrent_tasks, the worker thread is detached rather than joined, and the VM box is freed raw — so draining later only narrows the window (tried and discarded; it also frees the node but not the payload). The durable fix is at the poster side (drain-and-join, or a refcount taken by posters). Both sequential/test-worker-fshandles-*-on-termination.js expose it and are left out; the same class covers a lot of test/no-validate-leaksan.txt and the already-failing "cross-thread MessagePort post during worker shutdown" case in worker_threads.test.ts.

Why online was moved the way it was

Moving the whole dispatchOnline (state flip + event) before evaluation regressed six routing tests (enqueueToWorker buffers only while not Running, so postMessage from an 'online' handler drained into a worker with no listener yet); gating delivery on an entry-evaluated flag fixed those but the early Running flip alone still broke heap-profile, terminate-unrefed and two destruction tests. Running is overloaded three ways (online / deliverable / bookkeeping-valid), which is why only the event is posted early here and the flip is untouched. The removed comment claiming a parent online handler calling getHeapSnapshot() would see ERR_WORKER_NOT_RUNNING was wrong — postTaskToWorkerGlobalScope queues on Pending; confirmed by running it.


no test proof · iteration 51 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/tls/test-use-system-ca.test.ts, test/js/node/process/process.test.js, test/cli/run/cpu-prof.test.ts

Ports five Node.js v26.3.0 test/parallel worker tests that are absent from
the suite and pass against current main, byte-identical to upstream:

  test-worker-dns-terminate.js          terminate() with an in-flight dns.lookup
  test-worker-http2-stream-terminate.js terminate() with in-flight http2 streams
  test-worker-memory.js                 RSS does not grow across worker churn
  test-worker-unsupported-eval-on-url.mjs  eval:true rejected for a URL filename
  test-worker-cleanup-handles.js        handles are cleaned up on worker exit

Verified on a debug build of main (3/3 runs each), and again with
BUN_JSC_validateExceptionChecks=1 to match the asan shard. No source
changes: these cover behaviour Bun already implements, so they guard
against regressions rather than fix anything.

Note test-worker-unsupported-eval-on-url.mjs fails on 1.3.14 and passes on
main - the ERR_INVALID_ARG_VALUE message now matches Node exactly.

No-Verification-Needed: test-only change, no runtime surface to drive
@robobun

robobun commented Jul 16, 2026 •

Copy link
Copy Markdown
Collaborator
Updated 12:43 AM PT - Sep 8th, 2026

❌ @robobun, your commit 94cb78e has 1 failures in Build #112488 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 34424

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

bun-34424 --bun

…dy pass

  parallel/test-worker-dispose.mjs                            await using / Symbol.asyncDispose
  sequential/test-worker-fshandles-error-on-termination.js    terminate() with open FileHandles
  sequential/test-worker-fshandles-open-close-on-termination.js

These are the first test-worker-* files under test/js/node/test/sequential/;
discovery is glob-based so no manifest change is needed, and the runner
already sets BUN_FEATURE_FLAG_NO_ORPHANS=1 for that directory.

Verified 5/5 each under the runner's own env (bun run --config
bunfig.node-test.toml, NO_ORPHANS, BUN_JSC_validateExceptionChecks=1) and
on a release binary. Both fshandles tests are self-contained: no ports, no
chdir, no shared files, so they cannot interfere with other sequential tests.
Slowest is ~4.5s on a debug build against a 20s budget.

No-Verification-Needed: test-only change, no runtime surface to drive
@cirospaciari cirospaciari changed the title node:worker_threads: add 5 upstream Node worker tests that already pass node:worker_threads: add 8 upstream Node worker tests that already pass Jul 17, 2026
… from LeakSan

The x64-asan shard surfaced two pre-existing bugs in these new tests:

test-worker-dns-terminate.js hits a heap-use-after-free (READ of size 4,
thread T6) when a worker is terminated with a dns.lookup in flight:
GlobalData::drop tears the Resolver down by value while in-flight DNSLookups
still hold an IntrusiveRc, so the later deref reads the freed RefCount
(Cell<u32>). Dropping the test - this needs a real fix in the DNS teardown,
and it cannot be suppressed: worker VMs are destroyed on exit regardless of
BUN_DESTRUCT_VM_ON_EXIT (VirtualMachine.rs), and ASAN_OPTIONS cannot hide a
UAF. Linux-only; macOS uses lib_info rather than lib_c.

test-worker-fshandles-open-close-on-termination.js is a genuine leak
(ConcurrentTask boxed for a VM being torn down) and is delisted from
LeakSanitizer, which is what no-validate-leaksan.txt actually controls.

No-Verification-Needed: test-only change, no runtime surface to drive
@cirospaciari cirospaciari changed the title node:worker_threads: add 8 upstream Node worker tests that already pass node:worker_threads: add 7 upstream Node worker tests that already pass Jul 17, 2026
The asan shard flagged the sibling too, on a different allocation site: an
in-flight AsyncFSTask<Open> rather than JSC deferred work. Same class -
terminating a worker with work still in flight leaks the ConcurrentTask boxed
for it, because the VM is torn down before the task runs. Both tests pass;
only the exit-time leak check fails.
Comment thread test/no-validate-leaksan.txt Resolved
…g them

Reverts the no-validate-leaksan.txt entries and removes both
sequential/test-worker-fshandles-*-on-termination.js.

Needing a LeakSanitizer exemption means the test is not passing: terminating
a worker with work still in flight leaks the ConcurrentTask boxed for it (an
AsyncFSTask<Open>, or JSC deferred work via DeferredWorkTimer) because the VM
is torn down before the task runs. That is a real leak and should be fixed in
the teardown rather than hidden, so these two tests stay out until it is.

What remains is 5 parallel tests that pass with no exemptions.
@cirospaciari cirospaciari changed the title node:worker_threads: add 7 upstream Node worker tests that already pass node:worker_threads: add 5 upstream Node worker tests that already pass Jul 17, 2026
Node publishes the newly-constructed Worker on the 'worker_threads'
diagnostics channel at the end of the Worker constructor
(lib/internal/worker.js). Bun never did, so dc.subscribe('worker_threads')
was silently dead.

Resolve the channel at module load rather than lazily in the constructor:
diagnostics_channel keys its registry off a Map, so a lazy require would
build the channel out of whatever user code had tampered with by then
(this broke worker_threads.test.ts's tampered-Map-prototype test), and a
module-load strong ref also pins the channel against its WeakRefMap
registry so subscribe() cannot race a GC. http2.ts and _http_client.ts
already require diagnostics_channel at module scope.

Verified against the node v26.3.0 binary: identical payload ({ worker }),
no publish without subscribers, no fire when subscribing after
construction, one publish per Worker in order, unsubscribe stops it, and
a nested worker publishes on its own thread's channel. All 36 node
test-diagnostics-channel-*.js pass; vendored test-worker* is 105/2 vs
104/3 before.
@cirospaciari cirospaciari changed the title node:worker_threads: add 5 upstream Node worker tests that already pass node:worker_threads: publish the worker_threads diagnostics channel + 6 upstream Node tests Jul 17, 2026
…d timeOrigin

Three independent Node compat fixes, each with its upstream v26.3.0 test.
All three verified against the node v26.3.0 binary.

async_hooks WORKER init (test-worker-hasref.js)
  Bun delivered `init` for TickObject only, so a hook watching for WORKER
  resources never fired. Emit one from the Worker constructor into the same
  tickInitHooks array, exposing hasRef(): it follows ref()/unref() and reads
  back undefined once the thread has exited, as node's handle does. ref() and
  unref() no-op after exit rather than resurrecting it (lib/internal/worker.js
  nulls kHandle before emitting 'exit'). A throwing init hook is fatal, as in
  node, mirroring the TickObject site in ProcessObjectInternals.

  createHook also invoked `init` bare, so `this` was undefined inside it. Node
  calls init as a method on the AsyncHook instance
  (lib/internal/async_hooks.js), which is why `this.disable()` inside init
  works there and threw here.

Worker error clone (test-worker-error-stack-getter-throws.js)
  Cloning an Error reads `stack`, so a throwing Error.prepareStackTrace took
  the whole clone down and the error surfaced as Bun's pretty-printed text.
  Node drops just the unreadable stack (lib/internal/error_serdes.js
  TryGetAllProperties). Retry once with an own undefined stack, only on the
  failure path, only for an ErrorInstance, using
  setStackPropertyAlreadyMaterialized so the retry does not re-run the getter.

performance.timeOrigin (test-perf-hooks-worker-timeorigin.js)
  Each worker VM called Instant::now(), so a worker's timeOrigin drifted from
  the main thread's by the spawn delay (~265ms with a forced gap; node: 0.000).
  timeOrigin is the process start and every thread reports the same one, so
  capture it once. This also makes Bun.nanoseconds() match its own documented
  "nanoseconds since the process started".

Vendored test-worker* goes 105/2 -> 113/3 (the 3 are pre-existing and
unrelated); bun's worker_threads suite stays at its 2 baseline failures;
async_hooks and perf_hooks unchanged.
@cirospaciari cirospaciari changed the title node:worker_threads: publish the worker_threads diagnostics channel + 6 upstream Node tests node:worker_threads: 4 compat fixes (async_hooks WORKER, error clone, timeOrigin, diagnostics channel) + 9 upstream tests Jul 17, 2026
Node's BroadcastChannel#ref() returns `this` so it chains
(lib/internal/worker/io.js); Bun's returned undefined while unref() already
returned the channel, so the two disagreed in the same file.

This is the other half of #19810, which fixed the identical return-value bug in
unrefBody and left ref() behind. jsRef()/jsUnref() are mirrors — both void, both
guarding m_hasRef around an event-loop ref count — so refBody now matches
unrefBody exactly.

Verified against node v26.3.0: `bc.ref() === bc` and `bc.unref() === bc` on both,
and util.inspect(bc.ref()) is byte-identical. test/js/web/broadcastchannel 16/0.
@cirospaciari

Copy link
Copy Markdown
Member Author

@robobun adopt

@cirospaciari

Copy link
Copy Markdown
Member Author

@robobun fix conflicts

@cirospaciari
cirospaciari marked this pull request as ready for review July 17, 2026 18:24
@robobun

robobun commented Jul 17, 2026 •

Copy link
Copy Markdown
Collaborator

This branch last merged main at 94cb78e on Sep 8. It now conflicts with main in 22 files (660 commits behind), so it needs a merge before a review. Main has since taken one piece of it (#41383, online before the entry point). The other changes and the ten upstream tests are not on main.

The question for Ciro about the default root store is still open (#34424 (comment)). Comment here when you want the branch brought up to date with main, and I will do it.

Node reports 'online' once the worker thread has bootstrapped, before user
code (lib/internal/worker.js). Bun posted it only after the entry-point
promise settled, so a worker whose top-level never returns never reported
online at all:

    new Worker('while(true);', { eval: true })
    node: ONLINE fired | terminate() -> 1
    bun : no online, ever

dispatchOnline did two separable things: the Pending->Running flip under
m_pendingTasksMutex, which also gates message routing, and a postTaskToParent
of the open event. Only the second belongs before the entry point, so split
them: dispatchOnlineEvent() posts the event and is called ahead of the load;
dispatchOnline() keeps the state flip exactly where it was, leaving message
routing and fireEarlyMessages untouched. Moving both regresses the suite.

Removes a comment claiming the flip must precede the post or a parent 'online'
handler calling getHeapSnapshot() would see ERR_WORKER_NOT_RUNNING. It cannot:
postTaskToWorkerGlobalScope queues on Pending and returns true, rejecting only
for Closing/Closed, as JSWorker.cpp already documents. Verified by driving it —
getHeapSnapshot() from inside the online handler resolves.

Also fixes online-before-error: a worker whose entry throws now reports
["online","error"] like node, where it previously reported only ["error"].

Verified against node v26.3.0: spinning worker goes online and terminate()
resolves to 1; online fires exactly once; ordering vs a worker message and vs a
throwing entry both match. Failure set of the worker suite is unchanged
(strict subset of baseline); the new test fails 0/3 without the fix, 3/3 with.

Known gap left: a worker with an unresolvable specifier still skips 'online'
(node fires it) — the event goes out after entry resolution.
@coderabbitai

coderabbitai Bot commented Jul 17, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Changes

Worker runtime compatibility

Layer / File(s) Summary
Async hooks and worker state
src/js/node/async_hooks.ts, src/js/node/worker_threads.ts, src/js/internal/async_hooks_tick.ts, test/js/node/test/parallel/test-diagnostics-channel-worker-threads.js, test/js/node/test/parallel/test-worker-hasref.js
WORKER init hooks, diagnostics-channel publication, and hasRef() lifecycle tracking are implemented and tested.
Shared timing origin
src/jsc/VirtualMachine.rs, test/js/node/test/parallel/test-perf-hooks-worker-timeorigin.js
Virtual machines use a process-wide timing origin, with matching worker and parent time-origin coverage.
Online-event ordering
src/jsc/bindings/webcore/Worker.h, src/jsc/bindings/webcore/Worker.cpp, src/jsc/web_worker.rs, test/js/node/worker_threads/worker_threads.test.ts
The worker emits online before loading its entry point while retaining the later running-state transition.
Error and lifecycle handling
src/jsc/bindings/webcore/Worker.cpp, test/js/node/test/parallel/test-worker-error-stack-getter-throws.js, test/js/node/test/parallel/test-worker-cleanup-handles.js, test/js/node/test/parallel/test-worker-dispose.mjs, test/js/node/test/parallel/test-worker-http2-stream-terminate.js, test/js/node/test/parallel/test-worker-unsupported-eval-on-url.mjs
Error serialization retries after clearing an invalid stack, and worker cleanup, disposal, termination, and eval validation are covered.

BroadcastChannel reference chaining

Layer / File(s) Summary
Chained ref operation
src/jsc/bindings/webcore/JSBroadcastChannel.cpp
BroadcastChannel.prototype.ref() returns the wrapper after incrementing its reference count.

Node CLI short aliases

Layer / File(s) Summary
Short-alias parsing contract
src/clap/lib.rs, src/clap/streaming.rs, src/clap/comptime.rs
Clap carries configured short aliases and rewrites matching tokens before flag classification.
Node alias wiring
src/runtime/cli/Arguments.rs, src/runtime/node/node_process.rs, src/install/PackageManager/CommandLineArguments.rs
The -pe alias maps to -p for auto and run-as-node parsing, with default parser options applied elsewhere.

Worker CPU profiling

Layer / File(s) Summary
Worker exec-argv contract
src/jsc/VirtualMachine.rs, src/runtime/jsc_hooks.rs
Worker exec-argv parsing returns addon and CPU profiling options through a structured hook result.
Profiler inheritance and filenames
src/jsc/BunCPUProfiler.rs, src/runtime/cli/run_command.rs
Profiler configuration is inherited by workers, includes thread identifiers, clamps intervals, and produces worker-specific filenames.
Worker profiler integration
src/jsc/web_worker.rs, test/js/node/test/sequential/test-cpu-prof-dir-worker.js, test/js/node/test/sequential/test-cpu-prof-worker-argv.js
Worker startup applies profiling options and regression tests validate generated profiles.

Possibly related PRs

Suggested reviewers: robobun

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately summarizes the worker_threads compatibility changes and added tests. It is longer and more detailed than recommended, but it remains specific and related to the primary changes.
Description check ✅ Passed The description provides a detailed summary of the changes, verification context, known gaps, and test coverage. It does not use the template headings exactly, but it contains the required information…

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

@cirospaciari

Copy link
Copy Markdown
Member Author

@robobun heads-up — I resolved the conflicts locally, so you can drop your run to avoid a race. Your resolution started from c308af4b6b, but I pushed f1384cda99 (the 'online' timing fix) right after, so a merge built on the older head would drop it.

What conflicted and how I resolved it, in case it's useful: only src/js/node/worker_threads.ts, three hunks, all where #34338 and my changes touch the same lines. #34338 removed the #stdoutAutoPipe/#stderrAutoPipe fields and dropped the stdio port ref/unref out of ref()/unref() (ports now manage their own ref via incrementsPortRef). My side only ever added #hasRef bookkeeping there, so I took main's structure wholesale and kept just the two if (!this.#exited) this.#hasRef = ... lines plus the field. async_hooks.ts (#31825) and VirtualMachine.rs (#34293, #32498) auto-merged.

I verified the merge is a pure addition on top of main — git diff origin/main -- src/js/node/worker_threads.ts shows zero deleted lines, so nothing from #34338 or #31825 is reverted.

Also: #34338 fixed the {stdout:true} hang I'd documented in this PR's description as an open bug, so I'm dropping that section. Worth noting your fix got the part mine missed — I tried attaching the listener eagerly and it regressed ~20 tests because the listener re-refs the port; port.unref() immediately after the eager attach is the piece I didn't have.

Comment thread src/js/node/worker_threads.ts Resolved
Comment thread src/js/node/worker_threads.ts Resolved
Comment thread src/js/node/async_hooks.ts Resolved
Only src/js/node/worker_threads.ts conflicted, in three hunks where #34338
("don't hang when captured stdout/stderr is never consumed") and this branch
touch the same lines. #34338 removed the #stdoutAutoPipe/#stderrAutoPipe fields
and moved the stdio port ref/unref out of ref()/unref() — ports now manage their
own ref via makePortReadable's incrementsPortRef. This branch only added #hasRef
bookkeeping there, so main's structure is taken wholesale and only the two
`if (!this.#exited) this.#hasRef = ...` lines and the field are kept.

async_hooks.ts (#31825) and VirtualMachine.rs (#34293, #32498) auto-merged.

`git diff origin/main -- src/js/node/worker_threads.ts` is a pure addition:
zero deleted lines, so nothing from #34338 or #31825 is reverted.

Verified on the merge result: test-worker-hasref, test-worker-error-stack-
getter-throws, test-perf-hooks-worker-timeorigin, test-diagnostics-channel-
worker-threads and the new "online fires before the entry point finishes" all
pass; #34338's own repro still exits 0 like node; BroadcastChannel ref()/unref()
and the 'online' timing fix both still match node v26.3.0.
@cirospaciari cirospaciari changed the title node:worker_threads: 4 compat fixes (async_hooks WORKER, error clone, timeOrigin, diagnostics channel) + 9 upstream tests node:worker_threads: 6 compat fixes (online timing, async_hooks WORKER, error clone, timeOrigin, diagnostics channel, BroadcastChannel ref) + 9 upstream tests Jul 17, 2026
- drop test-worker-memory.js: RSS ratio assertion fails on macOS aarch64
  (builds 74530 and 74665), same bar as the fshandles tests
- emit the WORKER async_hooks init before the diagnostics_channel publish
  so the observable order matches node (AsyncWrap fires mid-constructor)
- hasRef() starts false when Bun's { ref: false } option is passed
- update the stale async_hooks_tick header comment (WORKER now flows there)
- test that BroadcastChannel ref()/unref() return the channel

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All five earlier findings are addressed in d7e1659 (test-worker-memory dropped, init/dc order swapped, #hasRef seeded from options.ref, async_hooks_tick header updated, BroadcastChannel ref/unref return test added) and I found nothing new. Deferring to a maintainer because the dispatchOnlineEvent/dispatchOnline split changes the Worker state-machine timing across threads, and the process_origin() OnceLock makes origin_timer process-global — both are the kind of lifecycle change worth a human sign-off, and build #74680 is still running.

What was reviewed:

  • dispatchOnlineEvent split: confirmed it only posts the open event; the Pending→Running flip and fireEarlyMessages stay where they were, so message routing/queueing is unchanged.
  • Error-clone retry in dispatchErrorWithValue: gated on !serialized && !scope.exception() + dynamicDowncast<ErrorInstance>, uses putDirect + setStackPropertyAlreadyMaterialized() so the retry can't re-enter prepareStackTrace; code read below still runs.
  • process_origin(): OnceLock<(Instant, u64)> is thread-safe; first VM (main thread) initialises it before any worker VM is created.
  • #emitAsyncHooksInit: snapshot-before-iterate matches node's tmp_array; throwing hook → process.exit(1) matches the existing TickObject site.
Extended reasoning...

Overview

Six Node-compat behaviour fixes across the worker stack plus eight vendored upstream tests and two Bun-side tests. Source changes: src/js/node/worker_threads.ts (diagnostics_channel publish, async_hooks WORKER init, #hasRef bookkeeping), src/js/node/async_hooks.ts (init hook this binding), src/js/internal/async_hooks_tick.ts (comment), src/jsc/VirtualMachine.rs (process-global origin_timer/origin_timestamp via OnceLock), src/jsc/bindings/webcore/Worker.{cpp,h} (split dispatchOnlineEvent from dispatchOnline, error-clone retry on throwing prepareStackTrace), src/jsc/web_worker.rs (call WebWorker__dispatchOnlineEvent before entry-point load), src/jsc/bindings/webcore/JSBroadcastChannel.cpp (ref() returns this).

Security risks

None identified. No auth/crypto/permissions surface. The error-clone retry mutates a worker-thread ErrorInstance whose thread is terminating; the diagnostics_channel is resolved at module load so it isn't built from tampered primordials (which the PR calls out and which the existing tampered-Map test would have caught).

Level of scrutiny

High. The dispatchOnlineEvent split touches the Worker state machine documented in Worker.h — the PR description records two rejected attempts that regressed the suite, and the final shape moves only the event post while leaving the Pending→Running flip and fireEarlyMessages in place. That reasoning checks out against the code (postTaskToWorkerGlobalScope queues on Pending, so a parent's online→getHeapSnapshot() still works), but it is a deliberate change to cross-thread lifecycle timing and a removed comment previously defended the opposite ordering. The process_origin() change makes per-VM state process-global, which is correct per Node/WHATWG but changes the meaning of origin_timer for every worker VM.

Other factors

All five of my previous inline findings were addressed in d7e1659 and the threads are resolved. The reviewer who set the bar for this PR ("we should not add any tests that dont pass here") has adopted it and resolved the merge; build #74680 on the latest commit is still in progress, so CI status on the final shape (particularly the online-timing change across all shards) isn't confirmed yet. Given the scope — C++/Rust worker lifecycle across threads — and the active maintainer engagement, this should get a human approval rather than a bot one.

`bun -pe "1+1"` printed a ReferenceError for `e`. `-p` is declared
`-p, --print <STR>` and the short parser accepts attached values, so `-pe X`
read as `-p` carrying the value `e` and evaluated that identifier. Node has the
same ambiguity and resolves it the same way: `-pe` is not a short at all, it's a
whole-token alias applied before short parsing (AddAlias, node_options.cc).

Adds an alias table to ParseOptions, applied in StreamingClap::parse_next_arg —
the one place a token is classified as a flag. Option values and `--` targets
are pulled straight off the iterator and never pass through it, so they stay
verbatim; node scopes its own lookup to the option-name branch for the same
reason. Only AutoCommand/RunAsNodeCommand pass the node table: `bun run -pe` and
`bunx -p` (where -p means --package) are unaffected.

process.execArgv re-parses argv against a set built from AUTO_PARAMS to find
value-taking flags. An alias is not a param, so `-pe` missed the set and the
code string was dropped from execArgv; the set now also takes any alias whose
target takes a value, derived from the same table rather than hardcoded.

Verified against node v26.3.0, matching byte-for-byte:
  bun -pe '1+1'                       -> 2
  bun -p '1+1' / -e 'console.log(3)'  -> 2 / 3       (unchanged)
  bun -- -pe                          -> Script not found "-pe"  (not "-p")
  bun -e -pe                          -> evaluates "-pe"         (value intact)
  bun script.js -pe x                 -> ["-pe","x"]             (argv intact)
  bun -pe 'process.execArgv'          -> ["-pe","..."]
test-preload-worker.js goes 0/6 -> 6/6; it needed only this.

Not addressed: `-pe=x`. Node rejects `-pe=3+3` and `-p=1+1` alike ('=' splitting
is long-flags-only there); bun evaluates them. That divergence predates this and
is orthogonal.
@cirospaciari cirospaciari changed the title node:worker_threads: 6 compat fixes (online timing, async_hooks WORKER, error clone, timeOrigin, diagnostics channel, BroadcastChannel ref) + 9 upstream tests node:worker_threads: 7 compat fixes (online timing, async_hooks WORKER, error clone, timeOrigin, diagnostics channel, BroadcastChannel ref, -pe) + 10 upstream tests Jul 17, 2026
@robobun

robobun commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Merged main (23 commits) in 3b0831e. Three files conflicted, all against #40592 (--no-ffi-cc), which lands on the worker execArgv parsing this branch reshaped. Summarizing since it is a semantic merge, not a textual one:

  • Main added --no-ffi-cc to its small WorkerExecArgvFlags { allow_addons, allow_ffi_cc } and a parse_worker_exec_argv_flags hook. This branch already replaced that hook with parse_worker_exec_argv, which returns WorkerExecArgv (addons, the per-thread CA flag, the cpu-prof options) and scans the whole execArgv. The resolution keeps this branch's struct and hook and folds main's flag into them: WorkerExecArgv.allow_ffi_cc: Option<bool>, set like allow_addons (Some(!seen) when the worker has its own execArgv), and web_worker.rs ANDs both with the parent's values, so a Worker cannot re-enable what its parent disabled. Main's struct and hook are gone; VirtualMachine::allow_ffi_cc() and the rest of Add --no-ffi-cc and make --no-addons disable bun:ffi cc() #40592 are unchanged.
  • cc.test.ts's new "disabling cc()" block passes here, including the three Worker cases (--no-ffi-cc inherited into a Worker with an empty execArgv, a Worker disabling cc() for itself through execArgv, a Worker not re-enabling it).

Also checked on the merged debug build: no-addons.test.ts, worker_threads.test.ts, perf_hooks, test-use-system-ca, cpu-prof.test.ts, worker.test.ts and broadcast-channel.test.ts (the one failure is the #37075 flood timing test), 53 vendored node files, rust:clippy and cargo fmt --check.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review found no issues

No high-confidence issues detected in this change.

Jarred-Sumner pushed a commit that referenced this pull request Sep 4, 2026
### Problem
- `@discordjs/ws` `WorkerShardingStrategy` hangs on Bun. It does `await
once(worker, "online")` and then attaches its `message` listener. Its
worker posts `{op: 7}` from top-level code. In Bun that message reaches
the parent before `online`, so no listener sees it and
`gateway.connect()` never settles. Node always delivers `online` first.
- The cause is in `WorkerMessagingProxy::workerGlobalScopeStarted`
(`src/jsc/bindings/webcore/WorkerMessagingProxy.cpp:395`). Bun posted
the `open` event (the `online` emit in `worker_threads.ts`) after the
entry had evaluated. A `parentPort.postMessage()` from the entry's top
level was already queued to the parent by then.

### Fix
- Add `WorkerMessagingProxy::workerThreadStarted()`. It posts the `open`
event and nothing else. `spin()` calls it before it resolves and loads
the entry point, which is where Node's bootstrap sends `UP_AND_RUNNING`.
It skips the post when the state is no longer `Pending` (a parent that
exits during worker startup moves it to `Closing`).
- The `Pending -> Running` transition stays in
`workerGlobalScopeStarted()`, after the entry has evaluated. Messages
and tasks the parent posts still wait for the entry graph, so
`postMessage()` or `getHeapSnapshot()` from an `online` handler is
queued and delivered as before.
- A worker whose entry does not resolve now reports `online` then
`error`, as Node does. The same ordering applies to the web `Worker`
`open` event.
- Verified: `test/js/node/worker_threads/worker_threads.test.ts` (3 new
tests) and `test/js/web/workers/worker.test.ts` (2 new tests). All 5
fail on 1.4.1. Also ran the rest of `test/js/web/workers/`,
`test/js/node/worker_threads/`, and the vendored `test-worker-*.js` Node
tests.

### Background
- `WorkerMessagingProxy` is the parent/worker relationship object. Its
`m_state` goes `Pending -> Running -> Closing -> Closed`. While
`Pending`, messages and tasks from the parent are buffered. `Running`
means the worker's entry has evaluated and they are routed directly.
- `parentPort.postMessage()` in a node worker travels over a
`MessageChannel` port pair that the `Worker` constructor creates
(`src/js/node/worker_threads.ts:898`). That path does not go through the
proxy, so it is ordered against the `open` task only by when each is
posted to the parent's loop.
- Prior work: #34424 (cirospaciari) moves the event the same way as part
of a much larger change. #32908 is an older attempt against code that no
longer exists. This PR is the narrow fix for #41375.
- Self-reviewed: 2 concerns raised, 2 addressed (the `Closing` guard and
the web `Worker` tests).

<details><summary>Notes</summary>

Minimal repro (from the issue, reduced):

```js
// worker.js
require("node:worker_threads").parentPort.postMessage("ready");
// main.mjs
const w = new Worker(new URL("./worker.js", import.meta.url));
w.on("online", () => order.push("online"));
w.on("message", m => order.push("message:" + m));
```

Bun 1.4.1: `message:ready, online`. Node 26: `online, message:ready`.
With this change Bun matches Node, and the full `@discordjs/ws` repro
(stub REST, `WorkerShardingStrategy({ shardsPerWorker: 4 })`) gets past
worker setup and the shards start connecting.

Three tests in `test/js/web/workers/` fail on this container on main and
on this branch alike (`a message flood from a worker does not starve the
parent's event loop`, `preload with an un-awaited import() does not open
message delivery before the entry runs`, `terminate() while dns.lookup()
is in flight`). They are timing limits of the debug/ASAN build here, not
related to this change.

`isOnline()` on the proxy still means `Running`. It has no callers.

Fixes #41375
</details>

<!-- robobun:evidence:begin -->

---

**[human-review]** gate passed · iteration 1 · 7 files touched

<details><summary>fails on main (without fix)</summary>

```console
ASAN without fix: BUILD FAILED (no junit output)
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/worker_threads/worker_threads.test.ts test/js/web/workers/worker.test.ts
ninja: Entering directory `/workspace/bun/build/debug'
[1/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-0.cpp.o
[2/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-5.cpp.o
[3/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-10.cpp.o
[4/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-2.cpp.o
[5/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-4.cpp.o
[6/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-7.cpp.o
[7/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-8.cpp.o
[8/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-1.cpp.o
[9/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-6.cpp.o
[10/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-11.cpp.o
[11/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-15.cpp.o
[12/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-18.cpp.o
[13/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-14.cpp.o
[14/123] cxx obj/unified/UnifiedSource-src_jsc_bindings-16.cpp.o
[15/123] cxx obj/u
... (truncated)

release without fix: all passed
bun test v1.4.1-canary.1 (0481e34)

test/js/web/workers/worker.test.ts:
(pass) web worker > preload > invalid file URL [0.20ms]
(pass) web worker > preload > string [2.32ms]
(pass) web worker > preload > array of 2 strings [2.52ms]
(pass) web worker > preload > array of string [1.82ms]
(pass) web worker > preload > error in preload doesn't crash parent [1.72ms]
(pass) web worker > worker [1.55ms]
(pass) web worker > worker-env [1.59ms]
(pass) web worker > worker-env: SHARE_ENV via the global Worker constructor [12.42ms]
(pass) web worker > worker-env: process.env reads inside a worker reflect the worker's own environment at runtime [8.36ms]
(pass) web worker > worker-env with a lot of properties [3.64ms]
(pass) web worker > argv / execArgv defaults [5.21ms]
(pass) web worker > argv / execArgv options [5.07ms]
(pass) web worker > sending 50 messages should just work [2.23ms]
(pass) web worker > worker with event listeners doesn't close event loop [8.79ms]
(pass) web worker > worker with event listeners doesn't close event loop 2 [7.89ms]
(pass) web worker > worker with process.exit [11.97ms]
(pass) web worker > worker event > is fired with the right object [0.15ms]
... (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/node/worker_threads/worker_threads.test.ts test/js/web/workers/worker.test.ts
bun test v1.4.1 (e0a2b82)

test/js/web/workers/worker.test.ts:
(pass) web worker > preload > invalid file URL [32.06ms]
(pass) web worker > preload > string [116.09ms]
(pass) web worker > preload > array of 2 strings [114.82ms]
(pass) web worker > preload > array of string [110.35ms]
(pass) web worker > preload > error in preload doesn't crash parent [106.56ms]
(pass) web worker > worker [105.83ms]
(pass) web worker > worker-env [109.61ms]
(pass) web worker > worker-env: SHARE_ENV via the global Worker constructor [651.05ms]
(pass) web worker > worker-env: process.env reads inside a worker reflect the worker's own environment at runtime [414.72ms]
(pass) web worker > worker-env with a lot of properties [289.74ms]
(pass) web worker > argv / execArgv defaults [371.01ms]
(pass) web worker > argv / execArgv options [376.89ms]
(pass) web worker > sending 50 messages should just work [133.43ms]
(pass) web worker > worker with event listeners doesn't close 
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 571ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/90] cxx obj/unified/UnifiedSource-src_jsc_bindings_node_http-0.cpp.o
[2/90] cxx obj/unified/UnifiedSource-src_jsc_bindings-5.cpp.o
[3/90] cxx obj/unified/UnifiedSource-src_jsc_bindings_node-0.cpp.o
[4/90] cxx obj/unified/UnifiedSource-src_jsc_bindings_node_crypto-0.cpp.o
[5/90] cxx obj/unified/UnifiedSource-src_jsc_bindings_node_crypto-1.cpp.o
[6/90] cxx obj/unified/UnifiedSource-src_jsc_bindings-0.cpp.o
[7/90] cxx obj/unified/UnifiedSource-src_jsc_bindings-1.cpp.o
[8/90] cxx obj/unified/UnifiedSource-src_jsc_bindings-2.cpp.o
[9/90] cxx obj/unified/UnifiedSource-src_jsc_bindings-3.cpp.o
[10/90] cxx obj/unified/UnifiedSource-src_jsc_bindings-4.cpp.o
[11/90] cxx obj/unified/UnifiedSource-src_jsc_bindings_webcore-1.cpp.o
[12/90] cxx obj/unified/UnifiedSource-src_jsc_bindings_webcrypto-0.cpp.o
[13/90] cxx obj/unified/UnifiedSource-src_jsc_bindings_webcore-2.cpp.o
[14/90] cxx obj/unified/UnifiedSource-src_jsc_bindings_webcore-3.cpp.o
[15/90] cxx obj/unified/UnifiedSource-src_jsc_bindings_webcrypto-1.cpp.o
[1
... (truncated)
```

</details>

<details><summary>diff hotspot</summary>

```
src/jsc/bindings/webcore/Worker.cpp                |  5 ++
 src/jsc/bindings/webcore/WorkerMessagingProxy.cpp  | 26 ++++---
 src/jsc/bindings/webcore/WorkerMessagingProxy.h    |  3 +
 src/jsc/web_worker.rs                              | 13 ++--
 test/js/node/test/parallel/test-sqlite-timeout.js  |  2 +-
 test/js/node/worker_threads/worker_threads.test.ts | 42 ++++++++++-
 test/js/web/workers/worker.test.ts                 | 85 ++++++++++++++++------
 7 files changed, 133 insertions(+), 43 deletions(-)
```

</details>

**gate history** · 1 passed · 1 rejected · iteration 1

<details><summary>evidence per changed file</summary>

```
file                                                reads  edits  tests
src/jsc/bindings/webcore/Worker.cpp                     1      2     22
src/jsc/bindings/webcore/WorkerMessagingProxy.cpp       3      6     22
src/jsc/bindings/webcore/WorkerMessagingProxy.h         2      3     22
src/jsc/web_worker.rs                                   3      8     22
test/js/node/test/parallel/test-sqlite-timeout.js       0      0     26
test/js/node/worker_threads/worker_threads.test.ts      1      4     13
test/js/web/workers/worker.test.ts                      3      4     12
```

</details>

<!-- robobun:evidence:end -->
@robobun

robobun commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Merged main (301 commits) in 94cb78e. Seven files conflicted; two of the conflicts are semantic, so summarizing for Ciro:

#41383 put this branch's online-before-entry change on main (it cites this PR). The resolution takes main's shape: workerThreadStarted() posts open while the state stays Pending, so the branch's extra State::Started, the wider isOnline() and the extra switch arm are gone, and the WorkerMessagingProxy / web_worker.rs delta against main is now only hasRef, ELU and the error.code carrier. The branch's two 'open' event order tests in worker.test.ts duplicated main's new open event block and are dropped; the one case main lacks (open fires for an entry that never returns) is folded into that block. online fires before the entry point finishes in worker_threads.test.ts stays, it blocks the entry on an Atomics wait and is distinct from main's three.

#40862 / #40825 rebuilt the root store (lazy DER sets, the Linux system loader in Rust) and decided that the default store trusts OpenSSL's default paths ($SSL_CERT_FILE, the hash directory) in every mode, pinned by tests that also pass on 1.4.1. That supersedes the efc5781 decision in this thread (flagless rejects SSL_CERT_FILE); please push back if you want that back, but it would now mean reverting main's pinned behaviour. The resolution keeps main's builder and changes two things in it: the system store joins per the decision of the thread the store is built for (the use_system_ca parameter, us_resolve_use_system_ca) instead of the process flag, and --use-openssl-ca still selects the default lookups instead of bundled plus system, as node's NewRootCertStore does. The two-slot shared store, us_is_shared_default_ca_store, the SSL_CTX ex_data stamp and the per-thread default client context are unchanged. get_peer_certificate's fallback takes main's us_ssl_ctx_has_user_ca test with the per-thread store. Since SSL_CERT_FILE is now in every default store, the hermetic Linux tests cannot use it as a system-only root any more: they now put the CA under a non-hash file name in SSL_CERT_DIR, which the Linux system loader reads and the default store's hashed-directory lookup does not. The --use-openssl-ca row of the connection matrix flips to rejected for the same reason. The description's store bullet is updated.

The seventh file, uws/lib.rs, only needed the us_ssl_ctx_use_system_ca extern to name crate::SslCtx after main dropped SSL_CTX from the local import list.

Checked on the merged debug build: test-use-system-ca (26 tests, main's new default-path and system-loader blocks included), worker_threads.test.ts, perf_hooks, worker.test.ts, fetch.tls, node-tls-cert (285 pass), 65 vendored node files (the CA and worker sets), rust:clippy, cargo fmt --check, prettier and clang-format.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review found no issues

No high-confidence issues detected in this change.

steipete added a commit to openclaw/bun that referenced this pull request Sep 14, 2026
### What does this PR do?

Integrates the 19 captured upstream compatibility PRs into the OpenClaw Bun fork, retaining their original commits as merge parents. The base is upstream `86771d09fd486a7256790d6f36602b683f7a19de`. This integration is separate from upstream PR review and does not publish a Bun release.

The two stacked PRs also bring their prerequisites: [worker support oven-sh#34424](oven-sh#34424) and [file-URL query handling oven-sh#35601](oven-sh#35601).

| Upstream PR | Captured head |
| --- | --- |
| [42349: fix(sqlite): allow workers to reuse custom library](oven-sh#42349) | `65924882863e` |
| [42374: fix(node:fs): preserve POSIX locks in realpath](oven-sh#42374) | `4df5e0600308` |
| [42446: fix(node:fs): preserve child rm permission errors](oven-sh#42446) | `17d1237bcbac` |
| [42469: fix(runtime): preserve encoded file URL path delimiters](oven-sh#42469) | `cc5b9fb06de9` |
| [42576: fix(node:https): support live secure context updates](oven-sh#42576) | `b8666fde28e6` |
| [42593: fix(worker_threads): preserve async context for worker events](oven-sh#42593) | `f72285db962b` |
| [42594: fix(node:https): wrap injected raw connections with TLS](oven-sh#42594) | `82a9d26cf2cc` |
| [42599: fix(node:os): observe runtime HOME changes](oven-sh#42599) | `772e4acb9263` |
| [42600: fix(worker_threads): preserve cloned error metadata](oven-sh#42600) | `7254eaec568c` |
| [42601: fix(node:path): honor replaced process.cwd](oven-sh#42601) | `e040ec4cf1c0` |
| [42607: fix(process): allow clearing exitCode](oven-sh#42607) | `bacfa9ee3cb3` |
| [42610: fix(node:http): uncork reused upgrade sockets](oven-sh#42610) | `33f89359c50a` |
| [42614: fix(node): resolve listen hosts before binding](oven-sh#42614) | `5b9ab5644122` |
| [42616: fix(node:module): synchronize builtin ESM exports](oven-sh#42616) | `aa78523549c1` |
| [42620: fix(worker_threads): apply execArgv preloads](oven-sh#42620) | `60fbb60c9a16` |
| [42621: fix(node:async_hooks): report timer lifecycles](oven-sh#42621) | `6e044db91d6b` |
| [42622: fix(node:http): align shutdown transport lifecycle](oven-sh#42622) | `98d5f813e8fe` |
| [42635: fix(node:fs): preserve Win32 semantics in recursive mkdir checks](oven-sh#42635) | `891eb8df52f3` |
| [42636: fix(runtime): derive data URL loaders from MIME](oven-sh#42636) | `4570e105f422` |

Integration repairs preserve newer upstream loop-init error handling, use current Rust loader/string-view interfaces, coordinate WORKER init hook mutations with timer/nextTick dispatch, apply TLS context updates made during pending listen, retain draining native listeners for force-close, and preserve literal filename delimiters across ESM/CommonJS resolution and lookup paths. Superseded C++ CommonJS key reconstruction is removed in favor of the shared resolver owner.

### How did you verify your code works?

- Fresh optimized macOS arm64 build: 1,687 passed, 37 existing skips, one existing todo, zero failures across the 22 selected suites, including standalone compilation.
- Debug/ASAN build and focused integration regressions passed. Its earlier full run passed 1,681 tests but hit an inherited standalone-compilation fixture limitation: the large debug template exceeded that test budget, and relocated output needs its ASAN sidecar. The optimized run covers that production flow; no sanitizer setting, test timeout, or skip was weakened.
- Ten directly affected vendored Node conformance files passed with retries disabled.
- All twelve Rust targets passed: zero failed and zero skipped. These are compilation checks, not native execution claims for every target.
- Oxlint, root TypeScript, Rust formatting, and `git diff --check` passed.
- Independent review is clean through P2. Confirmed integration regressions were repaired; an empty-query/fragment review claim was rejected using actual Node 26.8.2 behavior and protected by a regression.
- Repeated recursive-directory testing keeps its 200 optimized-build iterations and descriptor-leak checks, with a fixed nested fixture instead of scanning the growing source tree.

### Final CI corrections

The follow-up removes MIME decoding and response cork adapters whose last callers were replaced by the integrated PRs, documents raw-slice ownership immediately above the unsafe operations, and sorts HTTP exports. Workspace Clippy and formatting pass locally. The final debug/ASAN check passes 392 tests across the data-URL, worker-thread, and HTTP suites, with one existing skip and no failures. Independent review of this follow-up is clean through P2.

The first CI run also exposed two fork-service limitations: the issue-linking bot has no Anthropic credentials, and autofix.ci cannot push formatter changes without its GitHub App. The formatting change was applied locally.

Mordant's advisory `unchecked_construction` warning points to the existing server reload assignment of `user_routes_to_build`. That assignment moves fields from `new_config`, which `on_reload` obtains through `ServerConfig::from_js` before calling `on_reload_from_zig`; the integrated TLS setter also parses its replacement through `SSLConfig::from_js`. This is not an unchecked user-input path. Its baseline and enforcement were left intact; the three unused-helper findings were repaired.

The final optimized macOS arm64 build passes all four affected suites: **433 passed, one existing skip, zero failures** in 9.11 seconds, including standalone compilation. This supplements the initial 22-suite run (1,687 passed), ten vendored Node conformance files, and twelve Rust compilation targets. The final cleanup also passes **392 debug/ASAN tests** and workspace Clippy.

The reload validation path discussed above is visible at [ServerConfig::from_js before reload](https://github.com/openclaw/bun/blob/597b78c2c4b6a0e15b4b1724ab0e5ebff80f5678/src/runtime/server/server_body.rs#L2262), while [the flagged assignment](https://github.com/openclaw/bun/blob/597b78c2c4b6a0e15b4b1724ab0e5ebff80f5678/src/runtime/server/server_body.rs#L2208) transfers that parsed configuration.

Final hosted validation on `597b78c2c4b6a0e15b4b1724ab0e5ebff80f5678`: formatting, JavaScript/source lint, TypeScript types, package tests, Clippy, Miri, and lol-html tests passed. The [Rust workflow](https://github.com/openclaw/bun/actions/runs/34808804630) succeeded; its advisory Mordant job retains only the documented reload-validation false positive.
@robobun

robobun commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

A note for the merge order with #43089.

#43089 makes node:quic take the default CA store when no ca is given. TlsContext::new (src/runtime/node/quic/tls.rs) calls two new wrappers in src/uws/lib.rs, shared_default_ca_store() and default_ca_store(). Their declarations sit next to the us_get_shared_default_ca_store declaration that this PR changes.

This PR gives both getters a use_system_ca argument. So the two PRs conflict at those lines, and that is on purpose: a separate zero-argument declaration of us_get_default_ca_store would still build and link against the new definition.

The PR that merges second has to pass the decision of the thread from node:quic. If this PR merges first, I will do that on #43089.

robobun added a commit that referenced this pull request Oct 11, 2026
…me generator

v8.writeHeapSnapshot() with no path formatted its own file name in JS and
used Date#getMonth() as the month, so the name was one month behind (00 in
January). That formatter had no sequence number, so two calls in the same
second returned one name and the second overwrote the first snapshot.

The name now comes from write_diagnostic_filename, the port of node's
DiagnosticFilename that --cpu-prof and --heap-prof already use:
Heap.<yyyymmdd>.<hhmmss>.<pid>.<thread id>.<sequence>.heapsnapshot. The
generator takes the thread id, the local time and the sequence number as
arguments. The profilers draw their sequence numbers when they start, as
node does, so a snapshot written while the program runs gets a later
number than the profile.

WebWorker::thread_id() is the accessor from #34424.

Co-authored-by: Ciro Spaciari <ciro.spaciari@gmail.com>
@robobun

robobun commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

#45209 registers the on() and once() listeners of a MessagePort in the native listener list and deletes the wrapper functions in injectFakeEmitter. this in such a listener is the port there. If #45209 merges first, the two .$call hunks of this PR in wrapped() and onceWrapper() (src/js/node/worker_threads.ts) are not necessary, and they conflict with it.

robobun added a commit that referenced this pull request Oct 16, 2026
…me generator

v8.writeHeapSnapshot() with no path formatted its own file name in JS and
used Date#getMonth() as the month, so the name was one month behind (00 in
January). That formatter had no sequence number, so two calls in the same
second returned one name and the second overwrote the first snapshot.

The name now comes from write_diagnostic_filename, the port of node's
DiagnosticFilename that --cpu-prof and --heap-prof already use:
Heap.<yyyymmdd>.<hhmmss>.<pid>.<thread id>.<sequence>.heapsnapshot. The
generator takes the thread id, the local time and the sequence number as
arguments. The profilers draw their sequence numbers when they start, as
node does, so a snapshot written while the program runs gets a later
number than the profile.

WebWorker::thread_id() is the accessor from #34424.

Co-authored-by: Ciro Spaciari <ciro.spaciari@gmail.com>
@robobun

robobun commented Oct 17, 2026

Copy link
Copy Markdown
Collaborator

A note for the merge order: #45802 makes process.hrtime() read uv_hrtime(), as Node does. The test hrtime, uptime, Bun.nanoseconds and performance share the parent's origin in this PR brackets process.uptime(), Bun.nanoseconds() and performance.now() between two process.hrtime.bigint() readings. That holds only while hrtime counts from process start, and Node fails the same test. The two branches have no textual conflict there, so the PR that lands second must bracket each clock with readings of the same clock.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants