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
1 change: 0 additions & 1 deletion Cargo.lock

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

1 change: 0 additions & 1 deletion crates/backend/system-info/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ edition.workspace = true

[dependencies]
libc = "0.2"
rayon.workspace = true

[lints]
workspace = true
33 changes: 0 additions & 33 deletions crates/backend/system-info/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,36 +9,3 @@ pub fn peak_rss_bytes() -> u64 {
// ru_maxrss unit: bytes on macOS, KiB on Linux.
if cfg!(target_os = "macos") { max } else { max * 1024 }
}

/// Number of jobs [`flush_rayon`] pushes. Must exceed
/// `crossbeam_deque::deque::BLOCK_CAP` (currently 63 —
/// `crossbeam-deque-0.8.6/src/deque.rs:1191`).
const RAYON_FLUSH_JOBS: usize = 256;

/// Drain rayon's internal queues so they release any storage allocated during the
/// previous phase.
///
/// Rayon's global pool owns a `crossbeam_deque::Injector`, internally a linked list
/// of fixed-size blocks (`Block` and `Injector::push` —
/// `crossbeam-deque-0.8.6/src/deque.rs:1219` and `:1371`). A block is freed only
/// once its last slot has been consumed.
///
/// `rayon::join` from a non-worker thread reaches that injector via
/// `join` (`rayon-core-1.13.0/src/join/mod.rs:132`) ->
/// `registry::in_worker` (`registry.rs:946`) ->
/// `Registry::in_worker_cold` (`:517`) ->
/// `Registry::inject` (`:428`) -> `Injector::push`.
///
/// Under an arena allocator that recycles memory between phases (e.g. `zk-alloc`),
/// a block allocated *during* a phase points into a slab the next `begin_phase()`
/// will reuse. The next push then writes a `JobRef` straight through whatever the
/// application has placed on top, silently corrupting it.
///
/// Pushing more than `BLOCK_CAP` jobs while the arena is off forces the Injector
/// to allocate a fresh tail block (which lands in System), and forces workers to
/// steal the last slot of every preceding block (which destroys them).
pub fn flush_rayon() {
for _ in 0..RAYON_FLUSH_JOBS {
rayon::join(|| {}, || {});
}
}
34 changes: 30 additions & 4 deletions crates/backend/zk-alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ const SLACK: usize = 4; // SLACK absorbs the main thread and any non-rayon helpe
const MAX_THREADS: usize = NUM_THREADS + SLACK;
const REGION_SIZE: usize = SLAB_SIZE * MAX_THREADS;

/// Allocations smaller than this go to System even during active phases.
/// Routes registry / hashmap / injector-block-sized allocations away from the
/// arena, so library state that outlives a phase doesn't land in recycled
/// memory. Covers the known phase-crossing patterns: crossbeam_deque::Injector
/// blocks (~1.5 KB), tracing-subscriber Registry slot data (sub-KB), hashbrown
/// HashMap entries (sub-KB), rayon-core job stack frames (sub-KB).
///
/// TODO is there a cleaner way?
const MIN_ARENA_BYTES: usize = 4096;

#[derive(Debug)]
pub struct ZkAllocator;

Expand Down Expand Up @@ -106,12 +116,8 @@ pub fn begin_phase() {

/// Deactivates the arena. New allocations go to the system allocator; existing arena
/// pointers stay valid until the next `begin_phase()` resets the slabs.
///
/// Also calls [`system_info::flush_rayon`] to release any rayon/crossbeam storage
/// still referencing this phase's arena memory.
pub fn end_phase() {
ARENA_ACTIVE.store(false, Ordering::Release);
system_info::flush_rayon();
}

#[cold]
Expand Down Expand Up @@ -152,6 +158,15 @@ unsafe impl GlobalAlloc for ZkAllocator {
#[inline(always)]
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
if ARENA_ACTIVE.load(Ordering::Relaxed) {
// Small allocs bypass arena: registry slots / HashMap entries /
// injector-block-sized allocations from rayon/tracing libraries
// commonly outlive a phase. Routing them to System keeps them
// safe across begin_phase()/end_phase() boundaries.
//
// TODO is there a cleaner way?
if layout.size() < MIN_ARENA_BYTES {
return unsafe { std::alloc::System.alloc(layout) };
}
let generation = GENERATION.load(Ordering::Relaxed);
if ARENA_GEN.get() == generation {
let align = layout.align();
Expand Down Expand Up @@ -182,6 +197,17 @@ unsafe impl GlobalAlloc for ZkAllocator {
if new_size <= layout.size() {
return ptr;
}
// Sticky-System routing: if the original allocation came from System
// (small, or pre-phase, or routed by size-routing), keep the grown
// allocation in System too. Without this, a Vec allocated outside a
// phase that grows inside one would silently migrate into the arena
// and become subject to phase recycling.
let addr = ptr as usize;
let base = REGION_BASE.load(Ordering::Relaxed);
let in_arena = base != 0 && addr >= base && addr < base + REGION_SIZE;
if !in_arena {
return unsafe { std::alloc::System.realloc(ptr, layout, new_size) };
}
// SAFETY: new_size > layout.size() > 0, align unchanged from valid layout.
let new_layout = unsafe { Layout::from_size_align_unchecked(new_size, layout.align()) };
let new_ptr = unsafe { self.alloc(new_layout) };
Expand Down
4 changes: 3 additions & 1 deletion crates/backend/zk-alloc/tests/test_rayon.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
//! Regression test for the bug prevented by `system_info::flush_rayon`.
//! Regression test for arena/rayon corruption: rayon's `crossbeam_deque::Injector`
//! blocks (~1.5 KB) used to land in the arena and outlive a phase. Now prevented
//! by `MIN_ARENA_BYTES` size-routing in `ZkAllocator::alloc`.
use rayon::prelude::*;

Expand Down
Loading