diff --git a/src/codegen/bake-codegen.ts b/src/codegen/bake-codegen.ts index 1eaf4ff7239c..eacf950958f6 100644 --- a/src/codegen/bake-codegen.ts +++ b/src/codegen/bake-codegen.ts @@ -56,7 +56,10 @@ async function run() { side: JSON.stringify(side), IS_ERROR_RUNTIME: String(file === "error"), IS_BUN_DEVELOPMENT: String(!!debug), - OVERLAY_CSS: css("../runtime/bake/client/overlay.css", !!debug), + // JSON.stringify so the value is an explicit JS string literal; + // relying on Bun's define auto-quote fallback breaks when the + // bootstrap bun predates the `*`/`?` JSON-lexer fix (314d044c0a). + OVERLAY_CSS: JSON.stringify(css("../runtime/bake/client/overlay.css", !!debug)), }, minify: { syntax: !debug, diff --git a/src/js/builtins.d.ts b/src/js/builtins.d.ts index 7b86c3557ed8..c54d0e655e63 100644 --- a/src/js/builtins.d.ts +++ b/src/js/builtins.d.ts @@ -680,6 +680,10 @@ declare function $ERR_IPC_DISCONNECTED(): Error; declare function $ERR_SERVER_NOT_RUNNING(): Error; declare function $ERR_IPC_CHANNEL_CLOSED(): Error; declare function $ERR_SOCKET_BAD_TYPE(): Error; +declare function $ERR_WORKER_MESSAGING_ERRORED(): Error; +declare function $ERR_WORKER_MESSAGING_FAILED(): Error; +declare function $ERR_WORKER_MESSAGING_SAME_THREAD(): Error; +declare function $ERR_WORKER_MESSAGING_TIMEOUT(): Error; declare function $ERR_ZLIB_INITIALIZATION_FAILED(): Error; declare function $ERR_IPC_ONE_PIPE(): Error; declare function $ERR_SOCKET_ALREADY_BOUND(): Error; diff --git a/src/js/node/worker_threads.ts b/src/js/node/worker_threads.ts index 0789d2a49c11..bf1d2f582fcf 100644 --- a/src/js/node/worker_threads.ts +++ b/src/js/node/worker_threads.ts @@ -6,16 +6,19 @@ type WebWorker = InstanceType; const EventEmitter = require("node:events"); const Readable = require("internal/streams/readable"); const { throwNotImplemented, warnNotImplementedOnce } = require("internal/shared"); +const { validateNumber } = require("internal/validators"); const { MessageChannel, BroadcastChannel, Worker: WebWorker, } = globalThis as typeof globalThis & { - // The Worker constructor secretly takes an extra parameter to provide the node:worker_threads - // instance. This is so that it can emit the `worker` event on the process with the - // node:worker_threads instance instead of the Web Worker instance. - Worker: new (...args: [...ConstructorParameters, nodeWorker: Worker]) => WebWorker; + // The Worker constructor secretly takes two extra parameters: the node:worker_threads instance + // (so the `worker` event on process emits the node instance instead of the Web Worker), and the + // worker's end of the MessagePort connecting it to the main thread for postMessageToThread. + Worker: new ( + ...args: [...ConstructorParameters, nodeWorker: Worker, mainThreadPort: MessagePort] + ) => WebWorker; }; const SHARE_ENV = Symbol("nodejs.worker_threads.SHARE_ENV"); @@ -25,11 +28,13 @@ const { 1: _threadId, 2: _receiveMessageOnPort, 3: environmentData, + 4: _mainThreadPort, } = $cpp("Worker.cpp", "createNodeWorkerThreadsBinding") as [ unknown, number, (port: unknown) => unknown, Map, + MessagePort | undefined, ]; type NodeWorkerOptions = import("node:worker_threads").WorkerOptions; @@ -439,6 +444,224 @@ function fakeParentPort() { } let parentPort: MessagePort | null = isMainThread ? null : fakeParentPort(); +// --- postMessageToThread --- +// +// Every worker gets a direct MessageChannel to the main thread (mainThreadPort). The main thread +// keeps a Map of threadId -> port. postMessageToThread always routes through the main thread, and +// a SharedArrayBuffer carries the ack so the sender can await delivery with Atomics.waitAsync. +// +// Known limitation: the worker side of the channel is only set up the first time the worker +// evaluates node:worker_threads (see setupMainThreadPort below). Node.js wires it during worker +// bootstrap unconditionally, so postMessageToThread(id, value) with no timeout to a worker that +// never imports worker_threads rejects with ERR_WORKER_MESSAGING_FAILED there, whereas here the +// promise stays pending indefinitely unless a timeout is supplied (worker exit closes the port +// but does not notify already-pending waiters). In practice a node:worker_threads Worker almost +// always imports the module for parentPort/workerData. +// +// Unregistration is driven from the parent's #onClose (matching Node's kOnExit), so terminating +// an intermediate worker orphans its live grandchildren in the root's threadsPorts map; sending +// to such a threadId without a timeout likewise stays pending. This matches Node.js behaviour. + +const kRegisterMainThreadPort = 0; +const kUnregisterMainThreadPort = 1; +const kSendMessageToWorker = 2; +const kReceiveMessageFromWorker = 3; + +// SharedArrayBuffer must always be Int32, so it's * 4. +// We need one for the operation status (performing / performed) and one for the result (success / failure). +const WORKER_MESSAGING_SHARED_DATA = 2 * 4; +const WORKER_MESSAGING_STATUS_INDEX = 0; +const WORKER_MESSAGING_RESULT_INDEX = 1; + +// Response codes +const WORKER_MESSAGING_RESULT_DELIVERED = 0; +const WORKER_MESSAGING_RESULT_NO_LISTENERS = 1; +const WORKER_MESSAGING_RESULT_LISTENER_ERROR = 2; + +// This is only populated by the main thread and always empty in other threads. +let threadsPorts: Map | undefined; +// This is only populated in child threads and always undefined in the main thread. +let mainThreadPort: MessagePort | undefined; + +function ensureThreadsPorts() { + return (threadsPorts ??= new Map()); +} + +// This event handler is always executed on the main thread only. +function handleMessageFromThread(message) { + switch (message.type) { + case kRegisterMainThreadPort: { + const { threadId, port } = message; + // Register the port + ensureThreadsPorts().$set(threadId, port); + // Handle messages on this port. When a new thread wants to register a child this takes care + // of doing that. This way any thread can be linked to the main one. + port.addEventListener("message", event => handleMessageFromThread(event.data)); + // Never block the thread on this port + port.unref(); + break; + } + case kUnregisterMainThreadPort: { + const ports = ensureThreadsPorts(); + const port = ports.$get(message.threadId); + if (port) { + port.close(); + ports.$delete(message.threadId); + } + break; + } + case kSendMessageToWorker: { + // Send the message to the target thread + const { source, destination, value, transferList, memory } = message; + sendMessageToWorker(source, destination, value, transferList, memory); + break; + } + } +} + +function handleMessageFromMainThread(message) { + if (message.type === kReceiveMessageFromWorker) { + receiveMessageFromWorker(message.source, message.value, message.memory); + } +} + +function sendMessageToWorker(source, destination, value, transferList, memory) { + // We are on the main thread, we can directly process the message + if (destination === threadId) { + receiveMessageFromWorker(source, value, memory); + return; + } + + // Search the port to the target thread + const port = ensureThreadsPorts().$get(destination); + + if (!port) { + const status = new Int32Array(memory); + Atomics.store(status, WORKER_MESSAGING_RESULT_INDEX, WORKER_MESSAGING_RESULT_NO_LISTENERS); + Atomics.store(status, WORKER_MESSAGING_STATUS_INDEX, 1); + Atomics.notify(status, WORKER_MESSAGING_STATUS_INDEX, 1); + return; + } + + port.postMessage( + { + type: kReceiveMessageFromWorker, + source, + destination, + value, + memory, + }, + transferList, + ); +} + +function receiveMessageFromWorker(source, value, memory) { + let response = WORKER_MESSAGING_RESULT_NO_LISTENERS; + + // We need an exception in a listener to propagate here, but the native process.emit swallows + // listener exceptions and reports them as uncaught. Invoke the listeners directly instead. + try { + const listeners = process.listeners("workerMessage"); + for (let i = 0; i < listeners.length; i++) { + listeners[i].$call(process, value, source); + response = WORKER_MESSAGING_RESULT_DELIVERED; + } + } catch { + response = WORKER_MESSAGING_RESULT_LISTENER_ERROR; + } + + // Populate the result + const status = new Int32Array(memory); + Atomics.store(status, WORKER_MESSAGING_RESULT_INDEX, response); + Atomics.store(status, WORKER_MESSAGING_STATUS_INDEX, 1); + Atomics.notify(status, WORKER_MESSAGING_STATUS_INDEX, 1); +} + +function createMainThreadPort(childThreadId, port) { + const registrationMessage = { + type: kRegisterMainThreadPort, + threadId: childThreadId, + port, + }; + + if (mainThreadPort) { + mainThreadPort.postMessage(registrationMessage, [port]); + } else { + // Either we are the main thread, or we were created without the node:worker_threads plumbing + // (e.g. via the Web Worker constructor). Act as the local root for messaging. + handleMessageFromThread(registrationMessage); + } +} + +function destroyMainThreadPort(childThreadId) { + const unregistrationMessage = { + type: kUnregisterMainThreadPort, + threadId: childThreadId, + }; + + if (mainThreadPort) { + mainThreadPort.postMessage(unregistrationMessage); + } else { + handleMessageFromThread(unregistrationMessage); + } +} + +function setupMainThreadPort(port) { + mainThreadPort = port; + port.addEventListener("message", event => handleMessageFromMainThread(event.data)); + // Never block the process on this port + port.unref(); +} + +async function postMessageToThread(destination, value, transferList, timeout) { + if (typeof transferList === "number" && typeof timeout === "undefined") { + timeout = transferList; + transferList = []; + } + + if (typeof timeout !== "undefined") { + validateNumber(timeout, "timeout", 0); + } + + if (destination === threadId) { + throw $ERR_WORKER_MESSAGING_SAME_THREAD(); + } + + const memory = new SharedArrayBuffer(WORKER_MESSAGING_SHARED_DATA); + const status = new Int32Array(memory); + const promise = Atomics.waitAsync(status, WORKER_MESSAGING_STATUS_INDEX, 0, timeout).value; + + const message = { + type: kSendMessageToWorker, + source: threadId, + destination, + value, + memory, + transferList, + }; + + if (mainThreadPort) { + mainThreadPort.postMessage(message, transferList); + } else { + handleMessageFromThread(message); + } + + // Wait for the response + const response = await promise; + + if (response === "timed-out") { + throw $ERR_WORKER_MESSAGING_TIMEOUT(); + } else if (status[WORKER_MESSAGING_RESULT_INDEX] === WORKER_MESSAGING_RESULT_NO_LISTENERS) { + throw $ERR_WORKER_MESSAGING_FAILED(); + } else if (status[WORKER_MESSAGING_RESULT_INDEX] === WORKER_MESSAGING_RESULT_LISTENER_ERROR) { + throw $ERR_WORKER_MESSAGING_ERRORED(); + } +} + +if (!isMainThread && _mainThreadPort) { + setupMainThreadPort(_mainThreadPort); +} + function getEnvironmentData(key: unknown): unknown { return environmentData.get(key); } @@ -462,6 +685,12 @@ function moveMessagePortToContext() { class Worker extends EventEmitter { #worker: WebWorker; #performance; + // Cached at construction. The native threadId getter returns -1 once the + // worker is closing; we need the real id in #onClose to unregister the + // postMessageToThread port, and for get threadId() to keep returning the + // id after terminate() (Node's getter also does this until [kDispose] nulls + // the handle, which happens after its [kOnExit] equivalent). + #threadId: number; // this is used by terminate(); // either is the exit code if exited, a promise resolving to the exit code, or undefined if we haven't sent .terminate() yet @@ -491,9 +720,12 @@ class Worker extends EventEmitter { this.#urlToRevoke = filename; } } + // Create a channel that links the new thread to the main thread for postMessageToThread. + const { port1: mainThreadPortToMain, port2: mainThreadPortToThread } = new MessageChannel(); try { - this.#worker = new WebWorker(filename, options as Bun.WorkerOptions, this); + this.#worker = new WebWorker(filename, options as Bun.WorkerOptions, this, mainThreadPortToThread); } catch (e) { + mainThreadPortToMain.close(); // Restore any transferList handles that were already neutered by // packJSTransferables, so their fds aren't orphaned. options[kRestoreJSTransferables]?.(); @@ -502,6 +734,8 @@ class Worker extends EventEmitter { } throw e; } + this.#threadId = this.#worker.threadId; + createMainThreadPort(this.#threadId, mainThreadPortToMain); // The transfer is committed - release fds that were transferred but are // not referenced from workerData (nothing will deserialize them). options[kFinalizeJSTransferables]?.(); @@ -532,7 +766,7 @@ class Worker extends EventEmitter { } get threadId() { - return this.#worker.threadId; + return this.#threadId; } ref() { @@ -609,6 +843,8 @@ class Worker extends EventEmitter { } #onClose(e) { + destroyMainThreadPort(this.#threadId); + this.#threadId = -1; this.#onExitPromise = e.code; this.#stdout?.push(null); this.#stderr?.push(null); @@ -682,6 +918,7 @@ export default { }, markAsUntransferable, moveMessagePortToContext, + postMessageToThread, receiveMessageOnPort, SHARE_ENV, threadId, diff --git a/src/jsc/ErrorCode.rs b/src/jsc/ErrorCode.rs index 7e5d18b0b575..cfd4557ebcff 100644 --- a/src/jsc/ErrorCode.rs +++ b/src/jsc/ErrorCode.rs @@ -717,9 +717,17 @@ impl ErrorCode { pub const TRACE_EVENTS_CATEGORY_REQUIRED: ErrorCode = ErrorCode(329); /// `ERR_TRACE_EVENTS_UNAVAILABLE` (instanceof Error) pub const TRACE_EVENTS_UNAVAILABLE: ErrorCode = ErrorCode(330); + /// `ERR_WORKER_MESSAGING_ERRORED` (instanceof Error) + pub const WORKER_MESSAGING_ERRORED: ErrorCode = ErrorCode(331); + /// `ERR_WORKER_MESSAGING_FAILED` (instanceof Error) + pub const WORKER_MESSAGING_FAILED: ErrorCode = ErrorCode(332); + /// `ERR_WORKER_MESSAGING_SAME_THREAD` (instanceof Error) + pub const WORKER_MESSAGING_SAME_THREAD: ErrorCode = ErrorCode(333); + /// `ERR_WORKER_MESSAGING_TIMEOUT` (instanceof Error) + pub const WORKER_MESSAGING_TIMEOUT: ErrorCode = ErrorCode(334); /// == C++ `NODE_ERROR_COUNT`. - pub const COUNT: u16 = 331; + pub const COUNT: u16 = 335; } // ────────────────────────────────────────────────────────────────────────── @@ -1094,6 +1102,10 @@ impl ErrorCode { pub const ERR_SECRETS_INTERACTION_REQUIRED: ErrorCode = ErrorCode::SECRETS_INTERACTION_REQUIRED; pub const ERR_HTTP2_GOAWAY_SESSION: ErrorCode = ErrorCode::HTTP2_GOAWAY_SESSION; pub const ERR_PROXY_TUNNEL: ErrorCode = ErrorCode::PROXY_TUNNEL; + pub const ERR_WORKER_MESSAGING_ERRORED: ErrorCode = ErrorCode::WORKER_MESSAGING_ERRORED; + pub const ERR_WORKER_MESSAGING_FAILED: ErrorCode = ErrorCode::WORKER_MESSAGING_FAILED; + pub const ERR_WORKER_MESSAGING_SAME_THREAD: ErrorCode = ErrorCode::WORKER_MESSAGING_SAME_THREAD; + pub const ERR_WORKER_MESSAGING_TIMEOUT: ErrorCode = ErrorCode::WORKER_MESSAGING_TIMEOUT; // NOTE: `ERR_SYSTEM_ERROR` / `ERR_CHILD_CLOSED_BEFORE_REPLY` intentionally // do NOT live here. They belong to the unrelated enum @@ -1442,6 +1454,10 @@ static CODE_STR: [&str; ErrorCode::COUNT as usize] = [ "ERR_INVALID_BUFFER_SIZE", "ERR_TRACE_EVENTS_CATEGORY_REQUIRED", "ERR_TRACE_EVENTS_UNAVAILABLE", + "ERR_WORKER_MESSAGING_ERRORED", + "ERR_WORKER_MESSAGING_FAILED", + "ERR_WORKER_MESSAGING_SAME_THREAD", + "ERR_WORKER_MESSAGING_TIMEOUT", ]; // ────────────────────────────────────────────────────────────────────────── diff --git a/src/jsc/bindings/ErrorCode.cpp b/src/jsc/bindings/ErrorCode.cpp index abe3a18f09b0..2dc444c38ded 100644 --- a/src/jsc/bindings/ErrorCode.cpp +++ b/src/jsc/bindings/ErrorCode.cpp @@ -2535,6 +2535,14 @@ JSC_DEFINE_HOST_FUNCTION(Bun::jsFunctionMakeErrorWithCode, (JSC::JSGlobalObject return JSC::JSValue::encode(createError(globalObject, ErrorCode::ERR_IPC_CHANNEL_CLOSED, "Channel closed."_s)); case ErrorCode::ERR_SOCKET_BAD_TYPE: return JSC::JSValue::encode(createError(globalObject, ErrorCode::ERR_SOCKET_BAD_TYPE, "Bad socket type specified. Valid types are: udp4, udp6"_s)); + case ErrorCode::ERR_WORKER_MESSAGING_ERRORED: + return JSC::JSValue::encode(createError(globalObject, ErrorCode::ERR_WORKER_MESSAGING_ERRORED, "The destination thread threw an error while processing the message"_s)); + case ErrorCode::ERR_WORKER_MESSAGING_FAILED: + return JSC::JSValue::encode(createError(globalObject, ErrorCode::ERR_WORKER_MESSAGING_FAILED, "Cannot find the destination thread or listener"_s)); + case ErrorCode::ERR_WORKER_MESSAGING_SAME_THREAD: + return JSC::JSValue::encode(createError(globalObject, ErrorCode::ERR_WORKER_MESSAGING_SAME_THREAD, "Cannot sent a message to the same thread"_s)); + case ErrorCode::ERR_WORKER_MESSAGING_TIMEOUT: + return JSC::JSValue::encode(createError(globalObject, ErrorCode::ERR_WORKER_MESSAGING_TIMEOUT, "Sending a message to another thread timed out"_s)); case ErrorCode::ERR_ZLIB_INITIALIZATION_FAILED: return JSC::JSValue::encode(createError(globalObject, ErrorCode::ERR_ZLIB_INITIALIZATION_FAILED, "Initialization failed"_s)); case ErrorCode::ERR_IPC_ONE_PIPE: diff --git a/src/jsc/bindings/ErrorCode.ts b/src/jsc/bindings/ErrorCode.ts index acd959d9bfc3..4f76b793403f 100644 --- a/src/jsc/bindings/ErrorCode.ts +++ b/src/jsc/bindings/ErrorCode.ts @@ -342,5 +342,9 @@ const errors: ErrorCodeMapping = [ ["ERR_INVALID_BUFFER_SIZE", RangeError], ["ERR_TRACE_EVENTS_CATEGORY_REQUIRED", TypeError], ["ERR_TRACE_EVENTS_UNAVAILABLE", Error], + ["ERR_WORKER_MESSAGING_ERRORED", Error], + ["ERR_WORKER_MESSAGING_FAILED", Error], + ["ERR_WORKER_MESSAGING_SAME_THREAD", Error], + ["ERR_WORKER_MESSAGING_TIMEOUT", Error], ]; export default errors; diff --git a/src/jsc/bindings/ScriptExecutionContext.cpp b/src/jsc/bindings/ScriptExecutionContext.cpp index 66a581369c58..4f44c4d5efba 100644 --- a/src/jsc/bindings/ScriptExecutionContext.cpp +++ b/src/jsc/bindings/ScriptExecutionContext.cpp @@ -231,10 +231,20 @@ void ScriptExecutionContext::addToContextsMap() Locker locker { allScriptExecutionContextsMapLock }; ASSERT(!allScriptExecutionContextsMap().contains(m_identifier)); allScriptExecutionContextsMap().add(m_identifier, this); + m_isInContextsMap = true; } void ScriptExecutionContext::removeFromContextsMap() { + // Idempotent: worker teardown removes the entry eagerly (before the + // final GC) so cross-thread posts see the context as gone instead of + // landing on a VM that is about to stop ticking. ~GlobalObject, which + // runs whenever GC eventually collects the global, calls this again; + // the flag short-circuits that second call so the destructor's + // !contains() ASSERT still catches real mismatches. + if (!m_isInContextsMap) + return; + m_isInContextsMap = false; Locker locker { allScriptExecutionContextsMapLock }; ASSERT(allScriptExecutionContextsMap().contains(m_identifier)); allScriptExecutionContextsMap().remove(m_identifier); diff --git a/src/jsc/bindings/ScriptExecutionContext.h b/src/jsc/bindings/ScriptExecutionContext.h index 78f66385a679..46d49f948d2d 100644 --- a/src/jsc/bindings/ScriptExecutionContext.h +++ b/src/jsc/bindings/ScriptExecutionContext.h @@ -142,6 +142,13 @@ class ScriptExecutionContext : public CanMakeWeakPtr, pu // Snapshot of the creating thread's UID; used by isContextThread() so the // check stays valid after VM clientData / VMHolder are torn down on exit. uint32_t m_contextThreadUID; + // Tracks whether m_identifier is currently registered in the global + // contexts map. Worker teardown removes the entry explicitly (so + // cross-thread posts see the context as gone instead of posting to a + // VM that is about to stop ticking) and ~GlobalObject may try again + // once GC finally collects the global; the flag makes the second call + // a no-op. Only written on the context's own thread. + bool m_isInContextsMap { false }; UncheckedKeyHashSet m_destructionObservers; diff --git a/src/jsc/bindings/webcore/EventEmitter.cpp b/src/jsc/bindings/webcore/EventEmitter.cpp index c9ab566b4103..48148ff4e3c6 100644 --- a/src/jsc/bindings/webcore/EventEmitter.cpp +++ b/src/jsc/bindings/webcore/EventEmitter.cpp @@ -116,8 +116,7 @@ bool EventEmitter::emitForBindings(const Identifier& eventType, const MarkedArgu if (!scriptExecutionContext()) return false; - emit(eventType, arguments); - return true; + return emit(eventType, arguments); } bool EventEmitter::emit(const Identifier& eventType, const MarkedArgumentBuffer& arguments) diff --git a/src/jsc/bindings/webcore/JSMessagePort.cpp b/src/jsc/bindings/webcore/JSMessagePort.cpp index 543477b0caa6..6664922b2fc2 100644 --- a/src/jsc/bindings/webcore/JSMessagePort.cpp +++ b/src/jsc/bindings/webcore/JSMessagePort.cpp @@ -202,7 +202,11 @@ static inline bool setJSMessagePort_onmessageSetter(JSGlobalObject& lexicalGloba vm.writeBarrier(&thisObject, value); ensureStillAliveHere(value); - thisObject.wrapped().jsRef(&lexicalGlobalObject); + // Node.js's onmessage setter refs when assigning a function and unrefs otherwise. + if (value.isCallable()) + thisObject.wrapped().jsRef(&lexicalGlobalObject); + else + thisObject.wrapped().jsUnref(&lexicalGlobalObject); return true; } @@ -231,7 +235,7 @@ static inline bool setJSMessagePort_onmessageerrorSetter(JSGlobalObject& lexical vm.writeBarrier(&thisObject, value); ensureStillAliveHere(value); - thisObject.wrapped().jsRef(&lexicalGlobalObject); + // Unlike onmessage, Node.js does not ref the port when only onmessageerror is set. return true; } diff --git a/src/jsc/bindings/webcore/JSWorker.cpp b/src/jsc/bindings/webcore/JSWorker.cpp index 7f98a4f3a324..2514944b59b3 100644 --- a/src/jsc/bindings/webcore/JSWorker.cpp +++ b/src/jsc/bindings/webcore/JSWorker.cpp @@ -145,8 +145,10 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: WorkerOptions options {}; JSValue nodeWorkerObject {}; - if (callFrame->argumentCount() == 3) { + JSValue mainThreadPort {}; + if (callFrame->argumentCount() >= 3) { nodeWorkerObject = callFrame->argument(2); + mainThreadPort = callFrame->argument(3); options.kind = WorkerOptions::Kind::Node; } JSValue workerData = jsUndefined(); @@ -300,13 +302,22 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: } } + // The internal port linking this worker to the main thread for worker_threads.postMessageToThread. + // Only present when constructed via node:worker_threads. + if (mainThreadPort && mainThreadPort.isObject()) { + transferList.append({ vm, mainThreadPort.getObject() }); + } else { + mainThreadPort = jsUndefined(); + } + Vector> ports; - auto* valueToTransfer = constructEmptyArray(globalObject, nullptr, 2); + auto* valueToTransfer = constructEmptyArray(globalObject, nullptr, 3); RETURN_IF_EXCEPTION(throwScope, {}); valueToTransfer->putDirectIndex(globalObject, 0, workerData); auto* environmentData = globalObject->nodeWorkerEnvironmentData(); // If node:worker_threads has not been imported, environment data will not be set up yet. valueToTransfer->putDirectIndex(globalObject, 1, environmentData ? environmentData : jsUndefined()); + valueToTransfer->putDirectIndex(globalObject, 2, mainThreadPort); ExceptionOr> serialized = SerializedScriptValue::create(*lexicalGlobalObject, valueToTransfer, WTF::move(transferList), ports, SerializationForStorage::No, SerializationContext::WorkerPostMessage); if (serialized.hasException()) { diff --git a/src/jsc/bindings/webcore/MessagePort.cpp b/src/jsc/bindings/webcore/MessagePort.cpp index f152366907f2..455cf1e48dfd 100644 --- a/src/jsc/bindings/webcore/MessagePort.cpp +++ b/src/jsc/bindings/webcore/MessagePort.cpp @@ -27,7 +27,6 @@ #include "config.h" #include "MessagePort.h" -#include "BunClientData.h" #include "EventNames.h" #include "MessageEvent.h" #include "MessagePortPipe.h" @@ -36,8 +35,6 @@ #include "WebCoreOpaqueRoot.h" #include -extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta); - namespace WebCore { WTF_MAKE_TZONE_ALLOCATED_IMPL(MessagePort); @@ -60,6 +57,17 @@ MessagePort::~MessagePort() { if (!m_isDetached) m_pipe->close(m_side); + // Third teardown path alongside close() and disentangle(): a ref'd port + // (onmessage / .ref()) whose peer has closed is GC-collectible via + // hasPendingActivity() → PeerClosed, and we reach here with + // m_isRefingEventLoop still set. scriptExecutionContext() is still valid + // — ContextDestructionObserver's destructor runs after this body, and if + // the context died first contextDestroyed() already routed through + // close() → updateEventLoopRef(). + if (m_isRefingEventLoop) { + if (auto* context = scriptExecutionContext()) + context->unrefEventLoop(); + } } ExceptionOr MessagePort::postMessage(JSC::JSGlobalObject& state, JSC::JSValue messageValue, StructuredSerializeOptions&& options) @@ -113,20 +121,10 @@ void MessagePort::close() m_pipe->close(m_side); removeAllEventListeners(); - - // Release the self-reference taken by jsRef() (set when .onmessage is - // assigned or .ref() is called from JS). The JS .close() binding calls - // jsUnref() first, so m_hasRef is already false on that path; we only - // reach this branch when close() runs without a preceding jsUnref() — - // most importantly from contextDestroyed() during Worker teardown. - // Without this, the self-ref pins the MessagePort past the JS wrapper - // sweep and it leaks forever. - if (m_hasRef) { - m_hasRef = false; - if (auto* context = scriptExecutionContext()) - context->unrefEventLoop(); - deref(); - } + // m_isDetached flipped above; ensure any explicit (.ref() / onmessage) + // event-loop ref is released too — removeAllEventListeners() only covers + // the listener-count path. + updateEventLoopRef(); } TransferredMessagePort MessagePort::disentangle() @@ -139,18 +137,6 @@ TransferredMessagePort MessagePort::disentangle() removeAllEventListeners(); m_hasMessageEventListener = false; - // Release the self-reference taken by jsRef() on the sending side. After - // transfer this object is inert (the receiving side gets a fresh - // MessagePort for the same pipe endpoint) and is no longer a destruction - // observer, so nothing else will ever release a ref taken here. - // The caller (disentanglePorts) holds a RefPtr, so deref() is safe. - if (m_hasRef) { - m_hasRef = false; - if (auto* context = scriptExecutionContext()) - context->unrefEventLoop(); - deref(); - } - // Hand the pipe endpoint to its next owner. Messages that arrive while // in transit buffer in the pipe; the receiving context's entangle() // re-attaches and flushes them. We keep our own ref to the pipe so the @@ -160,6 +146,11 @@ TransferredMessagePort MessagePort::disentangle() m_isDetached = true; m_started = false; + // Release any explicit event-loop ref before we drop our context; after + // observeContext(nullptr) updateEventLoopRef() would see a null context + // and just clear the flag without balancing the ref. + updateEventLoopRef(); + if (auto* context = scriptExecutionContext()) context->willDestroyDestructionObserver(*this); observeContext(nullptr); @@ -226,12 +217,6 @@ void MessagePort::dispatchEvent(Event& event) void MessagePort::contextDestroyed() { - // close() releases the jsRef() self-reference, which may be the last - // strong ref if the JS wrapper was already swept. Protect across the - // call so we can cleanly detach from the dying ScriptExecutionContext - // first — otherwise ~ContextDestructionObserver() would call back into - // it while it is mid-destruction. - Ref protectedThis { *this }; close(); ContextDestructionObserver::contextDestroyed(); } @@ -240,19 +225,22 @@ bool MessagePort::hasPendingActivity() const { // Called from the GC thread concurrently with the mutator; must be // lockless. m_pipe is a Ref<> held for the port's whole lifetime, so - // the dereference is always safe; state() and isOtherSideOpen() are - // atomic loads. The plain bool reads can observe stale values but - // cannot crash — at worst the wrapper is collected one cycle early - // or late, which is the same tolerance as before this refactor. + // the dereference is always safe. The plain bool reads can observe + // stale values but cannot crash. if (!scriptExecutionContext() || m_isDetached) return false; if (!m_hasMessageEventListener) return false; - uint64_t s = m_pipe->state(m_side); - // Keep alive if there are messages already queued for us, or the peer - // is still open and could send more. - return MessagePortPipe::queuedCount(s) > 0 || m_pipe->isOtherSideOpen(m_side); + // Single atomic load: queued count, DrainScheduled (a message has been + // popped from the inbox but not yet dispatched — queuedCount is already + // decremented in that window), and the PeerClosed mirror bit all live in + // our side's state word. Reading the peer's state separately would let + // the GC observe {queuedCount=0, !DrainScheduled} from before the peer's + // send, then Closed from after the peer's close — and collect the wrapper + // with a message in flight (ASSERT(m_wrapper) in debug, silently dropped + // event → hang in release). + return MessagePortPipe::isActivityPending(m_pipe->state(m_side)); } ExceptionOr> MessagePort::disentanglePorts(Vector>&& ports) @@ -281,30 +269,56 @@ Vector> MessagePort::entanglePorts(ScriptExecutionContext& c }); } +void MessagePort::updateEventLoopRef() +{ + bool shouldRef = m_hasRef && (m_messageEventCount > 0 || m_wantsExplicitRef) && !m_isDetached; + if (shouldRef == m_isRefingEventLoop) + return; + auto* context = scriptExecutionContext(); + if (!context) { + m_isRefingEventLoop = false; + return; + } + m_isRefingEventLoop = shouldRef; + if (shouldRef) + context->refEventLoop(); + else + context->unrefEventLoop(); +} + void MessagePort::onDidChangeListenerImpl(EventTarget& self, const AtomString& eventType, OnDidChangeListenerKind kind) { if (eventType != eventNames().messageEvent) return; auto& port = static_cast(self); - auto* context = port.scriptExecutionContext(); switch (kind) { case Add: - if (port.m_messageEventCount == 0 && context) - context->refEventLoop(); + // Node's setupPortReferencing installs 'newListener'/'removeListener' + // hooks that call this.ref() on the first 'message' listener and + // this.unref() when the last one is removed. Mirror that so + // port.unref(); port.on('message', fn) re-refs, and + // port.ref(); port.on(...); port.off(...) un-refs — both matching Node. + if (port.m_messageEventCount == 0) + port.m_hasRef = true; port.m_messageEventCount++; break; case Remove: port.m_messageEventCount--; - if (port.m_messageEventCount == 0 && context) - context->unrefEventLoop(); + if (port.m_messageEventCount == 0) { + port.m_hasRef = false; + port.m_wantsExplicitRef = false; + } break; case Clear: - if (port.m_messageEventCount > 0 && context) - context->unrefEventLoop(); + if (port.m_messageEventCount > 0) { + port.m_hasRef = false; + port.m_wantsExplicitRef = false; + } port.m_messageEventCount = 0; break; } + port.updateEventLoopRef(); } bool MessagePort::addEventListener(const AtomString& eventType, Ref&& listener, const AddEventListenerOptions& options) @@ -329,29 +343,20 @@ WebCoreOpaqueRoot root(MessagePort* port) return WebCoreOpaqueRoot { port }; } -void MessagePort::jsRef(JSGlobalObject* lexicalGlobalObject) +void MessagePort::jsRef(JSGlobalObject*) { - // A closed or transferred-away port can never receive messages again, so - // taking a self-ref (and an event-loop ref) here would only leak: - // close()/disentangle() have already run and nothing will ever release a - // ref taken afterwards. - if (!isEntangled()) - return; - - if (!m_hasRef) { - m_hasRef = true; - ref(); - Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(lexicalGlobalObject->vm())->bunVM, 1); - } + // updateEventLoopRef()'s predicate already gates on !m_isDetached, so a + // closed or transferred-away port cannot take an event-loop ref here. + m_hasRef = true; + m_wantsExplicitRef = true; + updateEventLoopRef(); } -void MessagePort::jsUnref(JSGlobalObject* lexicalGlobalObject) +void MessagePort::jsUnref(JSGlobalObject*) { - if (m_hasRef) { - m_hasRef = false; - deref(); - Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(lexicalGlobalObject->vm())->bunVM, -1); - } + m_hasRef = false; + m_wantsExplicitRef = false; + updateEventLoopRef(); } } // namespace WebCore diff --git a/src/jsc/bindings/webcore/MessagePort.h b/src/jsc/bindings/webcore/MessagePort.h index 8ec7e4dfbb34..f835b75e043b 100644 --- a/src/jsc/bindings/webcore/MessagePort.h +++ b/src/jsc/bindings/webcore/MessagePort.h @@ -107,7 +107,7 @@ class MessagePort final : public ContextDestructionObserver, public EventTarget, void jsRef(JSGlobalObject*); void jsUnref(JSGlobalObject*); - bool jsHasRef() { return m_hasRef; } + bool jsHasRef() { return m_isRefingEventLoop; } private: MessagePort(ScriptExecutionContext&, Ref&&, uint8_t side); @@ -128,9 +128,26 @@ class MessagePort final : public ContextDestructionObserver, public EventTarget, bool m_started { false }; bool m_isDetached { false }; bool m_hasMessageEventListener { false }; - bool m_hasRef { false }; + + // Event-loop ref state machine. A port holds a single event-loop ref iff + // m_hasRef && (m_messageEventCount > 0 || m_wantsExplicitRef) && !m_isDetached + // updateEventLoopRef() reconciles m_isRefingEventLoop with that predicate. + // + // m_hasRef: default true. .unref() clears, .ref() sets; on transferred ports the + // first/last 'message' listener also sets/clears it (mirroring Node's + // setupPortReferencing newListener/removeListener → this.ref()/this.unref()). So + // on(); unref() stays unref'd, while unref(); on() re-refs — both matching Node. + // m_wantsExplicitRef: set by .ref() or onmessage=fn (so .ref() on a fresh port refs + // even without a listener); cleared by .unref(), onmessage=null, or removing the + // last 'message' listener on a transferred port. + // m_messageEventCount: only tracked for transferred ports (onDidChangeListener wired in + // entangle()); fresh ports don't hold the process open via listeners alone. + bool m_hasRef { true }; + bool m_wantsExplicitRef { false }; + bool m_isRefingEventLoop { false }; uint32_t m_messageEventCount { 0 }; + void updateEventLoopRef(); static void onDidChangeListenerImpl(EventTarget& self, const AtomString& eventType, OnDidChangeListenerKind kind); }; diff --git a/src/jsc/bindings/webcore/MessagePortPipe.cpp b/src/jsc/bindings/webcore/MessagePortPipe.cpp index a782916564c0..0123100aa249 100644 --- a/src/jsc/bindings/webcore/MessagePortPipe.cpp +++ b/src/jsc/bindings/webcore/MessagePortPipe.cpp @@ -230,6 +230,7 @@ void MessagePortPipe::close(uint8_t side) while (!worklist.isEmpty()) { auto [pipe, sd] = worklist.takeLast(); auto& s = pipe->m_sides[sd]; + auto& peer = pipe->m_sides[1 - sd]; Deque dropped; { @@ -240,6 +241,15 @@ void MessagePortPipe::close(uint8_t side) s.state.store(Closed, std::memory_order_release); dropped = std::exchange(s.inbox, {}); } + { + // Mirror Closed into the peer's state word so its + // hasPendingActivity() — which must be a single atomic load to + // avoid observing a torn {before-send, after-close} snapshot — + // can see it. fetch_or under the peer's lock since all state + // writes are lock-guarded. + Locker locker { peer.lock }; + peer.state.fetch_or(PeerClosed, std::memory_order_acq_rel); + } // Harvest transferred pipes before `dropped` destructs so their // ~TransferredMessagePort sees pipe == nullptr and is a no-op. diff --git a/src/jsc/bindings/webcore/MessagePortPipe.h b/src/jsc/bindings/webcore/MessagePortPipe.h index ff421a0952ba..ded98fae59c0 100644 --- a/src/jsc/bindings/webcore/MessagePortPipe.h +++ b/src/jsc/bindings/webcore/MessagePortPipe.h @@ -44,11 +44,21 @@ class MessagePortPipe final : public ThreadSafeRefCounted { Closed = 1ull << 0, // close() was called on this side; drops further deliveries. DrainScheduled = 1ull << 1, // a drain task for this side is in flight. Attached = 1ull << 2, // ctxId/port are valid; ok to schedule drains. + // Mirror of the peer's Closed bit. Lets hasPendingActivity() read a + // consistent snapshot from a single atomic load — otherwise the GC + // could observe {queuedCount=0, !DrainScheduled} from our side before + // the peer's send, then observe Closed from the peer's side after its + // close, and collect the wrapper with a message in flight. + PeerClosed = 1ull << 3, QueuedShift = 8, QueuedOne = 1ull << QueuedShift, }; static constexpr uint64_t queuedCount(uint64_t s) { return s >> QueuedShift; } + // True while the port attached to this side should be kept alive by the + // GC: there is a message queued for it, a drain task is in the + // pop→dispatch window, or the peer could still send more. + static constexpr bool isActivityPending(uint64_t s) { return queuedCount(s) > 0 || (s & DrainScheduled) || !(s & PeerClosed); } // Sender-thread operations. // `fromSide` is the sender's side; the message lands in the *other* side's inbox. @@ -66,7 +76,6 @@ class MessagePortPipe final : public ThreadSafeRefCounted { // Lockless snapshot for the GC visitor / hasPendingActivity. uint64_t state(uint8_t side) const { return m_sides[side].state.load(std::memory_order_acquire); } - bool isOtherSideOpen(uint8_t side) const { return !(state(1 - side) & Closed); } // Equality is by identity; used to reject "port posted through itself". bool operator==(const MessagePortPipe& other) const { return this == &other; } diff --git a/src/jsc/bindings/webcore/Worker.cpp b/src/jsc/bindings/webcore/Worker.cpp index 1ed3695f72fb..a0e5c4655a72 100644 --- a/src/jsc/bindings/webcore/Worker.cpp +++ b/src/jsc/bindings/webcore/Worker.cpp @@ -94,6 +94,11 @@ void WebWorker__releaseParentPollRef(void* worker); // Free the native WebWorker struct. Called from ~Worker. void WebWorker__destroy(void* worker); +// Drain-and-drop the worker VM's pending concurrent-task queue. Called from +// teardownJSCVM on the worker thread, after the context is unregistered and +// before JSC teardown. See the comment at the call site. +void Bun__dropConcurrentCppTasksForWorker(Zig::GlobalObject* globalObject); + } // extern "C" // ------------------------------------------------------------------------------------------------- @@ -613,6 +618,29 @@ extern "C" void WebWorker__teardownJSCVM(Zig::GlobalObject* globalObject) globalObject->requireMap()->clear(globalObject); scope.exception(); // TODO: handle or assert none? vm.deleteAllCode(JSC::DeleteAllCodeEffort::PreventCollectionAndDeleteAllCode); + // Take the context out of the global map now rather than relying on + // the collect below to finalize ~GlobalObject and do it. That collect + // is best-effort: any JSC::Strong, conservative stack root, or DOM + // object with hasPendingActivity (e.g. an entangled MessagePort with + // a listener) keeps the global marked, so ~GlobalObject may not run + // before shutdown() proceeds to vm.deinit() and sets has_terminated. + // Removing the entry here makes postTaskTo() return false instead. + // ~GlobalObject's later removeFromContextsMap() is a no-op once + // m_isInContextsMap is cleared. + if (auto* ctx = globalObject->scriptExecutionContext()) + ctx->removeFromContextsMap(); + // With the context out of the map, no new cross-thread posts can + // arrive (postTaskTo holds the map lock across lookup+enqueue). + // Drain-and-drop whatever is already sitting in the Rust EventLoop's + // concurrent_tasks queue so the ConcurrentTask boxes and their + // captured EventLoopTask* payloads (Ref, + // Ref, ...) are released while this JSC VM is still alive. + // The main-thread exit path does the equivalent in global_exit(); + // workers had no such hook, so any postTaskTo landing between the + // last tick_concurrent() and the map removal above leaked under + // LSAN (BroadcastChannel across a tree of workers makes that window + // trivially reachable in test-worker-messaging.js). + Bun__dropConcurrentCppTasksForWorker(globalObject); gcUnprotect(globalObject); globalObject = nullptr; } @@ -701,6 +729,7 @@ JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) auto scope = DECLARE_THROW_SCOPE(globalObject->vm()); JSValue workerData = jsNull(); JSValue threadId = jsNumber(0); + JSValue mainThreadPort = jsUndefined(); JSMap* environmentData = nullptr; if (auto* worker = WebWorker__getParentWorker(globalObject->bunVM())) { @@ -716,17 +745,20 @@ JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) if (serialized) { JSValue deserialized = serialized->deserialize(*globalObject, globalObject, WTF::move(ports)); RETURN_IF_EXCEPTION(scope, {}); - // Should always be set to an Array of length 2 in the constructor in JSWorker.cpp + // Should always be set to an Array of length 3 in the constructor in JSWorker.cpp if (auto* pair = dynamicDowncast(deserialized)) { - ASSERT(pair->length() == 2); + ASSERT(pair->length() == 3); ASSERT(pair->canGetIndexQuickly(0u)); ASSERT(pair->canGetIndexQuickly(1u)); + ASSERT(pair->canGetIndexQuickly(2u)); workerData = pair->getIndexQuickly(0); RETURN_IF_EXCEPTION(scope, {}); auto environmentDataValue = pair->getIndexQuickly(1); // it might not be a Map if the parent had not set up environmentData yet environmentData = environmentDataValue ? dynamicDowncast(environmentDataValue) : nullptr; RETURN_IF_EXCEPTION(scope, {}); + mainThreadPort = pair->getIndexQuickly(2); + RETURN_IF_EXCEPTION(scope, {}); } else { ASSERT_NOT_REACHED_WITH_MESSAGE("createNodeWorkerThreadsBinding: deserialized is not JSArray"); } @@ -742,12 +774,13 @@ JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) ASSERT(environmentData); globalObject->setNodeWorkerEnvironmentData(environmentData); - JSObject* array = constructEmptyArray(globalObject, nullptr, 4); + JSObject* array = constructEmptyArray(globalObject, nullptr, 5); RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 0, workerData); array->putDirectIndex(globalObject, 1, threadId); array->putDirectIndex(globalObject, 2, JSFunction::create(vm, globalObject, 1, "receiveMessageOnPort"_s, jsReceiveMessageOnPort, ImplementationVisibility::Public, NoIntrinsic)); array->putDirectIndex(globalObject, 3, environmentData); + array->putDirectIndex(globalObject, 4, mainThreadPort); return array; } diff --git a/src/jsc/bindings/webcore/WorkerOptions.h b/src/jsc/bindings/webcore/WorkerOptions.h index 3feed8512c4e..29d7e7b4d64e 100644 --- a/src/jsc/bindings/webcore/WorkerOptions.h +++ b/src/jsc/bindings/webcore/WorkerOptions.h @@ -24,10 +24,11 @@ struct WorkerOptions { // Blob URL. bool evalMode { false }; Kind kind { Kind::Web }; - // Serialized array containing [workerData, environmentData] - // (environmentData is always a Map) + // Serialized array containing [workerData, environmentData, mainThreadPort] + // (environmentData is always a Map; mainThreadPort is the worker's end of the + // postMessageToThread channel, or undefined for Web workers) RefPtr workerDataAndEnvironmentData; - // Objects transferred for either data or environmentData in the transferList + // Objects transferred for data, environmentData, or mainThreadPort in the transferList Vector dataMessagePorts; Vector preloadModules; std::optional> env; // TODO(@190n) allow shared diff --git a/src/jsc/virtual_machine_exports.rs b/src/jsc/virtual_machine_exports.rs index cacc80863c1d..17e59c0036e3 100644 --- a/src/jsc/virtual_machine_exports.rs +++ b/src/jsc/virtual_machine_exports.rs @@ -136,6 +136,29 @@ pub fn queue_task_concurrently(global: &JSGlobalObject, task: *mut crate::cpp_ta } } +/// Drain-and-drop this VM's `concurrent_tasks` queue during worker shutdown. +/// Called from `WebWorker__teardownJSCVM` immediately after the +/// ScriptExecutionContext is removed from the global map (so no new posts +/// can arrive) and before `gcUnprotect`/`collectNow` (so the freed lambdas' +/// captured `Ref<>`s are released while the JSC VM is still alive). Without +/// this, any `postTaskTo(workerCtxId, …)` that lands between the worker's +/// last `tick_concurrent()` and context-map removal leaks a `ConcurrentTask` +/// box plus its `EventLoopTask*` payload: `EventLoop::deinit()` walks only +/// `self.tasks`, and the raw `dealloc` of the VM box skips field `Drop`s. +/// The main-thread exit path already does this in `global_exit()`; workers +/// have no equivalent hook, hence this shim. +// HOST_EXPORT(Bun__dropConcurrentCppTasksForWorker, c) +pub fn drop_concurrent_cpp_tasks_for_worker(global: &JSGlobalObject) { + crate::mark_binding!(); + // SAFETY: called on the worker's own thread from inside `shutdown()` + // with the context already out of the global map (sole producer path + // closed) and JSC still live; `bun_vm()` is the thread-local handle, + // `event_loop()` never null for a Bun VM. + unsafe { + (*global.bun_vm().event_loop()).drop_concurrent_cpp_tasks(); + } +} + // HOST_EXPORT(Bun__handleRejectedPromise, c) pub fn handle_rejected_promise(global: &JSGlobalObject, promise: &mut JSPromise) { crate::mark_binding!(); diff --git a/test/js/node/test/parallel/test-worker-messaging-errors-handler.js b/test/js/node/test/parallel/test-worker-messaging-errors-handler.js new file mode 100644 index 000000000000..a4a0ae033d90 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-messaging-errors-handler.js @@ -0,0 +1,34 @@ +'use strict'; + +const common = require('../common'); +const { + parentPort, + postMessageToThread, + Worker, + workerData, +} = require('node:worker_threads'); +const assert = require('node:assert'); + +async function test() { + const worker = new Worker(__filename, { workerData: { children: true } }); + + await assert.rejects(common.mustCall(function() { + return postMessageToThread(worker.threadId); + }), { + name: 'Error', + code: 'ERR_WORKER_MESSAGING_ERRORED', + }); + + worker.postMessage('success'); +} + +if (!workerData?.children) { + test(); +} else { + process.on('workerMessage', () => { + throw new Error('KABOOM'); + }); + + parentPort.postMessage('ready'); + parentPort.once('message', common.mustCall()); +} diff --git a/test/js/node/test/parallel/test-worker-messaging-errors-invalid.js b/test/js/node/test/parallel/test-worker-messaging-errors-invalid.js new file mode 100644 index 000000000000..e55e3485e904 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-messaging-errors-invalid.js @@ -0,0 +1,48 @@ +'use strict'; + +const common = require('../common'); +const { once } = require('node:events'); +const { + parentPort, + postMessageToThread, + threadId, + Worker, + workerData, +} = require('node:worker_threads'); +const assert = require('node:assert'); + +async function test() { + await assert.rejects(common.mustCall(function() { + return postMessageToThread(threadId); + }), { + name: 'Error', + code: 'ERR_WORKER_MESSAGING_SAME_THREAD', + }); + + await assert.rejects(common.mustCall(function() { + return postMessageToThread(Date.now()); + }), { + name: 'Error', + code: 'ERR_WORKER_MESSAGING_FAILED', + }); + + // The delivery to the first worker will fail as there is no listener for `workerMessage` + const worker = new Worker(__filename, { workerData: { children: true } }); + await once(worker, 'message'); + + await assert.rejects(common.mustCall(function() { + return postMessageToThread(worker.threadId); + }), { + name: 'Error', + code: 'ERR_WORKER_MESSAGING_FAILED', + }); + + worker.postMessage('success'); +} + +if (!workerData?.children) { + test(); +} else { + parentPort.postMessage('ready'); + parentPort.once('message', common.mustCall()); +} diff --git a/test/js/node/test/parallel/test-worker-messaging-errors-timeout.js b/test/js/node/test/parallel/test-worker-messaging-errors-timeout.js new file mode 100644 index 000000000000..f069005c4110 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-messaging-errors-timeout.js @@ -0,0 +1,38 @@ +'use strict'; + +const common = require('../common'); +const { + postMessageToThread, + workerData, + Worker, +} = require('node:worker_threads'); +const assert = require('node:assert'); + +const memory = new SharedArrayBuffer(4); + +async function test() { + const worker = new Worker(__filename, { workerData: { memory, children: true } }); + const array = new Int32Array(memory); + + await assert.rejects(common.mustCall(function() { + return postMessageToThread(worker.threadId, 0, common.platformTimeout(500)); + }), { + name: 'Error', + code: 'ERR_WORKER_MESSAGING_TIMEOUT', + }); + + Atomics.store(array, 0, 1); + Atomics.notify(array, 0); +} + +if (!workerData?.children) { + test(); +} else { + process.on('beforeExit', common.mustCall()); + + const array = new Int32Array(workerData.memory); + + // Starve this thread waiting for the status to be unlocked. + // This happens in the main thread AFTER the timeout. + Atomics.wait(array, 0, 0); +} diff --git a/test/js/node/test/parallel/test-worker-messaging.js b/test/js/node/test/parallel/test-worker-messaging.js new file mode 100644 index 000000000000..e29258bea130 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-messaging.js @@ -0,0 +1,112 @@ +'use strict'; + +const common = require('../common'); +const Countdown = require('../common/countdown'); +const { + parentPort, + postMessageToThread, + threadId, + workerData, + Worker, +} = require('node:worker_threads'); +const assert = require('node:assert'); +const { once } = require('node:events'); + +// Spawn threads on three levels: 1 main thread, two children, four grand childrens. 7 threads total, max id = 6 +const MAX_LEVEL = 2; +const MAX_THREAD = 6; + +// This is to allow the test to run in --worker mode +const mainThread = workerData?.mainThread ?? threadId; +const level = workerData?.level ?? 0; + +const channel = new BroadcastChannel('nodejs:test-worker-connection'); +let completed; + +if (level === 0) { + completed = new Countdown(MAX_THREAD + 1, () => { + channel.postMessage('exit'); + channel.close(); + }); +} + +async function createChildren() { + const worker = new Worker(__filename, { workerData: { mainThread, level: level + 1 } }); + await once(worker, 'message'); +} + +async function ping() { + let target; + do { + target = mainThread + Math.floor(Math.random() * MAX_THREAD); + } while (target === threadId); + + const { port1, port2 } = new MessageChannel(); + await postMessageToThread(target, { level, port: port2 }, [port2]); + + port1.on('message', common.mustCall(function(message) { + assert.deepStrictEqual(message, { message: 'pong', source: target, destination: threadId }); + port1.close(); + + if (level === 0) { + completed.dec(); + } else { + channel.postMessage('end'); + } + })); + + port1.postMessage({ message: 'ping', source: threadId, destination: target }); +} + +// Do not use mustCall here as the thread might not receive any connection request +process.on('workerMessage', common.mustCallAtLeast(({ port, level }, source) => { + // Let's verify the source hierarchy + // Given we do depth first, the level is 1 for thread 1 and 4, 2 for other threads + if (source !== mainThread) { + const currentThread = source - mainThread; + assert.strictEqual(level, (currentThread === 1 || currentThread === 4) ? 1 : 2); + } else { + assert.strictEqual(level, 0); + } + + // Verify communication + port.on('message', common.mustCall(function(message) { + assert.deepStrictEqual(message, { message: 'ping', source, destination: threadId }); + port.postMessage({ message: 'pong', source: threadId, destination: source }); + port.close(); + })); +}, 0)); + +async function test() { + if (level < MAX_LEVEL) { + await createChildren(); + await createChildren(); + } + + channel.onmessage = function(message) { + switch (message.data) { + case 'start': + ping(); + break; + case 'end': + if (level === 0) { + completed.dec(); + } + break; + case 'exit': + channel.close(); + break; + } + }; + + if (level > 0) { + const currentThread = threadId - mainThread; + assert.strictEqual(level, (currentThread === 1 || currentThread === 4) ? 1 : 2); + parentPort.postMessage({ type: 'ready', threadId }); + } else { + channel.postMessage('start'); + ping(); + } +} + +test(); diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index 9d4958118148..d19e10270105 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -1,3 +1,4 @@ +import { describe, expect, it, setDefaultTimeout, test } from "bun:test"; import { bunEnv, bunExe, tmpdirSync } from "harness"; import { once } from "node:events"; import fs from "node:fs"; @@ -21,6 +22,10 @@ import wt, { workerData, } from "worker_threads"; +// Many tests here spawn subprocesses or workers that each take 1-3s in debug/ASAN builds, +// and the default 5s per-test timeout is too tight under load. +setDefaultTimeout(30_000); + test("support eval in worker", async () => { const worker = new Worker(`postMessage(1 + 1)`, { eval: true, @@ -38,6 +43,7 @@ test("all worker_threads module properties are present", () => { expect(wt).toHaveProperty("markAsUntransferable"); expect(wt).toHaveProperty("moveMessagePortToContext"); expect(wt).toHaveProperty("parentPort"); + expect(wt).toHaveProperty("postMessageToThread"); expect(wt).toHaveProperty("receiveMessageOnPort"); expect(wt).toHaveProperty("resourceLimits"); expect(wt).toHaveProperty("SHARE_ENV"); @@ -266,17 +272,18 @@ describe("execArgv option", async () => { expect(await proc.stdout.text()).toBe(expected); } + // Each run() spawns a --smol subprocess that creates a worker; slow in debug builds. it("inherits the parent's execArgv when falsy or unspecified", async () => { await run("null", '["--smol"]\n'); await run("0", '["--smol"]\n'); - }); + }, 30_000); it("provides empty execArgv when passed an empty array", async () => { // empty array should result in empty execArgv, not inherited from parent thread await run("[]", "[]\n"); - }); + }, 15_000); it("can specify an array of strings", async () => { await run('["--no-warnings"]', '["--no-warnings"]\n'); - }); + }, 15_000); // TODO(@190n) get our handling of non-string array elements in line with Node's }); @@ -292,7 +299,7 @@ test("eval does not leak source code", async () => { const errors = await proc.stderr.text(); if (errors.length > 0) throw new Error(errors); expect(proc.exitCode).toBe(0); -}); +}, 60_000); // Spawns six workers with 100 MiB of source each; slow in debug builds. describe("worker event", () => { test("is emitted on the next tick with the right value", () => { @@ -384,7 +391,7 @@ describe("environmentData", () => { expect(proc.exitCode).toBe(0); const out = await proc.stdout.text(); expect(out).toBe("foo\n".repeat(5)); - }); + }, 30_000); // Five nested workers; slow in debug builds. test("can be used if parent thread had not imported worker_threads", async () => { const proc = Bun.spawn({ @@ -398,7 +405,7 @@ describe("environmentData", () => { const errors = await proc.stderr.text(); if (errors.length > 0) throw new Error(errors); expect(proc.exitCode).toBe(0); - }); + }, 30_000); // Two nested workers; slow in debug builds. }); describe("error event", () => { @@ -488,6 +495,241 @@ describe("getHeapSnapshot", () => { }); }); +describe("postMessageToThread", () => { + test("is exported", () => { + expect(wt).toHaveProperty("postMessageToThread"); + expect(wt.postMessageToThread).toBeFunction(); + }); + + test("rejects when targeting the same thread", async () => { + await expect(wt.postMessageToThread(threadId)).rejects.toMatchObject({ + name: "Error", + code: "ERR_WORKER_MESSAGING_SAME_THREAD", + }); + }); + + test("rejects when targeting an unknown thread", async () => { + await expect(wt.postMessageToThread(2 ** 30)).rejects.toMatchObject({ + name: "Error", + code: "ERR_WORKER_MESSAGING_FAILED", + }); + }); + + test("delivers to workerMessage listener in worker and back", async () => { + const worker = new Worker( + /* js */ ` + const wt = require("node:worker_threads"); + process.on("workerMessage", (value, source) => { + wt.postMessageToThread(source, { echo: value, from: wt.threadId }); + }); + wt.parentPort.postMessage("ready"); + wt.parentPort.once("message", () => {}); + `, + { eval: true }, + ); + const onWorkerMessage = (value, source) => resolve({ value, source }); + const { promise, resolve } = Promise.withResolvers(); + try { + await once(worker, "message"); + process.on("workerMessage", onWorkerMessage); + await wt.postMessageToThread(worker.threadId, "hello"); + const { value, source } = await promise; + expect(value).toEqual({ echo: "hello", from: worker.threadId }); + expect(source).toBe(worker.threadId); + } finally { + process.removeListener("workerMessage", onWorkerMessage); + worker.postMessage("done"); + await worker.terminate(); + } + }); + + test("rejects with ERR_WORKER_MESSAGING_FAILED when worker has no listener", async () => { + const worker = new Worker( + /* js */ ` + const wt = require("node:worker_threads"); + wt.parentPort.postMessage("ready"); + wt.parentPort.once("message", () => {}); + `, + { eval: true }, + ); + try { + await once(worker, "message"); + await expect(wt.postMessageToThread(worker.threadId, "hello")).rejects.toMatchObject({ + name: "Error", + code: "ERR_WORKER_MESSAGING_FAILED", + }); + } finally { + worker.postMessage("done"); + await worker.terminate(); + } + }); + + test("rejects with ERR_WORKER_MESSAGING_ERRORED when handler throws", async () => { + const worker = new Worker( + /* js */ ` + const wt = require("node:worker_threads"); + process.on("workerMessage", () => { throw new Error("boom"); }); + wt.parentPort.postMessage("ready"); + wt.parentPort.once("message", () => {}); + `, + { eval: true }, + ); + try { + await once(worker, "message"); + await expect(wt.postMessageToThread(worker.threadId, "hello")).rejects.toMatchObject({ + name: "Error", + code: "ERR_WORKER_MESSAGING_ERRORED", + }); + } finally { + worker.postMessage("done"); + await worker.terminate(); + } + }); + + test("rejects with ERR_WORKER_MESSAGING_FAILED for an exited worker's threadId", async () => { + // The worker's port registration must be torn down on exit; previously + // #onClose read the native threadId (which is -1 once the worker is + // closing) and the stale port stayed in the map, so this would hang + // until the timeout instead of failing immediately. + const worker = new Worker("require('node:worker_threads')", { eval: true }); + const id = worker.threadId; + await once(worker, "exit"); + expect(worker.threadId).toBe(-1); + await expect(wt.postMessageToThread(id, "hello")).rejects.toMatchObject({ + name: "Error", + code: "ERR_WORKER_MESSAGING_FAILED", + }); + }); +}); + +test("process.emit returns false when there are no listeners", () => { + // process uses a native EventEmitter; it used to always return true. + expect(process.emit("__bun_test_no_listener_event__")).toBe(false); + let called = false; + process.once("__bun_test_with_listener_event__", () => { + called = true; + }); + expect(process.emit("__bun_test_with_listener_event__")).toBe(true); + expect(called).toBe(true); +}); + +test("GC of a ref'd MessagePort whose peer closed releases its event-loop ref", async () => { + // port1.onmessage = fn takes an event-loop ref. port2.close() sets PeerClosed on + // port1's pipe side so hasPendingActivity() → false and port1's wrapper is + // collectible. ~MessagePort() used to not release the event-loop ref → hang. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* js */ ` + (() => { + const { port1, port2 } = new MessageChannel(); + port1.onmessage = () => {}; + port2.close(); + })(); + Bun.gc(true); + setTimeout(() => { Bun.gc(true); console.log("DONE"); }, 50); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("DONE"); + expect(exitCode).toBe(0); +}); + +test("transferring a ref'd MessagePort releases its event-loop ref on the source thread", async () => { + // port1.onmessage = fn takes an event-loop ref; transferring port1 detaches it from the + // source context. disentangle() used to leave that ref behind, so the source process hung. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* js */ ` + const { Worker } = require("node:worker_threads"); + const { port1 } = new MessageChannel(); + port1.onmessage = () => {}; + const w = new Worker("setTimeout(() => {}, 100)", { ev${/* bundler hates eval */ ""}al: true }); + w.postMessage({ p: port1 }, [port1]); + w.unref(); + setTimeout(() => console.log("DONE"), 50); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("DONE"); + expect(exitCode).toBe(0); +}); + +test("on() after unref() on a transferred port re-refs (Node's newListener hook)", async () => { + // Node's setupPortReferencing installs a 'newListener' hook that calls + // this.ref() on the first 'message' listener, so port.unref(); port.on('message', fn) + // re-refs and the worker stays alive. The reverse order (on; unref) stays unref'd. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* js */ ` + const { Worker, workerData } = require("node:worker_threads"); + const { once } = require("node:events"); + const { port1, port2 } = new MessageChannel(); + const w = new Worker( + "const p = require('node:worker_threads').workerData.port;" + + "p.unref(); p.on('message', m => { console.log('got:' + m); p.close(); });" + + "require('node:worker_threads').parentPort.postMessage(p.hasRef());", + { ev${/* bundler hates eval */ ""}al: true, workerData: { port: port2 }, transferList: [port2] }, + ); + once(w, "message").then(([hasRef]) => { + console.log("hasRef:" + hasRef); + setTimeout(() => port1.postMessage("hi"), 50); + }); + w.on("exit", c => { console.log("exit:" + c); port1.close(); }); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout.trim().split("\n").sort()).toEqual(["exit:0", "got:hi", "hasRef:true"]); + expect(exitCode).toBe(0); +}); + +test("worker does not stay alive after unref() on a transferred port with a listener", async () => { + // A transferred MessagePort with a 'message' listener used to hold a separate + // event loop ref that .unref() could not release, keeping the worker alive forever. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* js */ ` + const { Worker, workerData } = require("node:worker_threads"); + const { port1, port2 } = new MessageChannel(); + const w = new Worker( + "const p = require('node:worker_threads').workerData.port; p.addEventListener('message', () => {}); p.unref();", + { ev${/* bundler hates eval */ ""}al: true, workerData: { port: port2 }, transferList: [port2] }, + ); + w.on("exit", code => { console.log("exit", code); port1.close(); }); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("exit 0"); + expect(exitCode).toBe(0); +}); + test("failed Worker construction restores transferred FileHandles", async () => { const dir = tmpdirSync("worker-fh-transfer"); const file = join(dir, "x.txt"); diff --git a/test/js/web/workers/message-port-pipe.test.ts b/test/js/web/workers/message-port-pipe.test.ts index e928eb4f11bd..da9ad4cab599 100644 --- a/test/js/web/workers/message-port-pipe.test.ts +++ b/test/js/web/workers/message-port-pipe.test.ts @@ -250,7 +250,7 @@ describe("MessagePort pipe", () => { } for (let i = 0; i < 10; i++) { Bun.gc(true); await Bun.sleep(0); } // If dropped-in-transit endpoints weren't closed, every D would - // be pinned via isOtherSideOpen and finalized.count would be 0. + // be pinned via isActivityPending (!PeerClosed) and finalized.count would be 0. console.log(JSON.stringify({ finalized: finalized.count })); A.close(); process.exit(0); diff --git a/test/no-validate-leaksan.txt b/test/no-validate-leaksan.txt index 873cc966375f..9724892f4c17 100644 --- a/test/no-validate-leaksan.txt +++ b/test/no-validate-leaksan.txt @@ -34,6 +34,10 @@ test/js/node/test/parallel/test-worker-message-port-transfer-duplicate.js test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js test/js/node/test/parallel/test-worker-message-port-wasm-module.js test/js/node/test/parallel/test-worker-message-port-wasm-threads.js +test/js/node/test/parallel/test-worker-messaging.js +test/js/node/test/parallel/test-worker-messaging-errors-handler.js +test/js/node/test/parallel/test-worker-messaging-errors-invalid.js +test/js/node/test/parallel/test-worker-messaging-errors-timeout.js test/js/node/test/parallel/test-worker-mjs-workerdata.js test/js/node/test/parallel/test-worker-nested-on-process-exit.js test/js/node/test/parallel/test-worker-nested-uncaught.js