Skip to content

worker_threads: start a node worker natively and build its stdio on first use - #47052

Open
robobun wants to merge 5 commits into
mainfrom
robobun/918fbecb/node-worker-native-start
Open

robobun wants to merge 5 commits into
mainfrom
robobun/918fbecb/node-worker-native-start

Conversation

@robobun

@robobun robobun commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Fix

  • The start sequence is native (NodeWorkerState::takeFromParent) and evaluates no builtin module: 6.5M.
  • The worker's console stays native. It posts to the parent's worker.stdout or worker.stderr port until the worker builds that stream.
  • process.stdin, stdout and stderr are getters that build the stream on the first read.
  • Verified: worker_threads.test.ts (107 new tests, 34 fail on main), node's test-worker-*.js, test/js/web/workers/. Self-reviewed: 75 concerns raised, 69 addressed (Notes).

Background

  • A node worker has five MessageChannels to its parent: stdin, stdout, stderr, the postMessageToThread hub and parentPort.
  • JSC compiles a builtin module from source in each VM that loads it. A worker is a VM.
  • Considered a lazy JS bootstrap: it still evaluates 9 module bodies (about 23M).

Downsides

  • A worker's console formats natively, as on the main thread and before 1.4. Code that depends on the util.inspect format of 1.4 worker logs sees Bun's format.
  • A read of process.stdout in a worker calls a native getter: 18 to 110 instructions. A first process.nextTick after hot code was optimized re-optimizes functions that read process.*: 11M instructions each, once.
  • WorkerOptions grows by 16 bytes per Worker. The binary grows by 32 KB.
Notes

Origin. No user report. A maintainer asked what a Worker's start does since 1.4.0 that it did not do on 1.3.14, with the empty file as the measure.

Cause. #31216 (1.4.0) added a JS bootstrap to every node:worker_threads worker. #39536 made it a native hook that requires node:worker_threads before preloads and the entry point. #39628 moved part of the module's own work to first use. What a worker on an empty file runs on its thread:

code blocks bytecode instructions builtin module bodies
main 84 47,327 27
PR 4 344 0
Web Worker (main and PR) 4 344 0

Measurements. Release builds (LTO) of the merge base a0cc096 and of this PR, linux x64. The unit is user instructions (perf_event_open, all threads). Per worker = slope between 100 and 300 sequential workers, median of 3 (spread under 0.3M).

Worker 1.3.14 main PR
empty file 17.59M 56.82M 6.51M
console.log once 18.08M 60.99M 7.20M
posts one message 46.52M 57.28M 19.04M
requires the module, parentPort.on, one reply 47.32M 58.03M 20.48M
requires the module, logs, posts 45.99M 61.30M 19.95M
one process.stdout.write 59.29M 29.47M
{ stdin: true }, stdin not used 57.37M 6.51M
console.log and a read of process.stdin 61.02M 27.81M
one postMessageToThread arrival 57.47M 8.26M
one chunk of worker.stdin 63.61M 35.03M
Web Worker 17.32M 5.91M 5.83M
  • Off the path, whole process, minimum of 20: bun <empty file> 7,087,669 to 7,086,756. require("node:worker_threads") on the main thread 20,940,668 to 20,214,984. A process that makes its first node Worker 100.1M to 45.8M. A process that makes one Web Worker: 18.74M and 18.72M (median of 40, spread 0.5M).
  • Per operation with the JIT off: port.start() 290.9 to 291.0. postMessage round trip 7,018 to 6,435. process.umask() on the main thread 396.0 to 396.0, in a worker 879 to 411. port.listenerCount() 2,036 to 2,100. port.on() plus off() 8,904 to 8,858.
  • console.log in a worker, per call: 14,395 to 12,962 (JIT), 80,310 to 43,300 (no JIT). process.stdout.write in a worker: 12,973 to 12,889 (JIT), 55,731 to 55,525 (no JIT). A read of process.stdout in a worker: 18.2 to 109.9 (JIT), 325 to 713 (no JIT).
  • Main-thread console, per call with the JIT off: log 1,602.8 to 1,598.8, error 1,604.8 to 1,600.8, count 1,266.3 to 1,274.3, timeLog 3,436.2 to 3,451.5, table 16,621 to 16,671, log() with no argument 755 to 711. With the JIT: log 1,408.4 to 1,404.1.
  • Sizes: Zig::GlobalObject 9,736 to 9,712 bytes. MessagePort 96 to 96. Process 136 to 136. WorkerOptions 128 to 144. One more internal module (8 bytes in each registry). Binary .text +27,328 bytes, .rodata +4,096, .bun_builtins -1,366, file +32,768.
  • Syscalls per worker (strace -f -c, slope): 813.5 to 485.5. Without futex, sched_yield, epoll and madvise: 79.0 to 61.1.
  • RSS per idle worker ((50 workers - 10) / 40): 2.97 MB to 1.37 MB (1.3.14: 2.19 MB). For a worker that loaded node:worker_threads: 2.99 MB to 1.85 MB (1.3.14: 3.39 MB).
  • Wall clock, cross-check only. Median life of an empty worker, 200 sequential, 5 interleaved rounds: main 32.3 to 34.9 ms, PR 13.3 to 16.0 ms, 1.3.14 23.8 to 28.5 ms. 100,000 console.log calls in a worker: main 439 to 513 ms, PR 322 to 419 ms. bench/snippets/worker-threads-start.mjs has six worker shapes.
  • Late first use. 100 optimized functions that read process.pid, then a first use of a lazy process property: +1.0G instructions for process.config on main, +1.1G on this PR. On this PR also +1.2G for the first process.nextTick, which the old bootstrap read at start. A first read of process.stdout after process.nextTick was used costs nothing (2,112M and 2,189M): the getter does not change the structure of process.

What the worker does at start now (NodeWorkerState::takeFromParent, called from WebWorker::spin()):

  • entangles the five channel ends that the parent's Worker made and passed through WorkerOptions::nodePorts,
  • starts parentPort and the hub port, held until the entry point settles,
  • puts native listeners on the stdin and hub ports,
  • switches the native console to the worker's stdout and stderr channels,
  • deserializes workerData and environmentData.

Console. The sink in ConsoleObject.rs is adapted from #39592 (credited in the commit). Unlike #39592, native code does not call into process.stdout before the worker has built that stream. Output goes as one chunk for each call, as the streams of 1.4 and of Node deliver it. When the worker builds process.stdout while the console has a message in flight, the stream waits for the ack of that message ahead of its first write. So 'drain' and write callbacks keep the order of main.

Console queue. What the console has written and the parent's reader has yet to acknowledge waits in segments of about 1 MiB, so no buffer comes near the 2 GiB that a WTF::Vector holds. An acknowledgement releases all that waits as one message (256 MiB at most), as the writev of a stream does on main and in Node. A synchronous exit posts one message for each segment. The queue has no limit, like the Writable's buffer on main and in Node: a worker whose parent never reads a captured worker.stdout keeps its output.

  • 5,000 MiB logged into such a worker: no abort, RSS 5,066 MiB. The first commit kept one Vector and aborted near 1.5 GiB. No test covers that: it needs 2 GiB of output.
  • 256 MiB of 100-byte log lines that nothing reads: the process holds 473 MiB (main: 2,610 MiB).
  • 1.5 million one-byte calls, then process.exit(): peak RSS 163 MiB (main: 1,062 MiB).
  • A 512 MiB backlog that the parent then reads: peak RSS 2,125 MiB (main: 2,780 MiB). At process.exit(): 683 MiB (main: 2,205 MiB).
  • With that queue an acknowledgement that releases 1 MiB, or a hand-off to process.stdout past 1 MiB, was built and measured too. The first left the output of a busy worker behind and lost it at terminate(). The second took a worker that logs 2 MiB in one loop from 240M to 448M instructions.

Stream builds. The build of a stdio stream runs nothing that user code can replace once the stream's constructor returned: native code puts the stream's message listener on the port, and the stream writes nothing to itself. So a MessagePort or Writable method that user code replaced does not run inside the build, and a read of a stream from inside its own build gives the same stream. User code can still run inside a build through an accessor on Object.prototype that a stream constructor hits. A console call from there waits in the queue, and the stream writes it when the build is done.

Behaviour changes against 1.4.x in a node worker. In each case the worker now behaves as Bun's main thread does, or as 1.3.x workers did.

  • console.* formats natively, not with util.inspect. console.timeLog() and console.timeEnd() write to stderr and console.trace() to stdout, as on the main thread.
  • console.write() and for await (const line of console) exist again and use fd 1 and fd 0 of the process, as in 1.3.x and as worker_threads: keep the native console in workers, routed through process.stdout/stderr #39592 chose.
  • console._stdout = stream does not redirect the console.
  • require("node:console") === console.
  • process has no 'exit' listener of its own (Node: 1, 1.4.x: 2).
  • Colors are decided when the worker starts, from FORCE_COLOR in the worker's env and the process's color setting. 1.4.x read the worker's process.env at each call.
  • A MessagePort has on() and once() only after node:worker_threads was loaded in that thread, as on the main thread. A port that arrives through process.on("workerMessage") loads the module first.
  • new globalThis.Worker(url, options, third) is a Web Worker. The third argument was internal.
  • node:events, the stream modules and node:worker_threads are evaluated at first use, in the realm as user code left it. The stdio streams take no option from Object.prototype and keep the highWaterMark and captureRejections defaults of worker start.
  • Object.entries(process) and { ...process } list stdin, stdout, stderr right after _exiting.
  • Races that the slow start hid. A postMessageToThread() sent right after new Worker() can reach a worker that already ended: the sender gets ERR_WORKER_MESSAGING_FAILED. An unref'd worker with a captured stdout that nothing reads keeps its parent alive once output arrived, as in Node. On main the parent usually went idle first.
  • The CommonJS entry's process.nextTick callbacks still run after its microtasks, as since 1.4.0 (Node and 1.3.14 run them first). A test.failing marks it.
  • process.stdout.writableLength does not count a console message that was posted before the worker first read process.stdout and that the parent has yet to acknowledge. On main and in Node the stream exists from the start and counts it.
  • Flow control is tighter than on main and in Node: the reader of a stdio port asks for more data only after it got some. On main a request that the parent's reader sent ahead of any output passed for the ack of the worker's first batch.
  • console.log after a node:vm timeout that landed in an earlier console call still prints. On main and in Node the stream stays mid-write.

Not changed. The main thread's console: byte-identical output for a corpus of 30 scripts in default, FORCE_COLOR and NO_COLOR runs. A bare console.error() on the main thread still writes its newline to stdout (#39592 changes that).

Relation to open PRs.

Self-review. Two review passes on the first commit and two on the second (the console queue and the stream builds). Not changed, with the reason:

  • console.write() and the console's async iterator stay bound to the process's fds in a worker: worker_threads: keep the native console in workers, routed through process.stdout/stderr #39592 made that choice, and 1.3.x did the same.
  • console.timeEnd() and timeLog() write to stderr: the worker has the main thread's console.
  • An unref'd worker with a captured, unread stdout pins an idle parent more often: the rule is main's and Node's, only the race moved.
  • A postMessageToThread() that is still queued when the entry point ends with an unhandled rejection gets no answer, as in Node. Main answered it from script that ran after the worker asked to stop. worker: take a listener's termination at the native boundary, end the tick pass on a stopped VM #45298 has the same ruling for its path. Answers from the hub side are a separate change.
  • A stream that a build made and did not record stays usable for code that kept it: a write to it is not acknowledged. That needs an accessor on Object.prototype that runs inside the build, reads the same stream there, and keeps the half-built object.
  • The hub's routing table stays a SafeMap: its require is what loads internal/primordials with the module on the main thread, and an existing test depends on that.

Tests. 107 new tests in worker_threads.test.ts: 89 in "node worker start sequence", 18 from #46921. On the merge base 34 of them fail. On the first commit of this PR 12 of the tests of the second commit fail. The file passes on a debug build with BUN_JSC_validateExceptionChecks=1, except for the flake of "terminate(): nothing of the worker's runs after the request" that #38194 fixes (1 of 8 runs here). Release build: test/js/node/worker_threads/ 286 pass, test/js/web/workers/ with broadcastchannel, web/console and node/events 567 pass, node's test-worker-*.js 105 of 106 (the same file fails on main), test-console*, test-event*, test-stdio*, test-stdout*, test-stdin* as on main. On this machine's debug build some tests of worker_destruction.test.ts, worker-late-completion.test.ts and module-graph-workers.test.ts pass only with a longer timeout, on main too.

Found on main during review, not changed here. worker.stdout.destroy() in the middle of a batch leaves the worker alive (Node and 1.4.4 let it exit). A late unhandled rejection in a worker sometimes does not reach the parent's 'error' listener. Both are reported separately.

Follow-ups, not in this PR. require("node:worker_threads") in a worker still evaluates the whole module (about 12M). A lazy Worker export and the emitter methods as a module of their own would cut that. Builtin bytecode for the plain binary (#35698) cuts every remaining first-use load.


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/workers/worker-late-completion.test.ts, test/js/node/worker_threads/worker_threads.test.ts

robobun and others added 4 commits October 27, 2026 08:09
`new Worker()` started the thread first and started the parent side of
the stdout, stderr and public ports after it. A worker that ended inside
that gap had its exit queued on the parent ahead of the first delivery
of those ports. The exit handler then closed the stdio ports with the
output still queued, and delivered the parentPort messages from inside
itself.

Start the three ports right before the thread starts. The worker's first
write or post then queues the port's drain, and the worker's port close
queues the delivery of the rest. Both come from the worker thread, ahead
of its exit task.
With the public port started before the thread, the port delivers what
the worker posted ahead of the worker's exit task. The drain in the exit
handler has no path left, so remove it.
A worker can transfer its parentPort to another thread. Its end of the
public channel then outlives the worker, so the parent's port gets no
peer-close flush, and one drain task delivers at most 1,024 messages.
The drain in the exit handler takes the rest. Restore it, and pin the
case with a test.
…irst use

Since 1.4.0 a node:worker_threads worker evaluated the node:worker_threads
module before its entry point. That ran 27 builtin module bodies and built
process.stdin, stdout, stderr and a Console in every worker.

The start sequence is now native (NodeWorkerState::takeFromParent) and
evaluates no builtin module:

- The parent passes the worker's ends of its five channels (stdin, stdout,
  stderr, the postMessageToThread hub, parentPort) through WorkerOptions,
  not inside workerData. node:worker_threads makes its native Worker through
  an internal function, so the public constructor takes no hidden argument.
- The console of a node worker stays the native one. Until the worker builds
  process.stdout or stderr, the console posts to the parent's port from a
  native queue, one chunk for each call, with the flow control of
  internal/worker/stdio. After that it writes through the stream as the
  Console of node:console does.
- process.stdin, stdout and stderr are getters on the worker's process. The
  first read builds the stream, and the stream takes over the batch that the
  console has in flight. A read does not change the structure of process.
- A message from the postMessageToThread hub, or a chunk for a
  { stdin: true } worker, builds its reader on arrival.
- The process restrictions of a worker are installed with the process object.
- A synchronous exit flushes stdio natively. process has no 'exit' listener of
  its own.

Related changes:

- The reader of a stdio port asks for more data only after it got some. The
  writer took an earlier request for the ack of its first batch.
- node:events defines EventEmitter.prototype.constructor. An assignment
  failed after Object.freeze(Object.prototype).
- The write of a Console to a stream whose errors it ignores is in
  internal/console_write, for node:console and for the worker's console.
- JSC's console.assert checks no exception after it calls the console
  client, so the client leaves no throw scope unchecked.

Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between c643ea8 and 31071fd.

Files selected for processing: 29
  • bench/snippets/worker-threads-start.mjs
  • src/js/builtins.d.ts
  • src/js/builtins/ConsoleObject.ts
  • src/js/builtins/ProcessObjectInternals.ts
  • src/js/internal/console_write.ts
  • src/js/internal/streams/state.ts
  • src/js/internal/worker/messaging.ts
  • src/js/internal/worker/stdio.ts
  • src/js/node/events.ts
  • src/js/node/worker_threads.ts
  • src/jsc/ConsoleObject.rs
  • src/jsc/bindings/BunProcess.cpp
  • src/jsc/bindings/ConsoleObject.cpp
  • src/jsc/bindings/NodeWorkerState.cpp
  • src/jsc/bindings/NodeWorkerState.h
  • src/jsc/bindings/ZigGlobalObject.cpp
  • src/jsc/bindings/ZigGlobalObject.h
  • src/jsc/bindings/webcore/JSWorker.cpp
  • src/jsc/bindings/webcore/JSWorker.h
  • src/jsc/bindings/webcore/MessagePort.cpp
  • src/jsc/bindings/webcore/MessagePort.h
  • src/jsc/bindings/webcore/Worker.cpp
  • src/jsc/bindings/webcore/Worker.h
  • src/jsc/bindings/webcore/WorkerOptions.h
  • src/jsc/web_worker.rs
  • test/js/node/worker_threads/parent-held-in-constructor-fixture.js
  • test/js/node/worker_threads/worker_threads.test.ts
  • test/js/web/workers/message-channel.test.ts
  • test/js/web/workers/worker-late-completion.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

Node worker startup now transfers worker data and dedicated channels through native worker state. The changes also update worker messaging, process streams, console output, and coverage for startup ordering, output, and shutdown behavior.

Changes

Node worker lifecycle

Layer / File(s) Summary
Worker creation and channel transfer
src/js/node/worker_threads.ts, src/js/builtins.d.ts, src/jsc/bindings/webcore/WorkerOptions.h, src/jsc/bindings/webcore/JSWorker.*, src/jsc/bindings/webcore/Worker.*, src/jsc/web_worker.rs, bench/snippets/worker-threads-start.mjs, test/js/node/worker_threads/worker_threads.test.ts
Worker construction now passes dedicated channels and transfer metadata to a native worker factory. The startup path transfers worker data before preloads and entry evaluation. Tests cover startup ordering, worker data, and early completion; the benchmark measures six worker source variants.
Native worker state and messaging
src/jsc/bindings/NodeWorkerState.*, src/jsc/bindings/ZigGlobalObject.*, src/jsc/bindings/webcore/MessagePort.*, src/jsc/bindings/webcore/Worker.cpp, src/js/internal/worker/messaging.ts, test/js/node/worker_threads/parent-held-in-constructor-fixture.js, test/js/node/worker_threads/worker_threads.test.ts, test/js/web/workers/*
NodeWorkerState stores worker data, ports, listeners, and startup flags. Entry settlement releases held port starts. Native bindings supply messaging state and dispatch incoming messages. Tests cover early worker completion, message delivery, port lifecycle, and late completion.
Worker process streams and console output
src/js/internal/worker/stdio.ts, src/js/internal/console_write.ts, src/js/internal/streams/state.ts, src/js/builtins/ConsoleObject.ts, src/js/builtins/ProcessObjectInternals.ts, src/js/node/events.ts, src/jsc/ConsoleObject.rs, src/jsc/bindings/BunProcess.cpp, src/jsc/bindings/ConsoleObject.cpp, test/js/node/worker_threads/worker_threads.test.ts
Worker process streams are created lazily from worker ports. Console writes use worker-specific formatting and queued output. Acknowledgements and process-exit handling flush pending output. Tests cover stream construction, backpressure, process restrictions, and console output.

Possibly related PRs

  • oven-sh/bun#46921: Starts parent-side worker ports before the worker thread, matching this change’s channel startup sequence.
  • oven-sh/bun#46997: Updates lazy worker stdio construction introduced by this change.
  • oven-sh/bun#46948: Changes worker error delivery relative to exit listeners, which is related to this change’s exit-time output handling.

Suggested reviewers: dylan-conway, jarred-sumner

Priority: Normal

Merge Risk: Minimal · up to 31071

This change moves Node worker startup and stdio setup to native code with lazy stream creation. The supplied evidence shows no concrete merge-blocking defect. Normal CI validation of the worker test suites remains the main check.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title "worker_threads: start a node worker natively and build its stdio on first use" matches the main change. The PR starts Node workers through a native path (NodeWorkerState::takeFromParent) in…
Description check Passed The description supplies a Problem section, a Fix section, measurements, background, downsides, and detailed verification notes. It covers the template's two required parts in substance. The template'…
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.
Full details: Title check

Explanation

The title "worker_threads: start a node worker natively and build its stdio on first use" matches the main change. The PR starts Node workers through a native path (NodeWorkerState::takeFromParent) instead of evaluating the node:worker_threads module, and it builds process.stdin/stdout/stderr lazily on first read. The title is concise, specific, and does not use vague terms. Its wording is somewhat long but still readable.

Full details: Description check

Explanation

The description supplies a Problem section, a Fix section, measurements, background, downsides, and detailed verification notes. It covers the template's two required parts in substance. The template's 'What does this PR do?' content appears as Problem/Fix/Background, and 'How did you verify your code works?' appears in the Verified line and the Tests section. The description is very long and dense, and the verification section is more detailed than the template asks. It also lists two platform-specific tests the author could not run locally, deferred to CI. The template headings are not used verbatim, which is a minor deviation.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@robobun

robobun commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

Status

How I reproduced it: a script makes 100 and then 300 node:worker_threads workers on an empty file, one after the other, on release builds. I count user instructions with perf_event_open and take the slope. Per worker: 1.3.14 17.6M, main 56.8M, this PR 6.5M. On main the worker evaluates node:worker_threads (27 builtin module bodies, three stdio streams, a Console) before its entry point.

PR: #47052

@robobun

robobun commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 12:36 PM PT - Oct 27th, 2026

✅ @robobun, your commit 31071fdedb194e27974e3b0b4711f4e87206c43b passed in Build #133839! 🎉


🧪   To try this PR locally:

bunx bun-pr 47052

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

bun-47052 --bun

@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.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked GC rooting of the new NodeWorkerState: its four WriteBarrier members are all appended in NodeWorkerState::visit, and the unique_ptr on Zig::GlobalObject is reached through the existing gcMembers table, so the new state replaces five global-object fields with one visited owner. The lazy process.stdout/stderr getters in BunProcess.cpp were also checked for the "process exists before start sequence" and setter-in-strict-mode concerns; neither reproduces on the code as written.

Extended reasoning...

The change moves node:worker_threads worker startup from a JS bootstrap into a native NodeWorkerState (C++), reworks how the five parent/worker MessagePorts are passed through WorkerOptions, makes process stdio lazy getters, and routes the worker's native console through the stdout/stderr ports with its own flow-control queue. It touches no auth, crypto or injection surface, but it is a large cross-language change to worker lifecycle, port entanglement and GC-rooted state. Two confirmed flow-control findings are posted inline, so a human review is already indicated; this note only records what else was examined.

Comment thread src/jsc/bindings/NodeWorkerState.cpp Resolved
Comment thread src/jsc/bindings/NodeWorkerState.h Resolved
The queue of a worker's console held all output that the parent's
reader had yet to acknowledge in one Vector. A Vector holds less than
2 GiB, so a worker whose parent did not read worker.stdout aborted the
process after about 1.5 GiB of console output. The queue now holds
segments of about 1 MiB. An acknowledgement still releases all that
waits as one message, up to 256 MiB. A synchronous exit posts a message
for each segment.

The build of process.stdin, stdout or stderr now runs nothing that user
code can replace once the stream's constructor returned. Native code
puts the stream's listener on the port. A stream that is made while the
console has a message in flight waits for the acknowledgement of that
message ahead of its first write, and no longer writes the message to
itself again. So a replaced MessagePort or Writable method that reads
the stream does not start a second build, and a read of a stream from
inside its own build gives one stream.

A console call from inside a build posted a message that the new stream
did not know of. The stream then took the acknowledgement of that
message for the one of its first write. Such a call now waits in the
queue. The stream writes it when the build is done. The queue posts it
when the build throws, and at once when the worker exits.

@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.

I re-reviewed after 31071fd and found no new bugs; the segmented ConsoleQueue and the m_buildingStream hold cover both points I raised inline earlier, and each has tests in worker_threads.test.ts. Given the scope (native worker start, MessagePort start holds, console sink rewiring, ~3k lines), a maintainer look is still worthwhile.

What was reviewed this pass:

  • NodeWorkerState.cpp stdioStream / writeConsole / sendConsoleQueue: the in-flight ack now maps to exactly one posted message; console calls during a build wait in the queue and are written through the stream after the build, or posted if the build throws.
  • ConsoleQueue::append / nextMessageSegments: per-segment Vector growth is capped at ~1 MiB with tryAppend, so a non-acking parent no longer trips the Vector size limit; the Deque itself still grows without bound, matching a Writable's buffer.
  • Loop ref balance on the stdio ports across sendConsoleQueue, didReceiveMessage, releaseConsoleQueue and the stream handoff in stdioStream — each jsRef has a matching jsUnref on the ack, exit, and build paths.
Extended reasoning...

The PR replaces the JS bootstrap of node:worker_threads workers with a native NodeWorkerState (C++), passes five MessagePort ends through WorkerOptions::nodePorts, reroutes the native console sink to the parent's stdout/stderr ports, and builds process.stdin/stdout/stderr lazily; it touches no auth, crypto or injection surface. The newest commit 31071fd addresses both inline findings from the prior run and adds tests for them, and this run's hunt exited on dry_streak with no new findings. The change is still large and complex native lifecycle code (port entanglement, start holds, exception scopes, loop ref counting) that a maintainer should weigh, so defer rather than approve.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants