From a63be92c11403a81321223fa6a20ccd92f717652 Mon Sep 17 00:00:00 2001 From: robobun Date: Thu, 16 Jul 2026 21:05:54 -0700 Subject: [PATCH 1/4] Defer termination at the reifyStaticProperty call sites and propagate builder exceptions (#282) * Defer termination across static-hashtable PropertyCallback builders LazyPropertyCallback runs inside reifyStaticProperty -> setUpStaticFunctionSlot -> getOwnPropertySlot, none of which check for exceptions after the builder. When worker.terminate() arms the termination trap while a builder enters JS, the trap fires inside the builder and the TerminationException stays pending after it returns; getOwnPropertySlot then returns true with an exception pending and trips EXCEPTION_ASSERT(!scope.exception() || !hasSlot) in JSValue::get / JSObject::getOwnPropertyDescriptor. LazyProperty::callFunc already wraps its initializer in DeferTerminationForAWhile for the same reason. Do the same for the PropertyCallback arm of reifyStaticProperty, covering both callers (setUpStaticFunctionSlot from getOwnPropertySlot and reifyAllStaticProperties from deleteProperty). * Move the defer to the call sites and propagate exceptions Per review: keep the include out of Lookup.h. DeferTerminationForAWhile now wraps the reifyStaticProperty call in setUpStaticFunctionSlot (Lookup.cpp) and the reify loop in JSObject::reifyAllStaticProperties. Also propagate a non-termination exception from a PropertyCallback builder: reifyStaticProperty skips the putDirect when the callback returns empty, setUpStaticFunctionSlot returns false (slot not found) when an exception is pending so JSValue::get / getOwnPropertyDescriptor's EXCEPTION_ASSERT(!scope.exception() || !result) holds, and reifyAllStaticProperties leaves the remaining properties lazy. * Use a ThrowScope instead of exceptionForInspection at the reify call sites exceptionForInspection() is for assertions/diagnostics only (every other use in JSC is inside ASSERT or #if ASSERT_ENABLED). It bypasses exception scope verification because it doesn't clear m_needExceptionCheck. Declare a ThrowScope and use RETURN_IF_EXCEPTION, matching the pattern at JSGlobalObject.cpp tryInstallSpeciesWatchpoint / JSBoundFunction.cpp nameSlow. In setUpStaticFunctionSlot the defer scope ends before the check so a suspended termination that is re-thrown on defer exit is observed and we return false, satisfying the caller's EXCEPTION_ASSERT(!scope.exception() || !hasSlot). --- Source/JavaScriptCore/runtime/JSObject.cpp | 9 ++++++++- Source/JavaScriptCore/runtime/Lookup.cpp | 13 ++++++++++++- Source/JavaScriptCore/runtime/Lookup.h | 5 +++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp index 3bbbd19d3b61d..85065e26c8963 100644 --- a/Source/JavaScriptCore/runtime/JSObject.cpp +++ b/Source/JavaScriptCore/runtime/JSObject.cpp @@ -2948,6 +2948,10 @@ void JSObject::reifyAllStaticProperties(JSGlobalObject* globalObject) if (!structure()->isDictionary()) convertToDictionary(vm); + // A PropertyCallback builder can enter JS; defer termination (like + // LazyProperty::callFunc) so it can't return with one pending. + auto scope = DECLARE_THROW_SCOPE(vm); + DeferTerminationForAWhile deferScope(vm); for (const ClassInfo* info = classInfo(); info; info = info->parentClass) { const HashTable* hashTable = info->staticPropHashTable; if (!hashTable) @@ -2957,8 +2961,11 @@ void JSObject::reifyAllStaticProperties(JSGlobalObject* globalObject) unsigned attributes; auto key = Identifier::fromString(vm, value.m_key); PropertyOffset offset = getDirectOffset(vm, key, attributes); - if (!isValidOffset(offset)) + if (!isValidOffset(offset)) { reifyStaticProperty(vm, hashTable->classForThis, key, value, *this); + // Leave the rest lazy on throw; the caller propagates. + RETURN_IF_EXCEPTION(scope, void()); + } } } diff --git a/Source/JavaScriptCore/runtime/Lookup.cpp b/Source/JavaScriptCore/runtime/Lookup.cpp index d0f861c34d3bc..fd4b6e3273316 100644 --- a/Source/JavaScriptCore/runtime/Lookup.cpp +++ b/Source/JavaScriptCore/runtime/Lookup.cpp @@ -20,6 +20,7 @@ #include "config.h" #include "Lookup.h" +#include "DeferTermination.h" #include "GetterSetter.h" #include "JSCInlines.h" #include @@ -58,7 +59,17 @@ bool setUpStaticFunctionSlot(VM& vm, const ClassInfo* classInfo, const HashTable if (thisObject->staticPropertiesReified()) return false; - reifyStaticProperty(vm, classInfo, propertyName, *entry, *thisObject); + auto scope = DECLARE_THROW_SCOPE(vm); + { + // A PropertyCallback builder can enter JS; defer termination (like + // LazyProperty::callFunc) so it can't return with one pending. + DeferTerminationForAWhile deferScope(vm); + reifyStaticProperty(vm, classInfo, propertyName, *entry, *thisObject); + } + // The builder may still throw a non-termination exception; report the + // slot as not found so JSValue::get / getOwnPropertyDescriptor's + // EXCEPTION_ASSERT(!scope.exception() || !result) holds. + RETURN_IF_EXCEPTION(scope, false); offset = thisObject->getDirectOffset(vm, propertyName, attributes); if (!isValidOffset(offset)) { diff --git a/Source/JavaScriptCore/runtime/Lookup.h b/Source/JavaScriptCore/runtime/Lookup.h index e2a95e996d1e1..a7134ccf7e0b9 100644 --- a/Source/JavaScriptCore/runtime/Lookup.h +++ b/Source/JavaScriptCore/runtime/Lookup.h @@ -538,6 +538,11 @@ inline void reifyStaticProperty(VM& vm, const ClassInfo* classInfo, const Proper if (value.attributes() & PropertyAttribute::PropertyCallback) { JSValue result = value.lazyPropertyCallback()(vm, &thisObj); + // A callback that enters JS may return empty with an exception pending; + // the two callers (setUpStaticFunctionSlot / reifyAllStaticProperties) + // check and propagate, so don't put an empty value in the slot here. + if (!result) [[unlikely]] + return; thisObj.putDirect(vm, propertyName, result, attributesForStructure(value.attributes())); return; } From 0006cf6093ef8fdaa7e32d236720ebe12c78fd44 Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 17 Jul 2026 14:00:31 -0700 Subject: [PATCH 2/4] Drop the ThrowScope from the reifyStaticProperty call sites (#306) ae5110d307 added DECLARE_THROW_SCOPE inside setUpStaticFunctionSlot and reifyAllStaticProperties so a PropertyCallback builder that throws can be detected and propagated. A ThrowScope's destructor unconditionally simulates a throw to its caller, so this made both functions look throwing to the exception-scope verifier. setUpStaticFunctionSlot runs on every first lookup of a static hashtable property via getOwnPropertySlot, and JSC-internal callers of that path (CodeBlock::finishCreation via initializeTemplateObjects, among others) never check afterward, so with validateExceptionChecks=1 they now crash even though nothing threw. reifyAllStaticProperties has the same issue for JSObject::deleteProperty. Keep the DeferTerminationForAWhile and the early-return-on-exception, but read vm.exceptionForInspection() directly instead of declaring a ThrowScope. That preserves the ae5110d307 behaviour (report the slot as not found when the builder left an exception pending, bail from the reify-all loop) without forcing a simulated throw onto callers that have never had to handle one. --- Source/JavaScriptCore/runtime/JSObject.cpp | 7 ++++--- Source/JavaScriptCore/runtime/Lookup.cpp | 8 +++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp index 85065e26c8963..0281bd75de9cf 100644 --- a/Source/JavaScriptCore/runtime/JSObject.cpp +++ b/Source/JavaScriptCore/runtime/JSObject.cpp @@ -2949,8 +2949,8 @@ void JSObject::reifyAllStaticProperties(JSGlobalObject* globalObject) convertToDictionary(vm); // A PropertyCallback builder can enter JS; defer termination (like - // LazyProperty::callFunc) so it can't return with one pending. - auto scope = DECLARE_THROW_SCOPE(vm); + // LazyProperty::callFunc) so it can't return with one pending. No + // ThrowScope here: JSObject::deleteProperty reaches this without one. DeferTerminationForAWhile deferScope(vm); for (const ClassInfo* info = classInfo(); info; info = info->parentClass) { const HashTable* hashTable = info->staticPropHashTable; @@ -2964,7 +2964,8 @@ void JSObject::reifyAllStaticProperties(JSGlobalObject* globalObject) if (!isValidOffset(offset)) { reifyStaticProperty(vm, hashTable->classForThis, key, value, *this); // Leave the rest lazy on throw; the caller propagates. - RETURN_IF_EXCEPTION(scope, void()); + if (vm.exceptionForInspection()) [[unlikely]] + return; } } } diff --git a/Source/JavaScriptCore/runtime/Lookup.cpp b/Source/JavaScriptCore/runtime/Lookup.cpp index fd4b6e3273316..ca6263ce92782 100644 --- a/Source/JavaScriptCore/runtime/Lookup.cpp +++ b/Source/JavaScriptCore/runtime/Lookup.cpp @@ -59,7 +59,6 @@ bool setUpStaticFunctionSlot(VM& vm, const ClassInfo* classInfo, const HashTable if (thisObject->staticPropertiesReified()) return false; - auto scope = DECLARE_THROW_SCOPE(vm); { // A PropertyCallback builder can enter JS; defer termination (like // LazyProperty::callFunc) so it can't return with one pending. @@ -68,8 +67,11 @@ bool setUpStaticFunctionSlot(VM& vm, const ClassInfo* classInfo, const HashTable } // The builder may still throw a non-termination exception; report the // slot as not found so JSValue::get / getOwnPropertyDescriptor's - // EXCEPTION_ASSERT(!scope.exception() || !result) holds. - RETURN_IF_EXCEPTION(scope, false); + // EXCEPTION_ASSERT(!scope.exception() || !result) holds. No ThrowScope + // here: a ThrowScope would simulate a throw on every first static-table + // lookup, and callers of getOwnPropertySlot don't check for one. + if (vm.exceptionForInspection()) [[unlikely]] + return false; offset = thisObject->getDirectOffset(vm, propertyName, attributes); if (!isValidOffset(offset)) { From 38ddd579345f1c6a5a773993f0030047418bba1d Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Mon, 13 Jul 2026 22:05:04 +0000 Subject: [PATCH 3/4] JSC: tolerate pending TerminationException in property-lookup assertions DeferTermination throws the TerminationException from its destructor, after a property-slot lookup has already succeeded. JSObject::get already tolerates this (hasPendingTerminationException in its assertion); three sibling sites did not and abort assertion builds when terminate() races script execution: - JSValue::get (both overloads): same tolerance + RETURN_IF_EXCEPTION, hit via LLInt get_by_id during module evaluation - JSObject::getOwnPropertyDescriptor: same, hit via Object.defineProperty - JSModuleLoader::continueDynamicImport: assertNoException -> assertNoExceptionExceptTermination (rejectWithCaughtException cannot clear a termination, so it stays pending by design) No exception is ever cleared; callers' existing checks unwind as usual. Reproduced with a worker terminate() stress loop: 3/320 aborts before, 0/320 + 0/240 after. --- .../runtime/JSCJSValuePropertyInlines.h | 14 +++++++++++--- Source/JavaScriptCore/runtime/JSModuleLoader.cpp | 5 ++++- Source/JavaScriptCore/runtime/JSObject.cpp | 6 +++++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/Source/JavaScriptCore/runtime/JSCJSValuePropertyInlines.h b/Source/JavaScriptCore/runtime/JSCJSValuePropertyInlines.h index 4b87b28e5d440..43a8c372ae718 100644 --- a/Source/JavaScriptCore/runtime/JSCJSValuePropertyInlines.h +++ b/Source/JavaScriptCore/runtime/JSCJSValuePropertyInlines.h @@ -46,9 +46,14 @@ ALWAYS_INLINE JSValue JSValue::get(JSGlobalObject* globalObject, PropertyName pr ALWAYS_INLINE JSValue JSValue::get(JSGlobalObject* globalObject, PropertyName propertyName, PropertySlot& slot) const { - auto scope = DECLARE_THROW_SCOPE(getVM(globalObject)); + VM& vm = getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); bool hasSlot = getPropertySlot(globalObject, propertyName, slot); - EXCEPTION_ASSERT(!scope.exception() || !hasSlot); + // A DeferTermination scope unwinding inside the slot lookup can throw the + // TerminationException after the property was found; tolerate it like + // JSObject::get does and let the caller's exception check unwind. + EXCEPTION_ASSERT(!scope.exception() || vm.hasPendingTerminationException() || !hasSlot); + RETURN_IF_EXCEPTION(scope, jsUndefined()); if (!hasSlot) return jsUndefined(); RELEASE_AND_RETURN(scope, slot.getValue(globalObject, propertyName)); @@ -136,7 +141,10 @@ ALWAYS_INLINE JSValue JSValue::get(JSGlobalObject* globalObject, unsigned proper object = asObject(asCell()); bool hasSlot = object->getPropertySlot(globalObject, propertyName, slot); - EXCEPTION_ASSERT(!scope.exception() || !hasSlot); + // See JSValue::get(JSGlobalObject*, PropertyName, PropertySlot&) above: + // termination can be thrown after a successful lookup. + EXCEPTION_ASSERT(!scope.exception() || getVM(globalObject).hasPendingTerminationException() || !hasSlot); + RETURN_IF_EXCEPTION(scope, jsUndefined()); if (!hasSlot) return jsUndefined(); RELEASE_AND_RETURN(scope, slot.getValue(globalObject, propertyName)); diff --git a/Source/JavaScriptCore/runtime/JSModuleLoader.cpp b/Source/JavaScriptCore/runtime/JSModuleLoader.cpp index 4ebd937f1f9bb..5a5919525f06b 100644 --- a/Source/JavaScriptCore/runtime/JSModuleLoader.cpp +++ b/Source/JavaScriptCore/runtime/JSModuleLoader.cpp @@ -1023,7 +1023,10 @@ void JSModuleLoader::continueDynamicImport(JSGlobalObject* globalObject, ModuleL // 1.a. Perform ! Call(promiseCapability.[[Reject]], undefined, « moduleCompletion.[[Value]] »). promise->reject(vm, (*exception)->value()); // 1.b. Return UNUSED. - scope.assertNoException(); + // The abrupt completion may be a resolution failure caused by the + // TerminationException, which rejectWithCaughtException cannot clear; + // it stays pending in the VM and the caller's exception check unwinds. + scope.assertNoExceptionExceptTermination(); return; } // 2. Let module be moduleCompletion.[[Value]]. diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp index 0281bd75de9cf..5dc46a6141ec1 100644 --- a/Source/JavaScriptCore/runtime/JSObject.cpp +++ b/Source/JavaScriptCore/runtime/JSObject.cpp @@ -3941,7 +3941,11 @@ bool JSObject::getOwnPropertyDescriptor(JSGlobalObject* globalObject, PropertyNa PropertySlot slot(this, PropertySlot::InternalMethodType::GetOwnProperty); bool result = methodTable()->getOwnPropertySlot(this, globalObject, propertyName, slot); - EXCEPTION_ASSERT_UNUSED(scope, !scope.exception() || !result); + // A DeferTermination scope unwinding inside the slot lookup can throw the + // TerminationException after the property was found; tolerate it like + // JSObject::get does and let the caller's exception check unwind. + EXCEPTION_ASSERT(!scope.exception() || vm.hasPendingTerminationException() || !result); + RETURN_IF_EXCEPTION(scope, false); if (!result) return false; From 5113fb40768cd01efdeace1137af58fab57e9a8d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 18 Jul 2026 17:15:27 +0000 Subject: [PATCH 4/4] JSModuleLoader: don't treat a TerminationException from resolve() as a resolution failure When a worker is terminated while hostLoadImportedModule is inside resolve(), the NeedTermination trap surfaces as a TerminationException. Returning early lets the caller's RETURN_IF_EXCEPTION unwind instead of caching the termination string in m_resolutionFailures and entering FinishLoadingImportedModule with the termination still pending on the VM. --- Source/JavaScriptCore/runtime/JSModuleLoader.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Source/JavaScriptCore/runtime/JSModuleLoader.cpp b/Source/JavaScriptCore/runtime/JSModuleLoader.cpp index 5a5919525f06b..44acae7883546 100644 --- a/Source/JavaScriptCore/runtime/JSModuleLoader.cpp +++ b/Source/JavaScriptCore/runtime/JSModuleLoader.cpp @@ -649,6 +649,11 @@ JSPromise* JSModuleLoader::hostLoadImportedModule(JSGlobalObject* globalObject, resolved = resolve(globalObject, specifier, referrerKey, scriptFetcher, useImportMap); // 9. If the previous step threw an exception, then: if (Exception* resolutionError = scope.exception()) { + // A TerminationException is not a resolution failure; bail without + // caching it or entering FinishLoadingImportedModule with it still + // pending on the VM. + if (vm.isTerminationException(resolutionError)) [[unlikely]] + return nullptr; attachErrorInfo(globalObject, resolutionError, nullptr, specifier, moduleRequest.type(), ModuleFailure::Kind::Instantiation); // Cache the resolution error so subsequent calls for the same specifier return the same error object. JSValue errorValue = resolutionError->value();