From b0a1d9dc88a6cc31e65ce864bea74c918c725e6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 5 Aug 2026 14:56:27 +0200 Subject: [PATCH 1/5] gc(diag): the sweep blocks line labeled every non-general block 'longlived' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The count is n_blocks - general_n — survivors + longlived + old — but the label said longlived, which reads as an 84 MB longlived-arena leak on a workload whose longlived arena holds 1 MB (measured while diagnosing #7438: the trace's per-region arena_bytes tells the truth, the DIAG line did not). --- crates/perry-codegen/src/codegen/clone_suffix_tests.rs | 5 ++++- crates/perry-codegen/src/codegen/mod.rs | 4 ++-- crates/perry-runtime/src/gc/oldgen.rs | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/perry-codegen/src/codegen/clone_suffix_tests.rs b/crates/perry-codegen/src/codegen/clone_suffix_tests.rs index 9af37e7eb9..4048953767 100644 --- a/crates/perry-codegen/src/codegen/clone_suffix_tests.rs +++ b/crates/perry-codegen/src/codegen/clone_suffix_tests.rs @@ -179,5 +179,8 @@ fn user_members_named_like_clone_suffixes_keep_their_own_symbols() { let add_clone = function_body(&ir, "perry_fn_clone_suffix_ts__add$typed_f64"); assert!(add_clone.contains("fadd"), "add's clone body is a + b"); let user_clone = function_body(&ir, "perry_fn_clone_suffix_ts__add__typed_f64$typed_f64"); - assert!(user_clone.contains("fmul"), "add__typed_f64's clone body is a * b"); + assert!( + user_clone.contains("fmul"), + "add__typed_f64's clone body is a * b" + ); } diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index afb6d40987..768e9c5c56 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -49,13 +49,13 @@ mod func_registry; mod function; // `pub(crate)` so `crate::linker` can read the inline-hot-small policy // (`inline_hot_small_enabled` / `inline_hot_small_hint_threshold`). +#[cfg(test)] +mod clone_suffix_tests; pub(crate) mod helpers; mod method; mod method_registry; mod module_globals_emit; #[cfg(test)] -mod clone_suffix_tests; -#[cfg(test)] mod number_exactness_tests; mod opts; mod spec_abi; diff --git a/crates/perry-runtime/src/gc/oldgen.rs b/crates/perry-runtime/src/gc/oldgen.rs index 6aae75062f..327f837155 100644 --- a/crates/perry-runtime/src/gc/oldgen.rs +++ b/crates/perry-runtime/src/gc/oldgen.rs @@ -966,7 +966,7 @@ fn legacy_sweep_with_age_bump_and_old_reclaim_targets( .filter(|&i| block_has_live[i]) .count(); eprintln!( - "[gc] blocks: general={} ({} live), longlived={} ({} live), freed_bytes={} retained_forwarded_stub_bytes={} retained_forwarded_stub_objects={}", + "[gc] blocks: general={} ({} live), non_general={} ({} live, survivors+longlived+old), freed_bytes={} retained_forwarded_stub_bytes={} retained_forwarded_stub_objects={}", resettable_general_n, live_general, n_blocks - resettable_general_n, @@ -1324,7 +1324,7 @@ impl ArenaSweepObjectsState { .filter(|&i| self.block_has_live[i]) .count(); eprintln!( - "[gc] blocks: general={} ({} live), longlived={} ({} live), freed_bytes={} retained_forwarded_stub_bytes={} retained_forwarded_stub_objects={}", + "[gc] blocks: general={} ({} live), non_general={} ({} live, survivors+longlived+old), freed_bytes={} retained_forwarded_stub_bytes={} retained_forwarded_stub_objects={}", self.resettable_general_n, live_general, self.block_has_live.len() - self.resettable_general_n, From e13ae07fdd5bc0f6f8c9f7d479b37b31b939d5fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 5 Aug 2026 15:24:22 +0200 Subject: [PATCH 2/5] =?UTF-8?q?arena:=20recycled-block=20pool=20=E2=80=94?= =?UTF-8?q?=20released=20blocks=20are=20reused,=20not=20round-tripped=20(#?= =?UTF-8?q?7438)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Block dealloc/realloc round-trips through the process allocator were the dominant term of tree.ts's scavenge-on peak RSS: each promoted-then-dropped cohort released its old-gen blocks and the next cohort's promotions landed in fresh allocator segments, so ever-dirtied pages grew with cumulative promotion volume — peak commit 257.5 MiB vs 140.5 MiB scavenge-off for a ~35 MB live set, while a cap matrix showed the young-cap dial barely moves RSS (64/32/16 MB caps -> 235/221/226 MB peak RSS). Reclaimed blocks now enter a capped 64 MB thread-local pool (MADV_FREE'd so the OS can take the pages under pressure) and the block reservation funnel reuses them before minting fresh mappings. No collection decision changes; thread teardown still frees for real. --- crates/perry-runtime/src/arena/block.rs | 70 +++++++++++++++++++++++++ crates/perry-runtime/src/arena/mod.rs | 8 +-- crates/perry-runtime/src/arena/reset.rs | 14 ++--- crates/perry-runtime/src/arena/tests.rs | 30 +++++++++++ 4 files changed, 111 insertions(+), 11 deletions(-) diff --git a/crates/perry-runtime/src/arena/block.rs b/crates/perry-runtime/src/arena/block.rs index 0fe5ae523a..f221555f2e 100644 --- a/crates/perry-runtime/src/arena/block.rs +++ b/crates/perry-runtime/src/arena/block.rs @@ -59,6 +59,68 @@ fn block_size_for(min_size: usize) -> usize { /// that `reserve_arena_block` then runs allocates blocks of its own through the /// non-injectable path, so the injected refusal cannot be consumed by the wrong /// allocation. +// --------------------------------------------------------------------------- +// #7438: recycled-block pool. +// +// Block dealloc/realloc round-trips through the process allocator were the +// dominant term of tree.ts's scavenge-on peak RSS: every promoted-then-dropped +// cohort released its old-gen blocks and the next cohort's promotions landed +// in FRESH allocator segments, so the union of ever-dirtied pages grew with +// cumulative promotion volume (~230 MB resident for a ~35 MB live set) while +// a cap matrix showed the young-cap dial barely moves RSS at all (64/32/16 MB +// caps → 235/221/226 MB). Recycling released blocks bounds ever-dirtied pages +// at the CONCURRENT high-water instead. +// +// Pooled blocks are `MADV_FREE`d so the OS can take the pages under memory +// pressure; contents are undefined on reuse, which every consumer tolerates +// (blocks are bump-filled from offset 0 and re-registered by the arena that +// adopts them). The pool is capped; overflow falls through to real dealloc, +// and thread teardown (`Arena::drop`) never pools. +// --------------------------------------------------------------------------- + +thread_local! { + static BLOCK_POOL: RefCell> = const { RefCell::new(Vec::new()) }; + static BLOCK_POOL_BYTES: Cell = const { Cell::new(0) }; +} + +/// Cap on pooled bytes. Sized to cover the young cap ceiling (64 MB) so a +/// phase change (cap scale shrinking, old reclaim) recycles rather than +/// round-trips; beyond it, blocks genuinely return to the allocator. +const BLOCK_POOL_CAP_BYTES: usize = 64 * 1024 * 1024; + +/// Offer a released block to the pool. Returns false (caller deallocs) when +/// the pool is full or the block is null. +pub(crate) fn block_pool_put(data: *mut u8, size: usize) -> bool { + if data.is_null() || size == 0 { + return false; + } + if BLOCK_POOL_BYTES.with(Cell::get).saturating_add(size) > BLOCK_POOL_CAP_BYTES { + return false; + } + #[cfg(unix)] + unsafe { + libc::madvise(data as *mut libc::c_void, size, libc::MADV_FREE); + } + BLOCK_POOL.with(|p| p.borrow_mut().push((data, size))); + BLOCK_POOL_BYTES.with(|c| c.set(c.get().saturating_add(size))); + true +} + +fn block_pool_take(size: usize) -> Option<*mut u8> { + let taken = BLOCK_POOL.with(|p| { + let mut pool = p.borrow_mut(); + let idx = pool.iter().rposition(|&(_, s)| s == size)?; + Some(pool.swap_remove(idx).0) + })?; + BLOCK_POOL_BYTES.with(|c| c.set(c.get().saturating_sub(size))); + Some(taken) +} + +#[cfg(test)] +pub(crate) fn block_pool_bytes_for_test() -> usize { + BLOCK_POOL_BYTES.with(Cell::get) +} + fn try_alloc_block(min_size: usize, injectable: bool) -> Option { let size = block_size_for(min_size); let layout = Layout::from_size_align(size, 16).unwrap(); @@ -68,6 +130,14 @@ fn try_alloc_block(min_size: usize, injectable: bool) -> Option { } #[cfg(not(test))] let _ = injectable; + if let Some(data) = block_pool_take(size) { + return Some(ArenaBlock { + data, + size, + offset: 0, + dead_cycles: 0, + }); + } let data = unsafe { alloc(layout) }; if data.is_null() { return None; diff --git a/crates/perry-runtime/src/arena/mod.rs b/crates/perry-runtime/src/arena/mod.rs index 4480e882ae..5dbaff7c99 100644 --- a/crates/perry-runtime/src/arena/mod.rs +++ b/crates/perry-runtime/src/arena/mod.rs @@ -29,14 +29,14 @@ pub(crate) use allocators::{ inactive_survivor_index, with_survivor_arena, with_survivor_arena_mut, }; pub(crate) use block::{ - arena_cell_alloc, old_gen_in_use_bytes_sub, Arena, ArenaBlock, ACTIVE_SURVIVOR, ARENA, - ARENA_TOTAL_BYTES, BLOCK_SIZE, FRESH_GENERAL_BLOCK_MIN_USED_BYTES, INLINE_STATE, + arena_cell_alloc, block_pool_put, old_gen_in_use_bytes_sub, Arena, ArenaBlock, ACTIVE_SURVIVOR, + ARENA, ARENA_TOTAL_BYTES, BLOCK_SIZE, FRESH_GENERAL_BLOCK_MIN_USED_BYTES, INLINE_STATE, LONGLIVED_ARENA, OLD_ARENA, OLD_GEN_IN_USE_BYTES, SURVIVOR_ARENA_0, SURVIVOR_ARENA_1, }; #[cfg(test)] pub(crate) use block::{ - force_next_block_alloc_failure, gc_trigger_arena_borrow_depth, gc_trigger_arena_calls, - reset_gc_trigger_arena_probe, + block_pool_bytes_for_test, force_next_block_alloc_failure, gc_trigger_arena_borrow_depth, + gc_trigger_arena_calls, reset_gc_trigger_arena_probe, }; pub(crate) use page_meta::{ address_span_overlaps_pages, register_block_space, register_old_object_pages, diff --git a/crates/perry-runtime/src/arena/reset.rs b/crates/perry-runtime/src/arena/reset.rs index dd11c5e153..4d4250d411 100644 --- a/crates/perry-runtime/src/arena/reset.rs +++ b/crates/perry-runtime/src/arena/reset.rs @@ -315,7 +315,7 @@ pub fn arena_reset_empty_blocks(block_has_live: &[bool]) -> ArenaResetStats { // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|t| t.set(t.get().saturating_sub(block.size))); @@ -591,7 +591,7 @@ impl ArenaResetEmptyBlocksState { // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|total| total.set(total.get().saturating_sub(size))); @@ -787,7 +787,7 @@ impl SurvivorArenaReclaimState { // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|total| total.set(total.get().saturating_sub(size))); @@ -1047,7 +1047,7 @@ impl OldArenaReclaimDeadBlocksState { // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|total| total.set(total.get().saturating_sub(size))); @@ -1142,7 +1142,7 @@ pub(crate) fn old_arena_reclaim_dead_blocks(block_has_live: &[bool]) -> ArenaRes // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|total| total.set(total.get().saturating_sub(size))); @@ -1245,7 +1245,7 @@ pub(crate) fn old_arena_reclaim_selected_dead_blocks( // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|total| total.set(total.get().saturating_sub(size))); @@ -1345,7 +1345,7 @@ fn reclaim_dead_survivor_arena_blocks( // #4665: in test builds keep freed blocks mapped (no munmap) so // unit tests holding raw GC pointers across a collection read stale // bytes instead of SIGSEGV-ing on an unmapped page. - if !cfg!(test) { + if !block_pool_put(block.data, block.size) && !cfg!(test) { std::alloc::dealloc(block.data, layout); } ARENA_TOTAL_BYTES.with(|total| total.set(total.get().saturating_sub(size))); diff --git a/crates/perry-runtime/src/arena/tests.rs b/crates/perry-runtime/src/arena/tests.rs index a404b6d688..c3bce7e42f 100644 --- a/crates/perry-runtime/src/arena/tests.rs +++ b/crates/perry-runtime/src/arena/tests.rs @@ -1103,3 +1103,33 @@ fn emergency_block_reclaim_runs_with_no_live_arena_borrow() { reallocate its `blocks` Vec underneath the borrow (#7022)" ); } + +/// #7438: a released block offered to the recycled-block pool is handed back +/// by the next same-size block reservation instead of a fresh allocator +/// mapping — the mechanism that bounds ever-dirtied pages at the concurrent +/// high-water instead of cumulative promotion volume. +#[test] +fn recycled_block_pool_reuses_released_blocks() { + let layout = std::alloc::Layout::from_size_align(BLOCK_SIZE, 16).unwrap(); + let raw = unsafe { std::alloc::alloc(layout) }; + assert!(!raw.is_null()); + let before = block_pool_bytes_for_test(); + assert!( + block_pool_put(raw, BLOCK_SIZE), + "pool must accept a block under its cap" + ); + assert_eq!(block_pool_bytes_for_test(), before + BLOCK_SIZE); + + // The reservation funnel must serve the pooled block back (LIFO) rather + // than minting a fresh mapping. + let block = crate::arena::block::reserve_arena_block(BLOCK_SIZE / 2); + assert_eq!( + block.data as usize, raw as usize, + "same-size reservation must reuse the pooled block" + ); + assert_eq!(block.size, BLOCK_SIZE); + assert_eq!(block.offset, 0); + assert_eq!(block_pool_bytes_for_test(), before); + // Hand it back to the allocator so the test doesn't leak the mapping. + unsafe { std::alloc::dealloc(block.data, layout) }; +} From 96153d5d4aa4514a863d41923f0638bf896ee7e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 5 Aug 2026 15:40:17 +0200 Subject: [PATCH 3/5] arena(pool): pin the cap at 64 MB with the measured tradeoff tree.ts peak RSS: no pool 225 MB, 64 MB pool 190 MB, 128 MB pool 210 MB - pooled pages are MADV_FREE'd but stay resident until the OS wants them, so an oversized pool holds free pages past the optimum. --- crates/perry-runtime/src/arena/block.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/perry-runtime/src/arena/block.rs b/crates/perry-runtime/src/arena/block.rs index f221555f2e..417c1d48f0 100644 --- a/crates/perry-runtime/src/arena/block.rs +++ b/crates/perry-runtime/src/arena/block.rs @@ -83,9 +83,13 @@ thread_local! { static BLOCK_POOL_BYTES: Cell = const { Cell::new(0) }; } -/// Cap on pooled bytes. Sized to cover the young cap ceiling (64 MB) so a -/// phase change (cap scale shrinking, old reclaim) recycles rather than -/// round-trips; beyond it, blocks genuinely return to the allocator. +/// Cap on pooled bytes: 64 MB, matching the young cap ceiling. Measured on +/// tree.ts (Mac mini M1, quiet): no pool -> 225 MB peak RSS; 64 MB pool -> +/// 190 MB; 128 MB pool -> 210 MB. Bigger is NOT better — pooled pages are +/// MADV_FREE'd but stay resident until the OS wants them, so an oversized +/// pool trades fresh-segment growth for held free pages past the optimum. +/// This is a cap, not a floor — the pool holds only blocks that were +/// actually released, and the OS can take every pooled page under pressure. const BLOCK_POOL_CAP_BYTES: usize = 64 * 1024 * 1024; /// Offer a released block to the pool. Returns false (caller deallocs) when From 63569a7f79840c9fc11c9a9f2ddcb4450c6dbe36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 5 Aug 2026 15:47:17 +0200 Subject: [PATCH 4/5] changelog: fragment for #7449 --- changelog.d/7449-recycled-block-pool.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog.d/7449-recycled-block-pool.md diff --git a/changelog.d/7449-recycled-block-pool.md b/changelog.d/7449-recycled-block-pool.md new file mode 100644 index 0000000000..a3bab2f8fc --- /dev/null +++ b/changelog.d/7449-recycled-block-pool.md @@ -0,0 +1,5 @@ +**Arena: recycled-block pool** (#7438 progress) — released 1 MB arena blocks were round-tripped through the process allocator, so each promoted-then-dropped cohort's replacement blocks landed in fresh mimalloc segments and the union of ever-dirtied pages grew with *cumulative promotion volume*: tree.ts peaked at 257.5 MiB committed scavenge-on vs 140.5 MiB scavenge-off for a ~35 MB live set, and a cap matrix showed the young-cap dial barely moves RSS (64/32/16 MB effective caps → 235/221/226 MB) while doubling wall time. + +Released blocks now enter a capped 64 MB thread-local pool and the single block-reservation funnel reuses them before minting fresh mappings. Pooled pages are `MADV_FREE`'d (the OS reclaims them under pressure; contents are undefined on reuse, which every consumer tolerates — blocks are bump-filled from offset 0 and re-registered by the adopting arena). The cap is measured, not guessed: no pool → 225 MB tree peak RSS, 64 MB → 190 MB, 128 MB → 210 MB (an oversized pool holds resident freed pages past the optimum). No collection decision changes — GC counters are byte-identical, so the pinned gc-ratchet baseline stands. Thread teardown still frees for real. + +Measured (dedicated M1 bench host, same-session A/B): tree.ts 225 → **190 MB** peak RSS at wall parity (9.85 s vs 9.96 s); deeplist 166 → 158 MB; churn/cycles/retain and the scavenge-off arm unchanged. Also relabels the sweep DIAG line that printed every non-general block as `longlived=` (it now says `non_general=` — the old label read as an 84 MB longlived leak on a workload whose longlived arena holds 1 MB). From 028a266815f89051696689611e6908b4894b2639 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 5 Aug 2026 16:03:37 +0200 Subject: [PATCH 5/5] arena(pool): give the recycled-block pool ownership of its blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The thread-local held a bare Vec<(*mut u8, usize)>, so a thread exiting with a non-empty pool ran the Vec's destructor — freeing the Vec's own buffer and stranding every block it pointed at, up to BLOCK_POOL_CAP_BYTES per thread. perry/thread's spawn/parallelMap give each agent its own arena and GC, so repeated spawns leaked without bound in the one change whose purpose is lowering RSS. Ownership lives on the pool value rather than in a drain called from Arena::drop: both are TLS destructors, their relative order is unspecified, and LocalKey::with panics once its own destructor has run, so a drain could be skipped exactly when it is needed. --- crates/perry-runtime/src/arena/block.rs | 42 ++++++++++++++++++++++--- crates/perry-runtime/src/arena/tests.rs | 35 +++++++++++++++++++++ 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/crates/perry-runtime/src/arena/block.rs b/crates/perry-runtime/src/arena/block.rs index 417c1d48f0..fcfc151cf1 100644 --- a/crates/perry-runtime/src/arena/block.rs +++ b/crates/perry-runtime/src/arena/block.rs @@ -78,8 +78,42 @@ fn block_size_for(min_size: usize) -> usize { // and thread teardown (`Arena::drop`) never pools. // --------------------------------------------------------------------------- +/// Owns the pooled blocks, so that a thread exiting with a non-empty pool +/// releases them instead of leaking up to [`BLOCK_POOL_CAP_BYTES`]. +/// +/// The ownership has to live *here* rather than in a drain called from +/// `Arena::drop`: both are TLS destructors, their relative order is not +/// specified, and `LocalKey::with` panics once its own destructor has run — +/// so a drain could be skipped exactly when it is needed. A `Drop` on the +/// pool's own value is order-independent by construction. +/// +/// Matters for `perry/thread`: `spawn`/`parallelMap` give every agent its own +/// arena and GC, so each exiting agent thread would otherwise strand its +/// pooled blocks — unbounded growth across repeated spawns, in the one change +/// whose purpose is lowering RSS. +struct BlockPool(Vec<(*mut u8, usize)>); + +impl Drop for BlockPool { + fn drop(&mut self) { + for &(data, size) in &self.0 { + if data.is_null() || size == 0 { + continue; + } + let layout = Layout::from_size_align(size, 16).unwrap(); + unsafe { + // #4665, mirroring `Arena::drop`: test builds keep freed blocks + // mapped so unit tests holding raw GC pointers across a + // collection read stale bytes instead of faulting. + if !cfg!(test) { + std::alloc::dealloc(data, layout); + } + } + } + } +} + thread_local! { - static BLOCK_POOL: RefCell> = const { RefCell::new(Vec::new()) }; + static BLOCK_POOL: RefCell = const { RefCell::new(BlockPool(Vec::new())) }; static BLOCK_POOL_BYTES: Cell = const { Cell::new(0) }; } @@ -105,7 +139,7 @@ pub(crate) fn block_pool_put(data: *mut u8, size: usize) -> bool { unsafe { libc::madvise(data as *mut libc::c_void, size, libc::MADV_FREE); } - BLOCK_POOL.with(|p| p.borrow_mut().push((data, size))); + BLOCK_POOL.with(|p| p.borrow_mut().0.push((data, size))); BLOCK_POOL_BYTES.with(|c| c.set(c.get().saturating_add(size))); true } @@ -113,8 +147,8 @@ pub(crate) fn block_pool_put(data: *mut u8, size: usize) -> bool { fn block_pool_take(size: usize) -> Option<*mut u8> { let taken = BLOCK_POOL.with(|p| { let mut pool = p.borrow_mut(); - let idx = pool.iter().rposition(|&(_, s)| s == size)?; - Some(pool.swap_remove(idx).0) + let idx = pool.0.iter().rposition(|&(_, s)| s == size)?; + Some(pool.0.swap_remove(idx).0) })?; BLOCK_POOL_BYTES.with(|c| c.set(c.get().saturating_sub(size))); Some(taken) diff --git a/crates/perry-runtime/src/arena/tests.rs b/crates/perry-runtime/src/arena/tests.rs index c3bce7e42f..d68af2088e 100644 --- a/crates/perry-runtime/src/arena/tests.rs +++ b/crates/perry-runtime/src/arena/tests.rs @@ -1133,3 +1133,38 @@ fn recycled_block_pool_reuses_released_blocks() { // Hand it back to the allocator so the test doesn't leak the mapping. unsafe { std::alloc::dealloc(block.data, layout) }; } + +/// A thread exiting with a non-empty pool must run the pool's own `Drop` +/// rather than stranding its blocks. Before the `BlockPool` newtype the +/// thread-local held a bare `Vec<(*mut u8, usize)>`, so the TLS destructor +/// freed the Vec's buffer and leaked every block it pointed at — up to +/// `BLOCK_POOL_CAP_BYTES` per exiting thread, which `perry/thread`'s +/// `spawn`/`parallelMap` create routinely. +/// +/// The dealloc itself is `cfg!(test)`-skipped (#4665, exactly as in +/// `Arena::drop`), so this asserts the destructor RUNS and that pools are +/// per-thread; it cannot observe the free. Its value is that a regression to +/// a bare `Vec` — or a drain called from another TLS destructor, whose +/// ordering is unspecified — still has to keep this path alive. +#[test] +fn block_pool_is_per_thread_and_drops_with_its_thread() { + let before = block_pool_bytes_for_test(); + let handle = std::thread::spawn(|| { + // Fresh thread => fresh pool. + assert_eq!(block_pool_bytes_for_test(), 0); + let layout = std::alloc::Layout::from_size_align(BLOCK_SIZE, 16).unwrap(); + let raw = unsafe { std::alloc::alloc(layout) }; + assert!(!raw.is_null()); + assert!( + block_pool_put(raw, BLOCK_SIZE), + "pool should accept the block" + ); + assert_eq!(block_pool_bytes_for_test(), BLOCK_SIZE); + // Thread exits here with a non-empty pool: BlockPool::drop must run. + }); + handle + .join() + .expect("spawned thread must exit cleanly, not double-free"); + // The other thread's pool never touched ours. + assert_eq!(block_pool_bytes_for_test(), before); +}