Skip to content

node:worker_threads: implement postMessageToThread [1bx5ty] - #29828

Closed
robobun wants to merge 51 commits into
mainfrom
farm/6140e626/worker-postMessageToThread
Closed

node:worker_threads: implement postMessageToThread [1bx5ty]#29828
robobun wants to merge 51 commits into
mainfrom
farm/6140e626/worker-postMessageToThread

Merge branch 'main' into farm/6140e626/worker-postMessageToThread

37c6af0
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Apr 29, 2026 in 37m 31s

Code review found 2 potential issues

Found 6 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 2
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/js/node/worker_threads.ts:410-413 postMessageToThread hangs forever when target worker never imports node:worker_threads
🟡 Nit src/bun.js/bindings/webcore/MessagePort.cpp:458-463 onmessageerror setter still refs the event loop via m_wantsExplicitRef

Annotations

Check warning on line 413 in src/js/node/worker_threads.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

postMessageToThread hangs forever when target worker never imports node:worker_threads

Because `setupMainThreadPort` only runs when the worker first evaluates `node:worker_threads` (while the parent eagerly registers the port in `threadsPorts` during the `Worker` constructor), `postMessageToThread(thatWorkerId, value)` with no timeout will return a promise that never settles if the target worker's script never imports `worker_threads`. In Node the messaging port is wired during worker bootstrap, so the same call rejects promptly with `ERR_WORKER_MESSAGING_FAILED` instead of hangin

Check warning on line 463 in src/bun.js/bindings/webcore/MessagePort.cpp

See this annotation in the file changed.

@claude claude / Claude Code Review

onmessageerror setter still refs the event loop via m_wantsExplicitRef

The `onmessageerror` setter (JSMessagePort.cpp:234) still calls `jsRef()`, which now sets `m_wantsExplicitRef = true` — so `port.onmessageerror = fn; port.hasRef()` returns `true` and the port refs the event loop, contrary to the resolved review thread's claim that this refactor makes Bun match Node (where it returns `false`). The header comment for `m_wantsExplicitRef` ("from .ref() or the onmessage setter") confirms `onmessageerror` was overlooked; the simplest fix is to drop the `jsRef()` cal