From f6049b84822b230eea008301e4b5e69a78169f90 Mon Sep 17 00:00:00 2001 From: robobun Date: Wed, 5 Aug 2026 10:33:34 +0000 Subject: [PATCH] HandleSet: assert the VM's API lock is held when mutating handles HandleSet::allocate, deallocate, and writeBarrier mutate m_strongList, which the GC's strong-handles marking constraint scans. The API lock is what orders those mutations with the scan, so mutating from a thread that does not hold it is a data race. Heap::protect/unprotect already assert exactly this for the protect set; this adds the same assertion to the Strong handle set. The scheduling-dependent crash this turns deterministic: a Strong captured by value in a cross-thread lambda, destroyed on the other thread, corrupts the owner VM's m_strongList (oven-sh/bun#30185). That class of bug previously surfaced only as a rare segfault or livelock in the marking constraint; with this assertion any debug build reports the violating call site on the first mutation. Debug-only: assertMayMutate() compiles to an empty inline in release builds. --- Source/JavaScriptCore/heap/HandleSet.cpp | 14 ++++++++++++++ Source/JavaScriptCore/heap/HandleSet.h | 9 +++++++++ 2 files changed, 23 insertions(+) diff --git a/Source/JavaScriptCore/heap/HandleSet.cpp b/Source/JavaScriptCore/heap/HandleSet.cpp index 70e35901cbdbc..383e009b67208 100644 --- a/Source/JavaScriptCore/heap/HandleSet.cpp +++ b/Source/JavaScriptCore/heap/HandleSet.cpp @@ -29,6 +29,7 @@ #include "HandleBlock.h" #include "HandleBlockInlines.h" #include "JSCJSValueInlines.h" +#include "VM.h" namespace JSC { @@ -93,4 +94,17 @@ bool HandleSet::isLiveNode(Node* node) } #endif // ENABLE(GC_VALIDATION) || ASSERT_ENABLED +#if ASSERT_ENABLED +void HandleSet::assertMayMutate() +{ + // Allocating, deallocating, or re-targeting a Strong handle mutates + // m_strongList, which the GC's strong-handles marking constraint scans. + // The VM's API lock is what orders those mutations with the scan, so a + // mutation from a thread that does not hold the lock is a data race. The + // classic way to get here is a Strong captured by value in a lambda that + // another thread destroys. + ASSERT_WITH_MESSAGE(m_vm.currentThreadIsHoldingAPILock(), "Strong handles may only be created, written, or destroyed while holding their VM's API lock"); +} +#endif // ASSERT_ENABLED + } // namespace JSC diff --git a/Source/JavaScriptCore/heap/HandleSet.h b/Source/JavaScriptCore/heap/HandleSet.h index 25819fc16540e..661e3cf99ea65 100644 --- a/Source/JavaScriptCore/heap/HandleSet.h +++ b/Source/JavaScriptCore/heap/HandleSet.h @@ -86,6 +86,12 @@ class HandleSet { JS_EXPORT_PRIVATE bool isLiveNode(Node*); #endif +#if ASSERT_ENABLED + JS_EXPORT_PRIVATE void assertMayMutate(); +#else + void assertMayMutate() { } +#endif + VM& m_vm; DoublyLinkedList m_blockList; @@ -106,6 +112,7 @@ inline VM& HandleSet::vm() inline HandleSlot HandleSet::allocate() { + assertMayMutate(); if (m_freeList.isEmpty()) grow(); @@ -116,6 +123,7 @@ inline HandleSlot HandleSet::allocate() inline void HandleSet::deallocate(HandleSlot handle) { + assertMayMutate(); HandleSet::Node* node = HandleNode::toHandleNode(handle); if (node->isOnList()) NodeList::remove(node); @@ -147,6 +155,7 @@ template void HandleSet::forEachStrongHandle(const Functor& fu template inline void HandleSet::writeBarrier(HandleSlot slot, JSValue value) { + assertMayMutate(); bool valueIsNonEmptyCell = value && (isCellOnly || value.isCell()); bool slotIsNonEmptyCell = *slot && (isCellOnly || slot->isCell()); if (valueIsNonEmptyCell == slotIsNonEmptyCell)