Skip to content

Back bun_jsc::Strong with JSC's StrongSet and remove StrongRootBlock #37842

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Back bun_jsc::Strong with JSC's StrongSet and remove StrongRootBlock #37842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions src/jsc/JSRef.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ use crate::{JSGlobalObject, JSValue, Strong};
/// See ServerWebSocket, UDPSocket, MySQLConnection, and ValkeyClient for examples.
///
/// `JsRef` is `!Send + !Sync` (transitively via `JSValue` and `Strong`): the
/// `StrongRootBlock` slot backing `Strong` hangs off the per-VM JSVMClientData
/// and must be dropped on the JS thread.
/// slot backing `Strong` lives in the VM's `JSC::StrongSet` and must be
/// dropped on the JS thread.
pub enum JsRef {
Weak(JSValue),
Strong(Strong),
Expand Down Expand Up @@ -149,7 +149,7 @@ impl JsRef {
match self {
JsRef::Weak(_) => {}
JsRef::Strong(_) => {
// `Strong`'s `Drop` releases the block slot when `*self` is
// `Strong`'s `Drop` releases the slot when `*self` is
// overwritten below, so no explicit deinit is needed.
}
JsRef::Finalized => {
Expand Down Expand Up @@ -220,7 +220,7 @@ impl JsRef {

pub fn finalize(&mut self) {
// Overwriting `*self` drops the prior variant (releasing the `Strong`
// block slot via its `Drop`), so no explicit deinit step is needed.
// slot via its `Drop`), so no explicit deinit step is needed.
// External `jsref.deinit()` callers become `*jsref = JsRef::empty()`.
*self = JsRef::Finalized;
}
Expand Down
82 changes: 41 additions & 41 deletions src/jsc/Strong.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use crate::{JSGlobalObject, JSValue};
pub struct Strong {
handle: NonNull<Impl>,
// NonNull<T> is already !Send + !Sync, matching the requirement that
// Strong must be dropped on the JS thread (the StrongRootBlock list hangs
// off the per-VM JSVMClientData).
// Strong must be dropped on the JS thread (the slot belongs to the VM's
// `JSC::StrongSet`, which is only touched under the JSLock).
}

impl Strong {
Expand All @@ -29,10 +29,12 @@ impl Strong {
result
}

/// Set a new value for the strong reference.
pub fn set(&mut self, global: &JSGlobalObject, new_value: JSValue) {
/// Set a new value for the strong reference. The slot already exists, so
/// `_global` is only taken to keep the signature interchangeable with
/// [`Optional::set`], which needs it to allocate one.
pub fn set(&mut self, _global: &JSGlobalObject, new_value: JSValue) {
debug_assert!(!new_value.is_empty());
Impl::set(self.handle, global, new_value);
Impl::set(self.handle, new_value);
}

/// Adopt an `Impl` handle allocated externally (e.g. by C++ bindgen glue),
Expand Down Expand Up @@ -147,7 +149,7 @@ impl Optional {
self.handle = Some(Impl::init(global, value));
return;
};
Impl::set(r, global, value);
Impl::set(r, value);
}
}

Expand All @@ -161,19 +163,18 @@ impl Drop for Optional {
}

bun_opaque::opaque_ffi! {
/// Opaque FFI handle to a `Bun::StrongRef` (one occupied slot in a
/// `Bun::StrongRootBlock`); see StrongRef.cpp.
/// Opaque FFI handle: points at the `JSC::JSValue` slot that
/// `Bun__StrongRef__new` allocated in the VM's `JSC::StrongSet` (the same
/// storage `JSC::Strong<>` uses); see StrongRef.cpp.
pub struct Impl;
}

impl Impl {
// Low 48 bits of the handle point at the `WriteBarrier<Unknown>` slot (a
// `JSC::JSValue`); see `encodeStrongRef` in StrongRef.cpp.
const SLOT_MASK: usize = (1usize << 48) - 1;

/// The slot holds exactly the `EncodedJSValue` bits (`JSC::JSValue` is one
/// 64-bit word), which is what [`JSValue`] is on this side.
#[inline(always)]
fn slot_ptr(this: NonNull<Impl>) -> *mut crate::DecodedJSValue {
(this.as_ptr() as usize & Self::SLOT_MASK) as *mut crate::DecodedJSValue
fn slot(this: NonNull<Impl>) -> NonNull<JSValue> {
this.cast()
}

pub(crate) fn init(global: &JSGlobalObject, value: JSValue) -> NonNull<Impl> {
Expand All @@ -183,22 +184,23 @@ impl Impl {

#[inline(always)]
pub fn get(this: NonNull<Impl>) -> JSValue {
// SAFETY: StrongRef.cpp guarantees the low 48 bits address a live
// `JSC::JSValue`-sized slot for the lifetime of the handle;
// `DecodedJSValue` is its `#[repr(C)]` ABI mirror.
unsafe { (*Self::slot_ptr(this)).encode() }
// SAFETY: the slot stays allocated until `destroy`, and only the JS
// thread touches it (`Strong` is !Send), so this read cannot race.
unsafe { Self::slot(this).read() }
}

pub fn set(this: NonNull<Impl>, global: &JSGlobalObject, value: JSValue) {
crate::mark_binding!();
Bun__StrongRef__set(Impl::opaque_ref(this.as_ptr()), global, value);
/// Plain store, like `JSC::Strong::set()`: the GC's strong-handle
/// constraint scans every slot of the set, so there is no barrier to run.
#[inline(always)]
pub fn set(this: NonNull<Impl>, value: JSValue) {
// SAFETY: as in `get`; the GC reads slots only with the mutator
// stopped, so a plain store is the same store `JSC::Strong` makes.
unsafe { Self::slot(this).write(value) };
}

#[inline(always)]
pub(crate) fn clear(this: NonNull<Impl>) {
// SAFETY: same slot-pointer invariant as `get`; clearing holds no
// barrier (WriteBarrier<Unknown>::clear() just stores encoded 0).
unsafe { Self::slot_ptr(this).cast::<i64>().write(0) };
Self::set(this, JSValue::ZERO);
}

/// SAFETY: `this` must be a valid handle from `init`; consumed here (do not reuse).
Expand All @@ -211,13 +213,13 @@ impl Impl {
this.as_ptr(),
);
}
// destructOnExit / WebWorker__teardownJSCVM unprotect the global and
// run a final full GC whose sweep-time finalizers (and
// deinit_runtime_state after ~VM) can drop `Strong`s; past that point
// the encoded StrongRootBlock cell may be unmarked or the heap freed.
// `is_shutting_down` is set before either path reaches the final
// collection, and the handle carries no allocation, so skipping the
// slot release is the whole of teardown. The Rust VM TLS outlives ~VM.
// The slot belongs to the VM's StrongSet, which ~VM frees (teardown
// phase C, see `VirtualMachine::teardown`), while runtime state that
// still owns `Strong`s is torn down after that (`deinit_runtime_state`,
// phase E). `is_shutting_down` is set before teardown starts, and from
// then on the slot simply dies with the set: the final collection is
// followed by ~VM's lastChanceToFinalize, so nothing it roots outlives
// the VM. The Rust VM TLS outlives ~VM.
match crate::virtual_machine::VirtualMachine::get_or_null() {
Some(vm) => {
// SAFETY: `get_or_null` returns the thread-local pointer set by
Expand All @@ -230,9 +232,9 @@ impl Impl {
// Off the JS thread. `Strong` is !Send, so reaching here means
// an `unsafe impl Send` wrapper carried one by value and
// dropped it on a pool thread; the slot (and its rooted value)
// leak until that VM's teardown. The block bitset/count are
// not thread-safe to touch here. Flag in debug so the owning
// wrapper can queue the drop back to the JS thread instead.
// leak until that VM's teardown. StrongSet::deallocate requires
// the JSLock, so it cannot be called from here. Flag in debug so
// the owning wrapper can queue the drop back to the JS thread.
debug_assert!(
false,
"bun_jsc::Strong dropped off the JS thread; slot leaks"
Expand All @@ -241,18 +243,16 @@ impl Impl {
}
}
// SAFETY: caller contract guarantees `this` is a live handle from
// `Bun__StrongRef__new`; C++ releases the block slot it encodes.
// `Bun__StrongRef__new`; C++ returns the slot to its StrongSet.
unsafe { Bun__StrongRef__delete(this.as_ptr()) };
}
}

// `Impl` and `JSGlobalObject` are opaque `UnsafeCell`-backed ZST handles, so
// `&Impl`/`&JSGlobalObject` are ABI-identical to non-null `*const T` and C++
// mutating through them (block slot write) is interior mutation invisible to
// Rust. The handle's low 48 bits point at the slot and the top 16 hold the
// index (no heap allocation); `delete` releases the slot and so stays `unsafe fn`.
// `JSGlobalObject` is an opaque `UnsafeCell`-backed ZST handle, so
// `&JSGlobalObject` is ABI-identical to a non-null `*const T`. `new` hands out
// a slot inside the VM's StrongSet (no heap allocation of its own); `delete`
// returns it and so stays `unsafe fn`.
unsafe extern "C" {
fn Bun__StrongRef__delete(this: *mut Impl);
safe fn Bun__StrongRef__new(global: &JSGlobalObject, value: JSValue) -> *mut Impl;
safe fn Bun__StrongRef__set(this: &Impl, global: &JSGlobalObject, value: JSValue);
}
2 changes: 1 addition & 1 deletion src/jsc/bindings/Bindgen/ExternTraits.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ template<> struct ExternTraits<JSC::JSValue> {
};

template<> struct ExternTraits<Bun::StrongRef> {
using ExternType = Bun::StrongRefImpl*;
using ExternType = JSC::JSValue*;

static ExternType convertToExtern(Bun::StrongRef&& cppValue)
{
Expand Down
46 changes: 0 additions & 46 deletions src/jsc/bindings/BunClientData.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <JavaScriptCore/HeapInlines.h>
#include <JavaScriptCore/IsoHeapCellType.h>
#include <JavaScriptCore/JSDestructibleObjectHeapCellType.h>
#include <JavaScriptCore/SimpleMarkingConstraint.h>
#include <JavaScriptCore/SubspaceInlines.h>
#include <JavaScriptCore/VM.h>
#include <wtf/MainThread.h>
Expand All @@ -26,7 +25,6 @@
#include "../../runtime/bake/BakeGlobalObject.h"
#include "napi_handle_scope.h"
#include "NativePromiseContext.h"
#include "StrongRootBlock.h"

namespace WebCore {
using namespace JSC;
Expand Down Expand Up @@ -125,50 +123,6 @@ void JSVMClientData::create(VM* vm, void* bunVM, bool isWorkerVM)
clientData->m_normalWorld = DOMWrapperWorld::create(*vm, DOMWrapperWorld::Type::Normal);

vm->heap.addMarkingConstraint(makeUnique<WebCore::DOMGCOutputConstraint>(*vm, clientData->heapData()));

// Root the StrongRootBlock list from the VM instead of any one global.
//
// JSC's collector alternates Fixpoint (world stopped: mutator suspended via
// finishChangingPhase / stopTheMutator; see worldShouldBeSuspended in
// CollectorPhase.cpp) with Concurrent (mutator running) phases. The
// constraint set is solved only during Fixpoint, so the lambda below never
// races the mutator's writes to m_strongRootBlockHead/Free.
//
// `GreyedByExecution` puts this in the "root" bucket
// (MarkingConstraintSet::didStartMarking tags it as an unexecuted root for
// iteration 1 and re-evaluates it on every return to Fixpoint after a
// mutator resumption), mirroring the "Sh" strong-handle constraint in
// Heap::addCoreConstraints: anything the mutator linked onto the list while
// running is picked up on the next Fixpoint.
//
// Eden vs. full: `appendUnbarriered` early-returns when `isMarked()`; eden
// keeps the previous full GC's `m_markingVersion`
// (MarkedSpace::beginMarking), so an old-gen head reads as marked and its
// `visitChildren` does not run on eden. Slots written into such a block
// since the last GC already fired `WriteBarrier::set` on the block cell
// (Heap::writeBarrierSlowPath -> addToRememberedSet -> m_mutatorMarkStack),
// and Fixpoint drains that stack to visit the dirtied block. A full GC
// bumps `m_markingVersion`, every cell reads unmarked, and the constraint
// seeds the whole `m_next` chain. Net: this body is O(1) per collection.
//
// `Concurrent` here means the constraint may run on a GC helper thread via
// MarkingConstraintSolver::runExecutionThread (MarkingConstraintSet still
// runs each constraint once per fixpoint iteration, gated by `m_executed`);
// it does not mean concurrent with the mutator. The lambda only reads three
// pointers and appends to the per-thread visitor, so helper-thread
// execution is safe. `clientData` outlives the Heap
// (~VM -> lastChanceToFinalize -> delete clientData), so the capture stays
// valid for every collection.
vm->heap.addMarkingConstraint(makeUnique<JSC::SimpleMarkingConstraint>(
"Srb", "Bun StrongRootBlocks",
MAKE_MARKING_CONSTRAINT_EXECUTOR_PAIR(([clientData](auto& visitor) {
JSC::SetRootMarkReasonScope rootScope(visitor, JSC::RootMarkReason::StrongHandles);
visitor.appendUnbarriered(clientData->m_strongRootBlockHead);
visitor.appendUnbarriered(clientData->m_strongRootBlockFree);
visitor.appendUnbarriered(clientData->m_strongRootBlockStructure);
})),
JSC::ConstraintVolatility::GreyedByExecution));

vm->m_typedArrayController = adoptRef(new WebCoreTypedArrayController(true));
clientData->builtinFunctions().exportNames();
}
Expand Down
16 changes: 0 additions & 16 deletions src/jsc/bindings/BunClientData.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,6 @@ namespace Zig {
class GlobalObject;
}

namespace Bun {
class StrongRootBlock;
}

namespace WebCore {
using namespace JSC;
using namespace Zig;
Expand Down Expand Up @@ -168,18 +164,6 @@ class JSVMClientData : public JSC::VM::ClientData {
ALWAYS_INLINE bool scriptAllowed() const { return Bun__VmHandle__scriptAllowedInline(vmHandleState); }
Bun::JSCTaskScheduler deferredWorkTimer;

// Linked list of StrongRootBlock cells backing bun_jsc::Strong handles
// (see StrongRootBlock.h). Raw pointers into the GC heap: they are rooted
// by a SimpleMarkingConstraint registered in JSVMClientData::create(), so
// no HandleSet node is needed and no GlobalObject owns them (ShadowRealm /
// node:vm / `bun test --isolate` globals share one list).
Bun::StrongRootBlock* m_strongRootBlockHead { nullptr };
Bun::StrongRootBlock* m_strongRootBlockFree { nullptr };
// Last block acquire() found room in; always on the active list (cleared by
// release() if unlinked), so it is already rooted via m_strongRootBlockHead.
Bun::StrongRootBlock* m_strongRootBlockCursor { nullptr };
JSC::Structure* m_strongRootBlockStructure { nullptr };

// Backing storage for Bun::IsolatedModuleCache (see IsolatedModuleCache.h).
// All access should go through that class. Stored as the JSC base type to
// avoid pulling ZigSourceProvider.h into this header; the cache class
Expand Down
82 changes: 9 additions & 73 deletions src/jsc/bindings/StrongRef.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,81 +1,17 @@
#include "root.h"
#include "StrongRef.h"
#include "StrongRootBlock.h"
#include "BunClientData.h"
#include <JavaScriptCore/StrongSet.h>

using Bun::StrongRefImpl;
using Bun::StrongRootBlock;

// Hot-path clientData lookup without the `downcast<JSVMClientData>`
// RELEASE_ASSERT virtual call; vm.clientData is unconditionally a
// JSVMClientData* in bun (set in JSVMClientData::create).
static ALWAYS_INLINE WebCore::JSVMClientData* clientDataFast(JSC::VM& vm)
{
ASSERT(WebCore::clientData(vm));
return static_cast<WebCore::JSVMClientData*>(vm.clientData);
}

// Handle layout for bun_jsc::Strong. The Rust side treats the return of
// Bun__StrongRef__new as an opaque non-null pointer; there is no heap
// allocation. The low 48 bits point at the WriteBarrier<Unknown> slot inside a
// StrongRootBlock (so Rust can read/clear the value with a direct pointer
// load, matching the old HandleSlot fast path), and the top 16 bits hold the
// slot index so `block` can be recovered as `slot - index*8 - slotsOffset()`.
// JSC cells live in the low 48 bits of the address space (the same invariant
// JSValue NaN-boxing and StructureID encoding rely on), and the slot index is
// bounded by StrongRootBlock::capacity.
static constexpr unsigned kStrongRefIndexShift = 48;
static constexpr uintptr_t kStrongRefSlotMask = (static_cast<uintptr_t>(1) << kStrongRefIndexShift) - 1;
static_assert(sizeof(uintptr_t) == 8, "StrongRef handle encoding requires 64-bit pointers");
static_assert(StrongRootBlock::capacity < (1u << (64 - kStrongRefIndexShift)), "slot index must fit in the top 16 bits");
static_assert(sizeof(StrongRootBlock::Slot) == sizeof(JSC::JSValue), "Rust Impl::get reads the slot as a JSValue");

static ALWAYS_INLINE StrongRefImpl* encodeStrongRef(StrongRootBlock* block, unsigned index)
{
uintptr_t slot = reinterpret_cast<uintptr_t>(block->slotAt(index));
ASSERT(!(slot & ~kStrongRefSlotMask));
return reinterpret_cast<StrongRefImpl*>(slot | (static_cast<uintptr_t>(index) << kStrongRefIndexShift));
}

static ALWAYS_INLINE unsigned decodeStrongRefIndex(StrongRefImpl* ref)
{
return static_cast<unsigned>(reinterpret_cast<uintptr_t>(ref) >> kStrongRefIndexShift);
}

static ALWAYS_INLINE StrongRootBlock* decodeStrongRefBlock(StrongRefImpl* ref)
{
uintptr_t slot = reinterpret_cast<uintptr_t>(ref) & kStrongRefSlotMask;
return reinterpret_cast<StrongRootBlock*>(slot - static_cast<uintptr_t>(decodeStrongRefIndex(ref)) * sizeof(StrongRootBlock::Slot) - StrongRootBlock::slotsOffset());
}

extern "C" StrongRefImpl* Bun__StrongRef__new(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue encodedValue)
{
auto& vm = JSC::getVM(globalObject);
unsigned index;
auto* block = StrongRootBlock::acquire(clientDataFast(vm), vm, index);
block->set(vm, index, JSC::JSValue::decode(encodedValue));
return encodeStrongRef(block, index);
}

extern "C" void Bun__StrongRef__set(StrongRefImpl* _Nonnull ref, JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue encodedValue)
extern "C" JSC::JSValue* Bun__StrongRef__new(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue encodedValue)
{
decodeStrongRefBlock(ref)->write(JSC::getVM(globalObject), decodeStrongRefIndex(ref), JSC::JSValue::decode(encodedValue));
JSC::HandleSlot slot = JSC::getVM(globalObject).heap.strongSet()->allocate();
// Plain store, like JSC::Strong::set(): the "Sh" marking constraint scans
// every StrongSet slot, so there is no barrier to run.
*slot = JSC::JSValue::decode(encodedValue);
return slot;
}

// The Rust caller (Strong.rs Impl::destroy) skips this call once
// VirtualMachine.is_shutting_down is true, so the block cell is guaranteed
// live here: destructOnExit / WebWorker__teardownJSCVM set that flag before
// their final collectNow, which is the only point the block can go dead while
// handles still exist.
extern "C" void Bun__StrongRef__delete(StrongRefImpl* _Nonnull ref)
extern "C" void Bun__StrongRef__delete(JSC::JSValue* _Nonnull slot)
{
auto* block = decodeStrongRefBlock(ref);
auto& vm = block->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
// the cursor sits at a full head).
clientData->m_strongRootBlockCursor = block;
if (block->clear(decodeStrongRefIndex(ref))) [[unlikely]]
StrongRootBlock::release(clientData, vm, block);
JSC::StrongSet::deallocate(slot);
}
Loading