diff --git a/scripts/build/deps/webkit.ts b/scripts/build/deps/webkit.ts index c9baf14351e6..0b1c956da743 100644 --- a/scripts/build/deps/webkit.ts +++ b/scripts/build/deps/webkit.ts @@ -3,7 +3,7 @@ * for local mode. Override via `--webkit-version=` to test a branch. * From https://github.com/oven-sh/WebKit releases. */ -export const WEBKIT_VERSION = "e2f13c6aa1cdaa885722c0cb55e609334a717d13"; +export const WEBKIT_VERSION = "f0f60fd2324817dae9656d8bf2fcae25ceaccc37"; /** * WebKit (JavaScriptCore) — the JS engine. diff --git a/src/codegen/cppbind.ts b/src/codegen/cppbind.ts index 4821b9207fc2..210115236420 100644 --- a/src/codegen/cppbind.ts +++ b/src/codegen/cppbind.ts @@ -739,7 +739,7 @@ function generateRustFn(fn: CppFn, rustRaw: string[], rustWrap: string[]): void ` // any raw-pointer args are forwarded under the wrapper's own \`unsafe fn\` contract.`, ` let __v = unsafe { raw::${fn.name}(${callArgsStr}) };`, ` __scope.assert_exception_presence_matches(${errCond});`, - ` if ${errCond} { Err(crate::JsError::Thrown) } else { Ok(${okExpr}) }`, + ` if ${errCond} { Err(crate::top_exception_scope::thrown(${gname})) } else { Ok(${okExpr}) }`, `}`, ); } diff --git a/src/jsc/JSRef.rs b/src/jsc/JSRef.rs index ee8a99116737..b7a98050d7ea 100644 --- a/src/jsc/JSRef.rs +++ b/src/jsc/JSRef.rs @@ -205,6 +205,10 @@ impl JsRef { } } + pub fn is_finalized(&self) -> bool { + matches!(self, JsRef::Finalized) + } + pub fn is_not_empty(&self) -> bool { match self { JsRef::Weak(weak) => !weak.is_empty_or_undefined_or_null(), diff --git a/src/jsc/TopExceptionScope.rs b/src/jsc/TopExceptionScope.rs index 39d93710b847..63d7f70595c2 100644 --- a/src/jsc/TopExceptionScope.rs +++ b/src/jsc/TopExceptionScope.rs @@ -584,6 +584,22 @@ impl ExceptionValidationScope { // so the validation scope's diagnostics point at the user's call site, and the // scope is RAII (dropped on every return path including `?`). +unsafe extern "C" { + // safe fn: `&JSGlobalObject` is ABI-identical to a non-null `JSGlobalObject*`; C++ only reads + // and writes its VM's termination flag. + safe fn Bun__VM__keepTerminationRequestWithPendingException(global: &JSGlobalObject); +} + +/// An FFI call into JSC came back with an exception pending. If it is a worker's +/// TerminationException that we are about to leave pending past the entry it unwound (see the C++ +/// side), keep JSC's termination-request flag in step with it. Cold path only. +#[cold] +#[inline(never)] +pub fn thrown(global: &JSGlobalObject) -> JsError { + Bun__VM__keepTerminationRequestWithPendingException(global); + JsError::Thrown +} + /// `[[ZIG_EXPORT(zero_is_throw)]]`: callee returns `JSValue::ZERO` ⟺ it threw. /// /// `src` is the diagnostic location for `BUN_JSC_dumpSimulatedThrows`; pass [`src!`](crate::src) @@ -600,7 +616,7 @@ pub fn call_zero_is_throw_at( let v = f(); scope.assert_exception_presence_matches(v == JSValue::ZERO); if v == JSValue::ZERO { - Err(JsError::Thrown) + Err(thrown(global)) } else { Ok(v) } @@ -629,7 +645,7 @@ pub fn call_false_is_throw_at( let mut scope = ExceptionValidationScope::init_guard_at(&mut storage, global, src); let v = f(); scope.assert_exception_presence_matches(!v); - if v { Ok(()) } else { Err(JsError::Thrown) } + if v { Ok(()) } else { Err(thrown(global)) } } /// `[[ZIG_EXPORT(false_is_throw)]]` — `#[track_caller]` convenience wrapper. @@ -650,7 +666,7 @@ pub fn call_null_is_throw_at( let mut scope = ExceptionValidationScope::init_guard_at(&mut storage, global, src); let v = f(); scope.assert_exception_presence_matches(v.is_null()); - NonNull::new(v).ok_or(JsError::Thrown) + NonNull::new(v).ok_or_else(|| thrown(global)) } /// `[[ZIG_EXPORT(null_is_throw)]]` — `#[track_caller]` convenience wrapper. @@ -681,7 +697,9 @@ pub fn call_check_slow_at( let mut storage = core::mem::MaybeUninit::uninit(); let mut scope = TopExceptionScope::init_guard_at(&mut storage, global, src); let r = f(); - scope.return_if_exception()?; + if scope.return_if_exception().is_err() { + return Err(thrown(global)); + } Ok(r) } #[cfg(not(any(debug_assertions, bun_asan)))] @@ -692,7 +710,7 @@ pub fn call_check_slow_at( // wrapper (reads `vm.m_exception` with trap check; same body as // `RETURN_IF_EXCEPTION` in C++). if crate::cpp::Bun__RETURN_IF_EXCEPTION(global) { - Err(JsError::Thrown) + Err(thrown(global)) } else { Ok(r) } diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index ac0001138b91..f7c0c80fb127 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -1596,15 +1596,25 @@ impl VirtualMachine { let mut dispatch = false; loop { while self.is_event_loop_alive() { + // A stop requested meanwhile (worker.terminate(), or process.exit() + // from a listener) ends the drain, as it ends the worker's main + // loop: what is still in flight is cancelled by teardown, and its + // completions would no longer be delivered to release the loop. + if !self.script_allowed() { + return; + } self.tick(); + if !self.script_allowed() { + return; + } self.auto_tick_active(); dispatch = true; } - // Same guard as on entry: a fatal throw during the inner drain - // must not re-dispatch. The main-thread case already hard-exits - // via `exit_on_uncaught_exception`; this covers workers. - if dispatch && self.unhandled_error_counter == 0 { + // Same guards as on entry: a fatal throw or a stop requested during + // the inner drain must not re-dispatch. The main-thread case already + // hard-exits via `exit_on_uncaught_exception`; this covers workers. + if dispatch && self.unhandled_error_counter == 0 && self.script_allowed() { ExitHandler::dispatch_on_before_exit(self); dispatch = false; diff --git a/src/jsc/bindings/ErrorCode.cpp b/src/jsc/bindings/ErrorCode.cpp index 6114d1d771e6..a0ec451de173 100644 --- a/src/jsc/bindings/ErrorCode.cpp +++ b/src/jsc/bindings/ErrorCode.cpp @@ -206,17 +206,26 @@ JSObject* ErrorCodeCache::createError(VM& vm, Zig::GlobalObject* globalObject, E } auto* structure = uncheckedDowncast(cache->internalField(static_cast(code)).get()); - auto* created_error = JSC::ErrorInstance::create(globalObject, structure, message, options, nullptr, JSC::RuntimeType::TypeNothing, data.type, true); + + // Convert the message and `cause` here rather than in ErrorInstance::create(JSGlobalObject*, ...), + // which hands back nullptr when that conversion is interrupted: every caller throws or rejects with + // what we return, so an object is always made. Whatever interrupted the conversion is dealt with + // below. + String messageString = message.isUndefined() ? String() : message.toWTFString(globalObject); + JSValue cause; + if (options.isObject() && !scope.exception()) + cause = asObject(options)->getIfPropertyExists(globalObject, vm.propertyNames->cause); if (auto* thrown_exception = scope.exception()) [[unlikely]] { - (void)scope.tryClearException(); - if (vm.hasPendingTerminationException()) [[unlikely]] - return created_error; - // TODO investigate what can throw here and whether it will throw non-objects - // (this is better than before where we would have returned nullptr from createError if any - // exception were thrown by ErrorInstance::create) - return uncheckedDowncast(thrown_exception->value()); + // A stopped worker's TerminationException stays pending for the caller's frame to report; the + // (message-less) error is still made. Anything else thrown while building the message (an + // OOM resolving a rope, a throwing `cause` getter) becomes the error, as before. + if (!vm.isTerminationException(thrown_exception)) { + (void)scope.tryClearException(); + if (auto* object = thrown_exception->value().getObject()) + return object; + } } - return created_error; + return JSC::ErrorInstance::create(vm, structure, messageString, cause, nullptr, JSC::RuntimeType::TypeNothing, data.type, true); } JSObject* createError(VM& vm, Zig::GlobalObject* globalObject, ErrorCode code, const String& message) @@ -240,7 +249,11 @@ JSObject* createError(VM& vm, JSC::JSGlobalObject* globalObject, ErrorCode code, return createError(vm, zigGlobalObject, code, message, jsUndefined()); auto* structure = createErrorStructure(vm, globalObject, errors[static_cast(code)].type, errors[static_cast(code)].name, errors[static_cast(code)].code); - return JSC::ErrorInstance::create(globalObject, structure, message, jsUndefined(), nullptr, JSC::RuntimeType::TypeNothing, errors[static_cast(code)].type, true); + auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); + String messageString = message.isUndefined() ? String() : message.toWTFString(globalObject); + if (scope.exception() && !vm.hasPendingTerminationException()) + (void)scope.tryClearException(); + return JSC::ErrorInstance::create(vm, structure, messageString, JSValue(), nullptr, JSC::RuntimeType::TypeNothing, errors[static_cast(code)].type, true); } JSC::JSObject* createError(VM& vm, Zig::GlobalObject* globalObject, ErrorCode code, JSValue message, JSValue options) diff --git a/src/jsc/bindings/NodeTimerObject.cpp b/src/jsc/bindings/NodeTimerObject.cpp index b534e41fedc0..6399894e99db 100644 --- a/src/jsc/bindings/NodeTimerObject.cpp +++ b/src/jsc/bindings/NodeTimerObject.cpp @@ -13,6 +13,9 @@ #include #include "JavaScriptCore/JSCJSValue.h" #include "AsyncContextFrame.h" + +extern "C" void Bun__VM__keepTerminationRequestWithPendingException(JSC::JSGlobalObject*); + namespace Bun { using namespace JSC; @@ -63,6 +66,7 @@ static bool call(JSGlobalObject* globalObject, JSValue timerObject, JSValue call auto* exception = scope.exception(); (void)scope.tryClearException(); Bun__reportUnhandledError(globalObject, JSValue::encode(exception)); + Bun__VM__keepTerminationRequestWithPendingException(globalObject); hadException = true; } diff --git a/src/jsc/bindings/Path.cpp b/src/jsc/bindings/Path.cpp index 91af4a73079f..84eb8c4171b7 100644 --- a/src/jsc/bindings/Path.cpp +++ b/src/jsc/bindings/Path.cpp @@ -141,15 +141,13 @@ JSC::JSValue createNodePathBinding(Zig::GlobalObject* globalObject) auto scope = DECLARE_THROW_SCOPE(vm); auto binding = constructEmptyArray(globalObject, nullptr, 2); RETURN_IF_EXCEPTION(scope, {}); - binding->putDirectIndex( - globalObject, - (unsigned)0, - Zig::createPath(globalObject, false)); + auto* posix = Zig::createPath(globalObject, false); RETURN_IF_EXCEPTION(scope, {}); - binding->putDirectIndex( - globalObject, - (unsigned)1, - Zig::createPath(globalObject, true)); + binding->putDirectIndex(globalObject, (unsigned)0, posix); + RETURN_IF_EXCEPTION(scope, {}); + auto* win32 = Zig::createPath(globalObject, true); + RETURN_IF_EXCEPTION(scope, {}); + binding->putDirectIndex(globalObject, (unsigned)1, win32); RETURN_IF_EXCEPTION(scope, {}); return binding; } diff --git a/src/jsc/bindings/TopExceptionScopeBinding.cpp b/src/jsc/bindings/TopExceptionScopeBinding.cpp index eb0f9e4397d1..9e196f62e0d2 100644 --- a/src/jsc/bindings/TopExceptionScopeBinding.cpp +++ b/src/jsc/bindings/TopExceptionScopeBinding.cpp @@ -38,10 +38,21 @@ extern "C" void TopExceptionScope__construct( #endif } +// A stopped worker keeps draining its tick with its TerminationException pending, past the outermost +// VMEntryScope that reset VM::hasTerminationRequest(); every Rust exception check lands in one of these, so +// re-mark the request where the exception is observed (see Bun__VM__keepTerminationRequestWithPendingException). +static inline JSC::Exception* keepTerminationRequest(JSC::VM& vm, JSC::Exception* exception) +{ + if (exception && vm.isTerminationException(exception) && !vm.hasTerminationRequest()) [[unlikely]] + vm.setHasTerminationRequest(); + return exception; +} + extern "C" JSC::Exception* TopExceptionScope__pureException(void* ptr) { ASSERT((uintptr_t)ptr % alignof(TopExceptionScope) == 0); - return static_cast(ptr)->exception(); + auto* scope = static_cast(ptr); + return keepTerminationRequest(scope->vm(), scope->exception()); } extern "C" JSC::Exception* TopExceptionScope__exceptionIncludingTraps(void* ptr) @@ -51,7 +62,7 @@ extern "C" JSC::Exception* TopExceptionScope__exceptionIncludingTraps(void* ptr) // this is different than `return scope->exception()` because `RETURN_IF_EXCEPTION` also checks // if there are traps that should throw an exception (like a termination request from another // thread) - RETURN_IF_EXCEPTION(*scope, scope->exception()); + RETURN_IF_EXCEPTION(*scope, keepTerminationRequest(scope->vm(), scope->exception())); return nullptr; } diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index acb42309607a..ca0613b6d856 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -3272,6 +3272,8 @@ extern "C" [[ZIG_EXPORT(nothrow)]] double JSC__JSGlobalObject__jsDateNow(JSC::JS // ====================== end conditional builtin globals ====================== +extern "C" void Bun__VM__keepTerminationRequestWithPendingException(JSC::JSGlobalObject*); + uint8_t GlobalObject::drainMicrotasks() { auto& vm = this->vm(); @@ -3279,6 +3281,7 @@ uint8_t GlobalObject::drainMicrotasks() if (auto* exception = scope.exception()) [[unlikely]] { if (vm.isTerminationException(exception)) [[unlikely]] { + Bun__VM__keepTerminationRequestWithPendingException(this); return 1; } @@ -3301,6 +3304,7 @@ uint8_t GlobalObject::drainMicrotasks() nextTickQueue->drain(vm, this); if (auto* exception = scope.exception()) { if (vm.isTerminationException(exception)) { + Bun__VM__keepTerminationRequestWithPendingException(this); return 1; } (void)scope.tryClearException(); @@ -3311,6 +3315,7 @@ uint8_t GlobalObject::drainMicrotasks() vm.drainMicrotasks(); if (auto* exception = scope.exception()) { if (vm.isTerminationException(exception)) { + Bun__VM__keepTerminationRequestWithPendingException(this); return 1; } (void)scope.tryClearException(); diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 82662920a933..ae5b28e37b6c 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -3892,9 +3892,18 @@ JSC::EncodedJSValue JSC__JSPromise__wrap(JSC::JSGlobalObject* globalObject, void arg0->rejectAsHandled(vm, JSC::JSValue::decode(JSValue2)); } -JSC::JSPromise* JSC__JSPromise__rejectedPromise(JSC::JSGlobalObject* arg0, JSC::EncodedJSValue JSValue1) +JSC::JSPromise* JSC__JSPromise__rejectedPromise(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue JSValue1) { - return JSC::JSPromise::rejectedPromise(arg0, JSC::JSValue::decode(JSValue1)); + auto value = JSC::JSValue::decode(JSValue1); + if (!value) [[unlikely]] { + // Building the rejection value threw — a stopped worker's pending TerminationException cuts + // error creation short. That exception is what the caller's frame reports; hand back an + // inert promise rather than reject with nothing. + auto& vm = JSC::getVM(globalObject); + ASSERT(vm.exceptionForInspection()); + return JSC::JSPromise::create(vm, globalObject->promiseStructure()); + } + return JSC::JSPromise::rejectedPromise(globalObject, value); } [[ZIG_EXPORT(check_slow)]] void JSC__JSPromise__resolve(JSC::JSPromise* arg0, JSC::JSGlobalObject* arg1, JSC::EncodedJSValue JSValue2) @@ -6632,6 +6641,20 @@ extern "C" double Bun__JSC__operationMathPow(double x, double y) return operationMathPow(x, y); } +// A stopped worker's TerminationException is kept pending after the JS entry it unwound has +// returned, until teardown clears or re-arms it (Bun__GlobalObject__clearExceptionsForExit / +// Zig__GlobalObject__forbidExecution). JSC resets its "termination in progress" flag when the +// outermost VMEntryScope exits and expects the two to agree while the exception is pending +// (VMTraps::deferTerminationSlow, VM::setException); its own clients never keep the exception past +// that point without also ceasing to touch the VM. Called where an entry has just come back with an +// exception: keep the flag for as long as we keep the exception. +extern "C" void Bun__VM__keepTerminationRequestWithPendingException(JSC::JSGlobalObject* globalObject) +{ + auto& vm = JSC::getVM(globalObject); + if (vm.hasPendingTerminationException() && !vm.hasTerminationRequest()) [[unlikely]] + vm.setHasTerminationRequest(); +} + #if !ENABLE(EXCEPTION_SCOPE_VERIFICATION) extern "C" [[ZIG_EXPORT(nothrow)]] __attribute__((__always_inline__)) bool Bun__RETURN_IF_EXCEPTION(JSC::JSGlobalObject* globalObject) { diff --git a/src/jsc/bindings/node/crypto/CryptoPrimes.cpp b/src/jsc/bindings/node/crypto/CryptoPrimes.cpp index 8a872d890634..472eb89275ad 100644 --- a/src/jsc/bindings/node/crypto/CryptoPrimes.cpp +++ b/src/jsc/bindings/node/crypto/CryptoPrimes.cpp @@ -3,12 +3,24 @@ #include "helpers.h" #include "CryptoUtil.h" #include "NodeValidator.h" +#include "BunClientData.h" namespace Bun { using namespace ncrypto; using namespace JSC; +// BN_generate_prime_ex / BN_is_prime_ex progress callback: returning false aborts. `safe` primes and +// awkward `add`/`rem` constraints can take unboundedly long, so stop as soon as the VM this is for has +// been asked to stop (worker terminate() / process.exit()) rather than hold its teardown — or, on the +// sync paths, the termination itself — until OpenSSL happens to finish. Readable from any thread. +static BignumPointer::PrimeCheckCallback whileScriptAllowed(JSGlobalObject* globalObject) +{ + return [clientData = WebCore::clientData(globalObject->vm())](int, int) -> bool { + return clientData->scriptAllowed(); + }; +} + CheckPrimeJobCtx::CheckPrimeJobCtx(ncrypto::BignumPointer candidate, int32_t checks) : m_candidate(WTF::move(candidate)) , m_checks(checks) @@ -25,20 +37,19 @@ extern "C" void Bun__CheckPrimeJobCtx__runTask(CheckPrimeJobCtx* ctx, JSGlobalOb } void CheckPrimeJobCtx::runTask(JSGlobalObject* lexicalGlobalObject) { - auto res = m_candidate.isPrime(m_checks, [](int32_t a, int32_t b) -> bool { - // TODO(dylan-conway): ideally we check for !vm->isShuttingDown() here - return true; - }); - - m_result = res != 0; + auto res = m_candidate.isPrime(m_checks, whileScriptAllowed(lexicalGlobalObject)); + m_failed = res < 0; + m_result = res > 0; } extern "C" void Bun__CheckPrimeJobCtx__runFromJS(CheckPrimeJobCtx* ctx, JSGlobalObject* lexicalGlobalObject, JSCallbackArgs* out) { *out = ctx->runFromJS(lexicalGlobalObject); } -JSCallbackArgs CheckPrimeJobCtx::runFromJS(JSGlobalObject*) +JSCallbackArgs CheckPrimeJobCtx::runFromJS(JSGlobalObject* globalObject) { + if (m_failed) [[unlikely]] + return { createError(globalObject, ErrorCode::ERR_CRYPTO_OPERATION_FAILED, "could not check prime"_s) }; return { jsUndefined(), jsBoolean(m_result) }; } @@ -97,12 +108,11 @@ JSC_DEFINE_HOST_FUNCTION(jsCheckPrimeSync, (JSC::JSGlobalObject * lexicalGlobalO } } - auto res = candidate.isPrime(checks, [](int32_t a, int32_t b) -> bool { - // TODO(dylan-conway): ideally we check for !vm->isShuttingDown() here - return true; - }); + auto res = candidate.isPrime(checks, whileScriptAllowed(lexicalGlobalObject)); + if (res < 0) [[unlikely]] + return ERR::CRYPTO_OPERATION_FAILED(scope, lexicalGlobalObject, "could not check prime"_s); - return JSValue::encode(jsBoolean(res != 0)); + return JSValue::encode(jsBoolean(res > 0)); } JSC_DEFINE_HOST_FUNCTION(jsCheckPrime, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame)) @@ -177,10 +187,7 @@ extern "C" void Bun__GeneratePrimeJobCtx__runTask(GeneratePrimeJobCtx* ctx, JSGl } void GeneratePrimeJobCtx::runTask(JSGlobalObject* lexicalGlobalObject) { - m_prime.generate({ .bits = m_size, .safe = m_safe, .add = m_add, .rem = m_rem }, [](int32_t a, int32_t b) -> bool { - // TODO(dylan-conway): ideally we check for !vm->isShuttingDown() here - return true; - }); + m_failed = !m_prime.generate({ .bits = m_size, .safe = m_safe, .add = m_add, .rem = m_rem }, whileScriptAllowed(lexicalGlobalObject)); } extern "C" void Bun__GeneratePrimeJobCtx__runFromJS(GeneratePrimeJobCtx* ctx, JSGlobalObject* lexicalGlobalObject, JSCallbackArgs* out) @@ -192,15 +199,19 @@ JSCallbackArgs GeneratePrimeJobCtx::runFromJS(JSGlobalObject* globalObject) auto& vm = globalObject->vm(); auto scope = DECLARE_THROW_SCOPE(vm); + if (m_failed) [[unlikely]] + return { createError(globalObject, ErrorCode::ERR_CRYPTO_OPERATION_FAILED, "could not generate prime"_s) }; JSValue result = GeneratePrimeJob::result(globalObject, scope, m_prime, m_bigint); - EXCEPTION_ASSERT(result.isEmpty() == !!scope.exception()); - if (scope.exception()) [[unlikely]] { - // The thrown Error, not the Exception cell (node parity). - JSValue err = scope.exception()->value(); - (void)scope.tryClearException(); - return { err }; + if (auto* exception = scope.exception()) [[unlikely]] { + // A stopped worker's termination (which a trap can raise anywhere in result(), even with a + // value made): leave it pending for then() to propagate. Anything else is this job's own + // failure and becomes the callback's `err` — the thrown Error, not the Exception cell + // (node parity). + if (!scope.tryClearException()) + return {}; + return { exception->value() }; } - + ASSERT(!result.isEmpty()); return { jsUndefined(), result }; } @@ -458,10 +469,8 @@ JSC_DEFINE_HOST_FUNCTION(jsGeneratePrimeSync, (JSC::JSGlobalObject * lexicalGlob return ERR::CRYPTO_OPERATION_FAILED(scope, lexicalGlobalObject, "could not generate prime"_s); } - prime.generate({ .bits = size, .safe = safe, .add = add, .rem = rem }, [](int32_t a, int32_t b) -> bool { - // TODO(dylan-conway): ideally we check for !vm->isShuttingDown() here - return true; - }); + if (!prime.generate({ .bits = size, .safe = safe, .add = add, .rem = rem }, whileScriptAllowed(lexicalGlobalObject))) [[unlikely]] + return ERR::CRYPTO_OPERATION_FAILED(scope, lexicalGlobalObject, "could not generate prime"_s); return JSValue::encode(GeneratePrimeJob::result(lexicalGlobalObject, scope, prime, bigint)); } diff --git a/src/jsc/bindings/node/crypto/CryptoPrimes.h b/src/jsc/bindings/node/crypto/CryptoPrimes.h index a60e34780189..3cab091f6261 100644 --- a/src/jsc/bindings/node/crypto/CryptoPrimes.h +++ b/src/jsc/bindings/node/crypto/CryptoPrimes.h @@ -19,6 +19,7 @@ struct CheckPrimeJobCtx { ncrypto::BignumPointer m_candidate; bool m_result { false }; + bool m_failed { false }; WTF_MAKE_TZONE_ALLOCATED(CheckPrimeJobCtx); }; @@ -42,6 +43,7 @@ struct GeneratePrimeJobCtx { ncrypto::BignumPointer m_add; ncrypto::BignumPointer m_rem; ncrypto::BignumPointer m_prime; + bool m_failed { false }; WTF_MAKE_TZONE_ALLOCATED(GeneratePrimeJobCtx); }; diff --git a/src/runtime/api/bun/subprocess.rs b/src/runtime/api/bun/subprocess.rs index 64343eae099e..e5ff663de543 100644 --- a/src/runtime/api/bun/subprocess.rs +++ b/src/runtime/api/bun/subprocess.rs @@ -441,6 +441,11 @@ impl Subprocess<'_> { if self.flags.get().contains(Flags::IS_SYNC) { return; } + // The wrapper is gone (finalize() closing stdio that a stopped worker + // left pending): there is nothing to keep alive or release. + if self.this_value.get().is_finalized() { + return; + } let has_pending = self.compute_has_pending_activity(); if cfg!(debug_assertions) { diff --git a/src/runtime/valkey_jsc/js_valkey.rs b/src/runtime/valkey_jsc/js_valkey.rs index c411219b998f..c40332f9b48a 100644 --- a/src/runtime/valkey_jsc/js_valkey.rs +++ b/src/runtime/valkey_jsc/js_valkey.rs @@ -1078,7 +1078,7 @@ impl JSValkeyClient { ) { return Ok(JSValue::UNDEFINED); } - self.client_mut().disconnect(); + self.client_mut().disconnect()?; Ok(JSValue::UNDEFINED) } @@ -1228,17 +1228,22 @@ impl JSValkeyClient { let client = self.client_mut(); client.flags.connection_promise_returns_client = false; client.flags.is_manually_closed = true; - let _close = scopeguard::guard(BackRef::new(self), |p| p.client_mut().close()); - if let Some(promise) = Js::connection_promise_get_cached(this_value) { - Js::connection_promise_set_cached( - this_value, - &global_object, - JSValue::ZERO, - ); - JSPromise::opaque_mut(promise.as_promise().unwrap()) - .reject(&global_object, Ok(error))?; - } - return self.client_mut().fail_with_js_value(&global_object, error); + let rejected = match Js::connection_promise_get_cached(this_value) { + Some(promise) => { + Js::connection_promise_set_cached( + this_value, + &global_object, + JSValue::ZERO, + ); + JSPromise::opaque_mut(promise.as_promise().unwrap()) + .reject(&global_object, Ok(error)) + } + None => Ok(()), + }; + let failed = + rejected.and_then(|()| client.fail_with_js_value(&global_object, error)); + let closed = self.client_mut().close(); + return failed.and(closed); } }; Js::hello_set_cached(this_value, &global_object, hello_value); @@ -1849,10 +1854,11 @@ impl SocketHandler { ) -> JsResult<()> { let _exit = this.vm().enter_event_loop_scope(); this.client_mut().flags.is_manually_closed = true; - let this_br = BackRef::new(this); - let _close = scopeguard::guard(this_br, |p| p.client_mut().close()); - this.client_mut() - .fail_with_js_value(&this.global_object, err_value) + let failed = this + .client_mut() + .fail_with_js_value(&this.global_object, err_value); + let closed = this.client_mut().close(); + failed.and(closed) } pub(crate) const ON_HANDSHAKE: Option< @@ -2023,7 +2029,7 @@ impl ValkeyDeferredClose { let ctx = self.ctx; // SAFETY: single-threaded; intrusive ref taken before enqueue guarantees liveness. unsafe { - (*ctx).client_mut().close(); + crate::dispatch::fold((*ctx).client_mut().close()); JSValkeyClient::deref(ctx.cast_mut()); } } diff --git a/src/runtime/valkey_jsc/valkey.rs b/src/runtime/valkey_jsc/valkey.rs index 43ebf9324c97..4172cf89a086 100644 --- a/src/runtime/valkey_jsc/valkey.rs +++ b/src/runtime/valkey_jsc/valkey.rs @@ -614,18 +614,23 @@ impl ValkeyClient { if !self.connection_ready() { self.flags.is_manually_closed = true; - self.close(); + let closed = self.close(); // unconditionally, whatever `val` is + return val.and(closed); } val } - pub fn close(&mut self) { + /// For a half-open socket this runs `on_close` itself (see below) and returns what its + /// `onclose` listener left pending; the caller propagates that like any other callback + /// result (or folds it if it is the trampoline), it is never folded here beneath a + /// frame that is still going to return its own `Err`. + pub fn close(&mut self) -> JsResult<()> { let socket = core::mem::replace( &mut self.socket, AnySocket::SocketTcp(uws::SocketTCP::detached()), ); if socket.is_closed() { - return; + return Ok(()); } // usockets does not dispatch `on_close`/`on_connect_error` when an // application explicitly closes a `us_socket_t` whose TCP connect @@ -642,10 +647,11 @@ impl ValkeyClient { socket.close(uws::CloseCode::Normal); if is_semi_socket { self.status = Status::Disconnected; - // A half-open socket never gets uSockets' close dispatch, so this is - // its trampoline for the event: fold what `onclose` left pending here. - crate::dispatch::fold(self.on_close()); + // A half-open socket never gets uSockets' close dispatch, so run the + // close event here. + return self.on_close(); } + Ok(()) } /// Handle connection closed event @@ -1492,12 +1498,13 @@ impl ValkeyClient { } /// Close the Valkey connection - pub(crate) fn disconnect(&mut self) { + pub(crate) fn disconnect(&mut self) -> JsResult<()> { self.flags.is_manually_closed = true; self.unregister_auto_flusher(); if self.status == Status::Connected || self.status == Status::Connecting { - self.close(); + return self.close(); } + Ok(()) } /// Get a writer for the connected socket diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 39e258c5d785..1589e4820a85 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -115,6 +115,10 @@ pub struct FetchTasklet { pub(crate) native_response: Option<*mut Response>, /// stream strong ref if any is available pub(crate) readable_stream_ref: ReadableStreamStrong, + /// A counted ref on that stream's ByteStream source for as long as this tasklet is its + /// `producer`, so unhooking goes through native memory we keep alive rather than through + /// the JS wrappers, which the VM's last sweep destroys in no particular order. + pub(crate) response_stream_source: Option>, pub(crate) request_headers: Headers, pub(crate) promise: jsc::JSPromiseStrong, pub(crate) concurrent_task: ConcurrentTask, @@ -1591,7 +1595,17 @@ impl FetchTasklet { readable: ReadableStream, ) { let this = Self::from_ctx(ctx); + this.clear_stream_handlers(); this.readable_stream_ref = ReadableStreamStrong::init(readable, global_this); + if let crate::webcore::readable_stream::Source::Bytes(bytes) = readable.ptr { + // SAFETY: the stream (held Strong above) owns a live ByteStream embedded in + // its Source; JS thread. + unsafe { + let source = (*bytes).parent(); + (*source).increment_count(); + this.response_stream_source = NonNull::new(source); + } + } // A ByteStream now drains scheduled_response_buffer per chunk; undo any // buffered-consumer reservation request so callback() stops growing it. this.is_buffering_body.store(false, Ordering::Release); @@ -1658,13 +1672,16 @@ impl FetchTasklet { } } - /// Clear every ByteStream.Source callback whose ctx is this FetchTasklet - /// before releasing `readable_stream_ref` — the stream can outlive us in JS. + /// Unhook this tasklet as the response ByteStream's producer before releasing + /// `readable_stream_ref` — the stream can outlive us in JS — and drop the ref that + /// kept the source's memory ours to write to. Touches no JS cell. fn clear_stream_handlers(&mut self) { - if let Some(readable) = self.readable_stream_ref.get(&self.global_this) { - if let Some(bytes) = readable.ptr.bytes() { - let source = bytes.parent_const(); - source.producer.set(SourceHandle::None); + if let Some(source) = self.response_stream_source.take() { + // SAFETY: counted ref taken in `on_readable_stream_available`; live until + // the `decrement_count` below, which may free it. + unsafe { + (*source.as_ptr()).producer.set(SourceHandle::None); + crate::webcore::byte_stream::Source::decrement_count(source.as_ptr()); } } } @@ -1676,7 +1693,7 @@ impl FetchTasklet { // reader.cancel() / body.cancel() aborts the fetch so the server sees the // close (Node/Deno/browsers abort unconditionally). abort_task() is idempotent. self.abort_task(); - self.ignore_remaining_response_body(false); + self.ignore_remaining_response_body(); } pub(crate) fn on_stream_drained(&self) { @@ -1813,7 +1830,7 @@ impl FetchTasklet { ) } - fn ignore_remaining_response_body(&mut self, from_finalizer: bool) { + fn ignore_remaining_response_body(&mut self) { bun_output::scoped_log!(FetchTasklet, "ignoreRemainingResponseBody"); // enabling streaming will make the http thread to drain into the main thread (aka stop buffering) // without a stream ref, response body or response instance alive it will just ignore the result @@ -1835,16 +1852,11 @@ impl FetchTasklet { } // we should not keep the process alive if we are ignoring the body self.poll_ref.unref(bun_io::js_vm_ctx()); - // When reached from `on_response_finalize` (a JSC Weak finalizer inside - // `WeakBlock::sweep`), `clear_stream_handlers()` must be skipped: it - // reaches `JSCell::classInfo()` via generated cached-value setters / - // `ReadableStreamTag__tagged`, and touching any cell during - // `MutatorState::Sweeping` is forbidden. The request-body sink is left - // for `clear_sink()` in `deinit()` (an event-loop task, outside sweep) - // to detach. - if !from_finalizer { - self.clear_stream_handlers(); - } + // Also fine from `on_response_finalize` (a JSC Weak finalizer inside + // `WeakBlock::sweep`): unhooking touches no JS cell. The + // request-body sink is left for `clear_sink()` in `deinit()` (an event-loop + // task, outside sweep) to detach. + self.clear_stream_handlers(); self.readable_stream_ref.deinit(); self.response.clear(); @@ -1907,6 +1919,7 @@ impl FetchTasklet { response: jsc::Weak::default(), native_response: None, readable_stream_ref: ReadableStreamStrong::default(), + response_stream_source: None, request_headers: fetch_options.headers, promise, concurrent_task: ConcurrentTask::default(), @@ -2632,11 +2645,11 @@ impl FetchTasklet { if let Some(promise) = locked.promise { if promise.is_empty_or_undefined_or_null() { // Scenario 2b. - this.ignore_remaining_response_body(true); + this.ignore_remaining_response_body(); } } else { // Scenario 3. - this.ignore_remaining_response_body(true); + this.ignore_remaining_response_body(); } } } diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index c1e666508640..88ab85eea2f4 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -683,3 +683,273 @@ test( }, timeout, ); + +// worker.terminate() landing while the worker was re-running its event loop for +// process.on('beforeExit') listeners (they scheduled more work) was never acted +// on: that inner drain only watched for the loop to go idle, and with the stop +// requested the in-flight work's completion is no longer delivered, so the +// worker slept in its loop forever and terminate() never settled. +test( + "terminate() while the worker drains work scheduled by 'beforeExit' stops it", + async () => { + using server = Bun.serve({ port: 0, fetch: () => new Promise(() => {}) }); + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("node:worker_threads"); + const w = new Worker( + "const { parentPort } = require('node:worker_threads');" + + "process.on('beforeExit', () => { fetch(process.env.HANG_URL).catch(() => {}); parentPort.postMessage('draining'); });" + + "process.on('exit', (c) => parentPort.postMessage('exit ' + c));", + { eval: true }, + ); + w.on("message", async (m) => { + if (m !== "draining") { console.log("unexpected", m); return; } + const code = await w.terminate(); + console.log("terminated", code); + }); + w.on("exit", (c) => console.log("exit", c)); + `, + ], + env: { ...bunEnv, HANG_URL: server.url.href }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout.trim().split("\n").sort()).toEqual(["exit 1", "terminated 1"]); + expect(exitCode).toBe(0); + }, + timeout, +); + +// For a debug build: host code that runs after the worker's own process.exit() +// unwound script — here a redis connect started in the same immediate tick as +// the exit, whose ECONNREFUSED then lands in that loop tick — builds JS error +// objects, initialising lazy structures under the TerminationException Bun keeps +// pending. JSC had already reset its termination-request flag when the entry the +// exception unwound exited, so DeferTermination asserted `vm.hasTerminationRequest()` +// (and dropped the pending termination in release); Bun now keeps the flag set +// for as long as it keeps the exception. +test.skipIf(!isDebug)( + "process.exit() with native error completions landing in the same tick does not trip DeferTermination", + async () => { + const workers = slow ? 8 : 24; + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("node:worker_threads"); + const src = + "const { parentPort } = require('node:worker_threads');" + + "Bun.file(process.execPath).slice(0, 100).json().catch(() => {});" + + "setImmediate(() => new Bun.RedisClient('redis://127.0.0.1:9', { connectionTimeout: 100, autoReconnect: false }).connect().catch(() => {}));" + + "parentPort.postMessage('up');" + + "setImmediate(() => process.exit(0));"; + let started = 0, exited = 0; + function again() { + if (started >= ${workers}) { + if (exited === ${workers}) console.log("PASS"); + return; + } + started++; + const w = new Worker(src, { eval: true }); + w.on("error", (e) => { console.error(e); process.exit(1); }); + w.on("exit", () => { exited++; again(); }); + } + again(); again(); + `, + ], + env: { ...bunEnv, UV_THREADPOOL_SIZE: "4" }, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("PASS\n"); + expect(exitCode).toBe(0); + }, + timeout, +); + +// worker.terminate() never stopped a worker parked in +// Atomics.wait() (sync-over-async worker pools park exactly there). JSC wakes +// the parked thread when termination is requested, but the wake-up predicate +// only looked at a flag the parked thread itself would have had to set, so it +// went back to sleep and terminate()'s promise never settled. +test( + "terminate() stops a worker blocked in Atomics.wait()", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("node:worker_threads"); + const w = new Worker( + "const { parentPort } = require('node:worker_threads');" + + "const i32 = new Int32Array(new SharedArrayBuffer(4));" + + "parentPort.postMessage('parking');" + + "Atomics.wait(i32, 0, 0);" + + "parentPort.postMessage('woke ' + Atomics.load(i32, 0));", + { eval: true }, + ); + w.on("message", async (m) => { + if (m !== "parking") { console.log("unexpected", m); process.exit(1); } + // The case of interest is terminate() landing once the worker is parked, for which there + // is no observable signal, so give it a moment; landing before it parks must pass too. + await Bun.sleep(100); + const code = await w.terminate(); + console.log("terminated", code); + }); + w.on("exit", (c) => console.log("exit", c)); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout.trim().split("\n").sort()).toEqual(["exit 1", "terminated 1"]); + expect(exitCode).toBe(0); + }, + timeout, +); + +// crypto.generatePrime()/generatePrimeSync()/checkPrime() with `safe: true` (or awkward add/rem +// constraints) can grind for minutes. The worker's teardown waits for its pool job, and the sync +// form cannot observe the termination at all, so terminate() used to hang for as long as BoringSSL +// took. The generation's progress callback now gives up once the worker has been asked to stop. +test( + "terminate() does not wait for a prime generation the worker no longer needs", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("node:worker_threads"); + async function run(body) { + const w = new Worker('require("node:worker_threads").parentPort.postMessage("go");' + body, { eval: true }); + w.on("error", (e) => { console.error(e); process.exit(1); }); + await new Promise((r) => w.once("message", r)); + await Bun.sleep(100); + return await w.terminate(); + } + (async () => { + const t = performance.now(); + const codes = await Promise.all([ + run('for (;;) require("node:crypto").generatePrimeSync(2048, { safe: true });'), + run('require("node:crypto").generatePrime(2048, { safe: true }, () => {}); setInterval(() => {}, 1000);'), + run('require("node:crypto").checkPrime((1n << 4423n) - 1n, { checks: 200 }, () => {}); setInterval(() => {}, 1000);'), + ]); + console.log(codes.join(","), performance.now() - t < 20000 ? "promptly" : "after " + Math.round(performance.now() - t) + "ms"); + })(); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("1,1,1 promptly\n"); + expect(exitCode).toBe(0); + }, + timeout, +); + +// A worker exiting while a Bun.spawn() child still has a pending pipe-backed stdin (a Blob the child +// never reads; the default stdin path on Windows, BUN_FEATURE_FLAG_DISABLE_MEMFD elsewhere): the +// Subprocess finalizer closed that writer, whose close path re-evaluated pending activity and tried to +// re-root the wrapper it had just marked finalized (debug assert). +test( + "worker exit with a spawned child's Blob stdin still pending", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("node:worker_threads"); + const w = new Worker(\` + const p = Bun.spawn([process.execPath, "-e", "setTimeout(() => {}, 3000)"], { stdin: new Blob([new Uint8Array(4 << 20)]), stdout: "ignore" }); + globalThis.keep = p; + setTimeout(() => process.exit(0), 50); + \`, { eval: true }); + w.on("error", (e) => { console.error(e); process.exit(1); }); + w.on("exit", (c) => { console.log("worker exit", c); process.exit(0); }); + `, + ], + env: { ...bunEnv, BUN_FEATURE_FLAG_DISABLE_MEMFD: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("worker exit 0\n"); + expect(exitCode).toBe(0); + }, + timeout, +); + +// A worker exiting with fetches that have both a streaming request body (whose sink cell holds the +// FetchTasklet) and a JS-touched response.body (a ByteStream source owned by another cell): the VM's +// last sweep destroys cells in no particular order, and the tasklet's teardown unhooked itself as the +// response stream's producer by writing through the stream's wrapper into a source that sweep had +// already freed (heap-use-after-free WRITE under ASAN). The tasklet now holds a counted ref on the +// source for as long as it is its producer. +test( + "worker exit with streaming-request-body fetches whose response.body was touched", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("node:worker_threads"); + const server = Bun.serve({ port: 0, fetch(req) { return new Response(req.body); } }); + const N = 6; + let done = 0; + for (let i = 0; i < N; i++) { + const w = new Worker(\` + const keep = []; + let touched = 0; + for (let j = 0; j < 8; j++) { + let ctrl; + const body = new ReadableStream({ start(c) { ctrl = c; c.enqueue(new Uint8Array(1024)); } }); + const p = fetch("\${server.url}", { method: "POST", body, duplex: "half" }) + .then((r) => { keep.push(r.body); const rd = r.body.getReader(); rd.read(); keep.push(rd); touched++; }) + .catch(() => {}); + keep.push(p, ctrl); + setInterval(() => { try { ctrl.enqueue(new Uint8Array(512)); } catch {} }, 5); + } + // Exit with that state alive; exit code 3 if no fetch ever reached it (the test would + // then not be exercising what it claims to). + setTimeout(() => process.exit(touched > 0 ? 0 : 3), 150 + \${(i * 13) % 60}); + \`, { eval: true }); + w.on("error", (e) => { console.error(e); process.exit(1); }); + w.on("exit", (code) => { + if (code !== 0) { console.error("worker exited " + code); process.exit(1); } + if (++done === N) { console.log("all exited"); server.stop(true); process.exit(0); } + }); + } + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("all exited\n"); + expect(exitCode).toBe(0); + }, + timeout, +);