From b3627082db542d36f375c8ffbfa9115c677e8770 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 6 Oct 2026 05:46:47 -0700 Subject: [PATCH 1/2] fix(jsc): wake passive collectors for synchronous GC A concurrent collector can already be parked for shared helpers when its mutator starts waiting for collection. Notify under the marking mutex and recheck the waiting bit before sleeping so the collector can drain its own work without relying on another heap to release helpers. Add a native regression that observes the actual collector and mutator parking addresses, holds shared helpers, and requires completion before helper release while preserving rooted objects. Include it in the existing native qualification gate. --- .../engine-gc-progress-probe.cpp | 251 ++++++++++++++++++ .github/openclaw/qualification/verify-sync.py | 5 + .github/openclaw/qualify-sync.sh | 6 + .github/openclaw/release-notes.md | 4 +- Source/JavaScriptCore/heap/Heap.cpp | 7 + Source/JavaScriptCore/heap/SlotVisitor.cpp | 7 + 6 files changed, 279 insertions(+), 1 deletion(-) create mode 100644 .github/openclaw/qualification/engine-gc-progress-probe.cpp diff --git a/.github/openclaw/qualification/engine-gc-progress-probe.cpp b/.github/openclaw/qualification/engine-gc-progress-probe.cpp new file mode 100644 index 0000000000000..6ff5a2655ae48 --- /dev/null +++ b/.github/openclaw/qualification/engine-gc-progress-probe.cpp @@ -0,0 +1,251 @@ +// Verify that a passive collector progresses when its mutator starts waiting. +// Observe existing engine state without production hooks or state mutation. +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + +// Explicit-instantiation arguments are exempt from member access checks. These +// probe-only accessors expose observation addresses, never mutate heap state. +template +struct ObserveMember { + friend typename Tag::Type observe(Tag) { return member; } +}; + +struct HeapCondition { + using Type = WTF::Condition JSC::Heap::*; + friend Type observe(HeapCondition); +}; +template struct ObserveMember; + +struct HeapWorldState { + using Type = WTF::Atomic JSC::Heap::*; + friend Type observe(HeapWorldState); +}; +template struct ObserveMember; + +struct ConditionWaitAddress { + using Type = WTF::Atomic WTF::Condition::*; + friend Type observe(ConditionWaitAddress); +}; +template struct ObserveMember; + +template +struct ObserveConstant { + friend unsigned observe(Tag) { return value; } +}; +struct MutatorWaitingBit { friend unsigned observe(MutatorWaitingBit); }; +template struct ObserveConstant; + +class HeldHelpers { +public: + void enter() + { + std::unique_lock lock(m_mutex); + ++m_entered; + m_changed.notify_all(); + m_changed.wait(lock, [&] { return m_released; }); + } + + bool waitUntilEntered(unsigned expected) + { + std::unique_lock lock(m_mutex); + return m_changed.wait_for(lock, std::chrono::seconds(10), [&] { return m_entered == expected; }); + } + + void release() + { + std::lock_guard lock(m_mutex); + m_released = true; + m_changed.notify_all(); + } + +private: + std::mutex m_mutex; + std::condition_variable m_changed; + unsigned m_entered { 0 }; + bool m_released { false }; +}; + +struct Parked { + bool collector { false }; + bool mutator { false }; + uintptr_t collectorThread { 0 }; + uintptr_t mutatorThread { 0 }; +}; + +Parked inspectParking(const void* conditionAddress, const void* worldAddress, uintptr_t expectedMutator) +{ + Parked result; + // The callback runs under ParkingLot locks: do not allocate, park, or log. + WTF::ParkingLot::forEach([&](uintptr_t thread, const void* address) { + if (address == conditionAddress) { + result.collector = true; + result.collectorThread = thread; + } + if (address == worldAddress && thread == expectedMutator) { + result.mutator = true; + result.mutatorThread = thread; + } + }); + return result; +} + +bool evaluate(JSGlobalContextRef context, const char* source, double* number = nullptr) +{ + JSStringRef script = JSStringCreateWithUTF8CString(source); + JSValueRef exception = nullptr; + JSValueRef value = JSEvaluateScript(context, script, nullptr, nullptr, 1, &exception); + JSStringRelease(script); + if (exception || !value) + return false; + if (number) + *number = JSValueToNumber(context, value, &exception); + return !exception; +} + +} // namespace + +int main() +{ + WTF::initializeMainThread(); + JSC::initialize(); + // Zero pause and utilization=1 force concurrent scheduling without an RNG + // outcome or allocation-time threshold. These are probe settings only. + if (!JSC::Options::setOptions("useConcurrentGC=true useStochasticMutatorScheduler=true minimumGCPauseMS=0 gcPauseScale=0 minimumMutatorUtilization=1 maximumMutatorUtilization=1 numberOfGCMarkers=2")) { + std::fprintf(stderr, "probe options rejected\n"); + return 2; + } + JSGlobalContextRef context = JSGlobalContextCreate(nullptr); + if (!context || !evaluate(context, "globalThis.gcProgressRoots = Array.from({length: 32768}, (_, i) => ({index:i, child:{value:i}})); gcProgressRoots.length")) { + std::fprintf(stderr, "probe heap setup failed\n"); + return 2; + } + + auto& vm = toJS(context)->vm(); + bool foundPassiveCollector = false; + bool foundLateWaitStall = false; + bool timedOut = false; + bool rootsSurvived = false; + bool completedBeforeHelperRelease = false; + unsigned helperCount = 0; + unsigned stalledWorldState = 0; + Parked stalledThreads; + const void* conditionAddress = nullptr; + const void* worldAddress = nullptr; + double elapsedMs = 0; + { + JSC::JSLockHolder lock(vm); + auto& heap = vm.heap; + // Settle setup collection while helpers are available. No prior cycle or + // unrelated VM is allowed to account for the later observed wait. + heap.collectNow(JSC::Sync, JSC::CollectionScope::Full); + auto& condition = heap.*observe(HeapCondition { }); + auto& world = heap.*observe(HeapWorldState { }); + conditionAddress = &(condition.*observe(ConditionWaitAddress { })); + worldAddress = &world; + const unsigned waitingBit = observe(MutatorWaitingBit { }); + const uintptr_t mutatorParkingThread = WTF::ParkingLot::currentThreadID(); + + WTF::RefPtr sharedPool = &JSC::heapHelperPool(); + helperCount = sharedPool->numberOfThreads(); + if (!helperCount) { + std::fprintf(stderr, "probe requires parallel GC helpers\n"); + return 2; + } + HeldHelpers gate; + WTF::ParallelHelperClient blocker(std::move(sharedPool)); + blocker.setFunction([&] { gate.enter(); }); + if (!gate.waitUntilEntered(helperCount)) { + gate.release(); + blocker.finish(); + std::fprintf(stderr, "could not occupy every shared helper\n"); + return 2; + } + + std::atomic invokeWait { false }; + std::atomic completed { false }; + std::atomic giveUp { false }; + auto begin = std::chrono::steady_clock::now(); + std::thread observer([&] { + const auto deadline = begin + std::chrono::seconds(10); + while (std::chrono::steady_clock::now() < deadline) { + auto parked = inspectParking(conditionAddress, worldAddress, mutatorParkingThread); + if (!foundPassiveCollector && parked.collector && !(world.load() & waitingBit)) { + foundPassiveCollector = true; + invokeWait.store(true, std::memory_order_release); + } + if (completed.load(std::memory_order_acquire)) { + completedBeforeHelperRelease = true; + gate.release(); + return; + } + if (foundPassiveCollector && parked.collector && parked.mutator && (world.load() & waitingBit)) { + // This is an observed wait cycle, not a timing inference. + foundLateWaitStall = true; + stalledThreads = parked; + stalledWorldState = world.load(); + gate.release(); // Recovery is not counted as successful proof. + return; + } + std::this_thread::yield(); + } + timedOut = true; + giveUp.store(true, std::memory_order_release); + gate.release(); + }); + + heap.collectAsync(JSC::CollectionScope::Full); + // This thread retains real mutator heap access. Service the engine's + // stop handshake until the observer sees the collector actually parked. + while (!invokeWait.load(std::memory_order_acquire) && !giveUp.load(std::memory_order_acquire)) { + heap.stopIfNecessary(); + heap.relinquishConn(); + std::this_thread::yield(); + } + if (invokeWait.load(std::memory_order_acquire)) { + heap.preventCollection(); + heap.allowCollection(); + completed.store(true, std::memory_order_release); + } + observer.join(); + blocker.finish(); + // Also settle the failure/control recovery before releasing the VM. + heap.collectNow(JSC::Sync, JSC::CollectionScope::Full); + double checksum = 0; + rootsSurvived = evaluate(context, "gcProgressRoots.length + gcProgressRoots[32767].child.value", &checksum) + && checksum == 65535; + elapsedMs = std::chrono::duration(std::chrono::steady_clock::now() - begin).count(); + } + JSGlobalContextRelease(context); + const bool passed = foundPassiveCollector && completedBeforeHelperRelease && !foundLateWaitStall && !timedOut && rootsSurvived; + std::printf("{\"passed\":%s,\"passiveCollectorObserved\":%s,\"lateWaitStallObserved\":%s,\"completedBeforeHelperRelease\":%s,\"timedOut\":%s,\"rootsSurvived\":%s,\"helperCount\":%u,\"worldStateAtStall\":%u,\"collectorParkingThreadId\":%llu,\"mutatorParkingThreadId\":%llu,\"conditionAddress\":\"%p\",\"worldAddress\":\"%p\",\"elapsedMs\":%.3f}\n", + passed ? "true" : "false", foundPassiveCollector ? "true" : "false", foundLateWaitStall ? "true" : "false", + completedBeforeHelperRelease ? "true" : "false", timedOut ? "true" : "false", rootsSurvived ? "true" : "false", + helperCount, stalledWorldState, static_cast(stalledThreads.collectorThread), + static_cast(stalledThreads.mutatorThread), conditionAddress, worldAddress, elapsedMs); + return passed ? 0 : foundLateWaitStall && rootsSurvived ? 1 : 2; +} diff --git a/.github/openclaw/qualification/verify-sync.py b/.github/openclaw/qualification/verify-sync.py index 27c311929738f..3f88a1fddbb79 100644 --- a/.github/openclaw/qualification/verify-sync.py +++ b/.github/openclaw/qualification/verify-sync.py @@ -25,6 +25,10 @@ assert len(cache)==4 and all(r['matches']==r['total']==56 for r in cache if r['arm']=='candidate') assert cache[-1]['arm']=='candidate' and cache[-1]['repeat']=='warm' and cache[-1]['cache_hit_proven'] is True assert len((out/'engine-limits.log').read_text().splitlines())==9 +gc_progress=json.loads((out/'engine-gc-progress.json').read_text()) +assert all(gc_progress[key] is True for key in ['passed','passiveCollectorObserved','completedBeforeHelperRelease','rootsSurvived']),'passive collector did not progress' +assert all(gc_progress[key] is False for key in ['lateWaitStallObserved','timedOut']),'passive collector stalled' +assert type(gc_progress['helperCount']) is int and gc_progress['helperCount']>0,'GC helper precondition missing' engine=json.loads((root/'engine-gate.json').read_text()) assert engine['passed'] and engine['source']==sys.argv[2] compatibility=json.loads((root/'compatibility-gate.json').read_text()) @@ -36,6 +40,7 @@ assert compatibility['only_manifest_changed_after_pairing'] and compatibility['memory_tests']==3 and compatibility['startup'] is True record={'passed':True,'base_commit':sys.argv[3],'prepared_tree':sys.argv[4], 'selected_files':len(selected),'result_rows':len(rows),'regressions':0,'tests':counts, + 'gc_progress':gc_progress, 'adapter_sha256':hashlib.sha256((root/'bun-sync-adapters.patch').read_bytes()).hexdigest()} (root/'sync-gate.json').write_text(json.dumps(record,indent=2)+'\n') gate=json.loads((root/'gate.json').read_text());gate['upstream_sync']=record;gate['engine']=engine;gate['compatibility']=compatibility diff --git a/.github/openclaw/qualify-sync.sh b/.github/openclaw/qualify-sync.sh index 3229a5884f3cd..84a7675153209 100644 --- a/.github/openclaw/qualify-sync.sh +++ b/.github/openclaw/qualify-sync.sh @@ -88,4 +88,10 @@ clang++-23 -std=c++23 -O2 -DNDEBUG -DBUILDING_WITH_CMAKE=1 -DHAVE_CONFIG_H=1 \ -Wl,--start-group "$WK"/lib/*.a -Wl,--end-group -lpthread -ldl -lm -latomic \ -o "$results/engine-limit-probe" > "$results/engine-limit-build.log" 2>&1 "$results/engine-limit-probe" > "$results/engine-limits.log" 2>&1 +clang++-23 -std=c++23 -O2 -DNDEBUG -DBUILDING_WITH_CMAKE=1 -DHAVE_CONFIG_H=1 \ + -I"$WK/include" -I"$WK/include/JavaScriptCore" -I"$WK/include/wtf" \ + "$inputs/engine-gc-progress-probe.cpp" "$repo/build/qualify-sync/obj/vendor/mimalloc/src/static.c.o" \ + -Wl,--start-group "$WK"/lib/*.a -Wl,--end-group -lpthread -ldl -lm -latomic \ + -o "$results/engine-gc-progress-probe" > "$results/engine-gc-progress-build.log" 2>&1 +"$results/engine-gc-progress-probe" > "$results/engine-gc-progress.json" 2> "$results/engine-gc-progress.log" python3 "$inputs/verify-sync.py" "$QUALIFICATION_DIR" "$SOURCE_SHA" "$BUN_BASE" "$SYNC_TREE" diff --git a/.github/openclaw/release-notes.md b/.github/openclaw/release-notes.md index 2985cbb7dcf74..6a5304ea70856 100644 --- a/.github/openclaw/release-notes.md +++ b/.github/openclaw/release-notes.md @@ -1,4 +1,6 @@ -This release delivers pending worker heap-limit termination promptly after GC. It invalidates optimized code at the mutator's collection epilogue so existing JavaScript safe points deliver the pending trap without repeated full collections. Full-GC managed live accounting, heap budgets and ArrayBuffer exclusions are unchanged. Native regressions cover termination after one over-cap full collection and near-limit survival; Bun qualification retains the worker OOM event contract and external-buffer controls. +This release wakes a passive garbage collector when its mutator starts waiting, allowing collection to progress when shared GC helpers are occupied elsewhere. A native regression checks the late-wait transition, completion before helper release, and survival of rooted objects. + +It also delivers pending worker heap-limit termination promptly after GC. It invalidates optimized code at the mutator's collection epilogue so existing JavaScript safe points deliver the pending trap without repeated full collections. Full-GC managed live accounting, heap budgets and ArrayBuffer exclusions are unchanged. Native regressions cover termination after one over-cap full collection and near-limit survival; Bun qualification retains the worker OOM event contract and external-buffer controls. It also fixes the JSC shell's mimalloc process-exit TLS race by closing admission and draining in-flight setters before deleting the pthread key. Deterministic native regressions cover late initialization and an already-admitted setter. Standard Bun builds disable this allocator teardown and are unaffected by that race. Thanks @dylan-conway for the diagnosis in oven-sh/WebKit#698. diff --git a/Source/JavaScriptCore/heap/Heap.cpp b/Source/JavaScriptCore/heap/Heap.cpp index 1947953da14be..4f6fa49ffec2f 100644 --- a/Source/JavaScriptCore/heap/Heap.cpp +++ b/Source/JavaScriptCore/heap/Heap.cpp @@ -2849,6 +2849,13 @@ void Heap::waitForCollector(const Func& func) } } + if (!done) { + // The collector may already be waiting passively for helper threads. Pair its + // predicate and wait with this notification, without holding m_threadLock. + Locker locker { m_markingMutex }; + m_markingConditionVariable.notifyAll(); + } + // If we're in a stop-the-world scenario, we need to wait for that even if done is true. unsigned oldState = m_worldState.load(); if (stopIfNecessarySlow(oldState)) diff --git a/Source/JavaScriptCore/heap/SlotVisitor.cpp b/Source/JavaScriptCore/heap/SlotVisitor.cpp index 6d2f83d719375..68b01fd52ad37 100644 --- a/Source/JavaScriptCore/heap/SlotVisitor.cpp +++ b/Source/JavaScriptCore/heap/SlotVisitor.cpp @@ -764,6 +764,13 @@ SlotVisitor::SharedDrainResult SlotVisitor::drainInParallelPassively(MonotonicTi return SharedDrainResult::Done; } + // A synchronous collector wait can begin after the entry check. Keep making + // progress even when the process-wide helper pool is serving another heap. + if (m_heap.m_worldState.load() & Heap::mutatorWaitingBit) { + locker.unlockEarly(); + return drainInParallel(timeout); + } + m_heap.m_markingConditionVariable.waitUntil(m_heap.m_markingMutex, timeout); } } From e4cb899b01d206792bfebe25c220fa6bfbc54e1a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 6 Oct 2026 06:03:18 -0700 Subject: [PATCH 2/2] test: retain the API lock through native probe teardown --- .github/openclaw/qualification/engine-gc-progress-probe.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/openclaw/qualification/engine-gc-progress-probe.cpp b/.github/openclaw/qualification/engine-gc-progress-probe.cpp index 6ff5a2655ae48..334a77c104c31 100644 --- a/.github/openclaw/qualification/engine-gc-progress-probe.cpp +++ b/.github/openclaw/qualification/engine-gc-progress-probe.cpp @@ -239,8 +239,9 @@ int main() rootsSurvived = evaluate(context, "gcProgressRoots.length + gcProgressRoots[32767].child.value", &checksum) && checksum == 65535; elapsedMs = std::chrono::duration(std::chrono::steady_clock::now() - begin).count(); + // Final VM destruction must retain the API lock and its active atom string table. + JSGlobalContextRelease(context); } - JSGlobalContextRelease(context); const bool passed = foundPassiveCollector && completedBeforeHelperRelease && !foundLateWaitStall && !timedOut && rootsSurvived; std::printf("{\"passed\":%s,\"passiveCollectorObserved\":%s,\"lateWaitStallObserved\":%s,\"completedBeforeHelperRelease\":%s,\"timedOut\":%s,\"rootsSurvived\":%s,\"helperCount\":%u,\"worldStateAtStall\":%u,\"collectorParkingThreadId\":%llu,\"mutatorParkingThreadId\":%llu,\"conditionAddress\":\"%p\",\"worldAddress\":\"%p\",\"elapsedMs\":%.3f}\n", passed ? "true" : "false", foundPassiveCollector ? "true" : "false", foundLateWaitStall ? "true" : "false",