Bump WebKit: assert the VM's API lock is held on HandleSet mutation

scripts/build/deps/webkit.ts

@@ -3,7 +3,7 @@
  * for local mode. Override via `--webkit-version=<hash>` 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.

src/jsc/bindings/JSCTestingHelpers.cpp

@@ -6,6 +6,13 @@
 #include <JavaScriptCore/JSString.h>
 #include "ZigGlobalObject.h"
 
+#if ASSERT_ENABLED
+#include "StrongRef.h"
+#include <JavaScriptCore/Strong.h>
+#include <JavaScriptCore/StrongInlines.h>
+#include <wtf/Threading.h>
+#endif
+
 #if OS(WINDOWS)
 #include <JavaScriptCore/ExecutableAllocator.h>
 #include <JavaScriptCore/JSBigInt.h>
@@ -64,6 +71,45 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionStartOfFixedExecutableMemoryPool,
 }
 #endif
 
+#if ASSERT_ENABLED
+// Deliberately mutates a strong handle owned by this VM from a spawned thread
+// that does not hold the API lock. The debug-only thread-affinity assertions
+// (JSC::HandleSet::assertMayMutate for JSC::Strong, the Bun__StrongRef__*
+// asserts for StrongRootBlock slots) must abort the process before the
+// mutation lands; the test asserting on that crash is what keeps the
+// detectors from rotting silently. Only compiled when the assertions are.
+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<JSC::JSObject> strong(vm, JSC::constructEmptyObject(globalObject));
+        Ref<Thread> 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 = 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 +133,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;
 }
 

src/jsc/bindings/StrongRef.cpp

@@ -48,9 +48,19 @@ static ALWAYS_INLINE StrongRootBlock* decodeStrongRefBlock(StrongRefImpl* ref)
     return reinterpret_cast<StrongRootBlock*>(slot - static_cast<uintptr_t>(decodeStrongRefIndex(ref)) * sizeof(StrongRootBlock::Slot) - StrongRootBlock::slotsOffset());
 }
 
+// The StrongRootBlock list hangs off JSVMClientData with no synchronization:
+// the "Srb" marking constraint (BunClientData.cpp) scans it assuming only the
+// mutator, holding the VM's API lock, ever writes it. These asserts are the
+// StrongRootBlock mirror of JSC::HandleSet::assertMayMutate, and check the
+// OWNER VM (the block's), so a thread that has some other VM's lock still
+// trips them.
+#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 +69,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 +83,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

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);
+  });
+}