diff --git a/scripts/build/deps/webkit.ts b/scripts/build/deps/webkit.ts index 87776fa6480d..a8407b517d30 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 = "e6e37cda216c0292ae68c30c84a9dc8601d0fba5"; +export const WEBKIT_VERSION = "autobuild-preview-pr-387-f6049b84"; /** * WebKit (JavaScriptCore) — the JS engine. diff --git a/src/jsc/bindings/JSCTestingHelpers.cpp b/src/jsc/bindings/JSCTestingHelpers.cpp index caca7766c736..c1bbecbc9f2d 100644 --- a/src/jsc/bindings/JSCTestingHelpers.cpp +++ b/src/jsc/bindings/JSCTestingHelpers.cpp @@ -6,6 +6,13 @@ #include #include "ZigGlobalObject.h" +#if ASSERT_ENABLED +#include "StrongRef.h" +#include +#include +#include +#endif + #if OS(WINDOWS) #include #include @@ -64,6 +71,43 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionStartOfFixedExecutableMemoryPool, } #endif +#if ASSERT_ENABLED +// Test hook: mutates a strong handle owned by this VM from a spawned thread +// without the API lock. The debug assertions in JSC::HandleSet and +// Bun__StrongRef__* must abort before the mutation lands; +// strong-handle-thread-guard.test.ts asserts on that crash. +JSC_DEFINE_HOST_FUNCTION(jsFunctionCrossThreadStrongHandleMutation, + (JSGlobalObject * globalObject, CallFrame* callframe)) +{ + auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + WTF::String kind = callframe->argument(0).toWTFString(globalObject); + RETURN_IF_EXCEPTION(scope, {}); + + if (kind == "strong"_s) { + // The #30185 shape: a by-value Strong capture destroyed off-thread. + JSC::Strong strong(vm, JSC::constructEmptyObject(globalObject)); + Ref thread = Thread::create("StrongHandleGuardTest"_s, [strong]() mutable { + strong.clear(); + }); + thread->waitForCompletion(); + return JSValue::encode(jsUndefined()); + } + + if (kind == "strongRef"_s) { + auto* ref = Bun__StrongRef__new(globalObject, JSValue::encode(JSC::constructEmptyObject(globalObject))); + Ref thread = Thread::create("StrongHandleGuardTest"_s, [ref]() { + Bun__StrongRef__delete(ref); + }); + thread->waitForCompletion(); + return JSValue::encode(jsUndefined()); + } + + throwTypeError(globalObject, scope, "Expected \"strong\" or \"strongRef\""_s); + return {}; +} +#endif + JSC::JSValue createJSCTestingHelpers(Zig::GlobalObject* globalObject) { auto& vm = JSC::getVM(globalObject); @@ -87,6 +131,13 @@ JSC::JSValue createJSCTestingHelpers(Zig::GlobalObject* globalObject) JSC::PropertyAttribute::DontDelete | 0); #endif +#if ASSERT_ENABLED + object->putDirectNativeFunction( + vm, globalObject, JSC::Identifier::fromString(vm, "crossThreadStrongHandleMutation"_s), 1, + jsFunctionCrossThreadStrongHandleMutation, ImplementationVisibility::Public, NoIntrinsic, + JSC::PropertyAttribute::DontDelete | 0); +#endif + return object; } diff --git a/src/jsc/bindings/StrongRef.cpp b/src/jsc/bindings/StrongRef.cpp index 5744b2593472..f83d3b65c66d 100644 --- a/src/jsc/bindings/StrongRef.cpp +++ b/src/jsc/bindings/StrongRef.cpp @@ -48,9 +48,16 @@ static ALWAYS_INLINE StrongRootBlock* decodeStrongRefBlock(StrongRefImpl* ref) return reinterpret_cast(slot - static_cast(decodeStrongRefIndex(ref)) * sizeof(StrongRootBlock::Slot) - StrongRootBlock::slotsOffset()); } +// The "Srb" marking constraint (BunClientData.cpp) scans the StrongRootBlock +// list without synchronization; holding the owner VM's API lock is what +// orders these mutations with that scan. Mirrors JSC::HandleSet's assertion. +#define ASSERT_STRONG_REF_MUTATION_ALLOWED(vm) \ + ASSERT_WITH_MESSAGE((vm).currentThreadIsHoldingAPILock(), "Bun::StrongRef handles may only be created, written, or destroyed while holding their VM's API lock") + extern "C" StrongRefImpl* Bun__StrongRef__new(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue encodedValue) { auto& vm = JSC::getVM(globalObject); + ASSERT_STRONG_REF_MUTATION_ALLOWED(vm); unsigned index; auto* block = StrongRootBlock::acquire(clientDataFast(vm), vm, index); block->set(vm, index, JSC::JSValue::decode(encodedValue)); @@ -59,7 +66,9 @@ extern "C" StrongRefImpl* Bun__StrongRef__new(JSC::JSGlobalObject* globalObject, extern "C" void Bun__StrongRef__set(StrongRefImpl* _Nonnull ref, JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue encodedValue) { - decodeStrongRefBlock(ref)->write(JSC::getVM(globalObject), decodeStrongRefIndex(ref), JSC::JSValue::decode(encodedValue)); + auto* block = decodeStrongRefBlock(ref); + ASSERT_STRONG_REF_MUTATION_ALLOWED(block->vm()); + block->write(JSC::getVM(globalObject), decodeStrongRefIndex(ref), JSC::JSValue::decode(encodedValue)); } // The Rust caller (Strong.rs Impl::destroy) skips this call once @@ -71,6 +80,7 @@ extern "C" void Bun__StrongRef__delete(StrongRefImpl* _Nonnull ref) { auto* block = decodeStrongRefBlock(ref); auto& vm = block->vm(); + ASSERT_STRONG_REF_MUTATION_ALLOWED(vm); auto* clientData = clientDataFast(vm); // This block just freed a slot, so the next acquire() should try it first // (covers the FIFO pattern where the oldest-armed block gets room while diff --git a/test/js/bun/jsc/strong-handle-thread-guard-fixture.js b/test/js/bun/jsc/strong-handle-thread-guard-fixture.js new file mode 100644 index 000000000000..fd52385268bb --- /dev/null +++ b/test/js/bun/jsc/strong-handle-thread-guard-fixture.js @@ -0,0 +1,7 @@ +import { jscInternals } from "bun:internal-for-testing"; + +// Violates the strong-handle thread-affinity contract on purpose; in a debug +// build the assertion must abort the process inside this call. +jscInternals.crossThreadStrongHandleMutation(process.argv[2]); + +console.log("survived"); diff --git a/test/js/bun/jsc/strong-handle-thread-guard.test.ts b/test/js/bun/jsc/strong-handle-thread-guard.test.ts new file mode 100644 index 000000000000..a683862c91b5 --- /dev/null +++ b/test/js/bun/jsc/strong-handle-thread-guard.test.ts @@ -0,0 +1,39 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, isDebug } from "harness"; +import path from "path"; + +// Each child deliberately mutates a strong handle owned by the main VM from a +// spawned thread that does not hold the VM's API lock. Debug builds carry +// assertions guarding exactly that (JSC::HandleSet::assertMayMutate for +// JSC::Strong, the Bun__StrongRef__* asserts for StrongRootBlock slots), so +// the child must die with the assertion's message before the mutation lands. +// +// This is the liveness check for those detectors: the previous guard for the +// #30185 cross-thread HandleSet race was a probabilistic crash workload that +// silently stopped detecting when GC scheduling changed (#35356, measured in +// #36952). If a WebKit bump or binding refactor drops the assertions, the +// child survives and this test fails instead of the coverage disappearing +// unnoticed. +// +// These crashes are intentional; keep them out of crash reporting so CI does +// not pin them on unrelated tests. +const noReportEnv = { ...bunEnv, BUN_CRASH_REPORT_URL: "", BUN_ENABLE_CRASH_REPORTING: "0" }; + +for (const [kind, message] of [ + ["strong", "Strong handles may only be created, written, or destroyed while holding their VM's API lock"], + ["strongRef", "Bun::StrongRef handles may only be created, written, or destroyed while holding their VM's API lock"], +] as const) { + test.if(isDebug)(`unlocked off-thread ${kind} mutation aborts with the guard's message`, async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), path.join(import.meta.dir, "strong-handle-thread-guard-fixture.js"), kind], + env: noReportEnv, + stdio: ["ignore", "pipe", "pipe"], + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toContain("ASSERTION FAILED"); + expect(stderr).toContain(message); + expect(stdout).not.toContain("survived"); + expect(exitCode).not.toBe(0); + }); +}