Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/build/deps/webkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
51 changes: 51 additions & 0 deletions src/jsc/bindings/JSCTestingHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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.
Comment thread
robobun marked this conversation as resolved.
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);
Expand All @@ -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;
}

Expand Down
12 changes: 11 additions & 1 deletion src/jsc/bindings/StrongRef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,16 @@ static ALWAYS_INLINE StrongRootBlock* decodeStrongRefBlock(StrongRefImpl* ref)
return reinterpret_cast<StrongRootBlock*>(slot - static_cast<uintptr_t>(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.
Comment thread
robobun marked this conversation as resolved.
#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));
Expand All @@ -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
Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions test/js/bun/jsc/strong-handle-thread-guard-fixture.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 39 additions & 0 deletions test/js/bun/jsc/strong-handle-thread-guard.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
}