fix(slab): O(1) aligned-page cross-thread frees + retire-on-live-count (closes #852)#859
Merged
Merged
Conversation
…t (no global hot-path lock) Distilled from #782 keeping its retire-on-live_count correctness, replacing the global-lock+linear-scan with aligned-page O(1) lookup + per-page lock-free remote-free queue (no hot-path contention). Each 64KB slab page is allocated aligned to its own size, so the owning page of any chunk is recovered by masking the pointer; a lock-free 3-level radix page map (cold-path writes only) tells slab_free whether a masked base is a real slab page or a plain heap pointer without dereferencing an unrelated address. Owner frees hit the thread-local free list; foreign frees CAS onto that page's lock-free MPSC remote-free stack. Pages carry refcount = handed-out chunks + owner guard; reclaim/destroy retire a page that still holds foreign-live chunks and free it when the final chunk returns, so a page holding a live tree-sitter lexer chunk is never freed under it. Keeps #782's resolve-worker terminal slab teardown. Closes the #852 use-after-free and the cross-thread invalid free. Reproduce-first guards in test_mem.c are RED on main (heap-use-after-free / bad-free under ASan) and GREEN with this fix. Closes #852 Supersedes #782 Co-authored-by: SS-42 <47749027+SS-42@users.noreply.github.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…ned-pages Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
… baseline The incr_full_index guard measured 2050-2072MB peak for the 1100-file Python index across CI runs after the retention/source-text-cap and RAM-tiering work shifted the x86 baseline up. The old 2048MB limit sat right on the line and flaked marginally (2050 vs 2048). Raise the x86 base budget to 2304MB to restore headroom; a genuine leak (GBs over) still trips it, and the ARM/large-page bumps to 2816 are unchanged. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(slab): O(1) aligned-page cross-thread frees + retire-on-live-count
Distilled from #782. Keeps #782's correctness (recognize cross-thread slab
frees; never free a page under a still-live tree-sitter chunk) but replaces its
global-lock + linear-scan hot path — the source of its own measured regression
(+12% @ 10 workers on the flagship workload) — with an O(1), lock-free hot path.
Closes #852. Supersedes #782.
The bugs
Tree-sitter's allocator callbacks are process-global, so a ≤64B chunk allocated
on parser thread A can be freed on thread B.
slab_owns()scanned only thefreeing thread's pages, so a cross-thread free missed A's pages and fell
through to
free(ptr)on a pointer interior to a malloc'd page → SIGABRT("pointer being freed was not allocated").
cbm_slab_destroy_thread()/cbm_slab_reclaim()freed a page while a still-live tree-sitter lexer referenced a chunk in it →
heap-use-after-free on the next
ts_lexer_goto.Design
1. O(1) page lookup via alignment (kills the scan). Each slab page is a
64KB region allocated aligned to its own size (
cbm_aligned_alloc=posix_memalign/_aligned_malloc). The page header lives at the alignedbase; chunks start at
SLAB_DATA_OFFSET(64B, 1023 usable chunks). The owningpage of any chunk is
(slab_page*)((uintptr_t)ptr & ~(SLAB_PAGE_SIZE-1))— noscan, no global registry needed for lookup.
2. Safe "is this a slab pointer?" test (lock-free).
slab_freealso receivesplain heap pointers (>64B), whose masked base may be unmapped — dereferencing a
"header" there would crash / trip ASan. A tiny 3-level radix page map keyed
by page number (covers all 48-bit addresses; 16KB static root + on-demand 8–16KB
tables) answers "is this masked base a registered slab page?" with lock-free
acquire loads only, never touching the queried pointer. Its tables are never
freed mid-run and stay reachable from the static root, so readers never touch
freed memory. Writes (page create/free/retire — cold paths only) take a
dedicated spinlock that the alloc/free hot path never touches.
3. Cross-thread free without a global hot-path lock (deferred remote free,
mimalloc/tcmalloc style). On free, recover the page in O(1):
owner == this thread→ push to the thread-local free list (no atomics on thelist). Fast path unchanged from the original slab.
(release), then
refcount--. The owner drains each owned page's remote listinto its free list on its next refill (acquire). No global lock.
4. Retire-on-live-count (the #852 fix; kept from #782). Each page carries
refcount = (handed-out chunks) + (1 owner guard while owned). Alloc++,every free
--.reclaim/destroydrop the owner guard: a page with no livechunks is freed immediately; a page still holding foreign-live chunks is
retired (
owner=NULL, left registered) and freed by whichever thread returnsits last chunk (the
refcount 1→0transition — a single atomic decision point,so exactly one thread frees, race-free). A chunk being freed keeps its own
page alive (refcount ≥ 1) until that very free, so the lock-free remote-free
CAS and the lock-free page-map read can never touch a page that is being freed
concurrently. #782's resolve-worker terminal teardown
(
cbm_destroy_thread_parser(); cbm_slab_destroy_thread();) is kept — it retiresresolve-worker pages so they are released instead of leaking with an owner
pointer into dead TLS.
Why not #782's approach
#782 fixed correctness with a global spinlock on every ≤64B alloc/free plus
an O(global_pages) linear scan on every op — reintroducing exactly the
cross-thread contention the thread-local slab was built to eliminate (author's
own numbers: main→PR +12.2% @ 10 workers, ~+7% @ default). This distillation
removes both: the hot path takes no global lock and performs no scan — page
lookup is a pointer mask, the is-slab test is a lock-free radix read, and the
foreign free is a single per-page CAS. Contention is eliminated by construction.
Reproduce-first evidence (ASan, macOS arm64)
Tests added to
tests/test_mem.c, all RED onorigin/main's allocator, GREENwith the fix:
slab_destroy_thread_with_live_chunk_no_uaf— heap-use-after-free: cross-suite thread-slab vs tree-sitter teardown (test-runner pipeline mem) #852 exact guard,deterministic single-thread (not cross-suite-order dependent). RED on main:
slab_cross_thread_free_is_safe— RED on main:slab_reclaim_with_foreign_live_chunk_is_safe,slab_destroy_with_foreign_live_chunk_is_safe— same two failure classes(reclaim/destroy frees a page while a foreign thread holds a live chunk).
GREEN with the fix (
memsuite): 35 passed, incl. all four new guards.Verification
make -f Makefile.cbm cbm— production-O2 -Werrorbuild clean.make -f Makefile.cbm lint-ci— cppcheck + clang-format + NOLINT clean.CBM_INDEX_SUPERVISOR=0 ./build/c/test-runner mem parallel pipeline— 273passed, ASan clean.
CBM_INDEX_SUPERVISOR=0 ./build/c/test-runner pipeline mem(the audit'srequested cross-suite run) — 250 passed, ASan clean: no heap-use-after-free,
no bad-free, no leaks.
Cross-platform:
cbm_aligned_alloc/cbm_aligned_freealready wrapposix_memalign/_aligned_malloc+_aligned_free; C11<stdatomic.h>isalready used in the tree (
mem.c).Co-authored-by: SS-42 47749027+SS-42@users.noreply.github.com