From 7e855d6cd35862293950a44576a341afb247c606 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 6 Oct 2026 05:54:40 -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/qualify.sh | 11 + .github/openclaw/release.py | 2 +- Source/JavaScriptCore/heap/Heap.cpp | 7 + Source/JavaScriptCore/heap/SlotVisitor.cpp | 7 + 5 files changed, 277 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/qualify.sh b/.github/openclaw/qualify.sh index ac68e8a4706f9..fc28e225d1cfd 100644 --- a/.github/openclaw/qualify.sh +++ b/.github/openclaw/qualify.sh @@ -78,6 +78,12 @@ 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 "$QUALIFICATION_DIR/engine-limit-probe" > "$QUALIFICATION_DIR/engine-limit-build.log" 2>&1 "$QUALIFICATION_DIR/engine-limit-probe" > "$QUALIFICATION_DIR/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" "$QUALIFICATION_DIR/bun/build/qualify-candidate/obj/vendor/mimalloc/src/static.c.o" \ + -Wl,--start-group "$WK"/lib/*.a -Wl,--end-group -lpthread -ldl -lm -latomic \ + -o "$QUALIFICATION_DIR/engine-gc-progress-probe" > "$QUALIFICATION_DIR/engine-gc-progress-build.log" 2>&1 +"$QUALIFICATION_DIR/engine-gc-progress-probe" > "$QUALIFICATION_DIR/engine-gc-progress.json" 2> "$QUALIFICATION_DIR/engine-gc-progress.log" "$candidate" "$inputs/module-context.mjs" matrix > "$QUALIFICATION_DIR/als-plugin.json" "$candidate" "$inputs/module-context.mjs" matrix --native-hooks > "$QUALIFICATION_DIR/als-native.json" python3 - "$QUALIFICATION_DIR" "$SOURCE_SHA" "$BUN_COMMIT" <<'PY' @@ -95,7 +101,12 @@ for name,minimum in [('proxy',4),('resource-limits',16),('namespace',41),('array passes=re.search(r'(\d+) pass',text) assert passes and int(passes[1])>=minimum, name+' missing passing cases' assert re.search(r'\b0 fail\b',text),name+' failures' +gc_progress=json.loads((root/'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' (root/'gate.json').write_text(json.dumps({'passed':True,'source':sys.argv[2],'bun_commit':sys.argv[3], 'selected_files':44,'result_files':46,'als_variants_per_mode':14,'proxy_minimum':4,'resource_minimum':16, + 'gc_progress':gc_progress, 'bun_adapter_sha256':hashlib.sha256((root/'bun-adapters.patch').read_bytes()).hexdigest()},indent=2)+'\n') PY diff --git a/.github/openclaw/release.py b/.github/openclaw/release.py index 9d057ee3cc2ee..7980b8c43cdc5 100644 --- a/.github/openclaw/release.py +++ b/.github/openclaw/release.py @@ -110,7 +110,7 @@ def publish(directory,source): raise ValueError('publishing requires reviewed source on an explicitly permitted owned branch') # Tag creation fails atomically if the name exists. Neither existing drafts nor releases are resumed. api('-X','POST',f'repos/{REPO}/git/refs','-f','ref=refs/tags/'+tag,'-f','sha='+source) - body=f'OpenClaw-built WebKit at `{source}`. All selected lanes and Linux qualification passed.\n\nBatch 1: Segmenter surrogate boundaries; opt-in Proxy global prototypes; module-loader AsyncLocalStorage propagation; per-VM worker heap and stack budgets. Thanks @steipete and @robobun for the source changes and upstream work.\n\nSource and licenses: LICENSE-SOURCES.txt. Checksums: SHA256SUMS and manifest.json. Full build and test provenance: provenance.tar.gz.\n\nRollback by restoring the prior Bun manifest and WebKit pin together; prior releases remain available.\n' + body=f'OpenClaw-built WebKit at `{source}`. All selected lanes and Linux qualification passed.\n\nThis release wakes a passive garbage collector when its mutator starts waiting, so collection progresses while shared GC helpers are occupied elsewhere. Native qualification requires completion before helper release and survival of rooted objects.\n\nBatch 1: Segmenter surrogate boundaries; opt-in Proxy global prototypes; module-loader AsyncLocalStorage propagation; per-VM worker heap and stack budgets. Thanks @steipete and @robobun for the source changes and upstream work.\n\nSource and licenses: LICENSE-SOURCES.txt. Checksums: SHA256SUMS and manifest.json. Full build and test provenance: provenance.tar.gz.\n\nRollback by restoring the prior Bun manifest and WebKit pin together; prior releases remain available.\n' draft=api('-X','POST',f'repos/{REPO}/releases','-f','tag_name='+tag,'-f','name='+tag,'-f','body='+body,'-F','draft=true') release_id=draft['id'] execute('gh','release','upload',tag,'-R',REPO,*[str(p) for p in sorted(directory.iterdir())]) 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 8971626b01534e079ef275d375ad331ecf8ccc42 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 6 Oct 2026 06:03:22 -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",