From 27230414bc1facf1c0e298edac1ebc5926aeff7e Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 1 Apr 2025 14:16:09 -0700 Subject: [PATCH 01/88] Emit online event before Worker entrypoint runs --- src/bun.js/web_worker.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index a8b6295520ce01..2a063b884b0ff7 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -403,6 +403,7 @@ pub const WebWorker = struct { assert(this.status.load(.acquire) == .start); this.setStatus(.starting); vm.preload = this.preloads; + WebWorker__dispatchOnline(this.cpp_worker, vm.global); var promise = vm.loadEntryPointForWebWorker(this.specifier) catch { this.flushLogs(); this.exitAndDeinit(); @@ -423,7 +424,6 @@ pub const WebWorker = struct { this.flushLogs(); log("[{d}] event loop start", .{this.execution_context_id}); - WebWorker__dispatchOnline(this.cpp_worker, vm.global); this.setStatus(.running); // don't run the GC if we don't actually need to From 8f3786d533029485a107449c13c8c1a293552f6e Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 3 Apr 2025 11:38:56 -0700 Subject: [PATCH 02/88] Accept callback in MessagePort.close --- src/bun.js/bindings/webcore/JSMessagePort.cpp | 4 ++-- src/bun.js/bindings/webcore/MessagePort.cpp | 17 ++++++++++++++--- src/bun.js/bindings/webcore/MessagePort.h | 2 +- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/bun.js/bindings/webcore/JSMessagePort.cpp b/src/bun.js/bindings/webcore/JSMessagePort.cpp index 45e80a8ba5ff3d..52431e15fb788d 100644 --- a/src/bun.js/bindings/webcore/JSMessagePort.cpp +++ b/src/bun.js/bindings/webcore/JSMessagePort.cpp @@ -326,10 +326,10 @@ static inline JSC::EncodedJSValue jsMessagePortPrototypeFunction_closeBody(JSC:: auto& vm = JSC::getVM(lexicalGlobalObject); auto throwScope = DECLARE_THROW_SCOPE(vm); UNUSED_PARAM(throwScope); - UNUSED_PARAM(callFrame); + auto callback = callFrame->argument(0); auto& impl = castedThis->wrapped(); impl.jsUnref(lexicalGlobalObject); - RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS(*lexicalGlobalObject, throwScope, [&]() -> decltype(auto) { return impl.close(); }))); + RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS(*lexicalGlobalObject, throwScope, [&]() -> decltype(auto) { return impl.close(lexicalGlobalObject, callback); }))); } JSC_DEFINE_HOST_FUNCTION(jsMessagePortPrototypeFunction_close, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame)) diff --git a/src/bun.js/bindings/webcore/MessagePort.cpp b/src/bun.js/bindings/webcore/MessagePort.cpp index 6c3b67c5f59b41..2ddba751216278 100644 --- a/src/bun.js/bindings/webcore/MessagePort.cpp +++ b/src/bun.js/bindings/webcore/MessagePort.cpp @@ -28,6 +28,8 @@ #include "MessagePort.h" #include "BunClientData.h" +#include "BunProcess.h" +#include "AsyncContextFrame.h" // #include "Document.h" #include "EventNames.h" // #include "Logging.h" @@ -143,7 +145,7 @@ MessagePort::~MessagePort() } if (m_entangled) - close(); + close(nullptr, {}); if (auto* context = scriptExecutionContext()) context->destroyedMessagePort(*this); @@ -232,7 +234,7 @@ void MessagePort::start() scriptExecutionContext()->processMessageWithMessagePortsSoon([pendingActivity = Ref { *this }] {}); } -void MessagePort::close() +void MessagePort::close(JSGlobalObject* lexicalGlobalObject, JSValue callback) { if (m_isDetached) return; @@ -241,6 +243,15 @@ void MessagePort::close() MessagePortChannelProvider::singleton().messagePortClosed(m_identifier); removeAllEventListeners(); + + if (lexicalGlobalObject && callback) { + auto* globalObject = defaultGlobalObject(lexicalGlobalObject); + if (globalObject && callback.isCallable()) { + if (auto* process = jsDynamicCast(globalObject->processObject())) { + process->queueNextTick(globalObject, AsyncContextFrame::withAsyncContextIfNeeded(globalObject, callback)); + } + } + } } void MessagePort::dispatchMessages() @@ -379,7 +390,7 @@ void MessagePort::contextDestroyed() { ASSERT(scriptExecutionContext()); - close(); + close(nullptr, {}); // ActiveDOMObject::contextDestroyed(); } diff --git a/src/bun.js/bindings/webcore/MessagePort.h b/src/bun.js/bindings/webcore/MessagePort.h index a5c9b8bc42c90b..98fff7bacb5f06 100644 --- a/src/bun.js/bindings/webcore/MessagePort.h +++ b/src/bun.js/bindings/webcore/MessagePort.h @@ -60,7 +60,7 @@ class MessagePort final : /* public ActiveDOMObject, */ public ContextDestructio ExceptionOr postMessage(JSC::JSGlobalObject&, JSC::JSValue message, StructuredSerializeOptions&&); void start(); - void close(); + void close(JSC::JSGlobalObject* lexicalGlobalObject, JSValue callback); void entangle(); // Returns nullptr if the passed-in vector is empty. From bc420e451776bb854c7ee876c999c19d17bca51d Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 7 Apr 2025 17:14:48 -0700 Subject: [PATCH 03/88] Pass test-worker-type-check.js --- src/bun.js/bindings/webcore/JSWorker.cpp | 11 ++++++- .../test/parallel/test-worker-type-check.js | 29 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 test/js/node/test/parallel/test-worker-type-check.js diff --git a/src/bun.js/bindings/webcore/JSWorker.cpp b/src/bun.js/bindings/webcore/JSWorker.cpp index 223aab42494006..87dfa63d6edcef 100644 --- a/src/bun.js/bindings/webcore/JSWorker.cpp +++ b/src/bun.js/bindings/webcore/JSWorker.cpp @@ -39,8 +39,10 @@ #include "JSDOMExceptionHandling.h" #include "JSDOMGlobalObjectInlines.h" #include "JSDOMOperation.h" +#include "JSDOMURL.h" #include "JSDOMWrapperCache.h" #include "JSEventListener.h" +#include "NodeValidator.h" #include "StructuredSerializeOptions.h" #include "JSWorkerOptions.h" #include "ScriptExecutionContext.h" @@ -123,7 +125,14 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: if (UNLIKELY(!context)) return throwConstructorScriptExecutionContextUnavailableError(*lexicalGlobalObject, throwScope, "Worker"_s); EnsureStillAliveScope argument0 = callFrame->uncheckedArgument(0); - auto scriptUrl = convert(*lexicalGlobalObject, argument0.value()); + String scriptUrl; + if (auto* url = jsDynamicCast(argument0.value())) { + scriptUrl = url->wrapped().href().string(); + } else if (argument0.value().isString()) { + scriptUrl = argument0.value().getString(lexicalGlobalObject); + } else { + return Bun::ERR::INVALID_ARG_TYPE(throwScope, lexicalGlobalObject, "filename"_s, "string or an instance of URL"_s, argument0.value()); + } RETURN_IF_EXCEPTION(throwScope, {}); EnsureStillAliveScope argument1 = callFrame->argument(1); diff --git a/test/js/node/test/parallel/test-worker-type-check.js b/test/js/node/test/parallel/test-worker-type-check.js new file mode 100644 index 00000000000000..9a718dfad055b4 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-type-check.js @@ -0,0 +1,29 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker_threads'); + +{ + [ + undefined, + null, + false, + 0, + Symbol('test'), + {}, + [], + () => {}, + ].forEach((val) => { + assert.throws( + () => new Worker(val), + { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError', + message: 'The "filename" argument must be of type string ' + + 'or an instance of URL.' + + common.invalidArgTypeHelper(val) + } + ); + }); +} From 354f2cc811fa876f909cf05c40d421edbc3e8093 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 7 Apr 2025 18:03:31 -0700 Subject: [PATCH 04/88] Use more appropriate #include for errors --- src/bun.js/bindings/webcore/JSWorker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/bindings/webcore/JSWorker.cpp b/src/bun.js/bindings/webcore/JSWorker.cpp index 87dfa63d6edcef..7fe8cef29a3915 100644 --- a/src/bun.js/bindings/webcore/JSWorker.cpp +++ b/src/bun.js/bindings/webcore/JSWorker.cpp @@ -42,7 +42,7 @@ #include "JSDOMURL.h" #include "JSDOMWrapperCache.h" #include "JSEventListener.h" -#include "NodeValidator.h" +#include "ErrorCode.h" #include "StructuredSerializeOptions.h" #include "JSWorkerOptions.h" #include "ScriptExecutionContext.h" From 638c4b4a880e4f45e621f54a24137e0fc279255d Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 7 Apr 2025 18:04:25 -0700 Subject: [PATCH 05/88] Correct receiveMessageOnPort error message --- src/bun.js/bindings/webcore/Worker.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index 38bcd175737296..1902785e4328fd 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -29,6 +29,7 @@ // #include "ContentSecurityPolicy.h" // #include "DedicatedWorkerGlobalScope.h" +#include "ErrorCode.h" #include "ErrorEvent.h" #include "Event.h" #include "EventNames.h" @@ -473,8 +474,7 @@ JSC_DEFINE_HOST_FUNCTION(jsReceiveMessageOnPort, (JSGlobalObject * lexicalGlobal auto port = callFrame->argument(0); if (!port.isObject()) { - throwTypeError(lexicalGlobalObject, scope, "the \"port\" argument must be a MessagePort instance"_s); - return {}; + return Bun::throwError(lexicalGlobalObject, scope, Bun::ErrorCode::ERR_INVALID_ARG_TYPE, "The \"port\" argument must be a MessagePort instance"_s); } if (auto* messagePort = jsDynamicCast(port)) { @@ -484,8 +484,7 @@ JSC_DEFINE_HOST_FUNCTION(jsReceiveMessageOnPort, (JSGlobalObject * lexicalGlobal return JSC::JSValue::encode(jsUndefined()); } - throwTypeError(lexicalGlobalObject, scope, "the \"port\" argument must be a MessagePort instance"_s); - return {}; + return Bun::throwError(lexicalGlobalObject, scope, Bun::ErrorCode::ERR_INVALID_ARG_TYPE, "The \"port\" argument must be a MessagePort instance"_s); } JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) From 9ba5b89eb496a6f07b337d23b06d7dbd2a123591 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 7 Apr 2025 18:04:39 -0700 Subject: [PATCH 06/88] Add passing test --- ...est-worker-message-port-receive-message.js | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 test/js/node/test/parallel/test-worker-message-port-receive-message.js diff --git a/test/js/node/test/parallel/test-worker-message-port-receive-message.js b/test/js/node/test/parallel/test-worker-message-port-receive-message.js new file mode 100644 index 00000000000000..bafcd3f7a7042f --- /dev/null +++ b/test/js/node/test/parallel/test-worker-message-port-receive-message.js @@ -0,0 +1,33 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { MessageChannel, receiveMessageOnPort } = require('worker_threads'); + +const { port1, port2 } = new MessageChannel(); + +const message1 = { hello: 'world' }; +const message2 = { foo: 'bar' }; + +// Make sure receiveMessageOnPort() works in a FIFO way, the same way it does +// when we’re using events. +assert.strictEqual(receiveMessageOnPort(port2), undefined); +port1.postMessage(message1); +port1.postMessage(message2); +assert.deepStrictEqual(receiveMessageOnPort(port2), { message: message1 }); +assert.deepStrictEqual(receiveMessageOnPort(port2), { message: message2 }); +assert.strictEqual(receiveMessageOnPort(port2), undefined); +assert.strictEqual(receiveMessageOnPort(port2), undefined); + +// Make sure message handlers aren’t called. +port2.on('message', common.mustNotCall()); +port1.postMessage(message1); +assert.deepStrictEqual(receiveMessageOnPort(port2), { message: message1 }); +port1.close(); + +for (const value of [null, 0, -1, {}, []]) { + assert.throws(() => receiveMessageOnPort(value), { + name: 'TypeError', + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "port" argument must be a MessagePort instance' + }); +} From dd7e5d478a2a930299d66a6bc6d12e34566ee375 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 8 Apr 2025 10:46:55 -0700 Subject: [PATCH 07/88] Use defaultGlobalObject --- src/bun.js/bindings/webcore/Worker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index 1902785e4328fd..f4852abf74747a 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -184,7 +184,7 @@ ExceptionOr> Worker::create(ScriptExecutionContext& context, const S void* impl = WebWorker__create( worker.ptr(), - jsCast(context.jsGlobalObject())->bunVM(), + defaultGlobalObject(context.jsGlobalObject())->bunVM(), nameStr, urlStr, &errorMessage, From b74476c2cc0cc37739a8567acee0b194ac48e6fd Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 8 Apr 2025 15:43:30 -0700 Subject: [PATCH 08/88] Fire worker event on process --- src/bun.js/bindings/BunProcess.cpp | 6 ++++ src/bun.js/bindings/BunProcess.h | 4 +++ src/bun.js/bindings/ZigGlobalObject.cpp | 10 ++++++ src/bun.js/bindings/ZigGlobalObject.h | 5 +++ .../bindings/webcore/DOMClientIsoSubspaces.h | 3 +- src/bun.js/bindings/webcore/DOMIsoSubspaces.h | 3 +- src/bun.js/bindings/webcore/JSWorker.cpp | 19 +++++++++-- src/bun.js/bindings/webcore/Worker.cpp | 3 +- src/js/node/worker_threads.ts | 18 +++++++++-- .../node/test/parallel/test-worker-event.js | 14 ++++++++ .../worker_threads/worker_threads.test.ts | 14 ++++++++ test/js/web/workers/worker.test.ts | 32 +++++++++++++++++++ 12 files changed, 124 insertions(+), 7 deletions(-) create mode 100644 test/js/node/test/parallel/test-worker-event.js diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index 840ea09dcde1c7..9cba52c7026f80 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -1,3 +1,4 @@ +#include "BoundEmitFunction.h" #include "ModuleLoader.h" #include "napi.h" @@ -3170,6 +3171,11 @@ void Process::queueNextTick(JSC::JSGlobalObject* globalObject, JSValue func, con this->queueNextTick(globalObject, argsBuffer); } +void Process::emitOnNextTick(Zig::GlobalObject* globalObject, ASCIILiteral eventName, JSValue event) +{ + queueNextTick(globalObject, BoundEmitFunction::create(getVM(globalObject), globalObject, this, eventName, event)); +} + extern "C" void Bun__Process__queueNextTick1(GlobalObject* globalObject, EncodedJSValue func, EncodedJSValue arg1) { auto process = jsCast(globalObject->processObject()); diff --git a/src/bun.js/bindings/BunProcess.h b/src/bun.js/bindings/BunProcess.h index 589105f52187b5..37c18e00c44846 100644 --- a/src/bun.js/bindings/BunProcess.h +++ b/src/bun.js/bindings/BunProcess.h @@ -55,6 +55,10 @@ class Process : public WebCore::JSEventEmitter { template void queueNextTick(JSC::JSGlobalObject* globalObject, JSValue func, const JSValue (&args)[NumArgs]); + // Some Node.js events want to be emitted on the next tick rather than synchronously. + // This is equivalent to `process.nextTick(() => process.emit(eventName, event))` from JavaScript. + void emitOnNextTick(Zig::GlobalObject* globalObject, ASCIILiteral eventName, JSValue event); + static JSValue emitWarning(JSC::JSGlobalObject* lexicalGlobalObject, JSValue warning, JSValue type, JSValue code, JSValue ctor); JSString* cachedCwd() { return m_cachedCwd.get(); } diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index 26c282c5cafbce..a398393e30c843 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -173,6 +173,7 @@ #include "ProcessBindingBuffer.h" #include "NodeValidator.h" #include "ProcessBindingFs.h" +#include "BoundEmitFunction.h" #include "JSBunRequest.h" #include "ServerRouteList.h" @@ -3499,6 +3500,13 @@ void GlobalObject::finishCreation(VM& vm) m_bigintStatFsValues.initLater([](const LazyProperty::Initializer& init) { init.set(JSC::JSBigInt64Array::create(init.owner, JSC::JSBigInt64Array::createStructure(init.vm, init.owner, init.owner->objectPrototype()), 7)); }); + m_BoundEmitFunctionStructure.initLater([](const LazyProperty::Initializer& init) { + init.set(Bun::BoundEmitFunction::createStructure(init.vm, init.owner)); + }); + m_nodeWorkerObjectSymbol.initLater([](const LazyProperty::Initializer& init) { + // This should not be visible to user code + init.set(Symbol::createWithDescription(init.vm, "node worker object symbol"_s)); + }); configureNodeVM(vm, this); @@ -4118,6 +4126,8 @@ void GlobalObject::visitChildrenImpl(JSCell* cell, Visitor& visitor) thisObject->m_bigintStatValues.visit(visitor); thisObject->m_statFsValues.visit(visitor); thisObject->m_bigintStatFsValues.visit(visitor); + thisObject->m_nodeWorkerObjectSymbol.visit(visitor); + thisObject->m_BoundEmitFunctionStructure.visit(visitor); thisObject->m_nodeErrorCache.visit(visitor); diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h index 60e3940bc6dd8f..d3d1aa62a40384 100644 --- a/src/bun.js/bindings/ZigGlobalObject.h +++ b/src/bun.js/bindings/ZigGlobalObject.h @@ -281,6 +281,7 @@ class GlobalObject : public Bun::GlobalScope { Structure* JSSocketAddressDTOStructure() const { return m_JSSocketAddressDTOStructure.getInitializedOnMainThread(this); } Structure* ImportMetaObjectStructure() const { return m_importMetaObjectStructure.getInitializedOnMainThread(this); } Structure* AsyncContextFrameStructure() const { return m_asyncBoundFunctionStructure.getInitializedOnMainThread(this); } + Structure* BoundEmitFunctionStructure() { return m_BoundEmitFunctionStructure.getInitializedOnMainThread(this); } JSWeakMap* vmModuleContextMap() const { return m_vmModuleContextMap.getInitializedOnMainThread(this); } @@ -512,6 +513,7 @@ class GlobalObject : public Bun::GlobalScope { JSObject* cryptoObject() const { return m_cryptoObject.getInitializedOnMainThread(this); } JSObject* JSDOMFileConstructor() const { return m_JSDOMFileConstructor.getInitializedOnMainThread(this); } + Symbol* nodeWorkerObjectSymbol() { return m_nodeWorkerObjectSymbol.getInitializedOnMainThread(this); } Bun::CommonStrings& commonStrings() { return m_commonStrings; } Bun::Http2CommonStrings& http2CommonStrings() { return m_http2_commongStrings; } @@ -618,6 +620,7 @@ class GlobalObject : public Bun::GlobalScope { LazyProperty m_importMetaObjectStructure; LazyProperty m_asyncBoundFunctionStructure; LazyProperty m_JSDOMFileConstructor; + LazyProperty m_BoundEmitFunctionStructure; LazyProperty m_JSCryptoKey; LazyProperty m_NapiExternalStructure; @@ -644,6 +647,8 @@ class GlobalObject : public Bun::GlobalScope { LazyProperty m_statFsValues; LazyProperty m_bigintStatFsValues; + LazyProperty m_nodeWorkerObjectSymbol; + // De-optimization once `require("module")._resolveFilename` is written to bool hasOverriddenModuleResolveFilenameFunction = false; // De-optimization once `require("module").wrapper` or `require("module").wrap` is written to diff --git a/src/bun.js/bindings/webcore/DOMClientIsoSubspaces.h b/src/bun.js/bindings/webcore/DOMClientIsoSubspaces.h index 159d2417cd4d90..6a5fd7f336e48c 100644 --- a/src/bun.js/bindings/webcore/DOMClientIsoSubspaces.h +++ b/src/bun.js/bindings/webcore/DOMClientIsoSubspaces.h @@ -66,6 +66,7 @@ class DOMClientIsoSubspaces { std::unique_ptr m_clientSubspaceForJSS3Bucket; std::unique_ptr m_clientSubspaceForJSS3File; std::unique_ptr m_clientSubspaceForJSX509Certificate; + std::unique_ptr m_clientSubspaceForBoundEmitFunction; #include "ZigGeneratedClasses+DOMClientIsoSubspaces.h" /* --- bun --- */ @@ -75,7 +76,7 @@ class DOMClientIsoSubspaces { std::unique_ptr m_clientSubspaceForDOMURL; std::unique_ptr m_clientSubspaceForURLSearchParams; std::unique_ptr m_clientSubspaceForURLSearchParamsIterator; - + std::unique_ptr m_clientSubspaceForCookie; std::unique_ptr m_clientSubspaceForCookieMap; std::unique_ptr m_clientSubspaceForCookieMapIterator; diff --git a/src/bun.js/bindings/webcore/DOMIsoSubspaces.h b/src/bun.js/bindings/webcore/DOMIsoSubspaces.h index 6b64f5d0f37e35..3d99f8d1c475b9 100644 --- a/src/bun.js/bindings/webcore/DOMIsoSubspaces.h +++ b/src/bun.js/bindings/webcore/DOMIsoSubspaces.h @@ -63,6 +63,7 @@ class DOMIsoSubspaces { std::unique_ptr m_subspaceForJSS3Bucket; std::unique_ptr m_subspaceForJSS3File; std::unique_ptr m_subspaceForJSX509Certificate; + std::unique_ptr m_subspaceForBoundEmitFunction; #include "ZigGeneratedClasses+DOMIsoSubspaces.h" /*-- BUN --*/ @@ -920,7 +921,7 @@ class DOMIsoSubspaces { std::unique_ptr m_subspaceForExposedToWorkerAndWindow; std::unique_ptr m_subspaceForURLSearchParams; std::unique_ptr m_subspaceForURLSearchParamsIterator; - + std::unique_ptr m_subspaceForCookie; std::unique_ptr m_subspaceForCookieMap; std::unique_ptr m_subspaceForCookieMapIterator; diff --git a/src/bun.js/bindings/webcore/JSWorker.cpp b/src/bun.js/bindings/webcore/JSWorker.cpp index 2988e3688f3276..1d9bc4f37505ee 100644 --- a/src/bun.js/bindings/webcore/JSWorker.cpp +++ b/src/bun.js/bindings/webcore/JSWorker.cpp @@ -58,6 +58,7 @@ #include #include #include "SerializedScriptValue.h" +#include "BunProcess.h" namespace WebCore { using namespace JSC; @@ -118,6 +119,7 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: auto& vm = JSC::getVM(lexicalGlobalObject); auto throwScope = DECLARE_THROW_SCOPE(vm); auto* castedThis = jsCast(callFrame->jsCallee()); + auto* globalObject = defaultGlobalObject(lexicalGlobalObject); ASSERT(castedThis); if (UNLIKELY(callFrame->argumentCount() < 1)) return throwVMError(lexicalGlobalObject, throwScope, createNotEnoughArgumentsError(lexicalGlobalObject)); @@ -135,6 +137,11 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: } RETURN_IF_EXCEPTION(throwScope, {}); EnsureStillAliveScope argument1 = callFrame->argument(1); + JSValue nodeWorkerObject {}; + if (JSValue::strictEqual(lexicalGlobalObject, callFrame->argument(2), globalObject->nodeWorkerObjectSymbol())) { + nodeWorkerObject = callFrame->argument(3); + } + RETURN_IF_EXCEPTION(throwScope, {}); auto options = WorkerOptions {}; @@ -230,7 +237,6 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: options.dataMessagePorts = WTFMove(transferredPorts); } - auto* globalObject = jsCast(lexicalGlobalObject); auto envValue = optionsObject->getIfPropertyExists(lexicalGlobalObject, Identifier::fromString(vm, "env"_s)); RETURN_IF_EXCEPTION(throwScope, {}); // for now, we don't permit SHARE_ENV, because the behavior isn't implemented @@ -323,6 +329,15 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: setSubclassStructureIfNeeded(lexicalGlobalObject, callFrame, asObject(jsValue)); RETURN_IF_EXCEPTION(throwScope, {}); + // Emit the 'worker' event on the process. If we are constructing a `node:worker_threads` + // worker, we emit the event with the worker_threads Worker object instead of the Web Worker. + JSValue workerToEmit = jsValue; + if (nodeWorkerObject) { + workerToEmit = nodeWorkerObject; + } + auto* process = jsCast(globalObject->processObject()); + process->emitOnNextTick(globalObject, "worker"_s, workerToEmit); + return JSValue::encode(jsValue); } JSC_ANNOTATE_HOST_FUNCTION(JSWorkerDOMConstructorConstruct, JSWorkerDOMConstructor::construct); @@ -694,4 +709,4 @@ Worker* JSWorker::toWrapped(JSC::VM&, JSC::JSValue value) return &wrapper->wrapped(); return nullptr; } -} +} // namespace WebCore diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index 26ced14488de23..d4b4b81435d571 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -518,11 +518,12 @@ JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) threadId = jsNumber(worker->clientIdentifier() - 1); } - JSObject* array = constructEmptyObject(globalObject, globalObject->objectPrototype(), 3); + JSObject* array = constructEmptyObject(globalObject, globalObject->objectPrototype(), 4); 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, globalObject->nodeWorkerObjectSymbol()); return array; } diff --git a/src/js/node/worker_threads.ts b/src/js/node/worker_threads.ts index 59cb63c6773eac..7c4a1cce23a194 100644 --- a/src/js/node/worker_threads.ts +++ b/src/js/node/worker_threads.ts @@ -11,7 +11,12 @@ const { MessageChannel, BroadcastChannel, Worker: WebWorker } = globalThis; const SHARE_ENV = Symbol("nodejs.worker_threads.SHARE_ENV"); const isMainThread = Bun.isMainThread; -const { 0: _workerData, 1: _threadId, 2: _receiveMessageOnPort } = $cpp("Worker.cpp", "createNodeWorkerThreadsBinding"); +const { + 0: _workerData, + 1: _threadId, + 2: _receiveMessageOnPort, + 3: kNodeWorkerObject, +} = $cpp("Worker.cpp", "createNodeWorkerThreadsBinding") as [unknown, number, (port: unknown) => unknown, symbol]; type NodeWorkerOptions = import("node:worker_threads").WorkerOptions; @@ -234,7 +239,16 @@ class Worker extends EventEmitter { } } try { - this.#worker = new WebWorker(filename, options); + // The native WebWorker constructor fires the 'worker' event on the process when the worker is + // successfully created. For worker_threads, we want to fire that event with the + // worker_threads object instead of the Web Worker object. + // + // This is implemented by accepting two extra arguments in the native constructor: a symbol to + // prove we are the real worker_threads constructor, and if so, the worker_threads object as + // the last parameter. + this.#worker = new (WebWorker as new ( + ...args: [...ConstructorParameters, nodeWorkerObjectSymbol: symbol, nodeWorker: Worker] + ) => WebWorker)(filename, options as Bun.WorkerOptions, kNodeWorkerObject, this); } catch (e) { if (this.#urlToRevoke) { URL.revokeObjectURL(this.#urlToRevoke); diff --git a/test/js/node/test/parallel/test-worker-event.js b/test/js/node/test/parallel/test-worker-event.js new file mode 100644 index 00000000000000..01e95ead8316cb --- /dev/null +++ b/test/js/node/test/parallel/test-worker-event.js @@ -0,0 +1,14 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { + Worker, + threadId: parentThreadId, +} = require('worker_threads'); + +process.on('worker', common.mustCall(({ threadId }) => { + assert.strictEqual(threadId, parentThreadId + 1); +})); + +new Worker('', { eval: true }); diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index 4e36578c460487..c4363c0c5a5a8a 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -291,3 +291,17 @@ test("eval does not leak source code", async () => { if (errors.length > 0) throw new Error(errors); expect(proc.exitCode).toBe(0); }); + +test("worker event", () => { + const { promise, resolve } = Promise.withResolvers(); + let worker: Worker | undefined = undefined; + let called = false; + process.once("worker", eventWorker => { + called = true; + expect(eventWorker as any).toBe(worker); + resolve(); + }); + worker = new Worker(new URL("data:text/javascript,")); + expect(called).toBeFalse(); + return promise; +}); diff --git a/test/js/web/workers/worker.test.ts b/test/js/web/workers/worker.test.ts index 219406c6ee47aa..d6d78083df4f75 100644 --- a/test/js/web/workers/worker.test.ts +++ b/test/js/web/workers/worker.test.ts @@ -269,6 +269,38 @@ describe("web worker", () => { done(); }); }); + + describe("worker event", () => { + test("is fired with the right object", () => { + const { promise, resolve } = Promise.withResolvers(); + let worker: Worker | undefined = undefined; + let called = false; + process.once("worker", eventWorker => { + called = true; + expect(eventWorker as any).toBe(worker); + resolve(); + }); + worker = new Worker(new URL("data:text/javascript,")); + expect(called).toBeFalse(); + return promise; + }); + + test("cannot fake a node:worker_threads Worker", () => { + const { promise, resolve } = Promise.withResolvers(); + let worker: Worker | undefined = undefined; + let called = false; + process.once("worker", eventWorker => { + called = true; + expect(eventWorker as any).toBe(worker); + resolve(); + }); + // make sure that the native constructor requires a secret symbol to emit a + // node:worker_threads object + worker = new (Worker as any)(new URL("data:text/javascript,"), {}, Symbol(), 5); + expect(called).toBeFalse(); + return promise; + }); + }); }); // TODO: move to node:worker_threads tests directory From cf5ad47ad5515c895e16c358bfbe64ec18ef5780 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 9 Apr 2025 14:34:55 -0700 Subject: [PATCH 09/88] Support worker environment data --- src/bun.js/bindings/ZigGlobalObject.cpp | 4 +- src/bun.js/bindings/ZigGlobalObject.h | 6 ++ src/bun.js/bindings/webcore/JSWorker.cpp | 76 ++++++++++--------- src/bun.js/bindings/webcore/Worker.cpp | 25 +++--- src/bun.js/bindings/webcore/WorkerOptions.h | 5 +- src/js/node/worker_threads.ts | 21 +++-- .../parallel/test-worker-environmentdata.js | 39 ++++++++++ 7 files changed, 124 insertions(+), 52 deletions(-) create mode 100644 test/js/node/test/parallel/test-worker-environmentdata.js diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index a398393e30c843..2734e28113d2e9 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -224,8 +224,6 @@ constexpr size_t DEFAULT_ERROR_STACK_TRACE_LIMIT = 10; Structure* createMemoryFootprintStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject); -extern "C" WebCore::Worker* WebWorker__getParentWorker(void*); - #ifndef BUN_WEBKIT_VERSION #ifndef ASSERT_ENABLED #warning "BUN_WEBKIT_VERSION is not defined. WebKit's cmakeconfig.h is supposed to define that. If you're building a release build locally, ignore this warning. If you're seeing this warning in CI, please file an issue." @@ -4021,6 +4019,8 @@ void GlobalObject::visitChildrenImpl(JSCell* cell, Visitor& visitor) visitor.append(thisObject->m_currentNapiHandleScopeImpl); + visitor.append(thisObject->m_nodeWorkerEnvironmentData); + thisObject->m_moduleResolveFilenameFunction.visit(visitor); thisObject->m_modulePrototypeUnderscoreCompileFunction.visit(visitor); thisObject->m_commonJSRequireESMFromHijackedExtensionFunction.visit(visitor); diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h index d3d1aa62a40384..42700360783e6d 100644 --- a/src/bun.js/bindings/ZigGlobalObject.h +++ b/src/bun.js/bindings/ZigGlobalObject.h @@ -437,6 +437,10 @@ class GlobalObject : public Bun::GlobalScope { // move them off the stack which will cause them to get collected if not in the handle scope. JSC::WriteBarrier m_currentNapiHandleScopeImpl; + // Supports getEnvironmentData() and setEnvironmentData(), and is cloned into newly-created + // Workers. Initialized in createNodeWorkerThreadsBinding. + WriteBarrier m_nodeWorkerEnvironmentData; + // The original, unmodified Error.prepareStackTrace. // // We set a default value for this to mimic Node.js behavior It is a @@ -513,7 +517,9 @@ class GlobalObject : public Bun::GlobalScope { JSObject* cryptoObject() const { return m_cryptoObject.getInitializedOnMainThread(this); } JSObject* JSDOMFileConstructor() const { return m_JSDOMFileConstructor.getInitializedOnMainThread(this); } + Symbol* nodeWorkerObjectSymbol() { return m_nodeWorkerObjectSymbol.getInitializedOnMainThread(this); } + JSMap* nodeWorkerEnvironmentData() { return m_nodeWorkerEnvironmentData.get(); } Bun::CommonStrings& commonStrings() { return m_commonStrings; } Bun::Http2CommonStrings& http2CommonStrings() { return m_http2_commongStrings; } diff --git a/src/bun.js/bindings/webcore/JSWorker.cpp b/src/bun.js/bindings/webcore/JSWorker.cpp index 1d9bc4f37505ee..2fc8186a301a76 100644 --- a/src/bun.js/bindings/webcore/JSWorker.cpp +++ b/src/bun.js/bindings/webcore/JSWorker.cpp @@ -59,6 +59,7 @@ #include #include "SerializedScriptValue.h" #include "BunProcess.h" +#include namespace WebCore { using namespace JSC; @@ -144,6 +145,8 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: RETURN_IF_EXCEPTION(throwScope, {}); auto options = WorkerOptions {}; + JSValue workerData; + Vector> transferList; if (JSObject* optionsObject = JSC::jsDynamicCast(argument1.value())) { if (auto nameValue = optionsObject->getIfPropertyExists(lexicalGlobalObject, vm.propertyNames->name)) { @@ -152,6 +155,7 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: RETURN_IF_EXCEPTION(throwScope, {}); } } + RETURN_IF_EXCEPTION(throwScope, {}); if (auto miniModeValue = optionsObject->getIfPropertyExists(lexicalGlobalObject, Identifier::fromString(vm, "smol"_s))) { options.mini = miniModeValue.toBoolean(lexicalGlobalObject); @@ -192,49 +196,26 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: } } - auto workerData = optionsObject->getIfPropertyExists(lexicalGlobalObject, Identifier::fromString(vm, "workerData"_s)); + workerData = optionsObject->getIfPropertyExists(lexicalGlobalObject, Identifier::fromString(vm, "workerData"_s)); if (!workerData) { workerData = optionsObject->getIfPropertyExists(lexicalGlobalObject, Identifier::fromString(vm, "data"_s)); + if (!workerData) workerData = jsUndefined(); } + RETURN_IF_EXCEPTION(throwScope, {}); - if (workerData) { - Vector> ports; - Vector> transferList; - - if (JSValue transferListValue = optionsObject->getIfPropertyExists(lexicalGlobalObject, Identifier::fromString(vm, "transferList"_s))) { - if (transferListValue.isObject()) { - JSC::JSObject* transferListObject = transferListValue.getObject(); - if (auto* transferListArray = jsDynamicCast(transferListObject)) { - for (unsigned i = 0; i < transferListArray->length(); i++) { - JSC::JSValue transferListValue = transferListArray->get(lexicalGlobalObject, i); - if (transferListValue.isObject()) { - JSC::JSObject* transferListObject = transferListValue.getObject(); - transferList.append(JSC::Strong(vm, transferListObject)); - } + if (JSValue transferListValue = optionsObject->getIfPropertyExists(lexicalGlobalObject, Identifier::fromString(vm, "transferList"_s))) { + if (transferListValue.isObject()) { + JSC::JSObject* transferListObject = transferListValue.getObject(); + if (auto* transferListArray = jsDynamicCast(transferListObject)) { + for (unsigned i = 0; i < transferListArray->length(); i++) { + JSC::JSValue transferListValue = transferListArray->get(lexicalGlobalObject, i); + if (transferListValue.isObject()) { + JSC::JSObject* transferListObject = transferListValue.getObject(); + transferList.append(JSC::Strong(vm, transferListObject)); } } } } - - ExceptionOr> serialized = SerializedScriptValue::create(*lexicalGlobalObject, workerData, WTFMove(transferList), ports, SerializationForStorage::No, SerializationContext::WorkerPostMessage); - if (serialized.hasException()) { - WebCore::propagateException(*lexicalGlobalObject, throwScope, serialized.releaseException()); - return encodedJSValue(); - } - - Vector transferredPorts; - - if (!ports.isEmpty()) { - auto disentangleResult = MessagePort::disentanglePorts(WTFMove(ports)); - if (disentangleResult.hasException()) { - WebCore::propagateException(*lexicalGlobalObject, throwScope, disentangleResult.releaseException()); - return encodedJSValue(); - } - transferredPorts = disentangleResult.releaseReturnValue(); - } - - options.data = serialized.releaseReturnValue(); - options.dataMessagePorts = WTFMove(transferredPorts); } auto envValue = optionsObject->getIfPropertyExists(lexicalGlobalObject, Identifier::fromString(vm, "env"_s)); @@ -311,6 +292,31 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: } } + Vector> ports; + auto* valueToTransfer = constructEmptyArray(globalObject, nullptr, 2); + valueToTransfer->putDirectIndex(globalObject, 0, workerData); + valueToTransfer->putDirectIndex(globalObject, 1, globalObject->nodeWorkerEnvironmentData()); + + ExceptionOr> serialized = SerializedScriptValue::create(*lexicalGlobalObject, valueToTransfer, WTFMove(transferList), ports, SerializationForStorage::No, SerializationContext::WorkerPostMessage); + if (serialized.hasException()) { + WebCore::propagateException(*lexicalGlobalObject, throwScope, serialized.releaseException()); + return encodedJSValue(); + } + + Vector transferredPorts; + + if (!ports.isEmpty()) { + auto disentangleResult = MessagePort::disentanglePorts(WTFMove(ports)); + if (disentangleResult.hasException()) { + WebCore::propagateException(*lexicalGlobalObject, throwScope, disentangleResult.releaseException()); + return encodedJSValue(); + } + transferredPorts = disentangleResult.releaseReturnValue(); + } + + options.workerDataAndEnvironmentData = serialized.releaseReturnValue(); + options.dataMessagePorts = WTFMove(transferredPorts); + RETURN_IF_EXCEPTION(throwScope, {}); auto object = Worker::create(*context, WTFMove(scriptUrl), WTFMove(options)); if constexpr (IsExceptionOr) diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index d4b4b81435d571..71cfcd89d57337 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -468,7 +468,7 @@ extern "C" void WebWorker__dispatchError(Zig::GlobalObject* globalObject, Worker worker->dispatchError(message.toWTFString(BunString::ZeroCopy)); } -extern "C" WebCore::Worker* WebWorker__getParentWorker(void*); +extern "C" WebCore::Worker* WebWorker__getParentWorker(void* bunVM); JSC_DEFINE_HOST_FUNCTION(jsReceiveMessageOnPort, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame)) { @@ -503,27 +503,34 @@ JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) auto scope = DECLARE_THROW_SCOPE(globalObject->vm()); JSValue workerData = jsNull(); JSValue threadId = jsNumber(0); + JSMap* environmentData = nullptr; if (auto* worker = WebWorker__getParentWorker(globalObject->bunVM())) { auto& options = worker->options(); - if (worker && options.data) { - auto ports = MessagePort::entanglePorts(*ScriptExecutionContext::getScriptExecutionContext(worker->clientIdentifier()), WTFMove(options.dataMessagePorts)); - RefPtr serialized = WTFMove(options.data); - JSValue deserialized = serialized->deserialize(*globalObject, globalObject, WTFMove(ports)); - RETURN_IF_EXCEPTION(scope, {}); - workerData = deserialized; - } + auto ports = MessagePort::entanglePorts(*ScriptExecutionContext::getScriptExecutionContext(worker->clientIdentifier()), WTFMove(options.dataMessagePorts)); + RefPtr serialized = WTFMove(options.workerDataAndEnvironmentData); + JSValue deserialized = serialized->deserialize(*globalObject, globalObject, WTFMove(ports)); + RETURN_IF_EXCEPTION(scope, {}); + // Should always be set to an Array of length 2 in the constructor in JSWorker.cpp + auto* pair = jsCast(deserialized); + ASSERT(pair->length() == 2); + workerData = pair->getIndexQuickly(0); + environmentData = jsCast(pair->getIndexQuickly(1)); // Main thread starts at 1 threadId = jsNumber(worker->clientIdentifier() - 1); + } else { + environmentData = JSMap::create(vm, globalObject->mapStructure()); } + globalObject->m_nodeWorkerEnvironmentData.set(vm, globalObject, environmentData); - JSObject* array = constructEmptyObject(globalObject, globalObject->objectPrototype(), 4); + JSObject* array = constructEmptyArray(globalObject, nullptr, 5); 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, globalObject->nodeWorkerObjectSymbol()); + array->putDirectIndex(globalObject, 4, environmentData); return array; } diff --git a/src/bun.js/bindings/webcore/WorkerOptions.h b/src/bun.js/bindings/webcore/WorkerOptions.h index 43249a3466a18c..f01d1bf5704e9a 100644 --- a/src/bun.js/bindings/webcore/WorkerOptions.h +++ b/src/bun.js/bindings/webcore/WorkerOptions.h @@ -16,7 +16,10 @@ struct WorkerOptions { // true, then we need to make sure that `process.argv` contains "[worker eval]" instead of the // Blob URL. bool evalMode { false }; - RefPtr data; + // Serialized array containing [workerData, environmentData] + // (environmentData is always a Map) + RefPtr workerDataAndEnvironmentData; + // Objects transferred for either data or environmentData in the transferList Vector dataMessagePorts; Vector preloadModules; std::optional> env; // TODO(@190n) allow shared diff --git a/src/js/node/worker_threads.ts b/src/js/node/worker_threads.ts index 7c4a1cce23a194..a962ea818e862a 100644 --- a/src/js/node/worker_threads.ts +++ b/src/js/node/worker_threads.ts @@ -16,7 +16,14 @@ const { 1: _threadId, 2: _receiveMessageOnPort, 3: kNodeWorkerObject, -} = $cpp("Worker.cpp", "createNodeWorkerThreadsBinding") as [unknown, number, (port: unknown) => unknown, symbol]; + 4: environmentData, +} = $cpp("Worker.cpp", "createNodeWorkerThreadsBinding") as [ + unknown, + number, + (port: unknown) => unknown, + symbol, + Map, +]; type NodeWorkerOptions = import("node:worker_threads").WorkerOptions; @@ -189,12 +196,16 @@ function fakeParentPort() { } let parentPort: MessagePort | null = isMainThread ? null : fakeParentPort(); -function getEnvironmentData() { - return process.env; +function getEnvironmentData(key: unknown): unknown { + return environmentData.get(key); } -function setEnvironmentData(env: any) { - process.env = env; +function setEnvironmentData(key: unknown, value: unknown): void { + if (value === undefined) { + environmentData.delete(key); + } else { + environmentData.set(key, value); + } } function markAsUntransferable() { diff --git a/test/js/node/test/parallel/test-worker-environmentdata.js b/test/js/node/test/parallel/test-worker-environmentdata.js new file mode 100644 index 00000000000000..5943666d060b36 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-environmentdata.js @@ -0,0 +1,39 @@ +'use strict'; +// Flags: --expose-internals + +require('../common'); +const { + Worker, + getEnvironmentData, + setEnvironmentData, + threadId, +} = require('worker_threads'); + +// BUN: skip using this internal module, it doesn't actually affect behavior of the test +// const { assignEnvironmentData } = require('internal/worker'); + +const { + deepStrictEqual, + strictEqual, +} = require('assert'); + +if (!process.env.HAS_STARTED_WORKER) { + process.env.HAS_STARTED_WORKER = 1; + setEnvironmentData('foo', 'bar'); + setEnvironmentData('hello', { value: 'world' }); + setEnvironmentData(1, 2); + strictEqual(getEnvironmentData(1), 2); + setEnvironmentData(1); // Delete it, key won't show up in the worker. + new Worker(__filename); + setEnvironmentData('hello'); // Delete it. Has no impact on the worker. +} else { + strictEqual(getEnvironmentData('foo'), 'bar'); + deepStrictEqual(getEnvironmentData('hello'), { value: 'world' }); + strictEqual(getEnvironmentData(1), undefined); + // assignEnvironmentData(undefined); // It won't setup any key. + strictEqual(getEnvironmentData(undefined), undefined); + + // Recurse to make sure the environment data is inherited + if (threadId <= 2) + new Worker(__filename); +} From 513c326f8d9b06993c3eaf2f7b3d41f3267bdb09 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 9 Apr 2025 14:54:39 -0700 Subject: [PATCH 10/88] Add test-worker-abort-on-uncaught-exception-terminate.js --- ...t-worker-abort-on-uncaught-exception-terminate.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 test/js/node/test/parallel/test-worker-abort-on-uncaught-exception-terminate.js diff --git a/test/js/node/test/parallel/test-worker-abort-on-uncaught-exception-terminate.js b/test/js/node/test/parallel/test-worker-abort-on-uncaught-exception-terminate.js new file mode 100644 index 00000000000000..de73b5e4e34e47 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-abort-on-uncaught-exception-terminate.js @@ -0,0 +1,12 @@ +// Flags: --abort-on-uncaught-exception +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker_threads'); + +// Tests that --abort-on-uncaught-exception does not apply to +// termination exceptions. + +const w = new Worker('while(true);', { eval: true }); +w.on('online', common.mustCall(() => w.terminate())); +w.on('exit', common.mustCall((code) => assert.strictEqual(code, 1))); From 38b20fb7e828ba421bccdb2da2474f2d9de23bf0 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 9 Apr 2025 15:24:08 -0700 Subject: [PATCH 11/88] Refine error message for duplicates in transferList --- .../webcore/SerializedScriptValue.cpp | 13 +++++++-- ...-worker-message-port-transfer-duplicate.js | 29 +++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 test/js/node/test/parallel/test-worker-message-port-transfer-duplicate.js diff --git a/src/bun.js/bindings/webcore/SerializedScriptValue.cpp b/src/bun.js/bindings/webcore/SerializedScriptValue.cpp index 152b97ac59c23e..5f681c0957f79d 100644 --- a/src/bun.js/bindings/webcore/SerializedScriptValue.cpp +++ b/src/bun.js/bindings/webcore/SerializedScriptValue.cpp @@ -5562,8 +5562,15 @@ ExceptionOr> SerializedScriptValue::create(JSGlobalOb #endif HashSet uniqueTransferables; for (auto& transferable : transferList) { - if (!uniqueTransferables.add(transferable.get()).isNewEntry) - return Exception { DataCloneError, "Duplicate transferable for structured clone"_s }; + if (!uniqueTransferables.add(transferable.get()).isNewEntry) { + if (toPossiblySharedArrayBuffer(vm, transferable.get())) { + return Exception { DataCloneError, "Transfer list contains duplicate ArrayBuffer"_s }; + } else if (JSMessagePort::toWrapped(vm, transferable.get())) { + return Exception { DataCloneError, "Transfer list contains duplicate MessagePort"_s }; + } else { + return Exception { DataCloneError, "Duplicate transferable for structured clone"_s }; + } + } if (auto arrayBuffer = toPossiblySharedArrayBuffer(vm, transferable.get())) { if (arrayBuffer->isDetached() || arrayBuffer->isShared()) @@ -5690,7 +5697,7 @@ ExceptionOr> SerializedScriptValue::create(JSGlobalOb if (arrayBufferContentsArray.hasException()) return arrayBufferContentsArray.releaseException(); - // auto backingStores = ImageBitmap::detachBitmaps(WTFMove(imageBitmaps)); + // auto backingStores = ImageBitmap::detachBitmaps(WTFMove(imageBitmaps)); #if ENABLE(OFFSCREEN_CANVAS_IN_WORKERS) Vector> detachedCanvases; diff --git a/test/js/node/test/parallel/test-worker-message-port-transfer-duplicate.js b/test/js/node/test/parallel/test-worker-message-port-transfer-duplicate.js new file mode 100644 index 00000000000000..ad0a2d8aca1f01 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-message-port-transfer-duplicate.js @@ -0,0 +1,29 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { MessageChannel } = require('worker_threads'); + +// Test that passing duplicate transferables in the transfer list throws +// DataCloneError exceptions. + +{ + const { port1, port2 } = new MessageChannel(); + port2.once('message', common.mustNotCall()); + + const port3 = new MessageChannel().port1; + assert.throws(() => { + port1.postMessage(port3, [port3, port3]); + }, /^DataCloneError: Transfer list contains duplicate MessagePort$/); + port1.close(); +} + +{ + const { port1, port2 } = new MessageChannel(); + port2.once('message', common.mustNotCall()); + + const buf = new Uint8Array(10); + assert.throws(() => { + port1.postMessage(buf, [buf.buffer, buf.buffer]); + }, /^DataCloneError: Transfer list contains duplicate ArrayBuffer$/); + port1.close(); +} From 288dcd02efa7b9036ae1f263f92fe63a9ba9c7fb Mon Sep 17 00:00:00 2001 From: 190n <7763597+190n@users.noreply.github.com> Date: Wed, 9 Apr 2025 22:26:13 +0000 Subject: [PATCH 12/88] `bun run clang-format` --- src/bun.js/bindings/webcore/SerializedScriptValue.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/bindings/webcore/SerializedScriptValue.cpp b/src/bun.js/bindings/webcore/SerializedScriptValue.cpp index 5f681c0957f79d..b743573e3aa115 100644 --- a/src/bun.js/bindings/webcore/SerializedScriptValue.cpp +++ b/src/bun.js/bindings/webcore/SerializedScriptValue.cpp @@ -5697,7 +5697,7 @@ ExceptionOr> SerializedScriptValue::create(JSGlobalOb if (arrayBufferContentsArray.hasException()) return arrayBufferContentsArray.releaseException(); - // auto backingStores = ImageBitmap::detachBitmaps(WTFMove(imageBitmaps)); + // auto backingStores = ImageBitmap::detachBitmaps(WTFMove(imageBitmaps)); #if ENABLE(OFFSCREEN_CANVAS_IN_WORKERS) Vector> detachedCanvases; From bd5f3aa77101fa02c4d5ce9e9d560587aed4b651 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 9 Apr 2025 17:37:18 -0700 Subject: [PATCH 13/88] Do not unref worker on main thread when terminated --- src/bun.js/web_worker.zig | 2 - .../parallel/test-worker-dns-terminate.js | 14 ++++++ .../test-worker-terminate-source-map.js | 45 +++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 test/js/node/test/parallel/test-worker-dns-terminate.js create mode 100644 test/js/node/test/parallel/test-worker-terminate-source-map.js diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index c256407a98492c..0134a463a155f8 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -496,8 +496,6 @@ pub const WebWorker = struct { if (this.vm) |vm| { vm.eventLoop().wakeup(); } - - this.setRefInternal(false); } /// This handles cleanup, emitting the "close" event, and deinit. diff --git a/test/js/node/test/parallel/test-worker-dns-terminate.js b/test/js/node/test/parallel/test-worker-dns-terminate.js new file mode 100644 index 00000000000000..da9d543c3b0be2 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-dns-terminate.js @@ -0,0 +1,14 @@ +'use strict'; +const common = require('../common'); +const { Worker } = require('worker_threads'); + +const w = new Worker(` +const dns = require('dns'); +dns.lookup('nonexistent.org', () => {}); +require('worker_threads').parentPort.postMessage('0'); +`, { eval: true }); + +w.on('message', common.mustCall(() => { + // This should not crash the worker during a DNS request. + w.terminate().then(common.mustCall()); +})); diff --git a/test/js/node/test/parallel/test-worker-terminate-source-map.js b/test/js/node/test/parallel/test-worker-terminate-source-map.js new file mode 100644 index 00000000000000..c855dab975be01 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-terminate-source-map.js @@ -0,0 +1,45 @@ +'use strict'; +const common = require('../common'); +const tmpdir = require('../common/tmpdir'); +const assert = require('assert'); + +// Attempts to test that the source map JS code run on process shutdown +// does not call any user-defined JS code. + +const { Worker, workerData, parentPort } = require('worker_threads'); + +if (!workerData) { + tmpdir.refresh(); + process.env.NODE_V8_COVERAGE = tmpdir.path; + + // Count the number of some calls that should not be made. + const callCount = new Int32Array(new SharedArrayBuffer(4)); + const w = new Worker(__filename, { workerData: { callCount } }); + w.on('message', common.mustCall(() => w.terminate())); + w.on('exit', common.mustCall(() => { + assert.strictEqual(callCount[0], 0); + })); + return; +} + +const { callCount } = workerData; + +function increaseCallCount() { callCount[0]++; } + +// Increase the call count when a forbidden method is called. +for (const property of ['_cache', 'lineLengths', 'url']) { + Object.defineProperty(Object.prototype, property, { + get: increaseCallCount, + set: increaseCallCount + }); +} +Object.getPrototypeOf([][Symbol.iterator]()).next = increaseCallCount; +Object.getPrototypeOf((new Map()).entries()).next = increaseCallCount; +Array.prototype[Symbol.iterator] = increaseCallCount; +Map.prototype[Symbol.iterator] = increaseCallCount; +Map.prototype.entries = increaseCallCount; +Object.keys = increaseCallCount; +Object.create = increaseCallCount; +Object.hasOwnProperty = increaseCallCount; + +parentPort.postMessage('done'); From b7c7627e4786f7bae1d751da33fc96f5027ae886 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 9 Apr 2025 18:36:17 -0700 Subject: [PATCH 14/88] Terminate the VM in Worker.requestTerminate() --- src/bun.js/web_worker.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index 0134a463a155f8..17e664eed24d2a 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -495,6 +495,7 @@ pub const WebWorker = struct { if (this.vm) |vm| { vm.eventLoop().wakeup(); + vm.jsc.notifyNeedTermination(); } } From 2d5900526152c51539fab3adc571435860470372 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 10 Apr 2025 11:05:05 -0700 Subject: [PATCH 15/88] Add missing files --- src/bun.js/bindings/BoundEmitFunction.cpp | 76 +++++++++++++++++++++++ src/bun.js/bindings/BoundEmitFunction.h | 43 +++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 src/bun.js/bindings/BoundEmitFunction.cpp create mode 100644 src/bun.js/bindings/BoundEmitFunction.h diff --git a/src/bun.js/bindings/BoundEmitFunction.cpp b/src/bun.js/bindings/BoundEmitFunction.cpp new file mode 100644 index 00000000000000..cb6f8ba1c679f2 --- /dev/null +++ b/src/bun.js/bindings/BoundEmitFunction.cpp @@ -0,0 +1,76 @@ +#include "BoundEmitFunction.h" + +#include "JavaScriptCore/FunctionPrototype.h" + +using namespace JSC; + +namespace Bun { + +BoundEmitFunction* BoundEmitFunction::create(VM& vm, Zig::GlobalObject* globalObject, WebCore::JSEventEmitter* target, WTF::ASCIILiteral eventName, JSValue event) +{ + auto* structure = globalObject->BoundEmitFunctionStructure(); + auto* function = new (NotNull, allocateCell(vm)) BoundEmitFunction( + vm, + structure, + eventName); + function->finishCreation(vm, target, event); + return function; +} + +BoundEmitFunction::BoundEmitFunction(VM& vm, Structure* structure, WTF::ASCIILiteral eventName) + : Base(vm, structure, functionCall) + , m_eventName(eventName) +{ +} + +void BoundEmitFunction::finishCreation(VM& vm, WebCore::JSEventEmitter* target, JSValue event) +{ + Base::finishCreation(vm, 0, "BoundEmitFunction"_s); + m_target.set(vm, this, target); + m_event.set(vm, this, event); +} + +JSC_DEFINE_HOST_FUNCTION(BoundEmitFunction::functionCall, (JSGlobalObject * globalObject, CallFrame* callFrame)) +{ + auto& vm = getVM(globalObject); + auto* function = jsCast(callFrame->jsCallee()); + MarkedArgumentBuffer args; + args.append(function->m_event.get()); + function->m_target->wrapped().emit(Identifier::fromString(vm, function->m_eventName), args); + return JSValue::encode(jsUndefined()); +} + +// for CREATE_METHOD_TABLE +namespace JSCastingHelpers = JSCastingHelpers; +const ClassInfo BoundEmitFunction::s_info = { + "BoundEmitFunction"_s, + &Base::s_info, + nullptr, + nullptr, + CREATE_METHOD_TABLE(BoundEmitFunction) +}; + +Structure* BoundEmitFunction::createStructure(VM& vm, JSGlobalObject* globalObject) +{ + return Structure::create( + vm, + globalObject, + globalObject->functionPrototype(), + TypeInfo(InternalFunctionType, StructureFlags), + info()); +} + +template +void BoundEmitFunction::visitChildrenImpl(JSCell* cell, Visitor& visitor) +{ + auto* fn = jsCast(cell); + ASSERT_GC_OBJECT_INHERITS(fn, info()); + Base::visitChildren(fn, visitor); + + visitor.append(fn->m_target); + visitor.append(fn->m_event); +} + +DEFINE_VISIT_CHILDREN(BoundEmitFunction); + +} // namespace Bun diff --git a/src/bun.js/bindings/BoundEmitFunction.h b/src/bun.js/bindings/BoundEmitFunction.h new file mode 100644 index 00000000000000..8a48537637f97a --- /dev/null +++ b/src/bun.js/bindings/BoundEmitFunction.h @@ -0,0 +1,43 @@ +#pragma once + +#include "root.h" +#include "JSEventEmitter.h" + +namespace Bun { + +// Callable wrapper around an event emitter, an event name, and a value. Will fire the specified +// event when called. Used to implement Process::emitOnNextTick. +class BoundEmitFunction : public JSC::InternalFunction { +public: + using Base = JSC::InternalFunction; + + static BoundEmitFunction* create(JSC::VM& vm, Zig::GlobalObject* globalObject, WebCore::JSEventEmitter* target, WTF::ASCIILiteral eventName, JSC::JSValue event); + static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject); + + template + static JSC::GCClient::IsoSubspace* subspaceFor(JSC::VM& vm) + { + if constexpr (mode == JSC::SubspaceAccess::Concurrently) + return nullptr; + return WebCore::subspaceForImpl( + vm, + [](auto& spaces) { return spaces.m_clientSubspaceForBoundEmitFunction.get(); }, + [](auto& spaces, auto&& space) { spaces.m_clientSubspaceForBoundEmitFunction = std::forward(space); }, + [](auto& spaces) { return spaces.m_subspaceForBoundEmitFunction.get(); }, + [](auto& spaces, auto&& space) { spaces.m_subspaceForBoundEmitFunction = std::forward(space); }); + } + + DECLARE_INFO; + DECLARE_VISIT_CHILDREN; + +private: + BoundEmitFunction(JSC::VM& vm, JSC::Structure* structure, WTF::ASCIILiteral eventName); + void finishCreation(JSC::VM& vm, WebCore::JSEventEmitter* target, JSC::JSValue event); + static JSC_DECLARE_HOST_FUNCTION(functionCall); + + JSC::WriteBarrier m_target; + WTF::ASCIILiteral m_eventName; + JSC::WriteBarrier m_event; +}; + +} // namespace Bun From 8c6aa4ed81920badf2a982c1e031fb9d0d567d4d Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 10 Apr 2025 11:41:47 -0700 Subject: [PATCH 16/88] Handle termination exception in next tick queue --- src/bun.js/bindings/JSNextTickQueue.cpp | 1 + .../test-worker-nexttick-terminate.js | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 test/js/node/test/parallel/test-worker-nexttick-terminate.js diff --git a/src/bun.js/bindings/JSNextTickQueue.cpp b/src/bun.js/bindings/JSNextTickQueue.cpp index 642ff260453b26..22613817c87a5f 100644 --- a/src/bun.js/bindings/JSNextTickQueue.cpp +++ b/src/bun.js/bindings/JSNextTickQueue.cpp @@ -88,6 +88,7 @@ void JSNextTickQueue::drain(JSC::VM& vm, JSC::JSGlobalObject* globalObject) } auto* drainFn = internalField(2).get().getObject(); auto throwScope = DECLARE_THROW_SCOPE(vm); + RETURN_IF_EXCEPTION(throwScope, ); MarkedArgumentBuffer drainArgs; JSC::call(globalObject, drainFn, drainArgs, "Failed to drain next tick queue"_s); } diff --git a/test/js/node/test/parallel/test-worker-nexttick-terminate.js b/test/js/node/test/parallel/test-worker-nexttick-terminate.js new file mode 100644 index 00000000000000..0e5d7e096c57ec --- /dev/null +++ b/test/js/node/test/parallel/test-worker-nexttick-terminate.js @@ -0,0 +1,25 @@ +'use strict'; +const common = require('../common'); +const { Worker } = require('worker_threads'); + +// Checks that terminating in the middle of `process.nextTick()` does not +// Crash the process. + +const w = new Worker(` +require('worker_threads').parentPort.postMessage('0'); +process.nextTick(() => { + while(1); +}); +`, { eval: true }); + +// Test deprecation of .terminate() with callback. +common.expectWarning( + 'DeprecationWarning', + 'Passing a callback to worker.terminate() is deprecated. ' + + 'It returns a Promise instead.', 'DEP0132'); + +w.on('message', common.mustCall(() => { + setTimeout(() => { + w.terminate(common.mustCall()).then(common.mustCall()); + }, 1); +})); From 2705a7baa3a65bdd1361c546b4707ab95f3876e2 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 10 Apr 2025 14:39:04 -0700 Subject: [PATCH 17/88] Use exit code 1 if Worker entry point fails to load --- src/bun.js/bindings/BunProcess.cpp | 2 +- src/bun.js/web_worker.zig | 1 + .../test-worker-vm-context-terminate.js | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 test/js/node/test/parallel/test-worker-vm-context-terminate.js diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index 9cba52c7026f80..b23331c25b1479 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -2168,7 +2168,7 @@ extern "C" void Bun__ForceFileSinkToBeSynchronousForProcessObjectStdio(JSC::JSGl static JSValue constructStdioWriteStream(JSC::JSGlobalObject* globalObject, int fd) { auto& vm = JSC::getVM(globalObject); - auto scope = DECLARE_THROW_SCOPE(vm); + auto scope = DECLARE_CATCH_SCOPE(vm); JSC::JSFunction* getStdioWriteStream = JSC::JSFunction::create(vm, globalObject, processObjectInternalsGetStdioWriteStreamCodeGenerator(vm), globalObject); JSC::MarkedArgumentBuffer args; diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index 17e664eed24d2a..cd69d35168e5d5 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -411,6 +411,7 @@ pub const WebWorker = struct { vm.preload = this.preloads; WebWorker__dispatchOnline(this.cpp_worker, vm.global); var promise = vm.loadEntryPointForWebWorker(this.specifier) catch { + vm.exit_handler.exit_code = 1; this.flushLogs(); this.exitAndDeinit(); return; diff --git a/test/js/node/test/parallel/test-worker-vm-context-terminate.js b/test/js/node/test/parallel/test-worker-vm-context-terminate.js new file mode 100644 index 00000000000000..23b58ba4db14d5 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-vm-context-terminate.js @@ -0,0 +1,19 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const vm = require('vm'); +const { Worker } = require('worker_threads'); + +// Do not use isMainThread so that this test itself can be run inside a Worker. +if (!process.env.HAS_STARTED_WORKER) { + process.env.HAS_STARTED_WORKER = 1; + const w = new Worker(__filename); + w.on('online', common.mustCall(() => { + setTimeout(() => w.terminate(), 50); + })); + w.on('error', common.mustNotCall()); + w.on('exit', common.mustCall((code) => assert.strictEqual(code, 1))); +} else { + while (true) + vm.runInNewContext(''); +} From e857f5429c33f22b42c3b5709b1176b8774b7f4c Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 10 Apr 2025 14:43:42 -0700 Subject: [PATCH 18/88] Allow termination exception while fetching builtin --- src/bun.js/bindings/JSCommonJSModule.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/bindings/JSCommonJSModule.cpp b/src/bun.js/bindings/JSCommonJSModule.cpp index d8d45de5b1f622..ff614b34bdb8f8 100644 --- a/src/bun.js/bindings/JSCommonJSModule.cpp +++ b/src/bun.js/bindings/JSCommonJSModule.cpp @@ -1258,7 +1258,7 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionRequireNativeModule, (JSGlobalObject * lexica if (res.success) return JSC::JSValue::encode(result); } - ASSERT_WITH_MESSAGE(false, "Failed to fetch builtin module %s", specifier.utf8().data()); + throwScope.assertNoExceptionExceptTermination(); return throwVMError(globalObject, throwScope, "Failed to fetch builtin module"_s); } From 7b4095017fd62cb3816a979713258a46e90f3e5c Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 12:29:54 -0700 Subject: [PATCH 19/88] Clean up exception handling in BunProcess.cpp --- src/bun.js/bindings/BunProcess.cpp | 51 ++++-------------------------- 1 file changed, 7 insertions(+), 44 deletions(-) diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index b23331c25b1479..41b151fff28151 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -2179,18 +2179,9 @@ static JSValue constructStdioWriteStream(JSC::JSGlobalObject* globalObject, int JSC::CallData callData = JSC::getCallData(getStdioWriteStream); - NakedPtr returnedException = nullptr; - auto result = JSC::profiledCall(globalObject, ProfilingReason::API, getStdioWriteStream, callData, globalObject->globalThis(), args, returnedException); - RETURN_IF_EXCEPTION(scope, {}); - - if (auto* exception = returnedException.get()) { -#if BUN_DEBUG - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); -#endif - scope.throwException(globalObject, exception->value()); - returnedException.clear(); - return {}; - } + auto result = JSC::profiledCall(globalObject, ProfilingReason::API, getStdioWriteStream, callData, globalObject->globalThis(), args); + scope.assertNoExceptionExceptTermination(); + CLEAR_AND_RETURN_IF_EXCEPTION(scope, jsUndefined()); ASSERT_WITH_MESSAGE(JSC::isJSArray(result), "Expected an array from getStdioWriteStream"); JSC::JSArray* resultObject = JSC::jsCast(result); @@ -2245,20 +2236,9 @@ static JSValue constructStdin(VM& vm, JSObject* processObject) args.append(jsNumber(static_cast(fdType))); JSC::CallData callData = JSC::getCallData(getStdioWriteStream); - NakedPtr returnedException = nullptr; - auto result = JSC::profiledCall(globalObject, ProfilingReason::API, getStdioWriteStream, callData, globalObject, args, returnedException); + auto result = JSC::profiledCall(globalObject, ProfilingReason::API, getStdioWriteStream, callData, globalObject, args); RETURN_IF_EXCEPTION(scope, {}); - - if (auto* exception = returnedException.get()) { -#if BUN_DEBUG - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); -#endif - scope.throwException(globalObject, exception->value()); - returnedException.clear(); - return {}; - } - - RELEASE_AND_RETURN(scope, result); + return result; } JSC_DEFINE_CUSTOM_GETTER(processThrowDeprecation, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::EncodedJSValue thisValue, JSC::PropertyName name)) @@ -2325,19 +2305,8 @@ static JSValue constructProcessChannel(VM& vm, JSObject* processObject) JSC::MarkedArgumentBuffer args; JSC::CallData callData = JSC::getCallData(getControl); - NakedPtr returnedException = nullptr; - auto result = JSC::profiledCall(globalObject, ProfilingReason::API, getControl, callData, globalObject->globalThis(), args, returnedException); + auto result = JSC::profiledCall(globalObject, ProfilingReason::API, getControl, callData, globalObject->globalThis(), args); RETURN_IF_EXCEPTION(scope, {}); - - if (auto* exception = returnedException.get()) { -#if BUN_DEBUG - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); -#endif - scope.throwException(globalObject, exception->value()); - returnedException.clear(); - return {}; - } - return result; } else { return jsUndefined(); @@ -3438,15 +3407,9 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionKill, (JSC::JSGlobalObject * globalObje args.append(jsNumber(signal)); JSC::CallData callData = JSC::getCallData(_killFn); - NakedPtr returnedException = nullptr; - auto result = JSC::profiledCall(globalObject, ProfilingReason::API, _killFn, callData, globalObject->globalThis(), args, returnedException); + auto result = JSC::profiledCall(globalObject, ProfilingReason::API, _killFn, callData, globalObject->globalThis(), args); RETURN_IF_EXCEPTION(scope, {}); - if (auto* exception = returnedException.get()) { - scope.throwException(globalObject, exception->value()); - returnedException.clear(); - return {}; - } auto err = result.toInt32(globalObject); if (err) { throwSystemError(scope, globalObject, "kill"_s, err); From 604ae717052ae1d7b7778d890a9a7f6653028369 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 12:30:16 -0700 Subject: [PATCH 20/88] Add more exception checks to JSNextTickQueue --- src/bun.js/bindings/JSNextTickQueue.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/bun.js/bindings/JSNextTickQueue.cpp b/src/bun.js/bindings/JSNextTickQueue.cpp index 22613817c87a5f..b7710d7751ac1e 100644 --- a/src/bun.js/bindings/JSNextTickQueue.cpp +++ b/src/bun.js/bindings/JSNextTickQueue.cpp @@ -76,21 +76,26 @@ bool JSNextTickQueue::isEmpty() void JSNextTickQueue::drain(JSC::VM& vm, JSC::JSGlobalObject* globalObject) { + auto throwScope = DECLARE_THROW_SCOPE(vm); bool mustResetContext = false; if (isEmpty()) { + RETURN_IF_EXCEPTION(throwScope, ); vm.drainMicrotasks(); + RETURN_IF_EXCEPTION(throwScope, ); mustResetContext = true; } if (!isEmpty()) { + RETURN_IF_EXCEPTION(throwScope, ); if (mustResetContext) { globalObject->m_asyncContextData.get()->putInternalField(vm, 0, jsUndefined()); + RETURN_IF_EXCEPTION(throwScope, ); } auto* drainFn = internalField(2).get().getObject(); - auto throwScope = DECLARE_THROW_SCOPE(vm); RETURN_IF_EXCEPTION(throwScope, ); MarkedArgumentBuffer drainArgs; JSC::call(globalObject, drainFn, drainArgs, "Failed to drain next tick queue"_s); + RETURN_IF_EXCEPTION(throwScope, ); } } From ed5966c49436408996585eaefa083f26dca5217b Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 13:00:11 -0700 Subject: [PATCH 21/88] Fix crash on workerData with no options --- src/bun.js/bindings/webcore/JSWorker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/bindings/webcore/JSWorker.cpp b/src/bun.js/bindings/webcore/JSWorker.cpp index 2fc8186a301a76..a6b966ac33d940 100644 --- a/src/bun.js/bindings/webcore/JSWorker.cpp +++ b/src/bun.js/bindings/webcore/JSWorker.cpp @@ -145,7 +145,7 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: RETURN_IF_EXCEPTION(throwScope, {}); auto options = WorkerOptions {}; - JSValue workerData; + JSValue workerData = jsUndefined(); Vector> transferList; if (JSObject* optionsObject = JSC::jsDynamicCast(argument1.value())) { From 5d8217fc07257179c7b18aaaf72accc604534760 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 13:16:21 -0700 Subject: [PATCH 22/88] Fix null environment data --- src/bun.js/bindings/webcore/JSWorker.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bun.js/bindings/webcore/JSWorker.cpp b/src/bun.js/bindings/webcore/JSWorker.cpp index a6b966ac33d940..d50511fec3351b 100644 --- a/src/bun.js/bindings/webcore/JSWorker.cpp +++ b/src/bun.js/bindings/webcore/JSWorker.cpp @@ -295,7 +295,11 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: Vector> ports; auto* valueToTransfer = constructEmptyArray(globalObject, nullptr, 2); valueToTransfer->putDirectIndex(globalObject, 0, workerData); - valueToTransfer->putDirectIndex(globalObject, 1, globalObject->nodeWorkerEnvironmentData()); + auto* environmentData = globalObject->nodeWorkerEnvironmentData(); + // If node:worker_threads has not been imported, environment data will not be set up yet. + if (environmentData) { + valueToTransfer->putDirectIndex(globalObject, 1, environmentData); + } ExceptionOr> serialized = SerializedScriptValue::create(*lexicalGlobalObject, valueToTransfer, WTFMove(transferList), ports, SerializationForStorage::No, SerializationContext::WorkerPostMessage); if (serialized.hasException()) { From 5aeb80dcad4cf2fe89bda702a92e7eee8eaf0428 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 14:25:40 -0700 Subject: [PATCH 23/88] Do not propagate termination exception to event loop --- src/bun.js/javascript.zig | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig index 6a6dbfd02b2f44..37f736d1e7a3e3 100644 --- a/src/bun.js/javascript.zig +++ b/src/bun.js/javascript.zig @@ -519,8 +519,10 @@ pub export fn Bun__reportUnhandledError(globalObject: *JSGlobalObject, value: JS JSC.markBinding(@src()); // This JSGlobalObject might not be the main script execution context // See the crash in https://github.com/oven-sh/bun/issues/9778 - const jsc_vm = JSC.VirtualMachine.get(); - _ = jsc_vm.uncaughtException(globalObject, value, false); + const vm = JSC.VirtualMachine.get(); + if (!value.isTerminationException(vm.jsc)) { + _ = vm.uncaughtException(globalObject, value, false); + } return .undefined; } From 91a58f47dab2de80d8a796e194d06add1263dc16 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 14:36:45 -0700 Subject: [PATCH 24/88] Un-todo "worker terminating forcefully properly interrupts" and fix expected exit code --- test/js/web/workers/worker.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/js/web/workers/worker.test.ts b/test/js/web/workers/worker.test.ts index d6d78083df4f75..6f994b8b2fe651 100644 --- a/test/js/web/workers/worker.test.ts +++ b/test/js/web/workers/worker.test.ts @@ -337,13 +337,13 @@ describe("worker_threads", () => { expect(code).toBe(2); }); - test.todo("worker terminating forcefully properly interrupts", async () => { + test("worker terminating forcefully properly interrupts", async () => { const worker = new wt.Worker(new URL("worker-fixture-while-true.js", import.meta.url).href, {}); await new Promise(done => { worker.on("message", () => done()); }); const code = await worker.terminate(); - expect(code).toBe(0); + expect(code).toBe(1); }); test("worker without argv/execArgv", async () => { From 097d298cd0c3237ae270f6e864c293e23b2af8a6 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 16:31:00 -0700 Subject: [PATCH 25/88] Resolve Worker entrypoint on worker thread instead of main --- src/bun.js/web_worker.zig | 30 +++++++++++++------ .../parallel/test-worker-esm-missing-main.js | 16 ++++++++++ 2 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 test/js/node/test/parallel/test-worker-esm-missing-main.js diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index cd69d35168e5d5..94ec676f080a30 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -20,8 +20,8 @@ pub const WebWorker = struct { parent_context_id: u32 = 0, parent: *JSC.VirtualMachine, - /// Already resolved. - specifier: []const u8 = "", + /// To be resolved on the Worker thread at startup, in spin(). + unresolved_specifier: []const u8, preloads: [][]const u8 = &.{}, store_fd: bool = false, arena: ?bun.MimallocArena = null, @@ -205,10 +205,6 @@ pub const WebWorker = struct { const preload_modules = if (preload_modules_ptr) |ptr| ptr[0..preload_modules_len] else &.{}; - const path = resolveEntryPointSpecifier(parent, spec_slice.slice(), error_message, &temp_log) orelse { - return null; - }; - var preloads = std.ArrayList([]const u8).initCapacity(bun.default_allocator, preload_modules_len) catch bun.outOfMemory(); for (preload_modules) |module| { const utf8_slice = module.toUTF8(bun.default_allocator); @@ -234,7 +230,7 @@ pub const WebWorker = struct { .execution_context_id = this_context_id, .mini = mini, .eval_mode = eval_mode, - .specifier = bun.default_allocator.dupe(u8, path) catch bun.outOfMemory(), + .unresolved_specifier = (spec_slice.toOwned(bun.default_allocator) catch bun.outOfMemory()).slice(), .store_fd = parent.transpiler.resolver.store_fd, .name = brk: { if (!name_str.isEmpty()) { @@ -324,7 +320,7 @@ pub const WebWorker = struct { fn deinit(this: *WebWorker) void { log("[{d}] deinit", .{this.execution_context_id}); this.parent_poll_ref.unrefConcurrently(this.parent); - bun.default_allocator.free(this.specifier); + bun.default_allocator.free(this.unresolved_specifier); for (this.preloads) |preload| { bun.default_allocator.free(preload); } @@ -410,7 +406,23 @@ pub const WebWorker = struct { this.setStatus(.starting); vm.preload = this.preloads; WebWorker__dispatchOnline(this.cpp_worker, vm.global); - var promise = vm.loadEntryPointForWebWorker(this.specifier) catch { + // resolve entrypoint + var resolve_error = bun.String.empty; + defer resolve_error.deref(); + const path = resolveEntryPointSpecifier(vm, this.unresolved_specifier, &resolve_error, vm.log) orelse { + vm.exit_handler.exit_code = 1; + if (vm.log.errors == 0 and !resolve_error.isEmpty()) { + const err = resolve_error.toUTF8(bun.default_allocator); + defer err.deinit(); + vm.log.addError(null, .Empty, err.slice()) catch bun.outOfMemory(); + } + this.flushLogs(); + this.exitAndDeinit(); + return; + }; + defer bun.default_allocator.free(path); + + var promise = vm.loadEntryPointForWebWorker(path) catch { vm.exit_handler.exit_code = 1; this.flushLogs(); this.exitAndDeinit(); diff --git a/test/js/node/test/parallel/test-worker-esm-missing-main.js b/test/js/node/test/parallel/test-worker-esm-missing-main.js new file mode 100644 index 00000000000000..dbcb050b77c061 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-esm-missing-main.js @@ -0,0 +1,16 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker_threads'); + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); +const missing = tmpdir.resolve('does-not-exist.js'); + +const worker = new Worker(missing); + +worker.on('error', common.mustCall((err) => { + // eslint-disable-next-line node-core/no-unescaped-regexp-dot + // BUN: this error comes from our bundler where it'd be impractical to rewrite all the errors to match Node + assert.match(err.message, /(Cannot find module|ModuleNotFound) .+does-not-exist.js/); +})); From 741c8d9974800c794c0fb1eddfbec5addeb2a261 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 16:41:54 -0700 Subject: [PATCH 26/88] new tests --- .../parallel/test-worker-heapdump-failure.js | 30 ++++++++++ ...est-worker-message-port-transfer-closed.js | 57 +++++++++++++++++++ .../test-worker-process-exit-async-module.js | 11 ++++ .../test-worker-terminate-microtask-loop.js | 19 +++++++ .../test-worker-terminate-ref-public-port.js | 12 ++++ ...r-voluntarily-exit-followed-by-addition.js | 18 ++++++ ...rker-voluntarily-exit-followed-by-throw.js | 23 ++++++++ 7 files changed, 170 insertions(+) create mode 100644 test/js/node/test/parallel/test-worker-heapdump-failure.js create mode 100644 test/js/node/test/parallel/test-worker-message-port-transfer-closed.js create mode 100644 test/js/node/test/parallel/test-worker-process-exit-async-module.js create mode 100644 test/js/node/test/parallel/test-worker-terminate-microtask-loop.js create mode 100644 test/js/node/test/parallel/test-worker-terminate-ref-public-port.js create mode 100644 test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-addition.js create mode 100644 test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-throw.js diff --git a/test/js/node/test/parallel/test-worker-heapdump-failure.js b/test/js/node/test/parallel/test-worker-heapdump-failure.js new file mode 100644 index 00000000000000..c5d24cdcf658a2 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-heapdump-failure.js @@ -0,0 +1,30 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker_threads'); +const { once } = require('events'); + +(async function() { + const w = new Worker('', { eval: true }); + + await once(w, 'exit'); + await assert.rejects(() => w.getHeapSnapshot(), { + name: 'Error', + code: 'ERR_WORKER_NOT_RUNNING' + }); +})().then(common.mustCall()); + +(async function() { + const worker = new Worker('setInterval(() => {}, 1000);', { eval: true }); + await once(worker, 'online'); + + [1, true, [], null, Infinity, NaN].forEach((i) => { + assert.throws(() => worker.getHeapSnapshot(i), { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError', + message: 'The "options" argument must be of type object.' + + common.invalidArgTypeHelper(i) + }); + }); + await worker.terminate(); +})().then(common.mustCall()); diff --git a/test/js/node/test/parallel/test-worker-message-port-transfer-closed.js b/test/js/node/test/parallel/test-worker-message-port-transfer-closed.js new file mode 100644 index 00000000000000..d8ec04cbd250be --- /dev/null +++ b/test/js/node/test/parallel/test-worker-message-port-transfer-closed.js @@ -0,0 +1,57 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { MessageChannel } = require('worker_threads'); + +// This tests various behaviors around transferring MessagePorts with closing +// or closed handles. + +const { port1, port2 } = new MessageChannel(); + +const arrayBuf = new ArrayBuffer(10); +port1.onmessage = common.mustNotCall(); +port2.onmessage = common.mustNotCall(); + +function testSingle(closedPort, potentiallyOpenPort) { + assert.throws(common.mustCall(() => { + potentiallyOpenPort.postMessage(null, [arrayBuf, closedPort]); + }), common.mustCall((err) => { + assert.strictEqual(err.name, 'DataCloneError'); + assert.strictEqual(err.message, + 'MessagePort in transfer list is already detached'); + assert.strictEqual(err.code, 25); + assert.ok(err instanceof Error); + + const DOMException = err.constructor; + assert.ok(err instanceof DOMException); + assert.strictEqual(DOMException.name, 'DOMException'); + + return true; + })); + + // arrayBuf must not be transferred, even though it is present earlier in the + // transfer list than the closedPort. + assert.strictEqual(arrayBuf.byteLength, 10); +} + +function testBothClosed() { + testSingle(port1, port2); + testSingle(port2, port1); +} + +// Even though the port handles may not be completely closed in C++ land, the +// observable behavior must be that the closing/detachment is synchronous and +// instant. + +port1.close(common.mustCall(testBothClosed)); +testSingle(port1, port2); +port2.close(common.mustCall(testBothClosed)); +testBothClosed(); + +function tickUnref(n, fn) { + if (n === 0) return fn(); + setImmediate(tickUnref, n - 1, fn).unref(); +} + +tickUnref(10, common.mustNotCall('The communication channel is still open')); diff --git a/test/js/node/test/parallel/test-worker-process-exit-async-module.js b/test/js/node/test/parallel/test-worker-process-exit-async-module.js new file mode 100644 index 00000000000000..38d4ad74c7bd85 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-process-exit-async-module.js @@ -0,0 +1,11 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker_threads'); + +// Regression for https://github.com/nodejs/node/issues/43182. +const w = new Worker(new URL('data:text/javascript,process.exit(1);await new Promise(()=>{ process.exit(2); })')); +w.on('exit', common.mustCall((code) => { + assert.strictEqual(code, 1); +})); diff --git a/test/js/node/test/parallel/test-worker-terminate-microtask-loop.js b/test/js/node/test/parallel/test-worker-terminate-microtask-loop.js new file mode 100644 index 00000000000000..b2351c5d0bb051 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-terminate-microtask-loop.js @@ -0,0 +1,19 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker_threads'); + +// Verify that `.terminate()` interrupts the microtask queue. + +const worker = new Worker(` +function loop() { Promise.resolve().then(loop); } loop(); +require('worker_threads').parentPort.postMessage('up'); +`, { eval: true }); + +worker.once('message', common.mustCall(() => { + setImmediate(() => worker.terminate()); +})); + +worker.once('exit', common.mustCall((code) => { + assert.strictEqual(code, 1); +})); diff --git a/test/js/node/test/parallel/test-worker-terminate-ref-public-port.js b/test/js/node/test/parallel/test-worker-terminate-ref-public-port.js new file mode 100644 index 00000000000000..4a2de785a36220 --- /dev/null +++ b/test/js/node/test/parallel/test-worker-terminate-ref-public-port.js @@ -0,0 +1,12 @@ +'use strict'; +const common = require('../common'); +const { Worker } = require('worker_threads'); + +// The actual test here is that the Worker does not keep the main thread +// running after it has been .terminate()’ed. + +const w = new Worker(` +const p = require('worker_threads').parentPort; +while(true) p.postMessage({})`, { eval: true }); +w.once('message', () => w.terminate()); +w.once('exit', common.mustCall()); diff --git a/test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-addition.js b/test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-addition.js new file mode 100644 index 00000000000000..9f152bd5c62b0c --- /dev/null +++ b/test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-addition.js @@ -0,0 +1,18 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker, isMainThread } = require('worker_threads'); + +if (isMainThread) { + const workerData = new Int32Array(new SharedArrayBuffer(4)); + new Worker(__filename, { + workerData, + }); + process.on('beforeExit', common.mustCall(() => { + assert.strictEqual(workerData[0], 0); + })); +} else { + const { workerData } = require('worker_threads'); + process.exit(); + workerData[0] = 1; +} diff --git a/test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-throw.js b/test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-throw.js new file mode 100644 index 00000000000000..92c4d5596cbddf --- /dev/null +++ b/test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-throw.js @@ -0,0 +1,23 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker, isMainThread } = require('worker_threads'); + +if (isMainThread) { + const workerData = new Int32Array(new SharedArrayBuffer(4)); + new Worker(__filename, { + workerData, + }); + process.on('beforeExit', common.mustCall(() => { + assert.strictEqual(workerData[0], 0); + })); +} else { + const { workerData } = require('worker_threads'); + try { + process.exit(); + throw new Error('xxx'); + // eslint-disable-next-line no-unused-vars + } catch (err) { + workerData[0] = 1; + } +} From 518345b8e723f1ec55e24831a1bed1d7acd1e11f Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 16:45:24 -0700 Subject: [PATCH 27/88] Delete TODO --- src/bun.js/web_worker.zig | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index 94ec676f080a30..69469c1f20cebf 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -497,7 +497,6 @@ pub const WebWorker = struct { /// Request a terminate (Called from main thread from worker.terminate(), or inside worker in process.exit()) /// The termination will actually happen after the next tick of the worker's loop. pub fn requestTerminate(this: *WebWorker) callconv(.C) void { - // TODO(@heimskr): make WebWorker termination more immediate. Currently, console.log after process.exit will go through if in a WebWorker. if (this.status.load(.acquire) == .terminated) { return; } From a9e62078dd47aa398e15de63829240fe304890ed Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 18:00:21 -0700 Subject: [PATCH 28/88] Rename Worker.requestTerminate -> notifyNeedTermination to align with JSC --- src/bun.js/bindings/webcore/Worker.cpp | 4 ++-- src/bun.js/node/types.zig | 2 +- src/bun.js/web_worker.zig | 11 +++++------ 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index 71cfcd89d57337..86634c550a194b 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -69,7 +69,7 @@ namespace WebCore { WTF_MAKE_TZONE_ALLOCATED_IMPL(Worker); -extern "C" void WebWorker__requestTerminate( +extern "C" void WebWorker__notifyNeedTermination( void* worker); static Lock allWorkersLock; @@ -263,7 +263,7 @@ void Worker::terminate() { // m_contextProxy.terminateWorkerGlobalScope(); m_terminationFlags.fetch_or(TerminateRequestedFlag); - WebWorker__requestTerminate(impl_); + WebWorker__notifyNeedTermination(impl_); } // const char* Worker::activeDOMObjectName() const diff --git a/src/bun.js/node/types.zig b/src/bun.js/node/types.zig index 2c2edb9cdb45e2..d634764b68e256 100644 --- a/src/bun.js/node/types.zig +++ b/src/bun.js/node/types.zig @@ -1894,7 +1894,7 @@ pub const Process = struct { var vm = globalObject.bunVM(); if (vm.worker) |worker| { vm.exit_handler.exit_code = code; - worker.requestTerminate(); + worker.notifyNeedTermination(); return; } diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index 69469c1f20cebf..d5d092800114ac 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -494,16 +494,15 @@ pub const WebWorker = struct { } } - /// Request a terminate (Called from main thread from worker.terminate(), or inside worker in process.exit()) - /// The termination will actually happen after the next tick of the worker's loop. - pub fn requestTerminate(this: *WebWorker) callconv(.C) void { + /// Request a terminate. Must be called from another thread (i.e. for worker.terminate()) + pub fn notifyNeedTermination(this: *WebWorker) callconv(.C) void { if (this.status.load(.acquire) == .terminated) { return; } if (this.setRequestedTerminate()) { return; } - log("[{d}] requestTerminate", .{this.execution_context_id}); + log("[{d}] notifyNeedTermination", .{this.execution_context_id}); if (this.vm) |vm| { vm.eventLoop().wakeup(); @@ -513,7 +512,7 @@ pub const WebWorker = struct { /// This handles cleanup, emitting the "close" event, and deinit. /// Only call after the VM is initialized AND on the same thread as the worker. - /// Otherwise, call `requestTerminate` to cause the event loop to safely terminate after the next tick. + /// Otherwise, call `notifyNeedTermination` to cause the event loop to safely terminate. pub fn exitAndDeinit(this: *WebWorker) noreturn { JSC.markBinding(@src()); this.setStatus(.terminated); @@ -557,7 +556,7 @@ pub const WebWorker = struct { comptime { @export(&create, .{ .name = "WebWorker__create" }); - @export(&requestTerminate, .{ .name = "WebWorker__requestTerminate" }); + @export(¬ifyNeedTermination, .{ .name = "WebWorker__notifyNeedTermination" }); @export(&setRef, .{ .name = "WebWorker__setRef" }); _ = WebWorker__updatePtr; } From 67b8cb282c5f62025bda4ed0234b945b68b97725 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 18:03:48 -0700 Subject: [PATCH 29/88] Fix not firing messages sent before Worker started --- src/bun.js/bindings/webcore/Worker.cpp | 11 +++++++++++ src/bun.js/bindings/webcore/Worker.h | 1 + src/bun.js/web_worker.zig | 4 +++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index 86634c550a194b..eb82391128e3bf 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -357,7 +357,12 @@ void Worker::dispatchOnline(Zig::GlobalObject* workerGlobalObject) } RELEASE_ASSERT(&thisContext->vm() == &workerGlobalObject->vm()); RELEASE_ASSERT(thisContext == workerGlobalObject->globalEventScope->scriptExecutionContext()); +} +void Worker::fireEarlyMessages(Zig::GlobalObject* workerGlobalObject) +{ + Locker lock(this->m_pendingTasksMutex); + auto* thisContext = workerGlobalObject->scriptExecutionContext(); if (workerGlobalObject->globalEventScope->hasActiveEventListeners(eventNames().messageEvent)) { auto tasks = std::exchange(this->m_pendingTasks, {}); lock.unlockEarly(); @@ -376,6 +381,7 @@ void Worker::dispatchOnline(Zig::GlobalObject* workerGlobalObject) }); } } + void Worker::dispatchError(WTF::String message) { @@ -455,6 +461,11 @@ extern "C" void WebWorker__dispatchOnline(Worker* worker, Zig::GlobalObject* glo worker->dispatchOnline(globalObject); } +extern "C" void WebWorker__fireEarlyMessages(Worker* worker, Zig::GlobalObject* globalObject) +{ + worker->fireEarlyMessages(globalObject); +} + extern "C" void WebWorker__dispatchError(Zig::GlobalObject* globalObject, Worker* worker, BunString message, JSC::EncodedJSValue errorValue) { JSValue error = JSC::JSValue::decode(errorValue); diff --git a/src/bun.js/bindings/webcore/Worker.h b/src/bun.js/bindings/webcore/Worker.h index 1c1c9924720a36..a71898eb81b737 100644 --- a/src/bun.js/bindings/webcore/Worker.h +++ b/src/bun.js/bindings/webcore/Worker.h @@ -94,6 +94,7 @@ class Worker final : public ThreadSafeRefCounted, public EventTargetWith void drainEvents(); void dispatchOnline(Zig::GlobalObject* workerGlobalObject); + void fireEarlyMessages(Zig::GlobalObject* workerGlobalObject); void dispatchError(WTF::String message); void dispatchExit(int32_t exitCode); ScriptExecutionContext* scriptExecutionContext() const final { return ContextDestructionObserver::scriptExecutionContext(); } diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index d5d092800114ac..b6ac0ccec3b4b0 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -52,7 +52,8 @@ pub const WebWorker = struct { }; extern fn WebWorker__dispatchExit(?*JSC.JSGlobalObject, *anyopaque, i32) void; - extern fn WebWorker__dispatchOnline(this: *anyopaque, *JSC.JSGlobalObject) void; + extern fn WebWorker__dispatchOnline(cpp_worker: *anyopaque, *JSC.JSGlobalObject) void; + extern fn WebWorker__fireEarlyMessages(cpp_worker: *anyopaque, *JSC.JSGlobalObject) void; extern fn WebWorker__dispatchError(*JSC.JSGlobalObject, *anyopaque, bun.String, JSValue) void; export fn WebWorker__getParentWorker(vm: *JSC.VirtualMachine) ?*anyopaque { @@ -443,6 +444,7 @@ pub const WebWorker = struct { this.flushLogs(); log("[{d}] event loop start", .{this.execution_context_id}); + WebWorker__fireEarlyMessages(this.cpp_worker, vm.global); this.setStatus(.running); // don't run the GC if we don't actually need to From e3d5602f180fdee421e742b7a9b6f620d9d0cd8c Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 18:04:25 -0700 Subject: [PATCH 30/88] Fix expectations in worker_threads test to align with Node --- test/js/web/workers/worker.test.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/test/js/web/workers/worker.test.ts b/test/js/web/workers/worker.test.ts index 6f994b8b2fe651..fc9597f3bf53f2 100644 --- a/test/js/web/workers/worker.test.ts +++ b/test/js/web/workers/worker.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { once } from "events"; import { bunEnv, bunExe } from "harness"; import path from "path"; import wt from "worker_threads"; @@ -381,14 +382,10 @@ describe("worker_threads", () => { test("worker with eval = false fails with code", async () => { let has_error = false; - try { - const worker = new wt.Worker("console.log('this should not get printed')", { eval: false }); - } catch (err) { - expect(err.constructor.name).toEqual("TypeError"); - expect(err.message).toMatch(/BuildMessage: ModuleNotFound.+/); - has_error = true; - } - expect(has_error).toBe(true); + const worker = new wt.Worker("console.log('this should not get printed')", { eval: false }); + const [err] = await once(worker, "error"); + expect(err.constructor.name).toEqual("Error"); + expect(err.message).toMatch(/BuildMessage: ModuleNotFound.+/); }); test("worker with eval = true succeeds with valid code", async () => { From 92845037e818a491217f9c90c142a9ae158fe22e Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 18:05:40 -0700 Subject: [PATCH 31/88] Add comment --- src/bun.js/bindings/webcore/Worker.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bun.js/bindings/webcore/Worker.h b/src/bun.js/bindings/webcore/Worker.h index a71898eb81b737..b5c8edd4509975 100644 --- a/src/bun.js/bindings/webcore/Worker.h +++ b/src/bun.js/bindings/webcore/Worker.h @@ -94,6 +94,7 @@ class Worker final : public ThreadSafeRefCounted, public EventTargetWith void drainEvents(); void dispatchOnline(Zig::GlobalObject* workerGlobalObject); + // Fire a 'message' event in the Worker for messages that were sent before the Worker started running void fireEarlyMessages(Zig::GlobalObject* workerGlobalObject); void dispatchError(WTF::String message); void dispatchExit(int32_t exitCode); From ecc3622de04c5bccf342bc3430f58d1ebaceada3 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 11 Apr 2025 18:42:06 -0700 Subject: [PATCH 32/88] Use code 0 for workers terminated before they start running --- src/bun.js/web_worker.zig | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index b6ac0ccec3b4b0..0bb606698d0af9 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -423,8 +423,10 @@ pub const WebWorker = struct { }; defer bun.default_allocator.free(path); - var promise = vm.loadEntryPointForWebWorker(path) catch { - vm.exit_handler.exit_code = 1; + var promise = vm.loadEntryPointForWebWorker(path) catch |e| { + if (e != error.WorkerTerminated) { + vm.exit_handler.exit_code = 1; + } this.flushLogs(); this.exitAndDeinit(); return; From 1a9412c0b83127be14d57eec1f6180dd0e618572 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 14 Apr 2025 15:29:36 -0700 Subject: [PATCH 33/88] Use right exit code for workers terminated at different times --- src/bun.js/node/types.zig | 2 ++ src/bun.js/web_worker.zig | 13 +++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/bun.js/node/types.zig b/src/bun.js/node/types.zig index d634764b68e256..ed56e2dffbb3a4 100644 --- a/src/bun.js/node/types.zig +++ b/src/bun.js/node/types.zig @@ -1894,6 +1894,8 @@ pub const Process = struct { var vm = globalObject.bunVM(); if (vm.worker) |worker| { vm.exit_handler.exit_code = code; + // TODO(@190n) we may need to use requestTerminate or throwTerminationException + // instead to terminate the worker sooner worker.notifyNeedTermination(); return; } diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index 0bb606698d0af9..fcc384176d1e55 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -423,10 +423,15 @@ pub const WebWorker = struct { }; defer bun.default_allocator.free(path); - var promise = vm.loadEntryPointForWebWorker(path) catch |e| { - if (e != error.WorkerTerminated) { - vm.exit_handler.exit_code = 1; - } + // If the worker is terminated before we even try to run any code, the exit code should be 0 + if (this.hasRequestedTerminate()) { + this.flushLogs(); + this.exitAndDeinit(); + return; + } + + var promise = vm.loadEntryPointForWebWorker(path) catch { + vm.exit_handler.exit_code = 1; this.flushLogs(); this.exitAndDeinit(); return; From 9b68b63efc8c2509eb8dd4770637efcf90d3850e Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 14 Apr 2025 16:16:40 -0700 Subject: [PATCH 34/88] Update test for behavior changes --- test/js/web/workers/worker_blob.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/js/web/workers/worker_blob.test.ts b/test/js/web/workers/worker_blob.test.ts index b3c7eaea3b3d9e..aaf58ae0ea3e26 100644 --- a/test/js/web/workers/worker_blob.test.ts +++ b/test/js/web/workers/worker_blob.test.ts @@ -57,9 +57,10 @@ test("TypeScript Worker from a Blob", async () => { }); test("Worker from a blob errors on invalid blob", async () => { - expect(() => { - new Worker("blob:i dont exist!"); - }).toThrow(); + const { promise, reject } = Promise.withResolvers(); + const worker = new Worker("blob:i dont exist!"); + worker.addEventListener("error", e => reject(e.message)); + expect(promise).rejects.toBe('BuildMessage: ModuleNotFound resolving "blob:i dont exist!" (entry point)'); }); test("Revoking an object URL after a Worker is created before it loads should throw an error", async () => { @@ -80,7 +81,7 @@ test("Revoking an object URL after a Worker is created before it loads should th worker.onerror = resolve; }); expect(result).toBeInstanceOf(ErrorEvent); - expect((result as ErrorEvent).message).toContain(url); + expect((result as ErrorEvent).message).toBe("BuildMessage: Blob URL is missing"); break; } catch (e) { if (attempt === 9) { From e9341342db07b35763ed65af7453403937dfc501 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 14 Apr 2025 16:30:02 -0700 Subject: [PATCH 35/88] Do not terminate worker in isMainThread test --- test/js/bun/util/main-worker-file.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/js/bun/util/main-worker-file.js b/test/js/bun/util/main-worker-file.js index 2d5c3aedde5ab0..3764803978776e 100644 --- a/test/js/bun/util/main-worker-file.js +++ b/test/js/bun/util/main-worker-file.js @@ -11,6 +11,4 @@ if (isMainThread) { }); await promise; - - worker.terminate(); } From 8e411f248e1e4b57fa5eb4ee295a17518f7b8e68 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 14 Apr 2025 18:20:59 -0700 Subject: [PATCH 36/88] WIP StatWatcher fixes --- src/bun.js/bindings/lldbhalp.cpp | 21 ++++++++++ src/bun.js/bindings/lldbhalp.h | 3 ++ src/bun.js/node/node.classes.ts | 1 - src/bun.js/node/node_fs_stat_watcher.zig | 49 +++++++++++++----------- src/bun.js/rare_data.zig | 12 +++--- src/ptr/ref_count.zig | 18 ++++----- 6 files changed, 64 insertions(+), 40 deletions(-) create mode 100644 src/bun.js/bindings/lldbhalp.cpp create mode 100644 src/bun.js/bindings/lldbhalp.h diff --git a/src/bun.js/bindings/lldbhalp.cpp b/src/bun.js/bindings/lldbhalp.cpp new file mode 100644 index 00000000000000..291335baaba75b --- /dev/null +++ b/src/bun.js/bindings/lldbhalp.cpp @@ -0,0 +1,21 @@ +#include "lldbhalp.h" + +namespace Bun { + +// ok for x86_64 and aarch64 +struct FrameEntry { + FrameEntry* next; + void (*return_address)(); +}; + +void (*get_trace_entry_at(void* frame_pointer, int idx))() +{ + if (!frame_pointer) return nullptr; + auto* entry = reinterpret_cast(frame_pointer); + for (int i = 0; i < idx; i++) { + entry = entry->next; + } + return entry->return_address; +} + +} // namespace Bun diff --git a/src/bun.js/bindings/lldbhalp.h b/src/bun.js/bindings/lldbhalp.h new file mode 100644 index 00000000000000..1c5af4a5202c35 --- /dev/null +++ b/src/bun.js/bindings/lldbhalp.h @@ -0,0 +1,3 @@ +namespace Bun { +void (*get_trace_entry_at(void* frame_pointer, int idx))(); +} diff --git a/src/bun.js/node/node.classes.ts b/src/bun.js/node/node.classes.ts index 72872c182216ac..419825ca7ec712 100644 --- a/src/bun.js/node/node.classes.ts +++ b/src/bun.js/node/node.classes.ts @@ -110,7 +110,6 @@ export default [ noConstructor: true, finalize: true, configurable: false, - hasPendingActivity: true, klass: {}, JSType: "0b11101110", proto: { diff --git a/src/bun.js/node/node_fs_stat_watcher.zig b/src/bun.js/node/node_fs_stat_watcher.zig index 3e979b1207e718..01739206b5b708 100644 --- a/src/bun.js/node/node_fs_stat_watcher.zig +++ b/src/bun.js/node/node_fs_stat_watcher.zig @@ -45,12 +45,25 @@ pub const StatWatcherScheduler = struct { .tag = .StatWatcherScheduler, }, + ref_count: RefCount, + + const RefCount = bun.ptr.ThreadSafeRefCount(StatWatcherScheduler, "ref_count", deinit, .{ .debug_name = "StatWatcherScheduler" }); + pub const ref = RefCount.ref; + pub const deref = RefCount.deref; + const WatcherQueue = UnboundedQueue(StatWatcher, .next); - pub fn init(allocator: std.mem.Allocator, vm: *bun.JSC.VirtualMachine) *StatWatcherScheduler { - const this = allocator.create(StatWatcherScheduler) catch bun.outOfMemory(); - this.* = .{ .main_thread = std.Thread.getCurrentId(), .vm = vm }; - return this; + pub fn init(vm: *bun.JSC.VirtualMachine) bun.ptr.RefPtr(StatWatcherScheduler) { + return .new(.{ + .ref_count = .init(), + .main_thread = std.Thread.getCurrentId(), + .vm = vm, + }); + } + + fn deinit(this: *StatWatcherScheduler) void { + bun.assertf(this.watchers.count == 0, "destroying StatWatcherScheduler while it still has {} watchers", .{this.watchers.count}); + bun.destroy(this); } pub fn append(this: *StatWatcherScheduler, watcher: *StatWatcher) void { @@ -72,6 +85,7 @@ pub const StatWatcherScheduler = struct { /// Update the current interval and set the timer (this function is thread safe) fn setInterval(this: *StatWatcherScheduler, interval: i32) void { + this.ref(); this.current_interval.store(interval, .monotonic); if (this.main_thread == std.Thread.getCurrentId()) { @@ -135,6 +149,8 @@ pub const StatWatcherScheduler = struct { pub fn workPoolCallback(task: *JSC.WorkPoolTask) void { var this: *StatWatcherScheduler = @alignCast(@fieldParentPtr("task", task)); + // ref'd when the timer was scheduled + defer this.deref(); // Instant.now will not fail on our target platforms. const now = std.time.Instant.now() catch unreachable; @@ -145,7 +161,6 @@ pub const StatWatcherScheduler = struct { var contain_watchers = false; while (iter.next()) |watcher| { if (watcher.closed) { - watcher.used_by_scheduler_thread.store(false, .release); continue; } contain_watchers = true; @@ -180,9 +195,6 @@ pub const StatWatcher = struct { /// Closed is set to true to tell the scheduler to remove from list and mark `used_by_scheduler_thread` as false. closed: bool, - /// When this is marked `false` this StatWatcher can get freed - used_by_scheduler_thread: std.atomic.Value(bool) = std.atomic.Value(bool).init(false), - path: [:0]u8, persistent: bool, bigint: bool, @@ -197,6 +209,8 @@ pub const StatWatcher = struct { last_stat: bun.Stat, last_jsvalue: JSC.Strong, + scheduler: bun.ptr.RefPtr(StatWatcherScheduler), + pub usingnamespace JSC.Codegen.JSStatWatcher; pub fn eventLoop(this: StatWatcher) *EventLoop { @@ -209,7 +223,7 @@ pub const StatWatcher = struct { pub fn deinit(this: *StatWatcher) void { log("deinit\n", .{}); - bun.assert(!this.hasPendingActivity()); + this.scheduler.deref(); if (this.persistent) { this.persistent = false; @@ -305,10 +319,6 @@ pub const StatWatcher = struct { return .undefined; } - pub fn hasPendingActivity(this: *StatWatcher) bool { - return this.used_by_scheduler_thread.load(.acquire); - } - /// Stops file watching but does not free the instance. pub fn close(this: *StatWatcher) void { if (this.persistent) { @@ -345,7 +355,6 @@ pub const StatWatcher = struct { const this = initial_stat_task.watcher; if (this.closed) { - this.used_by_scheduler_thread.store(false, .release); return; } @@ -368,28 +377,23 @@ pub const StatWatcher = struct { pub fn initialStatSuccessOnMainThread(this: *StatWatcher) void { if (this.closed) { - this.used_by_scheduler_thread.store(false, .release); return; } const jsvalue = statToJSStats(this.globalThis, &this.last_stat, this.bigint); this.last_jsvalue = JSC.Strong.create(jsvalue, this.globalThis); - const vm = this.globalThis.bunVM(); - vm.rareData().nodeFSStatWatcherScheduler(vm).append(this); + this.scheduler.data.append(this); } pub fn initialStatErrorOnMainThread(this: *StatWatcher) void { if (this.closed) { - this.used_by_scheduler_thread.store(false, .release); return; } const jsvalue = statToJSStats(this.globalThis, &this.last_stat, this.bigint); this.last_jsvalue = JSC.Strong.create(jsvalue, this.globalThis); - const vm = this.globalThis.bunVM(); - _ = StatWatcher.listenerGetCached(this.js_this).?.call( this.globalThis, .undefined, @@ -400,10 +404,9 @@ pub const StatWatcher = struct { ) catch |err| this.globalThis.reportActiveExceptionAsUnhandled(err); if (this.closed) { - this.used_by_scheduler_thread.store(false, .release); return; } - vm.rareData().nodeFSStatWatcherScheduler(vm).append(this); + this.scheduler.data.append(this); } /// Called from any thread @@ -474,12 +477,12 @@ pub const StatWatcher = struct { .js_this = .zero, .closed = false, .path = alloc_file_path, - .used_by_scheduler_thread = std.atomic.Value(bool).init(true), // Instant.now will not fail on our target platforms. .last_check = std.time.Instant.now() catch unreachable, // InitStatTask is responsible for setting this .last_stat = std.mem.zeroes(bun.Stat), .last_jsvalue = .empty, + .scheduler = vm.rareData().nodeFSStatWatcherScheduler(vm), }; errdefer this.deinit(); diff --git a/src/bun.js/rare_data.zig b/src/bun.js/rare_data.zig index de0f7ada7cde48..f83f4947721dd7 100644 --- a/src/bun.js/rare_data.zig +++ b/src/bun.js/rare_data.zig @@ -42,7 +42,7 @@ spawn_ipc_usockets_context: ?*uws.SocketContext = null, mime_types: ?bun.http.MimeType.Map = null, -node_fs_stat_watcher_scheduler: ?*StatWatcherScheduler = null, +node_fs_stat_watcher_scheduler: ?bun.ptr.RefPtr(StatWatcherScheduler) = null, listening_sockets_for_watch_mode: std.ArrayListUnmanaged(bun.FileDescriptor) = .{}, listening_sockets_for_watch_mode_lock: bun.Mutex = .{}, @@ -454,11 +454,11 @@ pub fn globalDNSResolver(rare: *RareData, vm: *JSC.VirtualMachine) *JSC.DNS.DNSR return &rare.global_dns_data.?.resolver; } -pub fn nodeFSStatWatcherScheduler(rare: *RareData, vm: *JSC.VirtualMachine) *StatWatcherScheduler { - return rare.node_fs_stat_watcher_scheduler orelse { - rare.node_fs_stat_watcher_scheduler = StatWatcherScheduler.init(vm.allocator, vm); - return rare.node_fs_stat_watcher_scheduler.?; - }; +pub fn nodeFSStatWatcherScheduler(rare: *RareData, vm: *JSC.VirtualMachine) bun.ptr.RefPtr(StatWatcherScheduler) { + return (rare.node_fs_stat_watcher_scheduler orelse init: { + rare.node_fs_stat_watcher_scheduler = StatWatcherScheduler.init(vm); + break :init rare.node_fs_stat_watcher_scheduler.?; + }).dupeRef(); } pub fn s3DefaultClient(rare: *RareData, globalThis: *JSC.JSGlobalObject) JSC.JSValue { diff --git a/src/ptr/ref_count.zig b/src/ptr/ref_count.zig index d7be1a085ec8d2..3a400992f3857c 100644 --- a/src/ptr/ref_count.zig +++ b/src/ptr/ref_count.zig @@ -129,10 +129,9 @@ pub fn RefCount(T: type, field_name: []const u8, destructor: fn (*T) void, optio // utility functions - pub fn hasOneRef(self: *T) bool { - const counter = getCounter(self); - counter.assertNonThreadSafeCountIsSingleThreaded(); - return counter.active_counts == 1; + pub fn hasOneRef(count: *const @This()) bool { + count.assertNonThreadSafeCountIsSingleThreaded(); + return count.active_counts == 1; } pub fn dumpActiveRefs(count: *@This()) void { @@ -234,9 +233,8 @@ pub fn ThreadSafeRefCount(T: type, field_name: []const u8, destructor: fn (*T) v // utility functions - pub fn hasOneRef(self: *T) bool { - if (enable_debug) getCounter(self).debug.assertValid(); - const counter = getCounter(self); + pub fn hasOneRef(counter: *const @This()) bool { + if (enable_debug) counter.debug.assertValid(); return counter.active_counts.load(.seq_cst) == 1; } @@ -316,9 +314,9 @@ pub fn RefPtr(T: type) type { pub fn adoptRef(raw_ptr: *T) @This() { if (enable_debug) { bun.assert(raw_ptr.ref_count.hasOneRef()); - bun.assert(!raw_ptr.ref_count.debug.isEmpty()); + raw_ptr.ref_count.debug.assertValid(); } - return uncheckedAndUnsafeInit(raw_ptr); + return uncheckedAndUnsafeInit(raw_ptr, @returnAddress()); } /// This will assert that ALL references are cleaned up by the time the allocation scope ends. @@ -392,7 +390,7 @@ pub fn DebugData(thread_safe: bool) type { .count_pointer = null, }; - fn assertValid(debug: *@This()) void { + fn assertValid(debug: *const @This()) void { bun.assert(debug.magic == .valid); } From 577f0c264ec7d8ec2cacee473fdc32c56fece614 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 15 Apr 2025 13:44:43 -0700 Subject: [PATCH 37/88] Fix overriding process.exit code --- src/bun.js/node/types.zig | 12 +++++------- src/bun.js/web_worker.zig | 20 ++++++++++++++------ 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/bun.js/node/types.zig b/src/bun.js/node/types.zig index ed56e2dffbb3a4..97c98bab96ad04 100644 --- a/src/bun.js/node/types.zig +++ b/src/bun.js/node/types.zig @@ -1892,17 +1892,15 @@ pub const Process = struct { // TODO(@190n) this may need to be noreturn pub fn exit(globalObject: *JSC.JSGlobalObject, code: u8) callconv(.c) void { var vm = globalObject.bunVM(); + vm.exit_handler.exit_code = code; if (vm.worker) |worker| { - vm.exit_handler.exit_code = code; // TODO(@190n) we may need to use requestTerminate or throwTerminationException // instead to terminate the worker sooner - worker.notifyNeedTermination(); - return; + worker.exit(); + } else { + vm.onExit(); + vm.globalExit(); } - - vm.exit_handler.exit_code = code; - vm.onExit(); - vm.globalExit(); } // TODO: switch this to using *bun.wtf.String when it is added diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index fcc384176d1e55..14a13de454905d 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -7,15 +7,13 @@ const JSValue = JSC.JSValue; const Async = bun.Async; const WTFStringImpl = @import("../string.zig").WTFStringImpl; -const Bool = std.atomic.Value(bool); - /// Shared implementation of Web and Node `Worker` pub const WebWorker = struct { /// null when haven't started yet vm: ?*JSC.VirtualMachine = null, - status: std.atomic.Value(Status) = std.atomic.Value(Status).init(.start), + status: std.atomic.Value(Status) = .init(.start), /// To prevent UAF, the `spin` function (aka the worker's event loop) will call deinit once this is set and properly exit the loop. - requested_terminate: Bool = Bool.init(false), + requested_terminate: std.atomic.Value(bool) = .init(false), execution_context_id: u32 = 0, parent_context_id: u32 = 0, parent: *JSC.VirtualMachine, @@ -44,6 +42,9 @@ pub const WebWorker = struct { argv: []const WTFStringImpl, execArgv: ?[]const WTFStringImpl, + /// Used to distinguish between terminate() called by exit(), and terminate() called for other reasons + exit_called: bool = false, + pub const Status = enum(u8) { start, starting, @@ -431,7 +432,8 @@ pub const WebWorker = struct { } var promise = vm.loadEntryPointForWebWorker(path) catch { - vm.exit_handler.exit_code = 1; + // If we called process.exit(), don't override the exit code + if (!this.exit_called) vm.exit_handler.exit_code = 1; this.flushLogs(); this.exitAndDeinit(); return; @@ -503,7 +505,13 @@ pub const WebWorker = struct { } } - /// Request a terminate. Must be called from another thread (i.e. for worker.terminate()) + /// Implement process.exit(). May only be called from the Worker thread. + pub fn exit(this: *WebWorker) void { + this.exit_called = true; + this.notifyNeedTermination(); + } + + /// Request a terminate from any thread. pub fn notifyNeedTermination(this: *WebWorker) callconv(.C) void { if (this.status.load(.acquire) == .terminated) { return; From 0abd2efcb5c56054cd31c93ae3bf5c998055693e Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 15 Apr 2025 14:03:01 -0700 Subject: [PATCH 38/88] Revert "Accept callback in MessagePort.close" This reverts commit 6943c562b37af11e48639356b3496ff8fe5d1a32. --- src/bun.js/bindings/webcore/JSMessagePort.cpp | 4 ++-- src/bun.js/bindings/webcore/MessagePort.cpp | 17 +++-------------- src/bun.js/bindings/webcore/MessagePort.h | 2 +- 3 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/bun.js/bindings/webcore/JSMessagePort.cpp b/src/bun.js/bindings/webcore/JSMessagePort.cpp index 52431e15fb788d..45e80a8ba5ff3d 100644 --- a/src/bun.js/bindings/webcore/JSMessagePort.cpp +++ b/src/bun.js/bindings/webcore/JSMessagePort.cpp @@ -326,10 +326,10 @@ static inline JSC::EncodedJSValue jsMessagePortPrototypeFunction_closeBody(JSC:: auto& vm = JSC::getVM(lexicalGlobalObject); auto throwScope = DECLARE_THROW_SCOPE(vm); UNUSED_PARAM(throwScope); - auto callback = callFrame->argument(0); + UNUSED_PARAM(callFrame); auto& impl = castedThis->wrapped(); impl.jsUnref(lexicalGlobalObject); - RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS(*lexicalGlobalObject, throwScope, [&]() -> decltype(auto) { return impl.close(lexicalGlobalObject, callback); }))); + RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS(*lexicalGlobalObject, throwScope, [&]() -> decltype(auto) { return impl.close(); }))); } JSC_DEFINE_HOST_FUNCTION(jsMessagePortPrototypeFunction_close, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame)) diff --git a/src/bun.js/bindings/webcore/MessagePort.cpp b/src/bun.js/bindings/webcore/MessagePort.cpp index 2ddba751216278..6c3b67c5f59b41 100644 --- a/src/bun.js/bindings/webcore/MessagePort.cpp +++ b/src/bun.js/bindings/webcore/MessagePort.cpp @@ -28,8 +28,6 @@ #include "MessagePort.h" #include "BunClientData.h" -#include "BunProcess.h" -#include "AsyncContextFrame.h" // #include "Document.h" #include "EventNames.h" // #include "Logging.h" @@ -145,7 +143,7 @@ MessagePort::~MessagePort() } if (m_entangled) - close(nullptr, {}); + close(); if (auto* context = scriptExecutionContext()) context->destroyedMessagePort(*this); @@ -234,7 +232,7 @@ void MessagePort::start() scriptExecutionContext()->processMessageWithMessagePortsSoon([pendingActivity = Ref { *this }] {}); } -void MessagePort::close(JSGlobalObject* lexicalGlobalObject, JSValue callback) +void MessagePort::close() { if (m_isDetached) return; @@ -243,15 +241,6 @@ void MessagePort::close(JSGlobalObject* lexicalGlobalObject, JSValue callback) MessagePortChannelProvider::singleton().messagePortClosed(m_identifier); removeAllEventListeners(); - - if (lexicalGlobalObject && callback) { - auto* globalObject = defaultGlobalObject(lexicalGlobalObject); - if (globalObject && callback.isCallable()) { - if (auto* process = jsDynamicCast(globalObject->processObject())) { - process->queueNextTick(globalObject, AsyncContextFrame::withAsyncContextIfNeeded(globalObject, callback)); - } - } - } } void MessagePort::dispatchMessages() @@ -390,7 +379,7 @@ void MessagePort::contextDestroyed() { ASSERT(scriptExecutionContext()); - close(nullptr, {}); + close(); // ActiveDOMObject::contextDestroyed(); } diff --git a/src/bun.js/bindings/webcore/MessagePort.h b/src/bun.js/bindings/webcore/MessagePort.h index 98fff7bacb5f06..a5c9b8bc42c90b 100644 --- a/src/bun.js/bindings/webcore/MessagePort.h +++ b/src/bun.js/bindings/webcore/MessagePort.h @@ -60,7 +60,7 @@ class MessagePort final : /* public ActiveDOMObject, */ public ContextDestructio ExceptionOr postMessage(JSC::JSGlobalObject&, JSC::JSValue message, StructuredSerializeOptions&&); void start(); - void close(JSC::JSGlobalObject* lexicalGlobalObject, JSValue callback); + void close(); void entangle(); // Returns nullptr if the passed-in vector is empty. From 3451e492b8316b987306ad6f3be896bbd121fbb9 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 15 Apr 2025 14:06:25 -0700 Subject: [PATCH 39/88] Remove failing test --- ...est-worker-message-port-transfer-closed.js | 57 ------------------- 1 file changed, 57 deletions(-) delete mode 100644 test/js/node/test/parallel/test-worker-message-port-transfer-closed.js diff --git a/test/js/node/test/parallel/test-worker-message-port-transfer-closed.js b/test/js/node/test/parallel/test-worker-message-port-transfer-closed.js deleted file mode 100644 index d8ec04cbd250be..00000000000000 --- a/test/js/node/test/parallel/test-worker-message-port-transfer-closed.js +++ /dev/null @@ -1,57 +0,0 @@ -'use strict'; - -const common = require('../common'); -const assert = require('assert'); -const { MessageChannel } = require('worker_threads'); - -// This tests various behaviors around transferring MessagePorts with closing -// or closed handles. - -const { port1, port2 } = new MessageChannel(); - -const arrayBuf = new ArrayBuffer(10); -port1.onmessage = common.mustNotCall(); -port2.onmessage = common.mustNotCall(); - -function testSingle(closedPort, potentiallyOpenPort) { - assert.throws(common.mustCall(() => { - potentiallyOpenPort.postMessage(null, [arrayBuf, closedPort]); - }), common.mustCall((err) => { - assert.strictEqual(err.name, 'DataCloneError'); - assert.strictEqual(err.message, - 'MessagePort in transfer list is already detached'); - assert.strictEqual(err.code, 25); - assert.ok(err instanceof Error); - - const DOMException = err.constructor; - assert.ok(err instanceof DOMException); - assert.strictEqual(DOMException.name, 'DOMException'); - - return true; - })); - - // arrayBuf must not be transferred, even though it is present earlier in the - // transfer list than the closedPort. - assert.strictEqual(arrayBuf.byteLength, 10); -} - -function testBothClosed() { - testSingle(port1, port2); - testSingle(port2, port1); -} - -// Even though the port handles may not be completely closed in C++ land, the -// observable behavior must be that the closing/detachment is synchronous and -// instant. - -port1.close(common.mustCall(testBothClosed)); -testSingle(port1, port2); -port2.close(common.mustCall(testBothClosed)); -testBothClosed(); - -function tickUnref(n, fn) { - if (n === 0) return fn(); - setImmediate(tickUnref, n - 1, fn).unref(); -} - -tickUnref(10, common.mustNotCall('The communication channel is still open')); From 10dba919461636ac6b66b669da4a1fe8db254cbe Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 15 Apr 2025 16:13:08 -0700 Subject: [PATCH 40/88] Use threadId -1 for exited Workers --- src/bun.js/bindings/webcore/JSWorker.cpp | 4 +++- src/bun.js/bindings/webcore/Worker.cpp | 10 ++++++++++ src/bun.js/bindings/webcore/Worker.h | 3 ++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/bun.js/bindings/webcore/JSWorker.cpp b/src/bun.js/bindings/webcore/JSWorker.cpp index d50511fec3351b..a5757f67363130 100644 --- a/src/bun.js/bindings/webcore/JSWorker.cpp +++ b/src/bun.js/bindings/webcore/JSWorker.cpp @@ -374,11 +374,13 @@ JSC_DEFINE_CUSTOM_GETTER(jsWorker_threadIdGetter, (JSGlobalObject * lexicalGloba if (UNLIKELY(!castedThis)) return JSValue::encode(jsUndefined()); + auto& worker = castedThis->wrapped(); + if (worker.isClosingOrTerminated()) return JSValue::encode(jsNumber(-1)); // Main thread starts at 1 // // Note that we cannot use posix thread ids here because we don't know their thread id until the thread starts // - return JSValue::encode(jsNumber(castedThis->wrapped().clientIdentifier() - 1)); + return JSValue::encode(jsNumber(worker.clientIdentifier() - 1)); } /* Hash table for prototype */ diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index eb82391128e3bf..5b0b89a0e25b38 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -292,6 +292,11 @@ void Worker::terminate() // } // } +bool Worker::wasTerminated() const +{ + return m_terminationFlags & TerminatedFlag; +} + bool Worker::hasPendingActivity() const { auto onlineClosingFlags = m_onlineClosingFlags.load(); @@ -302,6 +307,11 @@ bool Worker::hasPendingActivity() const return !(m_terminationFlags & TerminatedFlag); } +bool Worker::isClosingOrTerminated() const +{ + return m_onlineClosingFlags & ClosingFlag; +} + void Worker::dispatchEvent(Event& event) { if (!m_terminationFlags) diff --git a/src/bun.js/bindings/webcore/Worker.h b/src/bun.js/bindings/webcore/Worker.h index b5c8edd4509975..1aec9a7c95417d 100644 --- a/src/bun.js/bindings/webcore/Worker.h +++ b/src/bun.js/bindings/webcore/Worker.h @@ -68,8 +68,9 @@ class Worker final : public ThreadSafeRefCounted, public EventTargetWith using ThreadSafeRefCounted::ref; void terminate(); - bool wasTerminated() const { return m_terminationFlags & TerminatedFlag; } + bool wasTerminated() const; bool hasPendingActivity() const; + bool isClosingOrTerminated() const; bool updatePtr(); String identifier() const { return m_identifier; } From 01ecf802b71087c376fdd0412035049763feb009 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 15 Apr 2025 16:37:55 -0700 Subject: [PATCH 41/88] Fix test-worker-http2-generic-streams-terminate.js --- src/bun.js/bindings/JSGlobalObject.zig | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/bun.js/bindings/JSGlobalObject.zig b/src/bun.js/bindings/JSGlobalObject.zig index 229ce50fc2966d..79d91f37318b33 100644 --- a/src/bun.js/bindings/JSGlobalObject.zig +++ b/src/bun.js/bindings/JSGlobalObject.zig @@ -501,7 +501,10 @@ pub const JSGlobalObject = opaque { /// return global.reportActiveExceptionAsUnhandled(err); /// pub fn reportActiveExceptionAsUnhandled(this: *JSGlobalObject, err: bun.JSError) void { - _ = this.bunVM().uncaughtException(this, this.takeException(err), false); + const exception = this.takeException(err); + if (!exception.isTerminationException(this.vm())) { + _ = this.bunVM().uncaughtException(this, exception, false); + } } pub fn vm(this: *JSGlobalObject) *VM { From f54bfd7453658764fb20b502902d684ade55f596 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 16 Apr 2025 12:24:44 -0700 Subject: [PATCH 42/88] Do not call UDP socket error handler on termination exception --- src/bun.js/api/bun/udp_socket.zig | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/bun.js/api/bun/udp_socket.zig b/src/bun.js/api/bun/udp_socket.zig index 04a9f57a974c9a..2e67ad3999d99c 100644 --- a/src/bun.js/api/bun/udp_socket.zig +++ b/src/bun.js/api/bun/udp_socket.zig @@ -43,7 +43,7 @@ fn onDrain(socket: *uws.udp.Socket) callconv(.C) void { event_loop.enter(); defer event_loop.exit(); _ = callback.call(this.globalThis, this.thisValue, &.{this.thisValue}) catch |err| { - _ = this.callErrorHandler(.zero, &.{this.globalThis.takeException(err)}); + this.callErrorHandler(.zero, this.globalThis.takeException(err)); }; } @@ -111,7 +111,7 @@ fn onData(socket: *uws.udp.Socket, buf: *uws.udp.PacketBuffer, packets: c_int) c JSC.jsNumber(port), hostname_string.transferToJS(globalThis), }) catch |err| { - _ = udpSocket.callErrorHandler(.zero, &.{udpSocket.globalThis.takeException(err)}); + udpSocket.callErrorHandler(.zero, udpSocket.globalThis.takeException(err)); }; } } @@ -375,22 +375,21 @@ pub const UDPSocket = struct { pub fn callErrorHandler( this: *This, thisValue: JSValue, - err: []const JSValue, - ) bool { + err: JSValue, + ) void { const callback = this.config.on_error; const globalThis = this.globalThis; const vm = globalThis.bunVM(); if (callback == .zero) { - if (err.len > 0) - _ = vm.uncaughtException(globalThis, err[0], false); - - return false; + _ = vm.uncaughtException(globalThis, err, false); + return; + } + if (err.isTerminationException(vm.jsc)) { + return; } - _ = callback.call(globalThis, thisValue, err) catch |e| globalThis.reportActiveExceptionAsUnhandled(e); - - return true; + _ = callback.call(globalThis, thisValue, &.{err}) catch |e| globalThis.reportActiveExceptionAsUnhandled(e); } pub fn setBroadcast(this: *This, globalThis: *JSGlobalObject, callframe: *CallFrame) bun.JSError!JSValue { From 5f4b7e03fb19c7914ead85c6e646945aa223b814 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 16 Apr 2025 14:27:22 -0700 Subject: [PATCH 43/88] Add exception checks in evaluateCommonJSModuleOnce --- src/bun.js/bindings/JSCommonJSModule.cpp | 34 +++++++++--------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/src/bun.js/bindings/JSCommonJSModule.cpp b/src/bun.js/bindings/JSCommonJSModule.cpp index 15f751f626fab3..aea46b74983e17 100644 --- a/src/bun.js/bindings/JSCommonJSModule.cpp +++ b/src/bun.js/bindings/JSCommonJSModule.cpp @@ -128,18 +128,23 @@ static bool evaluateCommonJSModuleOnce(JSC::VM& vm, Zig::GlobalObject* globalObj globalObject->requireResolveFunctionUnbound(), moduleObject->filename(), ArgList(), 1, globalObject->commonStrings().resolveString(globalObject)); + RETURN_IF_EXCEPTION(scope, ); requireFunction = JSC::JSBoundFunction::create(vm, globalObject, globalObject->requireFunctionUnbound(), moduleObject, ArgList(), 1, globalObject->commonStrings().requireString(globalObject)); + RETURN_IF_EXCEPTION(scope, ); requireFunction->putDirect(vm, vm.propertyNames->resolve, resolveFunction, 0); + RETURN_IF_EXCEPTION(scope, ); moduleObject->putDirect(vm, WebCore::clientData(vm)->builtinNames().requirePublicName(), requireFunction, 0); + RETURN_IF_EXCEPTION(scope, ); moduleObject->hasEvaluated = true; }; if (UNLIKELY(Bun__VM__specifierIsEvalEntryPoint(globalObject->bunVM(), JSValue::encode(filename)))) { initializeModuleObject(); + scope.assertNoExceptionExceptTermination(); // Using same approach as node, `arguments` in the entry point isn't defined // https://github.com/nodejs/node/blob/592c6907bfe1922f36240e9df076be1864c3d1bd/lib/internal/process/execution.js#L92 @@ -148,15 +153,9 @@ static bool evaluateCommonJSModuleOnce(JSC::VM& vm, Zig::GlobalObject* globalObj globalObject->putDirect(vm, Identifier::fromString(vm, "module"_s), moduleObject, 0); globalObject->putDirect(vm, Identifier::fromString(vm, "__filename"_s), filename, 0); globalObject->putDirect(vm, Identifier::fromString(vm, "__dirname"_s), dirname, 0); - scope.assertNoException(); - WTF::NakedPtr returnedException; - JSValue result = JSC::evaluate(globalObject, code, jsUndefined(), returnedException); - if (UNLIKELY(returnedException)) { - scope.throwException(globalObject, returnedException.get()); - return false; - } - ASSERT(!scope.exception()); + JSValue result = JSC::evaluate(globalObject, code, jsUndefined()); + RETURN_IF_EXCEPTION(scope, false); ASSERT(result); Bun__VM__setEntryPointEvalResultCJS(globalObject->bunVM(), JSValue::encode(result)); @@ -164,13 +163,8 @@ static bool evaluateCommonJSModuleOnce(JSC::VM& vm, Zig::GlobalObject* globalObj RELEASE_AND_RETURN(scope, true); } - WTF::NakedPtr returnedException; - JSValue fnValue = JSC::evaluate(globalObject, code, jsUndefined(), returnedException); - if (UNLIKELY(returnedException)) { - scope.throwException(globalObject, returnedException.get()); - RELEASE_AND_RETURN(scope, false); - } - ASSERT(!scope.exception()); + JSValue fnValue = JSC::evaluate(globalObject, code, jsUndefined()); + RETURN_IF_EXCEPTION(scope, false); ASSERT(fnValue); JSObject* fn = fnValue.getObject(); @@ -209,13 +203,9 @@ static bool evaluateCommonJSModuleOnce(JSC::VM& vm, Zig::GlobalObject* globalObj // // fn(exports, require, module, __filename, __dirname) { /* code */ }(exports, require, module, __filename, __dirname) // - JSC::profiledCall(globalObject, ProfilingReason::API, fn, callData, moduleObject, args, returnedException); - if (UNLIKELY(returnedException)) { - scope.throwException(globalObject, returnedException.get()); - return false; - } - ASSERT(!scope.exception()); - RELEASE_AND_RETURN(scope, true); + JSC::profiledCall(globalObject, ProfilingReason::API, fn, callData, moduleObject, args); + RETURN_IF_EXCEPTION(scope, false); + return true; } bool JSCommonJSModule::load(JSC::VM& vm, Zig::GlobalObject* globalObject) From 4874af1e4dff05a27a424534e4ab0e847f182141 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 16 Apr 2025 16:15:37 -0700 Subject: [PATCH 44/88] cleanup --- src/bun.js/bindings/lldbhalp.cpp | 21 --------------------- src/bun.js/bindings/lldbhalp.h | 3 --- 2 files changed, 24 deletions(-) delete mode 100644 src/bun.js/bindings/lldbhalp.cpp delete mode 100644 src/bun.js/bindings/lldbhalp.h diff --git a/src/bun.js/bindings/lldbhalp.cpp b/src/bun.js/bindings/lldbhalp.cpp deleted file mode 100644 index 291335baaba75b..00000000000000 --- a/src/bun.js/bindings/lldbhalp.cpp +++ /dev/null @@ -1,21 +0,0 @@ -#include "lldbhalp.h" - -namespace Bun { - -// ok for x86_64 and aarch64 -struct FrameEntry { - FrameEntry* next; - void (*return_address)(); -}; - -void (*get_trace_entry_at(void* frame_pointer, int idx))() -{ - if (!frame_pointer) return nullptr; - auto* entry = reinterpret_cast(frame_pointer); - for (int i = 0; i < idx; i++) { - entry = entry->next; - } - return entry->return_address; -} - -} // namespace Bun diff --git a/src/bun.js/bindings/lldbhalp.h b/src/bun.js/bindings/lldbhalp.h deleted file mode 100644 index 1c5af4a5202c35..00000000000000 --- a/src/bun.js/bindings/lldbhalp.h +++ /dev/null @@ -1,3 +0,0 @@ -namespace Bun { -void (*get_trace_entry_at(void* frame_pointer, int idx))(); -} From 34370627a193a072cef3d1c26c46e681817a2d91 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 18 Apr 2025 14:20:56 -0700 Subject: [PATCH 45/88] Do not call uv_library_shutdown --- src/Global.zig | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Global.zig b/src/Global.zig index 221708b3bdfd07..a3fcd7d1377595 100644 --- a/src/Global.zig +++ b/src/Global.zig @@ -217,10 +217,6 @@ pub export fn Bun__onExit() void { std.mem.doNotOptimizeAway(&Bun__atexit); Output.Source.Stdio.restore(); - - if (Environment.isWindows) { - bun.windows.libuv.uv_library_shutdown(); - } } comptime { From 26c9443a299c161df1cfb57a381ec265d737d44a Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 18 Apr 2025 15:34:01 -0700 Subject: [PATCH 46/88] Add exception checks to createNodeURLBinding --- src/bun.js/bindings/NodeURL.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/bun.js/bindings/NodeURL.cpp b/src/bun.js/bindings/NodeURL.cpp index 15f0581e764e12..9257e6026f8496 100644 --- a/src/bun.js/bindings/NodeURL.cpp +++ b/src/bun.js/bindings/NodeURL.cpp @@ -146,16 +146,23 @@ JSC_DEFINE_HOST_FUNCTION(jsDomainToUnicode, (JSC::JSGlobalObject * globalObject, JSC::JSValue createNodeURLBinding(Zig::GlobalObject* globalObject) { VM& vm = globalObject->vm(); + auto scope = DECLARE_THROW_SCOPE(vm); auto binding = constructEmptyArray(globalObject, nullptr, 2); + RETURN_IF_EXCEPTION(scope, {}); + auto domainToAsciiFunction = JSC::JSFunction::create(vm, globalObject, 1, "domainToAscii"_s, jsDomainToASCII, ImplementationVisibility::Public); + RETURN_IF_EXCEPTION(scope, {}); + auto domainToUnicodeFunction = JSC::JSFunction::create(vm, globalObject, 1, "domainToUnicode"_s, jsDomainToUnicode, ImplementationVisibility::Public); + RETURN_IF_EXCEPTION(scope, {}); + ASSERT(binding && domainToAsciiFunction && domainToUnicodeFunction); binding->putByIndexInline( globalObject, (unsigned)0, - JSC::JSFunction::create(vm, globalObject, 1, "domainToAscii"_s, jsDomainToASCII, ImplementationVisibility::Public), + domainToAsciiFunction, false); binding->putByIndexInline( globalObject, (unsigned)1, - JSC::JSFunction::create(vm, globalObject, 1, "domainToUnicode"_s, jsDomainToUnicode, ImplementationVisibility::Public), + domainToUnicodeFunction, false); return binding; } From 2d51c9eccf6dce56ec43e5fdb17f0c70a166fe8d Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 18 Apr 2025 15:47:31 -0700 Subject: [PATCH 47/88] Run skipped test --- test/js/node/worker_threads/worker_destruction.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/node/worker_threads/worker_destruction.test.ts b/test/js/node/worker_threads/worker_destruction.test.ts index 083a2243bd09d5..4fe20a7709ea51 100644 --- a/test/js/node/worker_threads/worker_destruction.test.ts +++ b/test/js/node/worker_threads/worker_destruction.test.ts @@ -3,7 +3,7 @@ import "harness"; import { join } from "path"; describe("Worker destruction", () => { - const method = ["Bun.connect", "Bun.listen"]; + const method = ["Bun.connect", "Bun.listen", "fetch"]; test.each(method)("bun closes cleanly when %s is used in a Worker that is terminating", method => { expect([join(import.meta.dir, "worker_thread_check.ts"), method]).toRun(); }); From 7d20028188d5664ce6bac10369dbb12b66b66e75 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 18 Apr 2025 16:53:28 -0700 Subject: [PATCH 48/88] Add some exception checks --- src/bun.js/bindings/BunInjectedScriptHost.cpp | 19 ++++++++++++++++--- src/bun.js/bindings/BunString.cpp | 6 ++---- src/bun.js/bindings/webcore/Worker.cpp | 10 +++++++++- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/bun.js/bindings/BunInjectedScriptHost.cpp b/src/bun.js/bindings/BunInjectedScriptHost.cpp index 51e8ef1ef5ed91..8cda82b022dd13 100644 --- a/src/bun.js/bindings/BunInjectedScriptHost.cpp +++ b/src/bun.js/bindings/BunInjectedScriptHost.cpp @@ -54,12 +54,14 @@ static JSObject* objectForEventTargetListeners(VM& vm, JSGlobalObject* exec, Eve auto* scriptExecutionContext = eventTarget->scriptExecutionContext(); if (!scriptExecutionContext) return nullptr; + auto scope = DECLARE_THROW_SCOPE(vm); JSObject* listeners = nullptr; for (auto& eventType : eventTarget->eventTypes()) { unsigned listenersForEventIndex = 0; auto* listenersForEvent = constructEmptyArray(exec, nullptr); + RETURN_IF_EXCEPTION(scope, {}); for (auto& eventListener : eventTarget->eventListeners(eventType)) { if (!is(eventListener->callback())) @@ -74,6 +76,7 @@ static JSObject* objectForEventTargetListeners(VM& vm, JSGlobalObject* exec, Eve continue; auto* propertiesForListener = constructEmptyObject(exec); + RETURN_IF_EXCEPTION(scope, {}); propertiesForListener->putDirect(vm, Identifier::fromString(vm, "callback"_s), jsFunction); propertiesForListener->putDirect(vm, Identifier::fromString(vm, "capture"_s), jsBoolean(eventListener->useCapture())); propertiesForListener->putDirect(vm, Identifier::fromString(vm, "passive"_s), jsBoolean(eventListener->isPassive())); @@ -82,8 +85,10 @@ static JSObject* objectForEventTargetListeners(VM& vm, JSGlobalObject* exec, Eve } if (listenersForEventIndex) { - if (!listeners) + if (!listeners) { listeners = constructEmptyObject(exec); + RETURN_IF_EXCEPTION(scope, {}); + } listeners->putDirect(vm, Identifier::fromString(vm, eventType), listenersForEvent); } } @@ -121,6 +126,7 @@ JSValue BunInjectedScriptHost::getInternalProperties(VM& vm, JSGlobalObject* exe if (auto* worker = JSWorker::toWrapped(vm, value)) { unsigned index = 0; auto* array = constructEmptyArray(exec, nullptr); + RETURN_IF_EXCEPTION(scope, {}); String name = worker->name(); if (!name.isEmpty()) @@ -142,6 +148,7 @@ JSValue BunInjectedScriptHost::getInternalProperties(VM& vm, JSGlobalObject* exe if (type == JSDOMWrapperType) { if (auto* headers = jsDynamicCast(value)) { auto* array = constructEmptyArray(exec, nullptr); + RETURN_IF_EXCEPTION(scope, {}); constructDataProperties(vm, exec, array, WebCore::getInternalProperties(vm, exec, headers)); RETURN_IF_EXCEPTION(scope, {}); return array; @@ -149,6 +156,7 @@ JSValue BunInjectedScriptHost::getInternalProperties(VM& vm, JSGlobalObject* exe if (auto* formData = jsDynamicCast(value)) { auto* array = constructEmptyArray(exec, nullptr); + RETURN_IF_EXCEPTION(scope, {}); constructDataProperties(vm, exec, array, WebCore::getInternalProperties(vm, exec, formData)); RETURN_IF_EXCEPTION(scope, {}); return array; @@ -157,6 +165,7 @@ JSValue BunInjectedScriptHost::getInternalProperties(VM& vm, JSGlobalObject* exe } else if (type == JSAsJSONType) { if (auto* params = jsDynamicCast(value)) { auto* array = constructEmptyArray(exec, nullptr); + RETURN_IF_EXCEPTION(scope, {}); constructDataProperties(vm, exec, array, WebCore::getInternalProperties(vm, exec, params)); RETURN_IF_EXCEPTION(scope, {}); return array; @@ -164,6 +173,7 @@ JSValue BunInjectedScriptHost::getInternalProperties(VM& vm, JSGlobalObject* exe if (auto* cookie = jsDynamicCast(value)) { auto* array = constructEmptyArray(exec, nullptr); + RETURN_IF_EXCEPTION(scope, {}); constructDataProperties(vm, exec, array, WebCore::getInternalProperties(vm, exec, cookie)); RETURN_IF_EXCEPTION(scope, {}); return array; @@ -171,6 +181,7 @@ JSValue BunInjectedScriptHost::getInternalProperties(VM& vm, JSGlobalObject* exe if (auto* cookieMap = jsDynamicCast(value)) { auto* array = constructEmptyArray(exec, nullptr); + RETURN_IF_EXCEPTION(scope, {}); constructDataProperties(vm, exec, array, WebCore::getInternalProperties(vm, exec, cookieMap)); RETURN_IF_EXCEPTION(scope, {}); return array; @@ -181,11 +192,13 @@ JSValue BunInjectedScriptHost::getInternalProperties(VM& vm, JSGlobalObject* exe if (auto* eventTarget = JSEventTarget::toWrapped(vm, value)) { unsigned index = 0; auto* array = constructEmptyArray(exec, nullptr); + RETURN_IF_EXCEPTION(scope, {}); - if (auto* listeners = objectForEventTargetListeners(vm, exec, eventTarget)) + if (auto* listeners = objectForEventTargetListeners(vm, exec, eventTarget)) { array->putDirectIndex(exec, index++, constructInternalProperty(vm, exec, "listeners"_s, listeners)); + RETURN_IF_EXCEPTION(scope, {}); + } - RETURN_IF_EXCEPTION(scope, {}); return array; } diff --git a/src/bun.js/bindings/BunString.cpp b/src/bun.js/bindings/BunString.cpp index 6818f955e8622c..c6159b73083b97 100644 --- a/src/bun.js/bindings/BunString.cpp +++ b/src/bun.js/bindings/BunString.cpp @@ -429,13 +429,11 @@ extern "C" JSC::EncodedJSValue BunString__createArray( // Using tryCreateUninitialized here breaks stuff.. // https://github.com/oven-sh/bun/issues/3931 JSC::JSArray* array = constructEmptyArray(globalObject, nullptr, length); - if (!array) { - JSC::throwOutOfMemoryError(globalObject, throwScope); - RELEASE_AND_RETURN(throwScope, JSValue::encode(JSC::JSValue())); - } + RETURN_IF_EXCEPTION(throwScope, {}); for (size_t i = 0; i < length; ++i) { array->putDirectIndex(globalObject, i, Bun::toJS(globalObject, *ptr++)); + RETURN_IF_EXCEPTION(throwScope, {}); } return JSValue::encode(array); diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index 5b0b89a0e25b38..3b2b0f63989512 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -536,22 +536,30 @@ JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) auto* pair = jsCast(deserialized); ASSERT(pair->length() == 2); workerData = pair->getIndexQuickly(0); + RETURN_IF_EXCEPTION(scope, {}); environmentData = jsCast(pair->getIndexQuickly(1)); + RETURN_IF_EXCEPTION(scope, {}); // Main thread starts at 1 threadId = jsNumber(worker->clientIdentifier() - 1); } else { environmentData = JSMap::create(vm, globalObject->mapStructure()); + RETURN_IF_EXCEPTION(scope, {}); } globalObject->m_nodeWorkerEnvironmentData.set(vm, globalObject, environmentData); JSObject* array = constructEmptyArray(globalObject, nullptr, 5); - + RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 0, workerData); + RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 1, threadId); + RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 2, JSFunction::create(vm, globalObject, 1, "receiveMessageOnPort"_s, jsReceiveMessageOnPort, ImplementationVisibility::Public, NoIntrinsic)); + RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 3, globalObject->nodeWorkerObjectSymbol()); + RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 4, environmentData); + RETURN_IF_EXCEPTION(scope, {}); return array; } From 3d25e8680bdeb67b1ade22db59a05456df2c0247 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 21 Apr 2025 10:14:46 -0700 Subject: [PATCH 49/88] Add exception checks --- src/bun.js/bindings/BunProcess.cpp | 317 +++--------------- .../bindings/JSEnvironmentVariableMap.cpp | 2 + 2 files changed, 54 insertions(+), 265 deletions(-) diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index 41b151fff28151..ae620838f0b0b8 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -185,8 +185,10 @@ static JSValue constructPlatform(VM& vm, JSObject* processObject) static JSValue constructVersions(VM& vm, JSObject* processObject) { + auto scope = DECLARE_THROW_SCOPE(vm); auto* globalObject = processObject->globalObject(); JSC::JSObject* object = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 24); + RETURN_IF_EXCEPTION(scope, {}); object->putDirect(vm, JSC::Identifier::fromString(vm, "node"_s), JSC::JSValue(JSC::jsOwnedString(vm, makeAtomString(ASCIILiteral::fromLiteralUnsafe(REPORTED_NODEJS_VERSION))))); @@ -1476,274 +1478,11 @@ JSC_DEFINE_CUSTOM_SETTER(setProcessConnected, (JSC::JSGlobalObject * lexicalGlob static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalObject, const String& fileName) { + auto scope = DECLARE_THROW_SCOPE(vm); #if !OS(WINDOWS) - // macOS output: - // { - // header: { - // reportVersion: 3, - // event: 'JavaScript API', - // trigger: 'GetReport', - // filename: null, - // dumpEventTime: '2023-11-16T17:56:55Z', - // dumpEventTimeStamp: '1700186215013', - // processId: 18234, - // threadId: 0, - // cwd: '/Users/jarred/Code/bun', - // commandLine: [ 'node' ], - // nodejsVersion: 'v20.8.0', - // wordSize: 64, - // arch: 'arm64', - // platform: 'darwin', - // componentVersions: process.versions, - // release: { - // name: 'node', - // headersUrl: 'https://nodejs.org/download/release/v20.8.0/node-v20.8.0-headers.tar.gz', - // sourceUrl: 'https://nodejs.org/download/release/v20.8.0/node-v20.8.0.tar.gz' - // }, - // osName: 'Darwin', - // osRelease: '22.6.0', - // osVersion: 'Darwin Kernel Version 22.6.0: Wed Jul 5 22:22:05 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6000', - // osMachine: 'arm64', - // cpus: [], - // networkInterfaces: [], - // host: 'macbook.local' - // }, - // javascriptStack: { - // message: 'Error [ERR_SYNTHETIC]: JavaScript Callstack', - // stack: [ - // 'at new NodeError (node:internal/errors:406:5)', - // 'at Object.getReport (node:internal/process/report:36:13)', - // 'at REPL68:1:16', - // 'at Script.runInThisContext (node:vm:122:12)', - // 'at REPLServer.defaultEval (node:repl:594:29)', - // 'at bound (node:domain:432:15)', - // 'at REPLServer.runBound [as eval] (node:domain:443:12)', - // 'at REPLServer.onLine (node:repl:924:10)', - // 'at REPLServer.emit (node:events:526:35)' - // ], - // errorProperties: { code: 'ERR_SYNTHETIC' } - // }, - // javascriptHeap: { - // totalMemory: 5734400, - // executableMemory: 524288, - // totalCommittedMemory: 4931584, - // availableMemory: 4341838112, - // totalGlobalHandlesMemory: 8192, - // usedGlobalHandlesMemory: 8000, - // usedMemory: 4304384, - // memoryLimit: 4345298944, - // mallocedMemory: 147560, - // externalMemory: 2152593, - // peakMallocedMemory: 892416, - // nativeContextCount: 1, - // detachedContextCount: 0, - // doesZapGarbage: 0, - // heapSpaces: { - // read_only_space: [Object], - // new_space: [Object], - // old_space: [Object], - // code_space: [Object], - // shared_space: [Object], - // new_large_object_space: [Object], - // large_object_space: [Object], - // code_large_object_space: [Object], - // shared_large_object_space: [Object] - // } - // }, - // nativeStack: [ - // { - // pc: '0x0000000105293a44', - // symbol: 'node::GetNodeReport(node::Environment*, char const*, char const*, v8::Local, std::__1::basic_ostream>&) [/opt/homebrew/Cellar/node/20.8.0/bin/node]' - // }, - // ], - // resourceUsage: { - // free_memory: 14188216320, - // total_memory: 68719476736, - // rss: 40009728, - // available_memory: 14188216320, - // userCpuSeconds: 0.244133, - // kernelCpuSeconds: 0.058853, - // cpuConsumptionPercent: 1.16533, - // userCpuConsumptionPercent: 0.938973, - // kernelCpuConsumptionPercent: 0.226358, - // maxRss: 41697280, - // pageFaults: { IORequired: 1465, IONotRequired: 1689 }, - // fsActivity: { reads: 0, writes: 0 } - // }, - // libuv: [], - // workers: [], - // environmentVariables: { - // PATH: '', - // }, - // userLimits: { - // core_file_size_blocks: { soft: 0, hard: 'unlimited' }, - // data_seg_size_kbytes: { soft: 'unlimited', hard: 'unlimited' }, - // file_size_blocks: { soft: 'unlimited', hard: 'unlimited' }, - // max_locked_memory_bytes: { soft: 'unlimited', hard: 'unlimited' }, - // max_memory_size_kbytes: { soft: 'unlimited', hard: 'unlimited' }, - // open_files: { soft: 2147483646, hard: 2147483646 }, - // stack_size_bytes: { soft: 8372224, hard: 67092480 }, - // cpu_time_seconds: { soft: 'unlimited', hard: 'unlimited' }, - // max_user_processes: { soft: 10666, hard: 16000 }, - // virtual_memory_kbytes: { soft: 'unlimited', hard: 'unlimited' } - // }, - // sharedObjects: [ - // '/opt/homebrew/Cellar/node/20.8.0/bin/node', - // ] - - // linux: - // { - // header: { - // reportVersion: 3, - // event: 'JavaScript API', - // trigger: 'GetReport', - // filename: null, - // dumpEventTime: '2023-11-16T18:41:38Z', - // dumpEventTimeStamp: '1700188898941', - // processId: 1621753, - // threadId: 0, - // cwd: '/home/jarred', - // commandLine: [ 'node' ], - // nodejsVersion: 'v20.5.0', - // glibcVersionRuntime: '2.35', - // glibcVersionCompiler: '2.28', - // wordSize: 64, - // arch: 'x64', - // platform: 'linux', - // componentVersions: { - // acorn: '8.10.0', - // ada: '2.5.1', - // ares: '1.19.1', - // base64: '0.5.0', - // brotli: '1.0.9', - // cjs_module_lexer: '1.2.2', - // cldr: '43.1', - // icu: '73.2', - // llhttp: '8.1.1', - // modules: '115', - // napi: '9', - // nghttp2: '1.55.1', - // nghttp3: '0.7.0', - // ngtcp2: '0.8.1', - // node: '20.5.0', - // openssl: '3.0.9+quic', - // simdutf: '3.2.14', - // tz: '2023c', - // undici: '5.22.1', - // unicode: '15.0', - // uv: '1.46.0', - // uvwasi: '0.0.18', - // v8: '11.3.244.8-node.10', - // zlib: '1.2.13.1-motley' - // }, - // release: { - // name: 'node', - // headersUrl: 'https://nodejs.org/download/release/v20.5.0/node-v20.5.0-headers.tar.gz', - // sourceUrl: 'https://nodejs.org/download/release/v20.5.0/node-v20.5.0.tar.gz' - // }, - // osName: 'Linux', - // osRelease: '5.17.0-1016-oem', - // osVersion: '#17-Ubuntu SMP PREEMPT Mon Aug 22 11:31:08 UTC 2022', - // osMachine: 'x86_64', - // cpus: [ - // ], - // networkInterfaces: [ - - // ], - // host: 'jarred-desktop' - // }, - // javascriptStack: { - // message: 'Error [ERR_SYNTHETIC]: JavaScript Callstack', - // stack: [ - // 'at new NodeError (node:internal/errors:405:5)', - // 'at Object.getReport (node:internal/process/report:36:13)', - // 'at REPL18:1:16', - // 'at Script.runInThisContext (node:vm:122:12)', - // 'at REPLServer.defaultEval (node:repl:593:29)', - // 'at bound (node:domain:433:15)', - // 'at REPLServer.runBound [as eval] (node:domain:444:12)', - // 'at REPLServer.onLine (node:repl:923:10)', - // 'at REPLServer.emit (node:events:526:35)' - // ], - // errorProperties: { code: 'ERR_SYNTHETIC' } - // }, - // javascriptHeap: { - // totalMemory: 6696960, - // executableMemory: 262144, - // totalCommittedMemory: 6811648, - // availableMemory: 4339915016, - // totalGlobalHandlesMemory: 8192, - // usedGlobalHandlesMemory: 4416, - // usedMemory: 5251032, - // memoryLimit: 4345298944, - // mallocedMemory: 262312, - // externalMemory: 2120511, - // peakMallocedMemory: 521312, - // nativeContextCount: 2, - // detachedContextCount: 0, - // doesZapGarbage: 0, - // heapSpaces: { - // read_only_space: [Object], - // new_space: [Object], - // old_space: [Object], - // code_space: [Object], - // shared_space: [Object], - // new_large_object_space: [Object], - // large_object_space: [Object], - // code_large_object_space: [Object], - // shared_large_object_space: [Object] - // } - // }, - // nativeStack: [ - - // ], - // resourceUsage: { - // free_memory: 64445558784, - // total_memory: 67358441472, - // rss: 52109312, - // constrained_memory: 18446744073709552000, - // available_memory: 18446744073657442000, - // userCpuSeconds: 0.105635, - // kernelCpuSeconds: 0.033611, - // cpuConsumptionPercent: 4.64153, - // userCpuConsumptionPercent: 3.52117, - // kernelCpuConsumptionPercent: 1.12037, - // maxRss: 52150272, - // pageFaults: { IORequired: 26, IONotRequired: 3917 }, - // fsActivity: { reads: 3536, writes: 24 } - // }, - // uvthreadResourceUsage: { - // userCpuSeconds: 0.088644, - // kernelCpuSeconds: 0.005214, - // cpuConsumptionPercent: 3.1286, - // userCpuConsumptionPercent: 2.9548, - // kernelCpuConsumptionPercent: 0.1738, - // fsActivity: { reads: 3512, writes: 0 } - // }, - // libuv: [ - - // ], - // workers: [], - // environmentVariables: { - // }, - // userLimits: { - // core_file_size_blocks: { soft: 'unlimited', hard: 'unlimited' }, - // data_seg_size_kbytes: { soft: 'unlimited', hard: 'unlimited' }, - // file_size_blocks: { soft: 'unlimited', hard: 'unlimited' }, - // max_locked_memory_bytes: { soft: 8419803136, hard: 8419803136 }, - // max_memory_size_kbytes: { soft: 'unlimited', hard: 'unlimited' }, - // open_files: { soft: 1048576, hard: 1048576 }, - // stack_size_bytes: { soft: 8388608, hard: 'unlimited' }, - // cpu_time_seconds: { soft: 'unlimited', hard: 'unlimited' }, - // max_user_processes: { soft: 256637, hard: 256637 }, - // virtual_memory_kbytes: { soft: 'unlimited', hard: 'unlimited' } - // }, - // sharedObjects: [ - // - // ] - // } auto constructUserLimits = [&]() -> JSValue { JSC::JSObject* userLimits = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 11); + RETURN_IF_EXCEPTION(scope, {}); static constexpr int resourceLimits[] = { RLIMIT_CORE, @@ -1773,6 +1512,7 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb for (size_t i = 0; i < std::size(resourceLimits); i++) { JSC::JSObject* limitObject = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 2); + RETURN_IF_EXCEPTION(scope, {}); struct rlimit limit; getrlimit(resourceLimits[i], &limit); @@ -1793,6 +1533,7 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb auto constructResourceUsage = [&]() -> JSC::JSValue { JSC::JSObject* resourceUsage = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 11); + RETURN_IF_EXCEPTION(scope, {}); rusage usage; @@ -1810,12 +1551,14 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb resourceUsage->putDirect(vm, JSC::Identifier::fromString(vm, "maxRss"_s), JSC::jsNumber(usage.ru_maxrss), 0); JSC::JSObject* pageFaults = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 2); + RETURN_IF_EXCEPTION(scope, {}); pageFaults->putDirect(vm, JSC::Identifier::fromString(vm, "IORequired"_s), JSC::jsNumber(usage.ru_majflt), 0); pageFaults->putDirect(vm, JSC::Identifier::fromString(vm, "IONotRequired"_s), JSC::jsNumber(usage.ru_minflt), 0); resourceUsage->putDirect(vm, JSC::Identifier::fromString(vm, "pageFaults"_s), pageFaults, 0); JSC::JSObject* fsActivity = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 2); + RETURN_IF_EXCEPTION(scope, {}); fsActivity->putDirect(vm, JSC::Identifier::fromString(vm, "reads"_s), JSC::jsNumber(usage.ru_inblock), 0); fsActivity->putDirect(vm, JSC::Identifier::fromString(vm, "writes"_s), JSC::jsNumber(usage.ru_oublock), 0); @@ -1826,6 +1569,7 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb auto constructHeader = [&]() -> JSC::JSValue { JSC::JSObject* header = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype()); + RETURN_IF_EXCEPTION(scope, {}); header->putDirect(vm, JSC::Identifier::fromString(vm, "reportVersion"_s), JSC::jsNumber(3), 0); header->putDirect(vm, JSC::Identifier::fromString(vm, "event"_s), JSC::jsString(vm, String("JavaScript API"_s)), 0); @@ -1856,15 +1600,19 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb } header->putDirect(vm, JSC::Identifier::fromString(vm, "cwd"_s), JSC::jsString(vm, String::fromUTF8ReplacingInvalidSequences(std::span { reinterpret_cast(cwd), strlen(cwd) })), 0); + RETURN_IF_EXCEPTION(scope, {}); } header->putDirect(vm, JSC::Identifier::fromString(vm, "commandLine"_s), JSValue::decode(Bun__Process__getExecArgv(globalObject)), 0); + RETURN_IF_EXCEPTION(scope, {}); header->putDirect(vm, JSC::Identifier::fromString(vm, "nodejsVersion"_s), JSC::jsString(vm, String::fromLatin1(REPORTED_NODEJS_VERSION)), 0); header->putDirect(vm, JSC::Identifier::fromString(vm, "wordSize"_s), JSC::jsNumber(64), 0); header->putDirect(vm, JSC::Identifier::fromString(vm, "arch"_s), constructArch(vm, header), 0); header->putDirect(vm, JSC::Identifier::fromString(vm, "platform"_s), constructPlatform(vm, header), 0); header->putDirect(vm, JSC::Identifier::fromString(vm, "componentVersions"_s), constructVersions(vm, header), 0); + RETURN_IF_EXCEPTION(scope, {}); header->putDirect(vm, JSC::Identifier::fromString(vm, "release"_s), constructProcessReleaseObject(vm, header), 0); + RETURN_IF_EXCEPTION(scope, {}); { // uname @@ -1899,24 +1647,36 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb #endif header->putDirect(vm, Identifier::fromString(vm, "cpus"_s), JSC::constructEmptyArray(globalObject, nullptr), 0); + RETURN_IF_EXCEPTION(scope, {}); header->putDirect(vm, Identifier::fromString(vm, "networkInterfaces"_s), JSC::constructEmptyArray(globalObject, nullptr), 0); + RETURN_IF_EXCEPTION(scope, {}); return header; }; auto constructJavaScriptHeap = [&]() -> JSC::JSValue { JSC::JSObject* heap = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 16); + RETURN_IF_EXCEPTION(scope, {}); JSC::JSObject* heapSpaces = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 9); heapSpaces->putDirect(vm, JSC::Identifier::fromString(vm, "read_only_space"_s), JSC::constructEmptyObject(globalObject), 0); + RETURN_IF_EXCEPTION(scope, {}); heapSpaces->putDirect(vm, JSC::Identifier::fromString(vm, "new_space"_s), JSC::constructEmptyObject(globalObject), 0); + RETURN_IF_EXCEPTION(scope, {}); heapSpaces->putDirect(vm, JSC::Identifier::fromString(vm, "old_space"_s), JSC::constructEmptyObject(globalObject), 0); + RETURN_IF_EXCEPTION(scope, {}); heapSpaces->putDirect(vm, JSC::Identifier::fromString(vm, "code_space"_s), JSC::constructEmptyObject(globalObject), 0); + RETURN_IF_EXCEPTION(scope, {}); heapSpaces->putDirect(vm, JSC::Identifier::fromString(vm, "shared_space"_s), JSC::constructEmptyObject(globalObject), 0); + RETURN_IF_EXCEPTION(scope, {}); heapSpaces->putDirect(vm, JSC::Identifier::fromString(vm, "new_large_object_space"_s), JSC::constructEmptyObject(globalObject), 0); + RETURN_IF_EXCEPTION(scope, {}); heapSpaces->putDirect(vm, JSC::Identifier::fromString(vm, "large_object_space"_s), JSC::constructEmptyObject(globalObject), 0); + RETURN_IF_EXCEPTION(scope, {}); heapSpaces->putDirect(vm, JSC::Identifier::fromString(vm, "code_large_object_space"_s), JSC::constructEmptyObject(globalObject), 0); + RETURN_IF_EXCEPTION(scope, {}); heapSpaces->putDirect(vm, JSC::Identifier::fromString(vm, "shared_large_object_space"_s), JSC::constructEmptyObject(globalObject), 0); + RETURN_IF_EXCEPTION(scope, {}); heap->putDirect(vm, JSC::Identifier::fromString(vm, "totalMemory"_s), JSC::jsDoubleNumber(static_cast(WTF::ramSize())), 0); heap->putDirect(vm, JSC::Identifier::fromString(vm, "executableMemory"_s), jsNumber(0), 0); @@ -1939,6 +1699,7 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb auto constructUVThreadResourceUsage = [&]() -> JSC::JSValue { JSC::JSObject* uvthreadResourceUsage = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 6); + RETURN_IF_EXCEPTION(scope, {}); uvthreadResourceUsage->putDirect(vm, JSC::Identifier::fromString(vm, "userCpuSeconds"_s), JSC::jsNumber(0), 0); uvthreadResourceUsage->putDirect(vm, JSC::Identifier::fromString(vm, "kernelCpuSeconds"_s), JSC::jsNumber(0), 0); @@ -1947,6 +1708,7 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb uvthreadResourceUsage->putDirect(vm, JSC::Identifier::fromString(vm, "kernelCpuConsumptionPercent"_s), JSC::jsNumber(0), 0); JSC::JSObject* fsActivity = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 2); + RETURN_IF_EXCEPTION(scope, {}); fsActivity->putDirect(vm, JSC::Identifier::fromString(vm, "reads"_s), JSC::jsNumber(0), 0); fsActivity->putDirect(vm, JSC::Identifier::fromString(vm, "writes"_s), JSC::jsNumber(0), 0); @@ -1957,6 +1719,7 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb auto constructJavaScriptStack = [&]() -> JSC::JSValue { JSC::JSObject* javascriptStack = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 3); + RETURN_IF_EXCEPTION(scope, {}); javascriptStack->putDirect(vm, vm.propertyNames->message, JSC::jsString(vm, String("Error [ERR_SYNTHETIC]: JavaScript Callstack"_s)), 0); @@ -1982,15 +1745,19 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb } JSC::JSArray* stackArray = JSC::constructEmptyArray(globalObject, nullptr); + RETURN_IF_EXCEPTION(scope, {}); stack.split('\n', [&](const WTF::StringView& line) { stackArray->push(globalObject, JSC::jsString(vm, line.toString().trim(isASCIIWhitespace))); + RETURN_IF_EXCEPTION(scope, ); }); + RETURN_IF_EXCEPTION(scope, {}); javascriptStack->putDirect(vm, vm.propertyNames->stack, stackArray, 0); } JSC::JSObject* errorProperties = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 1); + RETURN_IF_EXCEPTION(scope, {}); errorProperties->putDirect(vm, JSC::Identifier::fromString(vm, "code"_s), JSC::jsString(vm, String("ERR_SYNTHETIC"_s)), 0); javascriptStack->putDirect(vm, JSC::Identifier::fromString(vm, "errorProperties"_s), errorProperties, 0); return javascriptStack; @@ -1998,6 +1765,7 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb auto constructSharedObjects = [&]() -> JSC::JSValue { JSC::JSObject* sharedObjects = JSC::constructEmptyArray(globalObject, nullptr); + RETURN_IF_EXCEPTION(scope, {}); // TODO: @@ -2006,6 +1774,7 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb auto constructLibUV = [&]() -> JSC::JSValue { JSC::JSObject* libuv = JSC::constructEmptyArray(globalObject, nullptr); + RETURN_IF_EXCEPTION(scope, {}); // TODO: @@ -2014,6 +1783,7 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb auto constructWorkers = [&]() -> JSC::JSValue { JSC::JSObject* workers = JSC::constructEmptyArray(globalObject, nullptr); + RETURN_IF_EXCEPTION(scope, {}); // TODO: @@ -2026,6 +1796,7 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb auto constructCpus = [&]() -> JSC::JSValue { JSC::JSObject* cpus = JSC::constructEmptyArray(globalObject, nullptr); + RETURN_IF_EXCEPTION(scope, {}); // TODO: @@ -2034,6 +1805,7 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb auto constructNetworkInterfaces = [&]() -> JSC::JSValue { JSC::JSObject* networkInterfaces = JSC::constructEmptyArray(globalObject, nullptr); + RETURN_IF_EXCEPTION(scope, {}); // TODO: @@ -2042,6 +1814,7 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb auto constructNativeStack = [&]() -> JSC::JSValue { JSC::JSObject* nativeStack = JSC::constructEmptyArray(globalObject, nullptr); + RETURN_IF_EXCEPTION(scope, {}); // TODO: @@ -2050,20 +1823,34 @@ static JSValue constructReportObjectComplete(VM& vm, Zig::GlobalObject* globalOb { JSC::JSObject* report = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 19); + RETURN_IF_EXCEPTION(scope, {}); report->putDirect(vm, JSC::Identifier::fromString(vm, "header"_s), constructHeader(), 0); + RETURN_IF_EXCEPTION(scope, {}); report->putDirect(vm, JSC::Identifier::fromString(vm, "javascriptStack"_s), constructJavaScriptStack(), 0); + RETURN_IF_EXCEPTION(scope, {}); report->putDirect(vm, JSC::Identifier::fromString(vm, "javascriptHeap"_s), constructJavaScriptHeap(), 0); + RETURN_IF_EXCEPTION(scope, {}); report->putDirect(vm, JSC::Identifier::fromString(vm, "nativeStack"_s), constructNativeStack(), 0); + RETURN_IF_EXCEPTION(scope, {}); report->putDirect(vm, JSC::Identifier::fromString(vm, "resourceUsage"_s), constructResourceUsage(), 0); + RETURN_IF_EXCEPTION(scope, {}); report->putDirect(vm, JSC::Identifier::fromString(vm, "uvthreadResourceUsage"_s), constructUVThreadResourceUsage(), 0); + RETURN_IF_EXCEPTION(scope, {}); report->putDirect(vm, JSC::Identifier::fromString(vm, "libuv"_s), constructLibUV(), 0); + RETURN_IF_EXCEPTION(scope, {}); report->putDirect(vm, JSC::Identifier::fromString(vm, "workers"_s), constructWorkers(), 0); + RETURN_IF_EXCEPTION(scope, {}); report->putDirect(vm, JSC::Identifier::fromString(vm, "environmentVariables"_s), constructEnvironmentVariables(), 0); + RETURN_IF_EXCEPTION(scope, {}); report->putDirect(vm, JSC::Identifier::fromString(vm, "userLimits"_s), constructUserLimits(), 0); + RETURN_IF_EXCEPTION(scope, {}); report->putDirect(vm, JSC::Identifier::fromString(vm, "sharedObjects"_s), constructSharedObjects(), 0); + RETURN_IF_EXCEPTION(scope, {}); report->putDirect(vm, JSC::Identifier::fromString(vm, "cpus"_s), constructCpus(), 0); + RETURN_IF_EXCEPTION(scope, {}); report->putDirect(vm, JSC::Identifier::fromString(vm, "networkInterfaces"_s), constructNetworkInterfaces(), 0); + RETURN_IF_EXCEPTION(scope, {}); return report; } diff --git a/src/bun.js/bindings/JSEnvironmentVariableMap.cpp b/src/bun.js/bindings/JSEnvironmentVariableMap.cpp index efff692442cf29..647d3b5c8671e8 100644 --- a/src/bun.js/bindings/JSEnvironmentVariableMap.cpp +++ b/src/bun.js/bindings/JSEnvironmentVariableMap.cpp @@ -293,6 +293,7 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) #if OS(WINDOWS) JSArray* keyArray = constructEmptyArray(globalObject, nullptr, count); + RETURN_IF_EXCEPTION(scope, {}); #endif static NeverDestroyed TZ = MAKE_STATIC_STRING_IMPL("TZ"); @@ -337,6 +338,7 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) ZigString nameStr = toZigString(name); if (Bun__getEnvValue(globalObject, &nameStr, &valueString)) { JSValue value = jsString(vm, Zig::toStringCopy(valueString)); + RETURN_IF_EXCEPTION(scope, {}); object->putDirectIndex(globalObject, *index, value, 0, PutDirectIndexLikePutDirect); } continue; From 981820f340b07bd88d24f1dfe14ce4459d8ccbf2 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 21 Apr 2025 15:08:10 -0700 Subject: [PATCH 50/88] Forbid termination exception in test-timeout-behavior --- test/cli/test/test-timeout-behavior.test.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/test/cli/test/test-timeout-behavior.test.ts b/test/cli/test/test-timeout-behavior.test.ts index f1ebe37943e0f5..c7188fe6330954 100644 --- a/test/cli/test/test-timeout-behavior.test.ts +++ b/test/cli/test/test-timeout-behavior.test.ts @@ -18,14 +18,18 @@ if (isFlaky && isLinux) { env: bunEnv, }); const [out, err, exitCode] = await Promise.all([new Response(stdout).text(), new Response(stderr).text(), exited]); - console.log(out); - console.log(err); + // merge outputs so that this test still works if we change which things are printed to stdout + // and which to stderr + const combined = out + err; // exit code should indicate failed tests, not abort or anything expect(exitCode).toBe(1); - expect(out).not.toContain("This should not be printed!"); - expect(err).toContain("killed 1 dangling process"); + expect(combined).not.toContain("This should not be printed!"); + expect(combined).toContain("killed 1 dangling process"); + // we should not expose the termination exception + expect(combined).not.toContain("Unhandled error between tests"); + expect(combined).not.toContain("JavaScript execution terminated"); // both tests should have run with the expected result - expect(err).toContain("(fail) test timeout kills dangling processes"); - expect(err).toContain("(pass) slow test after test timeout"); + expect(combined).toContain("(fail) test timeout kills dangling processes"); + expect(combined).toContain("(pass) slow test after test timeout"); }); } From 8c43780596f940115eab772d50d521218dcfb273 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 21 Apr 2025 15:31:09 -0700 Subject: [PATCH 51/88] Do not expose termination exception when test times out --- src/bun.js/test/jest.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bun.js/test/jest.zig b/src/bun.js/test/jest.zig index 2c228efbb1cb0b..1ca67492438141 100644 --- a/src/bun.js/test/jest.zig +++ b/src/bun.js/test/jest.zig @@ -1562,6 +1562,7 @@ pub const TestRunnerTask = struct { } }; + this.globalThis.clearTerminationException(); _ = vm.uncaughtException(this.globalThis, err, true); } From 84dd6378f0e551f6ec75f17b83e09585fbc2badc Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 21 Apr 2025 15:47:30 -0700 Subject: [PATCH 52/88] empty commit to see if zig fmt action is fixed From 8e54de3eb7f6fdae640efb36db7232912d4bf743 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 29 Apr 2025 18:15:29 -0700 Subject: [PATCH 53/88] clean up worker_thread_check.ts --- test/js/node/worker_threads/worker_thread_check.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/js/node/worker_threads/worker_thread_check.ts b/test/js/node/worker_threads/worker_thread_check.ts index 004786cad6e93a..ba4cb35f7b0a1e 100644 --- a/test/js/node/worker_threads/worker_thread_check.ts +++ b/test/js/node/worker_threads/worker_thread_check.ts @@ -7,9 +7,6 @@ const sleep = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); if (isMainThread) { let action = process.argv.at(-1); - if (process.argv.length === 2) { - action = "Bun.connect"; - } const server = Bun.serve({ port: 0, @@ -20,7 +17,7 @@ if (isMainThread) { let remaining = RUN_COUNT; while (remaining--) { - const promises = []; + const promises: Promise[] = []; for (let i = 0; i < CONCURRENCY; i++) { const worker = new Worker(import.meta.url, { @@ -31,16 +28,17 @@ if (isMainThread) { env: process.env, }); worker.ref(); - const { promise, resolve } = Promise.withResolvers(); + const { promise, resolve, reject } = Promise.withResolvers(); promises.push(promise); worker.on("online", () => { - sleep(1) + sleep(10000) .then(() => { return worker.terminate(); }) .finally(resolve); }); + worker.on("error", e => reject(e)); } await Promise.all(promises); From baaf33efaecfb8cc377c13f11083ee92f4e6a8d3 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 30 Apr 2025 11:49:45 -0700 Subject: [PATCH 54/88] Undo faster termination change and remove associated tests --- src/bun.js/web_worker.zig | 2 +- .../test-worker-nexttick-terminate.js | 25 ------------------- .../test-worker-terminate-microtask-loop.js | 19 -------------- .../test-worker-vm-context-terminate.js | 19 -------------- ...r-voluntarily-exit-followed-by-addition.js | 18 ------------- test/js/web/workers/worker.test.ts | 2 +- 6 files changed, 2 insertions(+), 83 deletions(-) delete mode 100644 test/js/node/test/parallel/test-worker-nexttick-terminate.js delete mode 100644 test/js/node/test/parallel/test-worker-terminate-microtask-loop.js delete mode 100644 test/js/node/test/parallel/test-worker-vm-context-terminate.js delete mode 100644 test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-addition.js diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index 52bb987c21669e..1b795f58614e34 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -523,7 +523,7 @@ pub const WebWorker = struct { if (this.vm) |vm| { vm.eventLoop().wakeup(); - vm.jsc.notifyNeedTermination(); + // TODO(@190n) notifyNeedTermination } } diff --git a/test/js/node/test/parallel/test-worker-nexttick-terminate.js b/test/js/node/test/parallel/test-worker-nexttick-terminate.js deleted file mode 100644 index 0e5d7e096c57ec..00000000000000 --- a/test/js/node/test/parallel/test-worker-nexttick-terminate.js +++ /dev/null @@ -1,25 +0,0 @@ -'use strict'; -const common = require('../common'); -const { Worker } = require('worker_threads'); - -// Checks that terminating in the middle of `process.nextTick()` does not -// Crash the process. - -const w = new Worker(` -require('worker_threads').parentPort.postMessage('0'); -process.nextTick(() => { - while(1); -}); -`, { eval: true }); - -// Test deprecation of .terminate() with callback. -common.expectWarning( - 'DeprecationWarning', - 'Passing a callback to worker.terminate() is deprecated. ' + - 'It returns a Promise instead.', 'DEP0132'); - -w.on('message', common.mustCall(() => { - setTimeout(() => { - w.terminate(common.mustCall()).then(common.mustCall()); - }, 1); -})); diff --git a/test/js/node/test/parallel/test-worker-terminate-microtask-loop.js b/test/js/node/test/parallel/test-worker-terminate-microtask-loop.js deleted file mode 100644 index b2351c5d0bb051..00000000000000 --- a/test/js/node/test/parallel/test-worker-terminate-microtask-loop.js +++ /dev/null @@ -1,19 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const { Worker } = require('worker_threads'); - -// Verify that `.terminate()` interrupts the microtask queue. - -const worker = new Worker(` -function loop() { Promise.resolve().then(loop); } loop(); -require('worker_threads').parentPort.postMessage('up'); -`, { eval: true }); - -worker.once('message', common.mustCall(() => { - setImmediate(() => worker.terminate()); -})); - -worker.once('exit', common.mustCall((code) => { - assert.strictEqual(code, 1); -})); diff --git a/test/js/node/test/parallel/test-worker-vm-context-terminate.js b/test/js/node/test/parallel/test-worker-vm-context-terminate.js deleted file mode 100644 index 23b58ba4db14d5..00000000000000 --- a/test/js/node/test/parallel/test-worker-vm-context-terminate.js +++ /dev/null @@ -1,19 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const vm = require('vm'); -const { Worker } = require('worker_threads'); - -// Do not use isMainThread so that this test itself can be run inside a Worker. -if (!process.env.HAS_STARTED_WORKER) { - process.env.HAS_STARTED_WORKER = 1; - const w = new Worker(__filename); - w.on('online', common.mustCall(() => { - setTimeout(() => w.terminate(), 50); - })); - w.on('error', common.mustNotCall()); - w.on('exit', common.mustCall((code) => assert.strictEqual(code, 1))); -} else { - while (true) - vm.runInNewContext(''); -} diff --git a/test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-addition.js b/test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-addition.js deleted file mode 100644 index 9f152bd5c62b0c..00000000000000 --- a/test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-addition.js +++ /dev/null @@ -1,18 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const { Worker, isMainThread } = require('worker_threads'); - -if (isMainThread) { - const workerData = new Int32Array(new SharedArrayBuffer(4)); - new Worker(__filename, { - workerData, - }); - process.on('beforeExit', common.mustCall(() => { - assert.strictEqual(workerData[0], 0); - })); -} else { - const { workerData } = require('worker_threads'); - process.exit(); - workerData[0] = 1; -} diff --git a/test/js/web/workers/worker.test.ts b/test/js/web/workers/worker.test.ts index fc9597f3bf53f2..d7f2f81869868a 100644 --- a/test/js/web/workers/worker.test.ts +++ b/test/js/web/workers/worker.test.ts @@ -338,7 +338,7 @@ describe("worker_threads", () => { expect(code).toBe(2); }); - test("worker terminating forcefully properly interrupts", async () => { + test.todo("worker terminating forcefully properly interrupts", async () => { const worker = new wt.Worker(new URL("worker-fixture-while-true.js", import.meta.url).href, {}); await new Promise(done => { worker.on("message", () => done()); From 18c19eab55615da68b78ba356379661a8e8e5404 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 30 Apr 2025 13:05:48 -0700 Subject: [PATCH 55/88] don't wait 10 seconds --- test/js/node/worker_threads/worker_thread_check.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/node/worker_threads/worker_thread_check.ts b/test/js/node/worker_threads/worker_thread_check.ts index ba4cb35f7b0a1e..8ee0eb5bc17e84 100644 --- a/test/js/node/worker_threads/worker_thread_check.ts +++ b/test/js/node/worker_threads/worker_thread_check.ts @@ -32,7 +32,7 @@ if (isMainThread) { promises.push(promise); worker.on("online", () => { - sleep(10000) + sleep(1) .then(() => { return worker.terminate(); }) From d630139a4e02b417cc4c52b45a0d0cd9dc3214bf Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 30 Apr 2025 13:13:51 -0700 Subject: [PATCH 56/88] delete test-worker-voluntarily-exit-followed-by-throw --- ...rker-voluntarily-exit-followed-by-throw.js | 23 ------------------- 1 file changed, 23 deletions(-) delete mode 100644 test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-throw.js diff --git a/test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-throw.js b/test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-throw.js deleted file mode 100644 index 92c4d5596cbddf..00000000000000 --- a/test/js/node/test/parallel/test-worker-voluntarily-exit-followed-by-throw.js +++ /dev/null @@ -1,23 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const { Worker, isMainThread } = require('worker_threads'); - -if (isMainThread) { - const workerData = new Int32Array(new SharedArrayBuffer(4)); - new Worker(__filename, { - workerData, - }); - process.on('beforeExit', common.mustCall(() => { - assert.strictEqual(workerData[0], 0); - })); -} else { - const { workerData } = require('worker_threads'); - try { - process.exit(); - throw new Error('xxx'); - // eslint-disable-next-line no-unused-vars - } catch (err) { - workerData[0] = 1; - } -} From 6d182f38f60d27890e84f8605613543238987792 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 30 Apr 2025 13:16:52 -0700 Subject: [PATCH 57/88] delete test-worker-process-exit-async-module --- .../parallel/test-worker-process-exit-async-module.js | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 test/js/node/test/parallel/test-worker-process-exit-async-module.js diff --git a/test/js/node/test/parallel/test-worker-process-exit-async-module.js b/test/js/node/test/parallel/test-worker-process-exit-async-module.js deleted file mode 100644 index 38d4ad74c7bd85..00000000000000 --- a/test/js/node/test/parallel/test-worker-process-exit-async-module.js +++ /dev/null @@ -1,11 +0,0 @@ -'use strict'; - -const common = require('../common'); -const assert = require('assert'); -const { Worker } = require('worker_threads'); - -// Regression for https://github.com/nodejs/node/issues/43182. -const w = new Worker(new URL('data:text/javascript,process.exit(1);await new Promise(()=>{ process.exit(2); })')); -w.on('exit', common.mustCall((code) => { - assert.strictEqual(code, 1); -})); From 721ac4c1e3b7b8e8bc5bae67e928b49fa105c022 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 30 Apr 2025 14:09:13 -0700 Subject: [PATCH 58/88] delete test-worker-abort-on-uncaught-exception-terminate --- ...t-worker-abort-on-uncaught-exception-terminate.js | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 test/js/node/test/parallel/test-worker-abort-on-uncaught-exception-terminate.js diff --git a/test/js/node/test/parallel/test-worker-abort-on-uncaught-exception-terminate.js b/test/js/node/test/parallel/test-worker-abort-on-uncaught-exception-terminate.js deleted file mode 100644 index de73b5e4e34e47..00000000000000 --- a/test/js/node/test/parallel/test-worker-abort-on-uncaught-exception-terminate.js +++ /dev/null @@ -1,12 +0,0 @@ -// Flags: --abort-on-uncaught-exception -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const { Worker } = require('worker_threads'); - -// Tests that --abort-on-uncaught-exception does not apply to -// termination exceptions. - -const w = new Worker('while(true);', { eval: true }); -w.on('online', common.mustCall(() => w.terminate())); -w.on('exit', common.mustCall((code) => assert.strictEqual(code, 1))); From 59ab6b51110ad03eae5a6a0349512009ab95ed74 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 1 May 2025 18:00:22 -0700 Subject: [PATCH 59/88] skip broken test --- test/js/node/worker_threads/worker_destruction.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/js/node/worker_threads/worker_destruction.test.ts b/test/js/node/worker_threads/worker_destruction.test.ts index 4fe20a7709ea51..5e483a5ba3f175 100644 --- a/test/js/node/worker_threads/worker_destruction.test.ts +++ b/test/js/node/worker_threads/worker_destruction.test.ts @@ -1,10 +1,14 @@ import { describe, expect, test } from "bun:test"; import "harness"; +import { isBroken } from "harness"; import { join } from "path"; describe("Worker destruction", () => { const method = ["Bun.connect", "Bun.listen", "fetch"]; - test.each(method)("bun closes cleanly when %s is used in a Worker that is terminating", method => { - expect([join(import.meta.dir, "worker_thread_check.ts"), method]).toRun(); + describe.each(method)("bun when %s is used in a Worker that is terminating", method => { + // fetch: ASAN failure + test.skipIf(isBroken && method == "fetch")("exits cleanly", () => { + expect([join(import.meta.dir, "worker_thread_check.ts"), method]).toRun(); + }); }); }); From a8dc6f41d2312cc43a53221b388c511e40db9b0b Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 1 May 2025 18:04:16 -0700 Subject: [PATCH 60/88] add ScriptExecutionContextIdentifier binding --- src/bun.js/bindings/JSGlobalObject.zig | 5 ++++ .../bindings/ScriptExecutionContext.cpp | 12 ++++++++++ src/bun.js/bindings/ScriptExecutionContext.h | 24 ++++++++++++++++++- src/bun.js/webcore.zig | 1 + src/bun.js/webcore/ScriptExecutionContext.zig | 18 ++++++++++++++ src/c-headers-for-zig.h | 2 ++ 6 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 src/bun.js/webcore/ScriptExecutionContext.zig diff --git a/src/bun.js/bindings/JSGlobalObject.zig b/src/bun.js/bindings/JSGlobalObject.zig index 42dbb7162b62a4..ba456213972517 100644 --- a/src/bun.js/bindings/JSGlobalObject.zig +++ b/src/bun.js/bindings/JSGlobalObject.zig @@ -846,6 +846,11 @@ pub const JSGlobalObject = opaque { return JSC.Error.INVALID_ARG_TYPE.fmt(global, fmt, args); } + pub fn scriptExecutionContextIdentifier(global: *JSC.JSGlobalObject) bun.webcore.ScriptExecutionContext.Identifier { + const c_global: *bun.c.JSC__JSGlobalObject = @ptrCast(global); + return @enumFromInt(bun.c.ScriptExecutionContextIdentifier__forGlobalObject(c_global)); + } + pub const Extern = [_][]const u8{ "create", "getModuleRegistryMap", "resetModuleRegistryMap" }; comptime { diff --git a/src/bun.js/bindings/ScriptExecutionContext.cpp b/src/bun.js/bindings/ScriptExecutionContext.cpp index 4b9e5ba42ab2a5..c324998da9c4a5 100644 --- a/src/bun.js/bindings/ScriptExecutionContext.cpp +++ b/src/bun.js/bindings/ScriptExecutionContext.cpp @@ -392,3 +392,15 @@ void ScriptExecutionContext::postTaskOnTimeout(FunctionscriptExecutionContext()->identifier(); +} + +JSC__JSGlobalObject* ScriptExecutionContextIdentifier__getGlobalObject(WebCore__ScriptExecutionContextIdentifier id) +{ + auto* context = WebCore::ScriptExecutionContext::getScriptExecutionContext(id); + if (context) return context->globalObject(); + return nullptr; +} diff --git a/src/bun.js/bindings/ScriptExecutionContext.h b/src/bun.js/bindings/ScriptExecutionContext.h index c17894f8921ac6..a7b800ab21916b 100644 --- a/src/bun.js/bindings/ScriptExecutionContext.h +++ b/src/bun.js/bindings/ScriptExecutionContext.h @@ -1,5 +1,13 @@ #pragma once +#include + +typedef uint32_t WebCore__ScriptExecutionContextIdentifier; + +#ifndef __cplusplus +typedef struct JSC__JSGlobalObject JSC__JSGlobalObject; +#else + #include "root.h" #include "ActiveDOMObject.h" #include @@ -23,6 +31,8 @@ struct us_socket_t; struct us_socket_context_t; struct us_loop_t; +using JSC__JSGlobalObject = JSC::JSGlobalObject; + namespace WebCore { class WebSocket; @@ -34,7 +44,7 @@ class EventLoopTask; class ContextDestructionObserver; -using ScriptExecutionContextIdentifier = uint32_t; +using ScriptExecutionContextIdentifier = WebCore__ScriptExecutionContextIdentifier; #if ENABLE(MALLOC_BREAKDOWN) DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(ScriptExecutionContext); @@ -208,3 +218,15 @@ class ScriptExecutionContext : public CanMakeWeakPtr, pu ScriptExecutionContext* executionContext(JSC::JSGlobalObject*); } + +#endif + +// Zig bindings +#ifdef __cplusplus +extern "C" { +#endif +WebCore__ScriptExecutionContextIdentifier ScriptExecutionContextIdentifier__forGlobalObject(JSC__JSGlobalObject*); +JSC__JSGlobalObject* ScriptExecutionContextIdentifier__getGlobalObject(WebCore__ScriptExecutionContextIdentifier); +#ifdef __cplusplus +} +#endif diff --git a/src/bun.js/webcore.zig b/src/bun.js/webcore.zig index e128398d2632b3..55e645a20bb5a7 100644 --- a/src/bun.js/webcore.zig +++ b/src/bun.js/webcore.zig @@ -35,6 +35,7 @@ pub const FetchHeaders = @import("bindings/FetchHeaders.zig").FetchHeaders; pub const ByteBlobLoader = @import("webcore/ByteBlobLoader.zig"); pub const ByteStream = @import("webcore/ByteStream.zig"); pub const FileReader = @import("webcore/FileReader.zig"); +pub const ScriptExecutionContext = @import("webcore/ScriptExecutionContext.zig"); pub const streams = @import("webcore/streams.zig"); pub const NetworkSink = streams.NetworkSink; diff --git a/src/bun.js/webcore/ScriptExecutionContext.zig b/src/bun.js/webcore/ScriptExecutionContext.zig new file mode 100644 index 00000000000000..f45e5e54014179 --- /dev/null +++ b/src/bun.js/webcore/ScriptExecutionContext.zig @@ -0,0 +1,18 @@ +const bun = @import("bun"); + +/// Safe handle to a JavaScript execution environment that may have exited. +/// Obtain with global_object.scriptExecutionContextIdentifier() +pub const Identifier = enum(bun.c.WebCore__ScriptExecutionContextIdentifier) { + _, + + /// Returns null if the context referred to by `self` no longer exists + pub fn globalObject(self: Identifier) ?*bun.jsc.JSGlobalObject { + return @ptrCast(bun.c.ScriptExecutionContextIdentifier__getGlobalObject(@intFromEnum(self))); + } + + /// Returns null if the context referred to by `self` no longer exists + pub fn bunVM(self: Identifier) ?*bun.jsc.VirtualMachine { + // concurrently because we expect these identifiers are mostly used by off-thread tasks + return (self.globalObject() orelse return null).bunVMConcurrently(); + } +}; diff --git a/src/c-headers-for-zig.h b/src/c-headers-for-zig.h index 90e2998161295d..96a4f61adf8367 100644 --- a/src/c-headers-for-zig.h +++ b/src/c-headers-for-zig.h @@ -69,3 +69,5 @@ #undef lstat #undef fstat #undef stat + +#include "bun.js/bindings/ScriptExecutionContext.h" From 5fc5c54f45ab96f3423fa68f9afb98da1d53d7c5 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 1 May 2025 18:13:41 -0700 Subject: [PATCH 61/88] replace translate-c symbol instead of ptrCasting --- build.zig | 60 ++++++++++--------- src/bun.js/bindings/JSGlobalObject.zig | 3 +- src/bun.js/webcore/ScriptExecutionContext.zig | 2 +- ...ranslate_c.zig => process_translate_c.zig} | 30 +++++++--- 4 files changed, 55 insertions(+), 40 deletions(-) rename src/codegen/{process_windows_translate_c.zig => process_translate_c.zig} (68%) diff --git a/build.zig b/build.zig index 3659c5936d4b29..c705f55b61aa82 100644 --- a/build.zig +++ b/build.zig @@ -507,34 +507,38 @@ fn getTranslateC(b: *Build, initial_target: std.Build.ResolvedTarget, optimize: translate_c.defineCMacroRaw(b.fmt("{s}={d}", .{ str, @intFromBool(value) })); } - if (target.result.os.tag == .windows) { - // translate-c is unable to translate the unsuffixed windows functions - // like `SetCurrentDirectory` since they are defined with an odd macro - // that translate-c doesn't handle. - // - // #define SetCurrentDirectory __MINGW_NAME_AW(SetCurrentDirectory) - // - // In these cases, it's better to just reference the underlying function - // directly: SetCurrentDirectoryW. To make the error better, a post - // processing step is applied to the translate-c file. - // - // Additionally, this step makes it so that decls like NTSTATUS and - // HANDLE point to the standard library structures. - const helper_exe = b.addExecutable(.{ - .name = "process_windows_translate_c", - .root_module = b.createModule(.{ - .root_source_file = b.path("src/codegen/process_windows_translate_c.zig"), - .target = b.graph.host, - .optimize = .Debug, - }), - }); - const in = translate_c.getOutput(); - const run = b.addRunArtifact(helper_exe); - run.addFileArg(in); - const out = run.addOutputFileArg("c-headers-for-zig.zig"); - return out; - } - return translate_c.getOutput(); + // translate-c is unable to translate the unsuffixed windows functions + // like `SetCurrentDirectory` since they are defined with an odd macro + // that translate-c doesn't handle. + // + // #define SetCurrentDirectory __MINGW_NAME_AW(SetCurrentDirectory) + // + // In these cases, it's better to just reference the underlying function + // directly: SetCurrentDirectoryW. To make the error better, a post + // processing step is applied to the translate-c file. + // + // Additionally, this step makes it so that decls like NTSTATUS and + // HANDLE point to the standard library structures. + // + // This is also used on all platforms to rewrite some `opaque` types. + // In most cases, we want to define our own `opaque` so that we can + // use it as a namespace that provides decls. Normally the translate-c + // output will include its own `opaque` types, which would require a + // `@ptrCast` to and from the type we define. Instead, we replace these + // types with their equivalents imported from Bun. + const helper_exe = b.addExecutable(.{ + .name = "process_translate_c", + .root_module = b.createModule(.{ + .root_source_file = b.path("src/codegen/process_translate_c.zig"), + .target = b.graph.host, + .optimize = .Debug, + }), + }); + const in = translate_c.getOutput(); + const run = b.addRunArtifact(helper_exe); + run.addFileArg(in); + const out = run.addOutputFileArg("c-headers-for-zig.zig"); + return out; } pub fn addBunObject(b: *Build, opts: *BunBuildOptions) *Compile { diff --git a/src/bun.js/bindings/JSGlobalObject.zig b/src/bun.js/bindings/JSGlobalObject.zig index ba456213972517..7bb09bef97dc0a 100644 --- a/src/bun.js/bindings/JSGlobalObject.zig +++ b/src/bun.js/bindings/JSGlobalObject.zig @@ -847,8 +847,7 @@ pub const JSGlobalObject = opaque { } pub fn scriptExecutionContextIdentifier(global: *JSC.JSGlobalObject) bun.webcore.ScriptExecutionContext.Identifier { - const c_global: *bun.c.JSC__JSGlobalObject = @ptrCast(global); - return @enumFromInt(bun.c.ScriptExecutionContextIdentifier__forGlobalObject(c_global)); + return @enumFromInt(bun.c.ScriptExecutionContextIdentifier__forGlobalObject(global)); } pub const Extern = [_][]const u8{ "create", "getModuleRegistryMap", "resetModuleRegistryMap" }; diff --git a/src/bun.js/webcore/ScriptExecutionContext.zig b/src/bun.js/webcore/ScriptExecutionContext.zig index f45e5e54014179..17397d9462d21a 100644 --- a/src/bun.js/webcore/ScriptExecutionContext.zig +++ b/src/bun.js/webcore/ScriptExecutionContext.zig @@ -7,7 +7,7 @@ pub const Identifier = enum(bun.c.WebCore__ScriptExecutionContextIdentifier) { /// Returns null if the context referred to by `self` no longer exists pub fn globalObject(self: Identifier) ?*bun.jsc.JSGlobalObject { - return @ptrCast(bun.c.ScriptExecutionContextIdentifier__getGlobalObject(@intFromEnum(self))); + return bun.c.ScriptExecutionContextIdentifier__getGlobalObject(@intFromEnum(self)); } /// Returns null if the context referred to by `self` no longer exists diff --git a/src/codegen/process_windows_translate_c.zig b/src/codegen/process_translate_c.zig similarity index 68% rename from src/codegen/process_windows_translate_c.zig rename to src/codegen/process_translate_c.zig index 8bbfd5cf57df4f..1bb6a0630e9362 100644 --- a/src/codegen/process_windows_translate_c.zig +++ b/src/codegen/process_translate_c.zig @@ -1,12 +1,23 @@ -// translate-c is unable to translate the unsuffixed windows functions -// like `SetCurrentDirectory` since they are defined with an odd macro -// that translate-c doesn't handle. -// -// #define SetCurrentDirectory __MINGW_NAME_AW(SetCurrentDirectory) -// -// In these cases, it's better to just reference the underlying function -// directly: SetCurrentDirectoryW. To make the error better, a post -// processing step is applied to the translate-c file. +//! translate-c is unable to translate the unsuffixed windows functions +//! like `SetCurrentDirectory` since they are defined with an odd macro +//! that translate-c doesn't handle. +//! +//! #define SetCurrentDirectory __MINGW_NAME_AW(SetCurrentDirectory) +//! +//! In these cases, it's better to just reference the underlying function +//! directly: SetCurrentDirectoryW. To make the error better, a post +//! processing step is applied to the translate-c file. +//! +//! Additionally, this step makes it so that decls like NTSTATUS and +//! HANDLE point to the standard library structures. +//! +//! This is also used on all platforms to rewrite some `opaque` types. +//! In most cases, we want to define our own `opaque` so that we can +//! use it as a namespace that provides decls. Normally the translate-c +//! output will include its own `opaque` types, which would require a +//! `@ptrCast` to and from the type we define. Instead, we replace these +//! types with their equivalents imported from Bun. + const std = @import("std"); const mem = std.mem; @@ -14,6 +25,7 @@ const symbol_replacements = std.StaticStringMap([]const u8).initComptime(&.{ &.{ "NTSTATUS", "@import(\"std\").os.windows.NTSTATUS" }, &.{ "HANDLE", "@import(\"std\").os.windows.HANDLE" }, &.{ "PHANDLE", "*HANDLE" }, + &.{ "JSC__JSGlobalObject", "@import(\"bun\").jsc.JSGlobalObject" }, }); pub fn main() !void { From c985ea3bf1ffbb2b45bf27272fb4e2b888a5db15 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 2 May 2025 12:17:36 -0700 Subject: [PATCH 62/88] error checking --- .../worker_threads/worker_thread_check.ts | 71 +++++++++---------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/test/js/node/worker_threads/worker_thread_check.ts b/test/js/node/worker_threads/worker_thread_check.ts index 8ee0eb5bc17e84..df171482228eae 100644 --- a/test/js/node/worker_threads/worker_thread_check.ts +++ b/test/js/node/worker_threads/worker_thread_check.ts @@ -5,8 +5,42 @@ import { Worker, isMainThread, workerData } from "worker_threads"; const sleep = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); +const actions = { + async ["Bun.connect"](port) { + await Bun.connect({ + hostname: "localhost", + port, + socket: { + open() {}, + error() {}, + data() {}, + drain() {}, + close() {}, + }, + }); + }, + async ["Bun.listen"](port) { + const server = Bun.listen({ + hostname: "localhost", + port: 0, + socket: { + open() {}, + error() {}, + data() {}, + drain() {}, + close() {}, + }, + }); + }, + async ["fetch"](port) { + const resp = await fetch("http://localhost:" + port); + await resp.blob(); + }, +}; + if (isMainThread) { let action = process.argv.at(-1); + if (actions[action!] === undefined) throw new Error("not found"); const server = Bun.serve({ port: 0, @@ -49,40 +83,5 @@ if (isMainThread) { } else { Bun.gc(true); const { action, port } = workerData; - - switch (action) { - case "Bun.connect": { - await Bun.connect({ - hostname: "localhost", - port, - socket: { - open() {}, - error() {}, - data() {}, - drain() {}, - close() {}, - }, - }); - break; - } - case "Bun.listen": { - const server = Bun.listen({ - hostname: "localhost", - port: 0, - socket: { - open() {}, - error() {}, - data() {}, - drain() {}, - close() {}, - }, - }); - break; - } - case "fetch": { - const resp = await fetch("http://localhost:" + port); - await resp.blob(); - break; - } - } + await actions[action](port); } From 35435f74aa358bdc98c2405af472015d471abcba Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 2 May 2025 12:46:53 -0700 Subject: [PATCH 63/88] finish merge --- src/bun.js/bindings/BunProcess.cpp | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index a48407896f820e..c7926cce62cc6f 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -2019,21 +2019,7 @@ static JSValue constructStdin(VM& vm, JSObject* processObject) auto result = JSC::profiledCall(globalObject, ProfilingReason::API, getStdioWriteStream, callData, globalObject, args); RETURN_IF_EXCEPTION(scope, {}); -<<<<<<< HEAD return result; -======= - - if (auto* exception = returnedException.get()) { -#if ASSERT_ENABLED - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); -#endif - scope.throwException(globalObject, exception->value()); - returnedException.clear(); - return {}; - } - - RELEASE_AND_RETURN(scope, result); ->>>>>>> main } JSC_DEFINE_CUSTOM_GETTER(processThrowDeprecation, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::EncodedJSValue thisValue, JSC::PropertyName name)) From b2cc48d450b227a21ee0cc96267a14007d4f50b2 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 2 May 2025 15:25:32 -0700 Subject: [PATCH 64/88] try unref --- src/bun.js/web_worker.zig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index 1b795f58614e34..17f43404aa46d3 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -525,6 +525,9 @@ pub const WebWorker = struct { vm.eventLoop().wakeup(); // TODO(@190n) notifyNeedTermination } + + // TODO(@190n) delete + this.setRefInternal(false); } /// This handles cleanup, emitting the "close" event, and deinit. From 38d3978b7b96d2bf77a8b754ecdb1544b1928c47 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 2 May 2025 15:27:42 -0700 Subject: [PATCH 65/88] no conflicts --- src/bun.js/bindings/BunProcess.cpp | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index c7926cce62cc6f..5e7df693889c63 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -2088,19 +2088,6 @@ static JSValue constructProcessChannel(VM& vm, JSObject* processObject) auto result = JSC::profiledCall(globalObject, ProfilingReason::API, getControl, callData, globalObject->globalThis(), args); RETURN_IF_EXCEPTION(scope, {}); -<<<<<<< HEAD -======= - - if (auto* exception = returnedException.get()) { -#if ASSERT_ENABLED - Zig::GlobalObject::reportUncaughtExceptionAtEventLoop(globalObject, exception); -#endif - scope.throwException(globalObject, exception->value()); - returnedException.clear(); - return {}; - } - ->>>>>>> main return result; } else { return jsUndefined(); From 97004c9bc39faa253584049b9d0e8935ad38975b Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 2 May 2025 17:00:15 -0700 Subject: [PATCH 66/88] remove broken tests --- .../parallel/test-worker-dns-terminate.js | 14 ------ .../parallel/test-worker-heapdump-failure.js | 30 ------------- .../test-worker-terminate-ref-public-port.js | 12 ----- .../test-worker-terminate-source-map.js | 45 ------------------- 4 files changed, 101 deletions(-) delete mode 100644 test/js/node/test/parallel/test-worker-dns-terminate.js delete mode 100644 test/js/node/test/parallel/test-worker-heapdump-failure.js delete mode 100644 test/js/node/test/parallel/test-worker-terminate-ref-public-port.js delete mode 100644 test/js/node/test/parallel/test-worker-terminate-source-map.js diff --git a/test/js/node/test/parallel/test-worker-dns-terminate.js b/test/js/node/test/parallel/test-worker-dns-terminate.js deleted file mode 100644 index da9d543c3b0be2..00000000000000 --- a/test/js/node/test/parallel/test-worker-dns-terminate.js +++ /dev/null @@ -1,14 +0,0 @@ -'use strict'; -const common = require('../common'); -const { Worker } = require('worker_threads'); - -const w = new Worker(` -const dns = require('dns'); -dns.lookup('nonexistent.org', () => {}); -require('worker_threads').parentPort.postMessage('0'); -`, { eval: true }); - -w.on('message', common.mustCall(() => { - // This should not crash the worker during a DNS request. - w.terminate().then(common.mustCall()); -})); diff --git a/test/js/node/test/parallel/test-worker-heapdump-failure.js b/test/js/node/test/parallel/test-worker-heapdump-failure.js deleted file mode 100644 index c5d24cdcf658a2..00000000000000 --- a/test/js/node/test/parallel/test-worker-heapdump-failure.js +++ /dev/null @@ -1,30 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const { Worker } = require('worker_threads'); -const { once } = require('events'); - -(async function() { - const w = new Worker('', { eval: true }); - - await once(w, 'exit'); - await assert.rejects(() => w.getHeapSnapshot(), { - name: 'Error', - code: 'ERR_WORKER_NOT_RUNNING' - }); -})().then(common.mustCall()); - -(async function() { - const worker = new Worker('setInterval(() => {}, 1000);', { eval: true }); - await once(worker, 'online'); - - [1, true, [], null, Infinity, NaN].forEach((i) => { - assert.throws(() => worker.getHeapSnapshot(i), { - code: 'ERR_INVALID_ARG_TYPE', - name: 'TypeError', - message: 'The "options" argument must be of type object.' + - common.invalidArgTypeHelper(i) - }); - }); - await worker.terminate(); -})().then(common.mustCall()); diff --git a/test/js/node/test/parallel/test-worker-terminate-ref-public-port.js b/test/js/node/test/parallel/test-worker-terminate-ref-public-port.js deleted file mode 100644 index 4a2de785a36220..00000000000000 --- a/test/js/node/test/parallel/test-worker-terminate-ref-public-port.js +++ /dev/null @@ -1,12 +0,0 @@ -'use strict'; -const common = require('../common'); -const { Worker } = require('worker_threads'); - -// The actual test here is that the Worker does not keep the main thread -// running after it has been .terminate()’ed. - -const w = new Worker(` -const p = require('worker_threads').parentPort; -while(true) p.postMessage({})`, { eval: true }); -w.once('message', () => w.terminate()); -w.once('exit', common.mustCall()); diff --git a/test/js/node/test/parallel/test-worker-terminate-source-map.js b/test/js/node/test/parallel/test-worker-terminate-source-map.js deleted file mode 100644 index c855dab975be01..00000000000000 --- a/test/js/node/test/parallel/test-worker-terminate-source-map.js +++ /dev/null @@ -1,45 +0,0 @@ -'use strict'; -const common = require('../common'); -const tmpdir = require('../common/tmpdir'); -const assert = require('assert'); - -// Attempts to test that the source map JS code run on process shutdown -// does not call any user-defined JS code. - -const { Worker, workerData, parentPort } = require('worker_threads'); - -if (!workerData) { - tmpdir.refresh(); - process.env.NODE_V8_COVERAGE = tmpdir.path; - - // Count the number of some calls that should not be made. - const callCount = new Int32Array(new SharedArrayBuffer(4)); - const w = new Worker(__filename, { workerData: { callCount } }); - w.on('message', common.mustCall(() => w.terminate())); - w.on('exit', common.mustCall(() => { - assert.strictEqual(callCount[0], 0); - })); - return; -} - -const { callCount } = workerData; - -function increaseCallCount() { callCount[0]++; } - -// Increase the call count when a forbidden method is called. -for (const property of ['_cache', 'lineLengths', 'url']) { - Object.defineProperty(Object.prototype, property, { - get: increaseCallCount, - set: increaseCallCount - }); -} -Object.getPrototypeOf([][Symbol.iterator]()).next = increaseCallCount; -Object.getPrototypeOf((new Map()).entries()).next = increaseCallCount; -Array.prototype[Symbol.iterator] = increaseCallCount; -Map.prototype[Symbol.iterator] = increaseCallCount; -Map.prototype.entries = increaseCallCount; -Object.keys = increaseCallCount; -Object.create = increaseCallCount; -Object.hasOwnProperty = increaseCallCount; - -parentPort.postMessage('done'); From ba115657d434e8a24d02559d6e8fb1aa86ab504e Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 2 May 2025 18:55:28 -0700 Subject: [PATCH 67/88] clean up fireEarlyMessages --- src/bun.js/bindings/webcore/Worker.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index 6dff266df90766..cae17f8f4feb2e 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -371,23 +371,20 @@ void Worker::dispatchOnline(Zig::GlobalObject* workerGlobalObject) void Worker::fireEarlyMessages(Zig::GlobalObject* workerGlobalObject) { - Locker lock(this->m_pendingTasksMutex); + auto tasks = [&]() { + Locker lock(this->m_pendingTasksMutex); + return std::exchange(this->m_pendingTasks, {}); + }(); auto* thisContext = workerGlobalObject->scriptExecutionContext(); if (workerGlobalObject->globalEventScope->hasActiveEventListeners(eventNames().messageEvent)) { - auto tasks = std::exchange(this->m_pendingTasks, {}); - lock.unlockEarly(); for (auto& task : tasks) { task(*thisContext); } } else { - auto tasks = std::exchange(this->m_pendingTasks, {}); - lock.unlockEarly(); - thisContext->postTask([tasks = WTFMove(tasks)](auto& ctx) mutable { for (auto& task : tasks) { task(ctx); } - tasks.clear(); }); } } From f832301455caae2b211c82ccd6773a3f2922a703 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Fri, 2 May 2025 18:55:34 -0700 Subject: [PATCH 68/88] try this --- src/bun.js/web_worker.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index 17f43404aa46d3..4571a7eeef8b6b 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -407,7 +407,6 @@ pub const WebWorker = struct { assert(this.status.load(.acquire) == .start); this.setStatus(.starting); vm.preload = this.preloads; - WebWorker__dispatchOnline(this.cpp_worker, vm.global); // resolve entrypoint var resolve_error = bun.String.empty; defer resolve_error.deref(); @@ -453,6 +452,7 @@ pub const WebWorker = struct { this.flushLogs(); log("[{d}] event loop start", .{this.execution_context_id}); + WebWorker__dispatchOnline(this.cpp_worker, vm.global); WebWorker__fireEarlyMessages(this.cpp_worker, vm.global); this.setStatus(.running); From b5bdf125c55f4be3739140eeecfd98f9f87e094b Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 5 May 2025 11:37:56 -0700 Subject: [PATCH 69/88] Add comment --- src/bun.js/web_worker.zig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index 4571a7eeef8b6b..6e3bdb0555844a 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -452,6 +452,8 @@ pub const WebWorker = struct { this.flushLogs(); log("[{d}] event loop start", .{this.execution_context_id}); + // TODO(@190n) call dispatchOnline earlier (basically as soon as spin() starts, before + // we start running JS) WebWorker__dispatchOnline(this.cpp_worker, vm.global); WebWorker__fireEarlyMessages(this.cpp_worker, vm.global); this.setStatus(.running); From 504e88e4b97f01e927fb216bda10dc500efb3b1a Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 5 May 2025 11:53:10 -0700 Subject: [PATCH 70/88] Remove nodeWorkerObjectSymbol --- src/bun.js/bindings/ZigGlobalObject.cpp | 4 ---- src/bun.js/bindings/ZigGlobalObject.h | 5 +---- src/bun.js/bindings/webcore/JSWorker.cpp | 4 ++-- src/bun.js/bindings/webcore/Worker.cpp | 6 ++---- src/js/node/worker_threads.ts | 26 +++++++++++------------- test/js/web/workers/worker.test.ts | 16 --------------- 6 files changed, 17 insertions(+), 44 deletions(-) diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index 4a3947b6809b19..654874508e5659 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -3374,10 +3374,6 @@ void GlobalObject::finishCreation(VM& vm) m_BoundEmitFunctionStructure.initLater([](const LazyProperty::Initializer& init) { init.set(Bun::BoundEmitFunction::createStructure(init.vm, init.owner)); }); - m_nodeWorkerObjectSymbol.initLater([](const LazyProperty::Initializer& init) { - // This should not be visible to user code - init.set(Symbol::createWithDescription(init.vm, "node worker object symbol"_s)); - }); configureNodeVM(vm, this); diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h index dc95adf34bc68c..9a44bcfafc9e24 100644 --- a/src/bun.js/bindings/ZigGlobalObject.h +++ b/src/bun.js/bindings/ZigGlobalObject.h @@ -602,9 +602,7 @@ class GlobalObject : public Bun::GlobalScope { V(public, LazyPropertyOfGlobalObject, m_statValues) \ V(public, LazyPropertyOfGlobalObject, m_bigintStatValues) \ V(public, LazyPropertyOfGlobalObject, m_statFsValues) \ - V(public, LazyPropertyOfGlobalObject, m_bigintStatFsValues) \ - \ - V(private, LazyPropertyOfGlobalObject, m_nodeWorkerObjectSymbol) + V(public, LazyPropertyOfGlobalObject, m_bigintStatFsValues) #define DECLARE_GLOBALOBJECT_GC_MEMBER(visibility, T, name) \ visibility: \ @@ -675,7 +673,6 @@ class GlobalObject : public Bun::GlobalScope { JSObject* cryptoObject() const { return m_cryptoObject.getInitializedOnMainThread(this); } JSObject* JSDOMFileConstructor() const { return m_JSDOMFileConstructor.getInitializedOnMainThread(this); } - Symbol* nodeWorkerObjectSymbol() { return m_nodeWorkerObjectSymbol.getInitializedOnMainThread(this); } JSMap* nodeWorkerEnvironmentData() { return m_nodeWorkerEnvironmentData.get(); } void setNodeWorkerEnvironmentData(JSMap* data); diff --git a/src/bun.js/bindings/webcore/JSWorker.cpp b/src/bun.js/bindings/webcore/JSWorker.cpp index a5757f67363130..55eb8de5a5da69 100644 --- a/src/bun.js/bindings/webcore/JSWorker.cpp +++ b/src/bun.js/bindings/webcore/JSWorker.cpp @@ -139,8 +139,8 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: RETURN_IF_EXCEPTION(throwScope, {}); EnsureStillAliveScope argument1 = callFrame->argument(1); JSValue nodeWorkerObject {}; - if (JSValue::strictEqual(lexicalGlobalObject, callFrame->argument(2), globalObject->nodeWorkerObjectSymbol())) { - nodeWorkerObject = callFrame->argument(3); + if (callFrame->argumentCount() == 3) { + nodeWorkerObject = callFrame->argument(2); } RETURN_IF_EXCEPTION(throwScope, {}); diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index cae17f8f4feb2e..3017304cdd8afe 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -545,7 +545,7 @@ JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) } globalObject->setNodeWorkerEnvironmentData(environmentData); - JSObject* array = constructEmptyArray(globalObject, nullptr, 5); + JSObject* array = constructEmptyArray(globalObject, nullptr, 4); RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 0, workerData); RETURN_IF_EXCEPTION(scope, {}); @@ -553,9 +553,7 @@ JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 2, JSFunction::create(vm, globalObject, 1, "receiveMessageOnPort"_s, jsReceiveMessageOnPort, ImplementationVisibility::Public, NoIntrinsic)); RETURN_IF_EXCEPTION(scope, {}); - array->putDirectIndex(globalObject, 3, globalObject->nodeWorkerObjectSymbol()); - RETURN_IF_EXCEPTION(scope, {}); - array->putDirectIndex(globalObject, 4, environmentData); + array->putDirectIndex(globalObject, 3, environmentData); RETURN_IF_EXCEPTION(scope, {}); return array; diff --git a/src/js/node/worker_threads.ts b/src/js/node/worker_threads.ts index a962ea818e862a..02faea4130a88e 100644 --- a/src/js/node/worker_threads.ts +++ b/src/js/node/worker_threads.ts @@ -7,7 +7,16 @@ const EventEmitter = require("node:events"); const { throwNotImplemented, warnNotImplementedOnce } = require("internal/shared"); const { validateObject, validateBoolean } = require("internal/validators"); -const { MessageChannel, BroadcastChannel, Worker: WebWorker } = globalThis; +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; +}; const SHARE_ENV = Symbol("nodejs.worker_threads.SHARE_ENV"); const isMainThread = Bun.isMainThread; @@ -15,13 +24,11 @@ const { 0: _workerData, 1: _threadId, 2: _receiveMessageOnPort, - 3: kNodeWorkerObject, - 4: environmentData, + 3: environmentData, } = $cpp("Worker.cpp", "createNodeWorkerThreadsBinding") as [ unknown, number, (port: unknown) => unknown, - symbol, Map, ]; @@ -250,16 +257,7 @@ class Worker extends EventEmitter { } } try { - // The native WebWorker constructor fires the 'worker' event on the process when the worker is - // successfully created. For worker_threads, we want to fire that event with the - // worker_threads object instead of the Web Worker object. - // - // This is implemented by accepting two extra arguments in the native constructor: a symbol to - // prove we are the real worker_threads constructor, and if so, the worker_threads object as - // the last parameter. - this.#worker = new (WebWorker as new ( - ...args: [...ConstructorParameters, nodeWorkerObjectSymbol: symbol, nodeWorker: Worker] - ) => WebWorker)(filename, options as Bun.WorkerOptions, kNodeWorkerObject, this); + this.#worker = new WebWorker(filename, options as Bun.WorkerOptions, this); } catch (e) { if (this.#urlToRevoke) { URL.revokeObjectURL(this.#urlToRevoke); diff --git a/test/js/web/workers/worker.test.ts b/test/js/web/workers/worker.test.ts index d7f2f81869868a..47925159708980 100644 --- a/test/js/web/workers/worker.test.ts +++ b/test/js/web/workers/worker.test.ts @@ -285,22 +285,6 @@ describe("web worker", () => { expect(called).toBeFalse(); return promise; }); - - test("cannot fake a node:worker_threads Worker", () => { - const { promise, resolve } = Promise.withResolvers(); - let worker: Worker | undefined = undefined; - let called = false; - process.once("worker", eventWorker => { - called = true; - expect(eventWorker as any).toBe(worker); - resolve(); - }); - // make sure that the native constructor requires a secret symbol to emit a - // node:worker_threads object - worker = new (Worker as any)(new URL("data:text/javascript,"), {}, Symbol(), 5); - expect(called).toBeFalse(); - return promise; - }); }); }); From 6dcc4ae8424f816b39f6ad1d504b237940eb50c6 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 5 May 2025 12:48:08 -0700 Subject: [PATCH 71/88] Address review feedback - remove some unneeded exception checks - use forEachInArrayLike instead of manual iteration - use global bunVM() helper --- src/bun.js/bindings/JSNextTickQueue.cpp | 1 - src/bun.js/bindings/NodeURL.cpp | 2 -- src/bun.js/bindings/webcore/JSWorker.cpp | 11 +++++------ src/bun.js/bindings/webcore/Worker.cpp | 2 +- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/bun.js/bindings/JSNextTickQueue.cpp b/src/bun.js/bindings/JSNextTickQueue.cpp index b7710d7751ac1e..baf178188234a0 100644 --- a/src/bun.js/bindings/JSNextTickQueue.cpp +++ b/src/bun.js/bindings/JSNextTickQueue.cpp @@ -92,7 +92,6 @@ void JSNextTickQueue::drain(JSC::VM& vm, JSC::JSGlobalObject* globalObject) RETURN_IF_EXCEPTION(throwScope, ); } auto* drainFn = internalField(2).get().getObject(); - RETURN_IF_EXCEPTION(throwScope, ); MarkedArgumentBuffer drainArgs; JSC::call(globalObject, drainFn, drainArgs, "Failed to drain next tick queue"_s); RETURN_IF_EXCEPTION(throwScope, ); diff --git a/src/bun.js/bindings/NodeURL.cpp b/src/bun.js/bindings/NodeURL.cpp index 9257e6026f8496..f75bb1ab212e34 100644 --- a/src/bun.js/bindings/NodeURL.cpp +++ b/src/bun.js/bindings/NodeURL.cpp @@ -150,9 +150,7 @@ JSC::JSValue createNodeURLBinding(Zig::GlobalObject* globalObject) auto binding = constructEmptyArray(globalObject, nullptr, 2); RETURN_IF_EXCEPTION(scope, {}); auto domainToAsciiFunction = JSC::JSFunction::create(vm, globalObject, 1, "domainToAscii"_s, jsDomainToASCII, ImplementationVisibility::Public); - RETURN_IF_EXCEPTION(scope, {}); auto domainToUnicodeFunction = JSC::JSFunction::create(vm, globalObject, 1, "domainToUnicode"_s, jsDomainToUnicode, ImplementationVisibility::Public); - RETURN_IF_EXCEPTION(scope, {}); ASSERT(binding && domainToAsciiFunction && domainToUnicodeFunction); binding->putByIndexInline( globalObject, diff --git a/src/bun.js/bindings/webcore/JSWorker.cpp b/src/bun.js/bindings/webcore/JSWorker.cpp index 55eb8de5a5da69..9a0509d9580fd3 100644 --- a/src/bun.js/bindings/webcore/JSWorker.cpp +++ b/src/bun.js/bindings/webcore/JSWorker.cpp @@ -207,13 +207,12 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: if (transferListValue.isObject()) { JSC::JSObject* transferListObject = transferListValue.getObject(); if (auto* transferListArray = jsDynamicCast(transferListObject)) { - for (unsigned i = 0; i < transferListArray->length(); i++) { - JSC::JSValue transferListValue = transferListArray->get(lexicalGlobalObject, i); - if (transferListValue.isObject()) { - JSC::JSObject* transferListObject = transferListValue.getObject(); - transferList.append(JSC::Strong(vm, transferListObject)); + JSC::forEachInArrayLike(globalObject, transferListArray, [&](JSValue transferValue) -> bool { + if (auto* transferObject = transferValue.getObject()) { + transferList.append({ vm, transferObject }); } - } + return true; + }); } } } diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index 3017304cdd8afe..91776a003a77db 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -191,7 +191,7 @@ ExceptionOr> Worker::create(ScriptExecutionContext& context, const S .value_or(std::span {}); void* impl = WebWorker__create( worker.ptr(), - defaultGlobalObject(context.jsGlobalObject())->bunVM(), + bunVM(context.jsGlobalObject()), nameStr, urlStr, &errorMessage, From bd50e622aa0e6376adb812b76ffa642ce28414c9 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 5 May 2025 13:06:18 -0700 Subject: [PATCH 72/88] Remove unnecessary exception check --- src/bun.js/bindings/JSNextTickQueue.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bun.js/bindings/JSNextTickQueue.cpp b/src/bun.js/bindings/JSNextTickQueue.cpp index baf178188234a0..190b5714b5d530 100644 --- a/src/bun.js/bindings/JSNextTickQueue.cpp +++ b/src/bun.js/bindings/JSNextTickQueue.cpp @@ -89,7 +89,6 @@ void JSNextTickQueue::drain(JSC::VM& vm, JSC::JSGlobalObject* globalObject) RETURN_IF_EXCEPTION(throwScope, ); if (mustResetContext) { globalObject->m_asyncContextData.get()->putInternalField(vm, 0, jsUndefined()); - RETURN_IF_EXCEPTION(throwScope, ); } auto* drainFn = internalField(2).get().getObject(); MarkedArgumentBuffer drainArgs; From 7b0774c5df875bb62c11707d024577c0a1771a85 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 5 May 2025 13:09:37 -0700 Subject: [PATCH 73/88] Check URLs are valid --- src/bun.js/bindings/webcore/JSWorker.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bun.js/bindings/webcore/JSWorker.cpp b/src/bun.js/bindings/webcore/JSWorker.cpp index 9a0509d9580fd3..d6936eb8d8235a 100644 --- a/src/bun.js/bindings/webcore/JSWorker.cpp +++ b/src/bun.js/bindings/webcore/JSWorker.cpp @@ -129,8 +129,8 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: return throwConstructorScriptExecutionContextUnavailableError(*lexicalGlobalObject, throwScope, "Worker"_s); EnsureStillAliveScope argument0 = callFrame->uncheckedArgument(0); String scriptUrl; - if (auto* url = jsDynamicCast(argument0.value())) { - scriptUrl = url->wrapped().href().string(); + if (auto* domUrl = jsDynamicCast(argument0.value()); domUrl && domUrl->wrapped().href().isValid()) { + scriptUrl = domUrl->wrapped().href().string(); } else if (argument0.value().isString()) { scriptUrl = argument0.value().getString(lexicalGlobalObject); } else { From e74482f4758fd2664369fce6a295bf43b7fc3877 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Mon, 5 May 2025 18:36:35 -0700 Subject: [PATCH 74/88] Use JSBoundFunction instead of BoundEmitFunction --- src/bun.js/bindings/BoundEmitFunction.cpp | 76 ------------------- src/bun.js/bindings/BoundEmitFunction.h | 43 ----------- src/bun.js/bindings/BunProcess.cpp | 11 ++- src/bun.js/bindings/ZigGlobalObject.cpp | 4 - src/bun.js/bindings/ZigGlobalObject.h | 2 - .../bindings/webcore/DOMClientIsoSubspaces.h | 1 - src/bun.js/bindings/webcore/DOMIsoSubspaces.h | 1 - src/bun.js/bindings/webcore/JSWorker.cpp | 1 + 8 files changed, 10 insertions(+), 129 deletions(-) delete mode 100644 src/bun.js/bindings/BoundEmitFunction.cpp delete mode 100644 src/bun.js/bindings/BoundEmitFunction.h diff --git a/src/bun.js/bindings/BoundEmitFunction.cpp b/src/bun.js/bindings/BoundEmitFunction.cpp deleted file mode 100644 index cb6f8ba1c679f2..00000000000000 --- a/src/bun.js/bindings/BoundEmitFunction.cpp +++ /dev/null @@ -1,76 +0,0 @@ -#include "BoundEmitFunction.h" - -#include "JavaScriptCore/FunctionPrototype.h" - -using namespace JSC; - -namespace Bun { - -BoundEmitFunction* BoundEmitFunction::create(VM& vm, Zig::GlobalObject* globalObject, WebCore::JSEventEmitter* target, WTF::ASCIILiteral eventName, JSValue event) -{ - auto* structure = globalObject->BoundEmitFunctionStructure(); - auto* function = new (NotNull, allocateCell(vm)) BoundEmitFunction( - vm, - structure, - eventName); - function->finishCreation(vm, target, event); - return function; -} - -BoundEmitFunction::BoundEmitFunction(VM& vm, Structure* structure, WTF::ASCIILiteral eventName) - : Base(vm, structure, functionCall) - , m_eventName(eventName) -{ -} - -void BoundEmitFunction::finishCreation(VM& vm, WebCore::JSEventEmitter* target, JSValue event) -{ - Base::finishCreation(vm, 0, "BoundEmitFunction"_s); - m_target.set(vm, this, target); - m_event.set(vm, this, event); -} - -JSC_DEFINE_HOST_FUNCTION(BoundEmitFunction::functionCall, (JSGlobalObject * globalObject, CallFrame* callFrame)) -{ - auto& vm = getVM(globalObject); - auto* function = jsCast(callFrame->jsCallee()); - MarkedArgumentBuffer args; - args.append(function->m_event.get()); - function->m_target->wrapped().emit(Identifier::fromString(vm, function->m_eventName), args); - return JSValue::encode(jsUndefined()); -} - -// for CREATE_METHOD_TABLE -namespace JSCastingHelpers = JSCastingHelpers; -const ClassInfo BoundEmitFunction::s_info = { - "BoundEmitFunction"_s, - &Base::s_info, - nullptr, - nullptr, - CREATE_METHOD_TABLE(BoundEmitFunction) -}; - -Structure* BoundEmitFunction::createStructure(VM& vm, JSGlobalObject* globalObject) -{ - return Structure::create( - vm, - globalObject, - globalObject->functionPrototype(), - TypeInfo(InternalFunctionType, StructureFlags), - info()); -} - -template -void BoundEmitFunction::visitChildrenImpl(JSCell* cell, Visitor& visitor) -{ - auto* fn = jsCast(cell); - ASSERT_GC_OBJECT_INHERITS(fn, info()); - Base::visitChildren(fn, visitor); - - visitor.append(fn->m_target); - visitor.append(fn->m_event); -} - -DEFINE_VISIT_CHILDREN(BoundEmitFunction); - -} // namespace Bun diff --git a/src/bun.js/bindings/BoundEmitFunction.h b/src/bun.js/bindings/BoundEmitFunction.h deleted file mode 100644 index 8a48537637f97a..00000000000000 --- a/src/bun.js/bindings/BoundEmitFunction.h +++ /dev/null @@ -1,43 +0,0 @@ -#pragma once - -#include "root.h" -#include "JSEventEmitter.h" - -namespace Bun { - -// Callable wrapper around an event emitter, an event name, and a value. Will fire the specified -// event when called. Used to implement Process::emitOnNextTick. -class BoundEmitFunction : public JSC::InternalFunction { -public: - using Base = JSC::InternalFunction; - - static BoundEmitFunction* create(JSC::VM& vm, Zig::GlobalObject* globalObject, WebCore::JSEventEmitter* target, WTF::ASCIILiteral eventName, JSC::JSValue event); - static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject); - - template - static JSC::GCClient::IsoSubspace* subspaceFor(JSC::VM& vm) - { - if constexpr (mode == JSC::SubspaceAccess::Concurrently) - return nullptr; - return WebCore::subspaceForImpl( - vm, - [](auto& spaces) { return spaces.m_clientSubspaceForBoundEmitFunction.get(); }, - [](auto& spaces, auto&& space) { spaces.m_clientSubspaceForBoundEmitFunction = std::forward(space); }, - [](auto& spaces) { return spaces.m_subspaceForBoundEmitFunction.get(); }, - [](auto& spaces, auto&& space) { spaces.m_subspaceForBoundEmitFunction = std::forward(space); }); - } - - DECLARE_INFO; - DECLARE_VISIT_CHILDREN; - -private: - BoundEmitFunction(JSC::VM& vm, JSC::Structure* structure, WTF::ASCIILiteral eventName); - void finishCreation(JSC::VM& vm, WebCore::JSEventEmitter* target, JSC::JSValue event); - static JSC_DECLARE_HOST_FUNCTION(functionCall); - - JSC::WriteBarrier m_target; - WTF::ASCIILiteral m_eventName; - JSC::WriteBarrier m_event; -}; - -} // namespace Bun diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index 5e7df693889c63..025df5907f8aec 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -1,4 +1,3 @@ -#include "BoundEmitFunction.h" #include "ModuleLoader.h" #include "napi.h" @@ -3075,7 +3074,15 @@ void Process::queueNextTick(JSC::JSGlobalObject* globalObject, JSValue func, con void Process::emitOnNextTick(Zig::GlobalObject* globalObject, ASCIILiteral eventName, JSValue event) { - queueNextTick(globalObject, BoundEmitFunction::create(getVM(globalObject), globalObject, this, eventName, event)); + auto& vm = getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + auto emit = this->get(globalObject, Identifier::fromString(vm, "emit"_s)); + RETURN_IF_EXCEPTION(scope, ); + if (!emit.getObject()) return; + + auto* bound = JSBoundFunction::create(vm, globalObject, emit.getObject(), this, {}, 0, nullptr); + JSValue args[] = { jsString(vm, String(eventName)), event }; + queueNextTick(globalObject, bound, args); } extern "C" void Bun__Process__queueNextTick1(GlobalObject* globalObject, EncodedJSValue func, EncodedJSValue arg1) diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index 58b2f229870d2d..3b1f10faee4316 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -182,7 +182,6 @@ #include "ProcessBindingBuffer.h" #include "NodeValidator.h" #include "ProcessBindingFs.h" -#include "BoundEmitFunction.h" #include "node/NodeTimers.h" #include "JSBunRequest.h" @@ -3371,9 +3370,6 @@ void GlobalObject::finishCreation(VM& vm) m_bigintStatFsValues.initLater([](const LazyProperty::Initializer& init) { init.set(JSC::JSBigInt64Array::create(init.owner, JSC::JSBigInt64Array::createStructure(init.vm, init.owner, init.owner->objectPrototype()), 7)); }); - m_BoundEmitFunctionStructure.initLater([](const LazyProperty::Initializer& init) { - init.set(Bun::BoundEmitFunction::createStructure(init.vm, init.owner)); - }); configureNodeVM(vm, this); diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h index 9a44bcfafc9e24..2baab5e59b146f 100644 --- a/src/bun.js/bindings/ZigGlobalObject.h +++ b/src/bun.js/bindings/ZigGlobalObject.h @@ -287,7 +287,6 @@ class GlobalObject : public Bun::GlobalScope { Structure* JSSocketAddressDTOStructure() const { return m_JSSocketAddressDTOStructure.getInitializedOnMainThread(this); } Structure* ImportMetaObjectStructure() const { return m_importMetaObjectStructure.getInitializedOnMainThread(this); } Structure* AsyncContextFrameStructure() const { return m_asyncBoundFunctionStructure.getInitializedOnMainThread(this); } - Structure* BoundEmitFunctionStructure() { return m_BoundEmitFunctionStructure.getInitializedOnMainThread(this); } JSWeakMap* vmModuleContextMap() const { return m_vmModuleContextMap.getInitializedOnMainThread(this); } @@ -574,7 +573,6 @@ class GlobalObject : public Bun::GlobalScope { V(private, LazyPropertyOfGlobalObject, m_processBindingFs) \ V(private, LazyPropertyOfGlobalObject, m_importMetaObjectStructure) \ V(private, LazyPropertyOfGlobalObject, m_asyncBoundFunctionStructure) \ - V(private, LazyPropertyOfGlobalObject, m_BoundEmitFunctionStructure) \ V(public, LazyPropertyOfGlobalObject, m_JSDOMFileConstructor) \ V(public, LazyPropertyOfGlobalObject, m_JSMIMEParamsConstructor) \ V(public, LazyPropertyOfGlobalObject, m_JSMIMETypeConstructor) \ diff --git a/src/bun.js/bindings/webcore/DOMClientIsoSubspaces.h b/src/bun.js/bindings/webcore/DOMClientIsoSubspaces.h index 965bbb5c9afbf4..bd97f654e9fedd 100644 --- a/src/bun.js/bindings/webcore/DOMClientIsoSubspaces.h +++ b/src/bun.js/bindings/webcore/DOMClientIsoSubspaces.h @@ -68,7 +68,6 @@ class DOMClientIsoSubspaces { std::unique_ptr m_clientSubspaceForJSS3Bucket; std::unique_ptr m_clientSubspaceForJSS3File; std::unique_ptr m_clientSubspaceForJSX509Certificate; - std::unique_ptr m_clientSubspaceForBoundEmitFunction; #include "ZigGeneratedClasses+DOMClientIsoSubspaces.h" /* --- bun --- */ diff --git a/src/bun.js/bindings/webcore/DOMIsoSubspaces.h b/src/bun.js/bindings/webcore/DOMIsoSubspaces.h index f62bada625d20d..a7b07760f9a6cc 100644 --- a/src/bun.js/bindings/webcore/DOMIsoSubspaces.h +++ b/src/bun.js/bindings/webcore/DOMIsoSubspaces.h @@ -65,7 +65,6 @@ class DOMIsoSubspaces { std::unique_ptr m_subspaceForJSS3Bucket; std::unique_ptr m_subspaceForJSS3File; std::unique_ptr m_subspaceForJSX509Certificate; - std::unique_ptr m_subspaceForBoundEmitFunction; #include "ZigGeneratedClasses+DOMIsoSubspaces.h" /*-- BUN --*/ diff --git a/src/bun.js/bindings/webcore/JSWorker.cpp b/src/bun.js/bindings/webcore/JSWorker.cpp index d6936eb8d8235a..57c59db339ab2f 100644 --- a/src/bun.js/bindings/webcore/JSWorker.cpp +++ b/src/bun.js/bindings/webcore/JSWorker.cpp @@ -346,6 +346,7 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: } auto* process = jsCast(globalObject->processObject()); process->emitOnNextTick(globalObject, "worker"_s, workerToEmit); + RETURN_IF_EXCEPTION(throwScope, {}); return JSValue::encode(jsValue); } From bed568b8634dec6f9cc86680b884845f38a0432d Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 6 May 2025 11:28:13 -0700 Subject: [PATCH 75/88] Undo bindings changes --- build.zig | 60 +++++++++---------- src/bun.js/bindings/JSGlobalObject.zig | 4 +- .../bindings/ScriptExecutionContext.cpp | 11 ++-- src/bun.js/bindings/ScriptExecutionContext.h | 24 +------- src/bun.js/webcore/ScriptExecutionContext.zig | 6 +- src/c-headers-for-zig.h | 2 - ..._c.zig => process_windows_translate_c.zig} | 30 +++------- 7 files changed, 51 insertions(+), 86 deletions(-) rename src/codegen/{process_translate_c.zig => process_windows_translate_c.zig} (68%) diff --git a/build.zig b/build.zig index 506a7d29d0a851..04afe96fe87c19 100644 --- a/build.zig +++ b/build.zig @@ -508,38 +508,34 @@ fn getTranslateC(b: *Build, initial_target: std.Build.ResolvedTarget, optimize: translate_c.defineCMacroRaw(b.fmt("{s}={d}", .{ str, @intFromBool(value) })); } - // translate-c is unable to translate the unsuffixed windows functions - // like `SetCurrentDirectory` since they are defined with an odd macro - // that translate-c doesn't handle. - // - // #define SetCurrentDirectory __MINGW_NAME_AW(SetCurrentDirectory) - // - // In these cases, it's better to just reference the underlying function - // directly: SetCurrentDirectoryW. To make the error better, a post - // processing step is applied to the translate-c file. - // - // Additionally, this step makes it so that decls like NTSTATUS and - // HANDLE point to the standard library structures. - // - // This is also used on all platforms to rewrite some `opaque` types. - // In most cases, we want to define our own `opaque` so that we can - // use it as a namespace that provides decls. Normally the translate-c - // output will include its own `opaque` types, which would require a - // `@ptrCast` to and from the type we define. Instead, we replace these - // types with their equivalents imported from Bun. - const helper_exe = b.addExecutable(.{ - .name = "process_translate_c", - .root_module = b.createModule(.{ - .root_source_file = b.path("src/codegen/process_translate_c.zig"), - .target = b.graph.host, - .optimize = .Debug, - }), - }); - const in = translate_c.getOutput(); - const run = b.addRunArtifact(helper_exe); - run.addFileArg(in); - const out = run.addOutputFileArg("c-headers-for-zig.zig"); - return out; + if (target.result.os.tag == .windows) { + // translate-c is unable to translate the unsuffixed windows functions + // like `SetCurrentDirectory` since they are defined with an odd macro + // that translate-c doesn't handle. + // + // #define SetCurrentDirectory __MINGW_NAME_AW(SetCurrentDirectory) + // + // In these cases, it's better to just reference the underlying function + // directly: SetCurrentDirectoryW. To make the error better, a post + // processing step is applied to the translate-c file. + // + // Additionally, this step makes it so that decls like NTSTATUS and + // HANDLE point to the standard library structures. + const helper_exe = b.addExecutable(.{ + .name = "process_windows_translate_c", + .root_module = b.createModule(.{ + .root_source_file = b.path("src/codegen/process_windows_translate_c.zig"), + .target = b.graph.host, + .optimize = .Debug, + }), + }); + const in = translate_c.getOutput(); + const run = b.addRunArtifact(helper_exe); + run.addFileArg(in); + const out = run.addOutputFileArg("c-headers-for-zig.zig"); + return out; + } + return translate_c.getOutput(); } pub fn addBunObject(b: *Build, opts: *BunBuildOptions) *Compile { diff --git a/src/bun.js/bindings/JSGlobalObject.zig b/src/bun.js/bindings/JSGlobalObject.zig index 521730d1ed0201..72eeebf4798a87 100644 --- a/src/bun.js/bindings/JSGlobalObject.zig +++ b/src/bun.js/bindings/JSGlobalObject.zig @@ -857,8 +857,10 @@ pub const JSGlobalObject = opaque { return JSC.Error.INVALID_ARG_TYPE.fmt(global, fmt, args); } + extern fn ScriptExecutionContextIdentifier__forGlobalObject(global: *JSC.JSGlobalObject) u32; + pub fn scriptExecutionContextIdentifier(global: *JSC.JSGlobalObject) bun.webcore.ScriptExecutionContext.Identifier { - return @enumFromInt(bun.c.ScriptExecutionContextIdentifier__forGlobalObject(global)); + return @enumFromInt(ScriptExecutionContextIdentifier__forGlobalObject(global)); } pub const Extern = [_][]const u8{ "create", "getModuleRegistryMap", "resetModuleRegistryMap" }; diff --git a/src/bun.js/bindings/ScriptExecutionContext.cpp b/src/bun.js/bindings/ScriptExecutionContext.cpp index c324998da9c4a5..effbd96cc921e0 100644 --- a/src/bun.js/bindings/ScriptExecutionContext.cpp +++ b/src/bun.js/bindings/ScriptExecutionContext.cpp @@ -391,16 +391,17 @@ void ScriptExecutionContext::postTaskOnTimeout(FunctionscriptExecutionContext()->identifier(); } -JSC__JSGlobalObject* ScriptExecutionContextIdentifier__getGlobalObject(WebCore__ScriptExecutionContextIdentifier id) +extern "C" JSC::JSGlobalObject* ScriptExecutionContextIdentifier__getGlobalObject(ScriptExecutionContextIdentifier id) { - auto* context = WebCore::ScriptExecutionContext::getScriptExecutionContext(id); + auto* context = ScriptExecutionContext::getScriptExecutionContext(id); if (context) return context->globalObject(); return nullptr; } + +} // namespace WebCore diff --git a/src/bun.js/bindings/ScriptExecutionContext.h b/src/bun.js/bindings/ScriptExecutionContext.h index a7b800ab21916b..c17894f8921ac6 100644 --- a/src/bun.js/bindings/ScriptExecutionContext.h +++ b/src/bun.js/bindings/ScriptExecutionContext.h @@ -1,13 +1,5 @@ #pragma once -#include - -typedef uint32_t WebCore__ScriptExecutionContextIdentifier; - -#ifndef __cplusplus -typedef struct JSC__JSGlobalObject JSC__JSGlobalObject; -#else - #include "root.h" #include "ActiveDOMObject.h" #include @@ -31,8 +23,6 @@ struct us_socket_t; struct us_socket_context_t; struct us_loop_t; -using JSC__JSGlobalObject = JSC::JSGlobalObject; - namespace WebCore { class WebSocket; @@ -44,7 +34,7 @@ class EventLoopTask; class ContextDestructionObserver; -using ScriptExecutionContextIdentifier = WebCore__ScriptExecutionContextIdentifier; +using ScriptExecutionContextIdentifier = uint32_t; #if ENABLE(MALLOC_BREAKDOWN) DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(ScriptExecutionContext); @@ -218,15 +208,3 @@ class ScriptExecutionContext : public CanMakeWeakPtr, pu ScriptExecutionContext* executionContext(JSC::JSGlobalObject*); } - -#endif - -// Zig bindings -#ifdef __cplusplus -extern "C" { -#endif -WebCore__ScriptExecutionContextIdentifier ScriptExecutionContextIdentifier__forGlobalObject(JSC__JSGlobalObject*); -JSC__JSGlobalObject* ScriptExecutionContextIdentifier__getGlobalObject(WebCore__ScriptExecutionContextIdentifier); -#ifdef __cplusplus -} -#endif diff --git a/src/bun.js/webcore/ScriptExecutionContext.zig b/src/bun.js/webcore/ScriptExecutionContext.zig index 17397d9462d21a..2a3a7096953adb 100644 --- a/src/bun.js/webcore/ScriptExecutionContext.zig +++ b/src/bun.js/webcore/ScriptExecutionContext.zig @@ -1,13 +1,15 @@ const bun = @import("bun"); +extern fn ScriptExecutionContextIdentifier__getGlobalObject(id: u32) ?*bun.jsc.JSGlobalObject; + /// Safe handle to a JavaScript execution environment that may have exited. /// Obtain with global_object.scriptExecutionContextIdentifier() -pub const Identifier = enum(bun.c.WebCore__ScriptExecutionContextIdentifier) { +pub const Identifier = enum(u32) { _, /// Returns null if the context referred to by `self` no longer exists pub fn globalObject(self: Identifier) ?*bun.jsc.JSGlobalObject { - return bun.c.ScriptExecutionContextIdentifier__getGlobalObject(@intFromEnum(self)); + return ScriptExecutionContextIdentifier__getGlobalObject(@intFromEnum(self)); } /// Returns null if the context referred to by `self` no longer exists diff --git a/src/c-headers-for-zig.h b/src/c-headers-for-zig.h index 96a4f61adf8367..90e2998161295d 100644 --- a/src/c-headers-for-zig.h +++ b/src/c-headers-for-zig.h @@ -69,5 +69,3 @@ #undef lstat #undef fstat #undef stat - -#include "bun.js/bindings/ScriptExecutionContext.h" diff --git a/src/codegen/process_translate_c.zig b/src/codegen/process_windows_translate_c.zig similarity index 68% rename from src/codegen/process_translate_c.zig rename to src/codegen/process_windows_translate_c.zig index 1bb6a0630e9362..8bbfd5cf57df4f 100644 --- a/src/codegen/process_translate_c.zig +++ b/src/codegen/process_windows_translate_c.zig @@ -1,23 +1,12 @@ -//! translate-c is unable to translate the unsuffixed windows functions -//! like `SetCurrentDirectory` since they are defined with an odd macro -//! that translate-c doesn't handle. -//! -//! #define SetCurrentDirectory __MINGW_NAME_AW(SetCurrentDirectory) -//! -//! In these cases, it's better to just reference the underlying function -//! directly: SetCurrentDirectoryW. To make the error better, a post -//! processing step is applied to the translate-c file. -//! -//! Additionally, this step makes it so that decls like NTSTATUS and -//! HANDLE point to the standard library structures. -//! -//! This is also used on all platforms to rewrite some `opaque` types. -//! In most cases, we want to define our own `opaque` so that we can -//! use it as a namespace that provides decls. Normally the translate-c -//! output will include its own `opaque` types, which would require a -//! `@ptrCast` to and from the type we define. Instead, we replace these -//! types with their equivalents imported from Bun. - +// translate-c is unable to translate the unsuffixed windows functions +// like `SetCurrentDirectory` since they are defined with an odd macro +// that translate-c doesn't handle. +// +// #define SetCurrentDirectory __MINGW_NAME_AW(SetCurrentDirectory) +// +// In these cases, it's better to just reference the underlying function +// directly: SetCurrentDirectoryW. To make the error better, a post +// processing step is applied to the translate-c file. const std = @import("std"); const mem = std.mem; @@ -25,7 +14,6 @@ const symbol_replacements = std.StaticStringMap([]const u8).initComptime(&.{ &.{ "NTSTATUS", "@import(\"std\").os.windows.NTSTATUS" }, &.{ "HANDLE", "@import(\"std\").os.windows.HANDLE" }, &.{ "PHANDLE", "*HANDLE" }, - &.{ "JSC__JSGlobalObject", "@import(\"bun\").jsc.JSGlobalObject" }, }); pub fn main() !void { From 3eb6a3f52a5ad20b3f5409d88710007800e892ee Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 6 May 2025 12:08:36 -0700 Subject: [PATCH 76/88] Add some exception checks --- src/bun.js/bindings/webcore/JSWorker.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/bun.js/bindings/webcore/JSWorker.cpp b/src/bun.js/bindings/webcore/JSWorker.cpp index 57c59db339ab2f..675d10041a610c 100644 --- a/src/bun.js/bindings/webcore/JSWorker.cpp +++ b/src/bun.js/bindings/webcore/JSWorker.cpp @@ -287,12 +287,14 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: RETURN_IF_EXCEPTION(scope, ); execArgv.append(str); }); + RETURN_IF_EXCEPTION(throwScope, {}); options.execArgv.emplace(WTFMove(execArgv)); } } Vector> ports; auto* valueToTransfer = constructEmptyArray(globalObject, nullptr, 2); + 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. From 51a5f4e4d1f4e1769071dfd09e74e188c7163819 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 7 May 2025 11:14:26 -0700 Subject: [PATCH 77/88] Delete unnecessary overload --- src/bun.js/bindings/ZigGlobalObject.cpp | 5 ----- src/bun.js/bindings/ZigGlobalObject.h | 1 - 2 files changed, 6 deletions(-) diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index 3b1f10faee4316..5e534e5e107168 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -1300,11 +1300,6 @@ void GlobalObject::destroy(JSCell* cell) static_cast(cell)->GlobalObject::~GlobalObject(); } -WebCore::ScriptExecutionContext* GlobalObject::scriptExecutionContext() -{ - return m_scriptExecutionContext; -} - WebCore::ScriptExecutionContext* GlobalObject::scriptExecutionContext() const { return m_scriptExecutionContext; diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h index 2baab5e59b146f..827c99f0c926f7 100644 --- a/src/bun.js/bindings/ZigGlobalObject.h +++ b/src/bun.js/bindings/ZigGlobalObject.h @@ -164,7 +164,6 @@ class GlobalObject : public Bun::GlobalScope { bool worldIsNormal() const { return m_worldIsNormal; } static ptrdiff_t offsetOfWorldIsNormal() { return OBJECT_OFFSETOF(GlobalObject, m_worldIsNormal); } - WebCore::ScriptExecutionContext* scriptExecutionContext(); WebCore::ScriptExecutionContext* scriptExecutionContext() const; void queueTask(WebCore::EventLoopTask* task); From 05ab6065a97951fae326e5f115b4c0c93c6d1898 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 7 May 2025 11:16:36 -0700 Subject: [PATCH 78/88] Use helper methods for errors --- src/bun.js/bindings/webcore/Worker.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index 91776a003a77db..52bc650937b09a 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -501,7 +501,7 @@ JSC_DEFINE_HOST_FUNCTION(jsReceiveMessageOnPort, (JSGlobalObject * lexicalGlobal auto port = callFrame->argument(0); if (!port.isObject()) { - return Bun::throwError(lexicalGlobalObject, scope, Bun::ErrorCode::ERR_INVALID_ARG_TYPE, "The \"port\" argument must be a MessagePort instance"_s); + return Bun::ERR::INVALID_ARG_TYPE(scope, lexicalGlobalObject, "port"_s, "MessagePort"_s, port); } if (auto* messagePort = jsDynamicCast(port)) { @@ -511,7 +511,7 @@ JSC_DEFINE_HOST_FUNCTION(jsReceiveMessageOnPort, (JSGlobalObject * lexicalGlobal return JSC::JSValue::encode(jsUndefined()); } - return Bun::throwError(lexicalGlobalObject, scope, Bun::ErrorCode::ERR_INVALID_ARG_TYPE, "The \"port\" argument must be a MessagePort instance"_s); + return Bun::ERR::INVALID_ARG_TYPE(scope, lexicalGlobalObject, "port"_s, "MessagePort"_s, port); } JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) From 84e6cae0d7bff0e10b30fd3ce45122494cbc9d41 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 7 May 2025 11:18:02 -0700 Subject: [PATCH 79/88] Delete some exception checks --- src/bun.js/bindings/webcore/Worker.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index 52bc650937b09a..be0fea12103444 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -548,14 +548,9 @@ JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) JSObject* array = constructEmptyArray(globalObject, nullptr, 4); RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 0, workerData); - RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 1, threadId); - RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 2, JSFunction::create(vm, globalObject, 1, "receiveMessageOnPort"_s, jsReceiveMessageOnPort, ImplementationVisibility::Public, NoIntrinsic)); - RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 3, environmentData); - RETURN_IF_EXCEPTION(scope, {}); - return array; } From d613137428a1534ebb21c04cfd5684f07744f20f Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 7 May 2025 11:47:39 -0700 Subject: [PATCH 80/88] review suggestion --- src/bun.js/bindings/ScriptExecutionContext.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bun.js/bindings/ScriptExecutionContext.cpp b/src/bun.js/bindings/ScriptExecutionContext.cpp index effbd96cc921e0..d5076a60a79b33 100644 --- a/src/bun.js/bindings/ScriptExecutionContext.cpp +++ b/src/bun.js/bindings/ScriptExecutionContext.cpp @@ -400,8 +400,8 @@ extern "C" ScriptExecutionContextIdentifier ScriptExecutionContextIdentifier__fo extern "C" JSC::JSGlobalObject* ScriptExecutionContextIdentifier__getGlobalObject(ScriptExecutionContextIdentifier id) { auto* context = ScriptExecutionContext::getScriptExecutionContext(id); - if (context) return context->globalObject(); - return nullptr; + if (!context) return nullptr; + return context->globalObject(); } } // namespace WebCore From a588751c018e8e4ed08bb676b96b80fbae035744 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 7 May 2025 12:00:02 -0700 Subject: [PATCH 81/88] slop --- src/bun.js/api/bun/udp_socket.zig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bun.js/api/bun/udp_socket.zig b/src/bun.js/api/bun/udp_socket.zig index a17b4b767db1d2..9422b9542b4acf 100644 --- a/src/bun.js/api/bun/udp_socket.zig +++ b/src/bun.js/api/bun/udp_socket.zig @@ -381,11 +381,11 @@ pub const UDPSocket = struct { const globalThis = this.globalThis; const vm = globalThis.bunVM(); - if (callback == .zero) { - _ = vm.uncaughtException(globalThis, err, false); + if (err.isTerminationException(vm.jsc)) { return; } - if (err.isTerminationException(vm.jsc)) { + if (callback == .zero) { + _ = vm.uncaughtException(globalThis, err, false); return; } From dd8083cff4b757d445e3eccdefbf703f7a00d02e Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 7 May 2025 14:04:43 -0700 Subject: [PATCH 82/88] Split up assertions in createNodeURLBinding --- src/bun.js/bindings/NodeURL.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/bun.js/bindings/NodeURL.cpp b/src/bun.js/bindings/NodeURL.cpp index f75bb1ab212e34..86b1e70bea227b 100644 --- a/src/bun.js/bindings/NodeURL.cpp +++ b/src/bun.js/bindings/NodeURL.cpp @@ -149,9 +149,11 @@ JSC::JSValue createNodeURLBinding(Zig::GlobalObject* globalObject) auto scope = DECLARE_THROW_SCOPE(vm); auto binding = constructEmptyArray(globalObject, nullptr, 2); RETURN_IF_EXCEPTION(scope, {}); + ASSERT(binding); auto domainToAsciiFunction = JSC::JSFunction::create(vm, globalObject, 1, "domainToAscii"_s, jsDomainToASCII, ImplementationVisibility::Public); + ASSERT(domainToAsciiFunction); auto domainToUnicodeFunction = JSC::JSFunction::create(vm, globalObject, 1, "domainToUnicode"_s, jsDomainToUnicode, ImplementationVisibility::Public); - ASSERT(binding && domainToAsciiFunction && domainToUnicodeFunction); + ASSERT(domainToUnicodeFunction); binding->putByIndexInline( globalObject, (unsigned)0, From 6ec45e311aac2276c7f6e7e6c9e05b478a68568d Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Wed, 7 May 2025 15:46:14 -0700 Subject: [PATCH 83/88] Use one helper function to emit events on the next tick --- src/bun.js/bindings/BunProcess.cpp | 31 ++++++--- src/bun.js/bindings/BunProcess.h | 3 + .../emit-non-function-fixture.js | 22 +++++++ .../worker_threads/worker_threads.test.ts | 63 +++++++++++++++---- 4 files changed, 101 insertions(+), 18 deletions(-) create mode 100644 test/js/node/worker_threads/emit-non-function-fixture.js diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index 025df5907f8aec..b908fdb9e9d8cc 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -2593,6 +2593,7 @@ void Process::visitChildrenImpl(JSCell* cell, Visitor& visitor) thisObject->m_memoryUsageStructure.visit(visitor); thisObject->m_bindingUV.visit(visitor); thisObject->m_bindingNatives.visit(visitor); + thisObject->m_emitHelperFunction.visit(visitor); } DEFINE_VISIT_CHILDREN(Process); @@ -3075,14 +3076,9 @@ void Process::queueNextTick(JSC::JSGlobalObject* globalObject, JSValue func, con void Process::emitOnNextTick(Zig::GlobalObject* globalObject, ASCIILiteral eventName, JSValue event) { auto& vm = getVM(globalObject); - auto scope = DECLARE_THROW_SCOPE(vm); - auto emit = this->get(globalObject, Identifier::fromString(vm, "emit"_s)); - RETURN_IF_EXCEPTION(scope, ); - if (!emit.getObject()) return; - - auto* bound = JSBoundFunction::create(vm, globalObject, emit.getObject(), this, {}, 0, nullptr); + auto* function = m_emitHelperFunction.getInitializedOnMainThread(this); JSValue args[] = { jsString(vm, String(eventName)), event }; - queueNextTick(globalObject, bound, args); + queueNextTick(globalObject, function, args); } extern "C" void Bun__Process__queueNextTick1(GlobalObject* globalObject, EncodedJSValue func, EncodedJSValue arg1) @@ -3381,6 +3377,24 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionLoadBuiltinModule, (JSGlobalObject * gl RELEASE_AND_RETURN(scope, JSValue::encode(jsUndefined())); } +JSC_DEFINE_HOST_FUNCTION(Process_functionEmitHelper, (JSGlobalObject * globalObject, CallFrame* callFrame)) +{ + auto& vm = JSC::getVM(globalObject); + auto* zigGlobalObject = defaultGlobalObject(globalObject); + auto* process = zigGlobalObject->processObject(); + auto scope = DECLARE_THROW_SCOPE(vm); + auto emit = process->get(globalObject, Identifier::fromString(vm, "emit"_s)); + RETURN_IF_EXCEPTION(scope, {}); + auto callData = JSC::getCallData(emit); + if (callData.type == CallData::Type::None) { + scope.throwException(globalObject, createNotAFunctionError(globalObject, emit)); + return {}; + } + auto ret = JSC::call(globalObject, emit, callData, process, callFrame); + RETURN_IF_EXCEPTION(scope, {}); + return JSValue::encode(ret); +} + extern "C" void Process__emitMessageEvent(Zig::GlobalObject* global, EncodedJSValue value) { auto* process = static_cast(global->processObject()); @@ -3531,6 +3545,9 @@ void Process::finishCreation(JSC::VM& vm) m_bindingNatives.initLater([](const JSC::LazyProperty::Initializer& init) { init.set(Bun::ProcessBindingNatives::create(init.vm, ProcessBindingNatives::createStructure(init.vm, init.owner->globalObject()))); }); + m_emitHelperFunction.initLater([](const JSC::LazyProperty::Initializer& init) { + init.set(JSFunction::create(init.vm, init.owner->globalObject(), 2, "emit"_s, Process_functionEmitHelper, ImplementationVisibility::Private)); + }); putDirect(vm, vm.propertyNames->toStringTagSymbol, jsString(vm, String("process"_s)), 0); putDirect(vm, Identifier::fromString(vm, "_exiting"_s), jsBoolean(false), 0); diff --git a/src/bun.js/bindings/BunProcess.h b/src/bun.js/bindings/BunProcess.h index 2b7f9a1e88dce8..cf6c0c71dad83d 100644 --- a/src/bun.js/bindings/BunProcess.h +++ b/src/bun.js/bindings/BunProcess.h @@ -22,6 +22,9 @@ class Process : public WebCore::JSEventEmitter { LazyProperty m_memoryUsageStructure; LazyProperty m_bindingUV; LazyProperty m_bindingNatives; + // Function that looks up "emit" on "process" and calls it with the provided arguments + // Only used by internal code via passing to queueNextTick + LazyProperty m_emitHelperFunction; WriteBarrier m_uncaughtExceptionCaptureCallback; WriteBarrier m_nextTickFunction; // https://github.com/nodejs/node/blob/2eff28fb7a93d3f672f80b582f664a7c701569fb/lib/internal/bootstrap/switches/does_own_process_state.js#L113-L116 diff --git a/test/js/node/worker_threads/emit-non-function-fixture.js b/test/js/node/worker_threads/emit-non-function-fixture.js new file mode 100644 index 00000000000000..7f6cf6dfc50954 --- /dev/null +++ b/test/js/node/worker_threads/emit-non-function-fixture.js @@ -0,0 +1,22 @@ +import { Worker } from "node:worker_threads"; +import assert from "node:assert"; + +const { promise, resolve, reject } = Promise.withResolvers(); + +process.on("worker", assert.fail); +process.once("uncaughtException", exception => { + try { + assert.strictEqual(exception.name, "TypeError"); + assert(exception.message.includes("5 is not a function"), "message should include '5 is not a function'"); + resolve(); + } catch (e) { + reject(e); + } +}); + +// this will emit the "worker" event on the next tick +new Worker("", { eval: true }); +// override it for when we try to emit the event and look up "emit" +process.emit = 5; +// wait for the error +await promise; diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index c4363c0c5a5a8a..709414fd0d5e3f 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -292,16 +292,57 @@ test("eval does not leak source code", async () => { expect(proc.exitCode).toBe(0); }); -test("worker event", () => { - const { promise, resolve } = Promise.withResolvers(); - let worker: Worker | undefined = undefined; - let called = false; - process.once("worker", eventWorker => { - called = true; - expect(eventWorker as any).toBe(worker); - resolve(); +describe("worker event", () => { + test("is emitted on the next tick with the right value", () => { + const { promise, resolve } = Promise.withResolvers(); + let worker: Worker | undefined = undefined; + let called = false; + process.once("worker", eventWorker => { + called = true; + expect(eventWorker as any).toBe(worker); + resolve(); + }); + worker = new Worker(new URL("data:text/javascript,")); + expect(called).toBeFalse(); + return promise; + }); + + test("uses an overridden process.emit function", async () => { + const previousEmit = process.emit; + try { + const { promise, resolve, reject } = Promise.withResolvers(); + let worker: Worker | undefined; + // should not actually emit the event + process.on("worker", expect.unreachable); + worker = new Worker("", { eval: true }); + // should look up process.emit on the next tick, not synchronously during the Worker constructor + (process as any).emit = (event, value) => { + try { + expect(event).toBe("worker"); + expect(value).toBe(worker); + resolve(); + } catch (e) { + reject(e); + } + }; + await promise; + } finally { + process.emit = previousEmit; + process.off("worker", expect.unreachable); + } + }); + + test("throws if process.emit is not a function", async () => { + const proc = Bun.spawn({ + cmd: [bunExe(), "emit-non-function-fixture.js"], + env: bunEnv, + cwd: __dirname, + stderr: "pipe", + stdout: "ignore", + }); + await proc.exited; + const errors = await new Response(proc.stderr).text(); + if (errors.length > 0) throw new Error(errors); + expect(proc.exitCode).toBe(0); }); - worker = new Worker(new URL("data:text/javascript,")); - expect(called).toBeFalse(); - return promise; }); From a52b52a8003d834890f43fc6c95ec7ed7dc8c4eb Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 8 May 2025 14:39:59 -0700 Subject: [PATCH 84/88] Fix environmentData crash --- src/bun.js/bindings/webcore/JSWorker.cpp | 4 +--- src/bun.js/bindings/webcore/Worker.cpp | 9 +++++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/bun.js/bindings/webcore/JSWorker.cpp b/src/bun.js/bindings/webcore/JSWorker.cpp index 675d10041a610c..0250768c45c695 100644 --- a/src/bun.js/bindings/webcore/JSWorker.cpp +++ b/src/bun.js/bindings/webcore/JSWorker.cpp @@ -298,9 +298,7 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor:: 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. - if (environmentData) { - valueToTransfer->putDirectIndex(globalObject, 1, environmentData); - } + valueToTransfer->putDirectIndex(globalObject, 1, environmentData ? environmentData : jsUndefined()); ExceptionOr> serialized = SerializedScriptValue::create(*lexicalGlobalObject, valueToTransfer, WTFMove(transferList), ports, SerializationForStorage::No, SerializationContext::WorkerPostMessage); if (serialized.hasException()) { diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index be0fea12103444..f930069e5b6d8f 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -532,17 +532,22 @@ JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) // Should always be set to an Array of length 2 in the constructor in JSWorker.cpp auto* pair = jsCast(deserialized); ASSERT(pair->length() == 2); + ASSERT(pair->canGetIndexQuickly(0u)); + ASSERT(pair->canGetIndexQuickly(1u)); workerData = pair->getIndexQuickly(0); RETURN_IF_EXCEPTION(scope, {}); - environmentData = jsCast(pair->getIndexQuickly(1)); + // it might not be a Map if the parent had not set up environmentData yet + environmentData = jsDynamicCast(pair->getIndexQuickly(1)); RETURN_IF_EXCEPTION(scope, {}); // Main thread starts at 1 threadId = jsNumber(worker->clientIdentifier() - 1); - } else { + } + if (!environmentData) { environmentData = JSMap::create(vm, globalObject->mapStructure()); RETURN_IF_EXCEPTION(scope, {}); } + ASSERT(environmentData); globalObject->setNodeWorkerEnvironmentData(environmentData); JSObject* array = constructEmptyArray(globalObject, nullptr, 4); From f2cacbca91bed2e579c8d6dd5bc243392d9f1301 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 8 May 2025 14:46:34 -0700 Subject: [PATCH 85/88] Add test --- .../environmentdata-empty-fixture.js | 20 +++++++ .../environmentdata-inherit-fixture.js | 13 +++++ .../worker_threads/worker_threads.test.ts | 53 +++++++++++++++++++ 3 files changed, 86 insertions(+) create mode 100644 test/js/node/worker_threads/environmentdata-empty-fixture.js create mode 100644 test/js/node/worker_threads/environmentdata-inherit-fixture.js diff --git a/test/js/node/worker_threads/environmentdata-empty-fixture.js b/test/js/node/worker_threads/environmentdata-empty-fixture.js new file mode 100644 index 00000000000000..a17907be651d39 --- /dev/null +++ b/test/js/node/worker_threads/environmentdata-empty-fixture.js @@ -0,0 +1,20 @@ +// when the main thread's environmentData has not been set up (because worker_threads was not imported) +// child threads should still be able to use environmentData + +const innerWorkerSrc = /* js */ ` + const assert = require("assert"); + const { getEnvironmentData } = require("worker_threads"); + assert.strictEqual(getEnvironmentData("foo"), "bar"); +`; + +const outerWorkerSrc = /* js */ ` + const { Worker, setEnvironmentData } = require("worker_threads"); + setEnvironmentData("foo", "bar"); + new Worker(${"`"}${innerWorkerSrc}${"`"}, { eval: true }).on("error", e => { + throw e; + }); +`; + +new Worker("data:text/javascript," + outerWorkerSrc).addEventListener("error", e => { + throw e; +}); diff --git a/test/js/node/worker_threads/environmentdata-inherit-fixture.js b/test/js/node/worker_threads/environmentdata-inherit-fixture.js new file mode 100644 index 00000000000000..5e9475728847ce --- /dev/null +++ b/test/js/node/worker_threads/environmentdata-inherit-fixture.js @@ -0,0 +1,13 @@ +const { Worker, getEnvironmentData, setEnvironmentData, workerData, isMainThread } = require("worker_threads"); + +if (isMainThread) { + // this value should be passed all the way down even through worker threads that don't call setEnvironmentData + setEnvironmentData("inherited", "foo"); + new Worker(__filename, { workerData: { depth: 0 } }); +} else { + console.log(getEnvironmentData("inherited")); + const { depth } = workerData; + if (depth + 1 < 5) { + new Worker(__filename, { workerData: { depth: depth + 1 } }); + } +} diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index 709414fd0d5e3f..ab0a1a63cbaaec 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -1,4 +1,5 @@ import { bunEnv, bunExe } from "harness"; +import { once } from "node:events"; import fs from "node:fs"; import { join, relative, resolve } from "node:path"; import wt, { @@ -346,3 +347,55 @@ describe("worker event", () => { expect(proc.exitCode).toBe(0); }); }); + +describe("environmentData", () => { + test("can pass a value to a child", async () => { + setEnvironmentData("foo", new Map([["hello", "world"]])); + const worker = new Worker( + /* js */ ` + const { getEnvironmentData, parentPort } = require("worker_threads"); + parentPort.postMessage(getEnvironmentData("foo")); + `, + { eval: true }, + ); + const [msg] = await once(worker, "message"); + expect(msg).toEqual(new Map([["hello", "world"]])); + }); + + test("child modifications do not affect parent", async () => { + const worker = new Worker('require("worker_threads").setEnvironmentData("does_not_exist", "foo")', { eval: true }); + const [code] = await once(worker, "exit"); + expect(code).toBe(0); + expect(getEnvironmentData("does_not_exist")).toBeUndefined(); + }); + + test("is deeply inherited", async () => { + const proc = Bun.spawn({ + cmd: [bunExe(), "environmentdata-inherit-fixture.js"], + env: bunEnv, + cwd: __dirname, + stderr: "pipe", + stdout: "pipe", + }); + await proc.exited; + const errors = await new Response(proc.stderr).text(); + if (errors.length > 0) throw new Error(errors); + expect(proc.exitCode).toBe(0); + const out = await new Response(proc.stdout).text(); + expect(out).toBe("foo\n".repeat(5)); + }); + + test("can be used if parent thread had not imported worker_threads", async () => { + const proc = Bun.spawn({ + cmd: [bunExe(), "environmentdata-empty-fixture.js"], + env: bunEnv, + cwd: __dirname, + stderr: "pipe", + stdout: "ignore", + }); + await proc.exited; + const errors = await new Response(proc.stderr).text(); + if (errors.length > 0) throw new Error(errors); + expect(proc.exitCode).toBe(0); + }); +}); From 725ef9fb0f209ac24c06d5759ca0d7e0009a8dbf Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 8 May 2025 15:48:34 -0700 Subject: [PATCH 86/88] Revert "Use helper methods for errors" This reverts commit 05ab6065a97951fae326e5f115b4c0c93c6d1898. --- src/bun.js/bindings/webcore/Worker.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index f930069e5b6d8f..e08cb923d51256 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -501,7 +501,7 @@ JSC_DEFINE_HOST_FUNCTION(jsReceiveMessageOnPort, (JSGlobalObject * lexicalGlobal auto port = callFrame->argument(0); if (!port.isObject()) { - return Bun::ERR::INVALID_ARG_TYPE(scope, lexicalGlobalObject, "port"_s, "MessagePort"_s, port); + return Bun::throwError(lexicalGlobalObject, scope, Bun::ErrorCode::ERR_INVALID_ARG_TYPE, "The \"port\" argument must be a MessagePort instance"_s); } if (auto* messagePort = jsDynamicCast(port)) { @@ -511,7 +511,7 @@ JSC_DEFINE_HOST_FUNCTION(jsReceiveMessageOnPort, (JSGlobalObject * lexicalGlobal return JSC::JSValue::encode(jsUndefined()); } - return Bun::ERR::INVALID_ARG_TYPE(scope, lexicalGlobalObject, "port"_s, "MessagePort"_s, port); + return Bun::throwError(lexicalGlobalObject, scope, Bun::ErrorCode::ERR_INVALID_ARG_TYPE, "The \"port\" argument must be a MessagePort instance"_s); } JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) From 326f9e0884f696ee809a845e162cfa7a27bcc700 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 8 May 2025 17:44:28 -0700 Subject: [PATCH 87/88] Fix RefPtr error and debug logs --- src/ptr/ref_count.zig | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/ptr/ref_count.zig b/src/ptr/ref_count.zig index 84ca5469be64c8..48a117b2dfe6fb 100644 --- a/src/ptr/ref_count.zig +++ b/src/ptr/ref_count.zig @@ -229,30 +229,30 @@ pub fn ThreadSafeRefCount(T: type, field_name: []const u8, destructor: fn (*T) v pub fn ref(self: *T) void { const counter = getCounter(self); if (enable_debug) counter.debug.assertValid(); - const new_count = counter.active_counts.fetchAdd(1, .seq_cst); + const old_count = counter.active_counts.fetchAdd(1, .seq_cst); if (comptime bun.Environment.enable_logs) { scope.log("0x{x} ref {d} -> {d}", .{ @intFromPtr(self), - new_count - 1, - new_count, + old_count, + old_count + 1, }); } - bun.debugAssert(new_count > 0); + bun.debugAssert(old_count > 0); } pub fn deref(self: *T) void { const counter = getCounter(self); if (enable_debug) counter.debug.assertValid(); - const new_count = counter.active_counts.fetchSub(1, .seq_cst); + const old_count = counter.active_counts.fetchSub(1, .seq_cst); if (comptime bun.Environment.enable_logs) { scope.log("0x{x} deref {d} -> {d}", .{ @intFromPtr(self), - new_count + 1, - new_count, + old_count, + old_count - 1, }); } - bun.debugAssert(new_count > 0); - if (new_count == 1) { + bun.debugAssert(old_count > 0); + if (old_count == 1) { if (enable_debug) { counter.debug.deinit(std.mem.asBytes(self), @returnAddress()); } @@ -275,7 +275,7 @@ pub fn ThreadSafeRefCount(T: type, field_name: []const u8, destructor: fn (*T) v pub fn dumpActiveRefs(count: *@This()) void { if (enable_debug) { - const ptr: *T = @fieldParentPtr(field_name, count); + const ptr: *T = @alignCast(@fieldParentPtr(field_name, count)); count.debug.dump(@typeName(T), ptr, count.active_counts.load(.seq_cst)); } } From 90f6978b2e59f5d76574127f626a93d4c2189065 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 8 May 2025 17:45:42 -0700 Subject: [PATCH 88/88] Fix StatWatcher errors --- src/bun.js/node/node_fs_stat_watcher.zig | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/bun.js/node/node_fs_stat_watcher.zig b/src/bun.js/node/node_fs_stat_watcher.zig index 9a5dc706589561..ee888f340333ec 100644 --- a/src/bun.js/node/node_fs_stat_watcher.zig +++ b/src/bun.js/node/node_fs_stat_watcher.zig @@ -71,7 +71,9 @@ pub const StatWatcherScheduler = struct { bun.assert(watcher.closed == false); bun.assert(watcher.next == null); + watcher.ref(); this.watchers.push(watcher); + log("push watcher {x} -> {d} watchers", .{ @intFromPtr(watcher), this.watchers.count }); const current = this.getInterval(); if (current == 0 or current > watcher.interval) { // we are not running or the new watcher has a smaller interval @@ -155,12 +157,14 @@ pub const StatWatcherScheduler = struct { const now = std.time.Instant.now() catch unreachable; var batch = this.watchers.popBatch(); + log("pop batch of {d} -> {d} watchers", .{ batch.count, this.watchers.count }); var iter = batch.iterator(); var min_interval: i32 = std.math.maxInt(i32); var closest_next_check: u64 = @intCast(min_interval); var contain_watchers = false; while (iter.next()) |watcher| { if (watcher.closed) { + watcher.deref(); continue; } contain_watchers = true; @@ -176,6 +180,7 @@ pub const StatWatcherScheduler = struct { } min_interval = @min(min_interval, watcher.interval); this.watchers.push(watcher); + log("reinsert {x} -> {d} watchers", .{ @intFromPtr(watcher), this.watchers.count }); } if (contain_watchers) { @@ -196,7 +201,9 @@ pub const StatWatcher = struct { ctx: *VirtualMachine, - /// Closed is set to true to tell the scheduler to remove from list and mark `used_by_scheduler_thread` as false. + ref_count: RefCount, + + /// Closed is set to true to tell the scheduler to remove from list and deref. closed: bool, path: [:0]u8, persistent: bool, @@ -214,6 +221,10 @@ pub const StatWatcher = struct { scheduler: bun.ptr.RefPtr(StatWatcherScheduler), + const RefCount = bun.ptr.ThreadSafeRefCount(StatWatcher, "ref_count", deinit, .{ .debug_name = "StatWatcher" }); + pub const ref = RefCount.ref; + pub const deref = RefCount.deref; + pub const js = JSC.Codegen.JSStatWatcher; pub const toJS = js.toJS; pub const fromJS = js.fromJS; @@ -228,8 +239,7 @@ pub const StatWatcher = struct { } pub fn deinit(this: *StatWatcher) void { - log("deinit\n", .{}); - this.scheduler.deref(); + log("deinit {x}", .{@intFromPtr(this)}); if (this.persistent) { this.persistent = false; @@ -343,7 +353,9 @@ pub const StatWatcher = struct { /// If the scheduler is not using this, free instantly, otherwise mark for being freed. pub fn finalize(this: *StatWatcher) void { log("Finalize\n", .{}); - this.deinit(); + this.closed = true; + this.scheduler.deref(); + this.deref(); // but don't deinit until the scheduler drops its reference } pub const InitialStatTask = struct { @@ -485,6 +497,7 @@ pub const StatWatcher = struct { .last_stat = std.mem.zeroes(bun.Stat), .last_jsvalue = .empty, .scheduler = vm.rareData().nodeFSStatWatcherScheduler(vm), + .ref_count = .init(), }; errdefer this.deinit();