diff --git a/.github/workflows/cci.yml b/.github/workflows/cci.yml index e37448e..67333a7 100644 --- a/.github/workflows/cci.yml +++ b/.github/workflows/cci.yml @@ -41,12 +41,21 @@ jobs: run: cd c && make test-set-atomic - name: Build and test first-fit allocator run: cd c && make test-first-fit + - name: Build and test first-fit allocator (atomic) + if: runner.os != 'Windows' + run: cd c && make test-first-fit-atomic - name: Build and test ghost-tree allocator run: cd c && make test-ghost-tree - name: Build and test slab allocator run: cd c && make test-slab + - name: Build and test slab allocator (atomic) + if: runner.os != 'Windows' + run: cd c && make test-slab-atomic - name: Build and test checked-slab allocator run: cd c && make test-checked-slab + - name: Build and test checked-slab allocator (atomic) + if: runner.os != 'Windows' + run: cd c && make test-checked-slab-atomic - name: Build and test bytevec run: cd c && make test-bytevec - name: Build alloc libraries (all feature variants) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ef2b3b9..51c53d6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -53,4 +53,8 @@ jobs: - name: Test with set and atomic run: cargo test --features "set atomic" - name: Test Release with set and atomic - run: cargo test --release --features "set atomic" \ No newline at end of file + run: cargo test --release --features "set atomic" + - name: Test with alloc, set and atomic + run: cargo test --features "alloc set atomic" -- --skip alloc_fuzz_tests + - name: Test Release with alloc, set and atomic + run: cargo test --release --features "alloc set atomic" -- --skip alloc_fuzz_tests \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 0589faa..0493ab9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed + +- **`SlabBStackAllocator` version bumped to 0.1.1** (`alloc` + `set` features): Magic number updated from `ALSL\x00\x01\x00\x00` to `ALSL\x00\x01\x01\x00`. Reflects the addition of `atomic` / `Sync` support. Existing 0.1.x files remain fully compatible (only the first 6 bytes are checked on open). +- **`CheckedSlabBStackAllocator` version bumped to 0.1.1** (`alloc` + `set` features): Magic number updated from `ALCK\x00\x01\x00\x00` to `ALCK\x00\x01\x01\x00`. Reflects the addition of `atomic` / `Sync` support. Existing 0.1.x files remain fully compatible (only the first 6 bytes are checked on open). +- **`SlabBStackAllocator` is `Send + Sync` with the `atomic` feature** (`alloc` + `set` features): Without `atomic`, free-list mutations read then write `free_head` as separate `BStack` calls — a TOCTOU race that can hand the same block to two callers. With `atomic`, an internal mutex serialises free-list pop/push; tail operations use `BStack::try_discard` / `BStack::try_extend_zeros` (atomic check-and-act under `BStack`'s own write lock, no allocator lock needed). Non-tail paths lock only around `push_free_blocks`. +- **`CheckedSlabBStackAllocator` is `Send + Sync` with the `atomic` feature** (`alloc` + `set` features): Same mutex model. Free-list pop in `alloc` is lock-scoped; tail extend runs lock-free. `dealloc` uses `try_discard` for the tail path without the lock; free-list push is locked. In `realloc`, the grow path uses `try_extend_zeros` lock-free; the shrink path holds the lock across tail-check + overhead-write + discard (overhead must be committed before truncation for crash safety). `recover` holds the lock for its full duration. + +--- + ## [0.2.3] - 2026-06-01 ### Added diff --git a/README.md b/README.md index e218f2b..c5cc765 100644 --- a/README.md +++ b/README.md @@ -842,7 +842,7 @@ bstack = { version = "0.2", features = ["alloc", "set"] } user data offset 48 (arena start) ``` -* **`magic`** — `"ALSL\x00\x01\x00\x00"` (version 0.1). +* **`magic`** — `"ALSL\x00\x01\x01\x00"` (version 0.1.1). * **`block_size`** — fixed size of every arena block, little-endian `u64`. * **`free_head`** — payload offset of the first free block's first byte, or `0` (sentinel). * **Free block** — first 8 bytes hold the payload offset of the next free block (`u64` LE, `0` = end of list); remaining bytes belong to the caller when live. @@ -870,6 +870,14 @@ Slab blocks at the tail are added to the free list (not discarded) so they can b Each free-list mutation is two `BStack` calls: write the next-pointer into the block, then update `free_head` in the header. A crash between the two calls leaks the block being added or removed but leaves the rest of the free list intact. No recovery scan is required on reopen. +#### Thread safety + +`SlabBStackAllocator` is always **`Send`** — ownership can be transferred to another thread. + +Without the `atomic` feature it is **not `Sync`**: free-list mutations require a read then a write of `free_head` as separate `BStack` calls — a TOCTOU race under concurrent `&self` access that can result in two callers receiving the same block. + +With the `atomic` feature it **is `Sync`**. An internal mutex serialises all compound operations that span multiple `BStack` calls: free-list pop/push and the tail-length check that precedes any `extend` or `discard`. The tail-path operations themselves use `try_discard` / `try_extend_zeros`, which check-and-act atomically under `BStack`'s own write lock without needing the allocator lock. + #### Constructors | Constructor | Stack | Effect | @@ -922,7 +930,7 @@ bstack = { version = "0.2", features = ["alloc", "set"] } user data offset 48 (arena start) ``` -* **`magic`** — `"ALCK\x00\x01\x00\x00"` (version 0.1). +* **`magic`** — `"ALCK\x00\x01\x01\x00"` (version 0.1.1). * **`block_size`** — `data_size + 8`, little-endian `u64`. * **`free_head`** — block start offset of the first free block, or `0` (sentinel; no valid block starts at offset 0). @@ -963,6 +971,14 @@ Multi-block requests always extend the tail — the free list holds single block Free-list mutations write block payloads before updating `free_head`. The overhead high bit is flipped to "in use" only after the block's data is clean, so a crash at any intermediate point leaks at most one block without corrupting the free list. A linear arena scan can reconstruct a valid free list from scratch if needed. +#### Thread safety + +`CheckedSlabBStackAllocator` is always **`Send`** — ownership can be transferred to another thread. + +Without the `atomic` feature it is **not `Sync`**: free-list mutations read then write `free_head` as separate `BStack` calls — a TOCTOU race under concurrent `&self` access. + +With the `atomic` feature it **is `Sync`**. An internal mutex serialises all compound operations: free-list pop/push, the tail-length checks preceding `extend` or `discard`, and the `recover` scan. Tail deallocations use `try_discard` (atomically checks tail and removes bytes under `BStack`'s own write lock) without needing the allocator lock. The shrink path acquires the lock before the tail check because the overhead must be written before discarding. + #### Constructors | Constructor | Stack | Effect | diff --git a/c/Makefile b/c/Makefile index 33c3c41..3466dd9 100644 --- a/c/Makefile +++ b/c/Makefile @@ -52,10 +52,20 @@ TEST_GT_OBJ = test_ghost_tree.o TEST_SL_BIN = test_slab$(EXE_EXT) TEST_SL_OBJ = test_slab.o +# Slab allocator test compiled with SET+ATOMIC, exercising the mutex and the +# try_discard / try_extend_zeros tail paths under concurrent access. +TEST_SL_ATOMIC_OBJ = test_slab-atomic.o +TEST_SL_ATOMIC_BIN = test_slab-atomic$(EXE_EXT) + # Checked-slab allocator test (requires BSTACK_FEATURE_SET). TEST_CSL_BIN = test_checked_slab$(EXE_EXT) TEST_CSL_OBJ = test_checked_slab.o +# Checked-slab allocator test compiled with SET+ATOMIC, exercising the mutex +# and the try_discard / try_extend_zeros tail paths under concurrent access. +TEST_CSL_ATOMIC_OBJ = test_checked_slab-atomic.o +TEST_CSL_ATOMIC_BIN = test_checked_slab-atomic$(EXE_EXT) + # ByteVec — requires BSTACK_FEATURE_SET. OBJ_BYTEVEC = bstack_bytevec.o LIB_BYTEVEC_SET = libbstack-bytevec-set.a @@ -70,7 +80,7 @@ MOVE_COW_BIN = ../examples/move_and_cow$(EXE_EXT) LINKED_LIST_BIN = ../examples/linked_list$(EXE_EXT) CHECKSUMMED_CACHE_BIN = ../examples/checksummed_cache$(EXE_EXT) -.PHONY: all test test-set test-atomic test-set-atomic test-first-fit test-first-fit-atomic test-ghost-tree test-slab test-checked-slab test-bytevec leaks clean alloc \ +.PHONY: all test test-set test-atomic test-set-atomic test-first-fit test-first-fit-atomic test-ghost-tree test-slab test-slab-atomic test-checked-slab test-checked-slab-atomic test-bytevec leaks clean alloc \ example example-basic example-buffer_reuse example-journal \ example-reading example-vec_store example-hashmap example-atomic_ops example-move_and_cow \ example-linked_list example-checksummed_cache @@ -165,6 +175,13 @@ $(TEST_SL_BIN): $(TEST_SL_OBJ) $(LIB_ALLOC_SET) test-slab: $(TEST_SL_BIN) $(RUN)$(TEST_SL_BIN) +# Same slab test compiled against the SET+ATOMIC alloc lib, exercising the +# in-memory mutex and the try_discard / try_extend_zeros tail paths. +test-slab-atomic: $(LIB_ALLOC_SET_ATOMIC) + $(CC) $(CFLAGS) -DBSTACK_FEATURE_SET -DBSTACK_FEATURE_ATOMIC -c -o $(TEST_SL_ATOMIC_OBJ) test_slab.c + $(CC) $(CFLAGS) -DBSTACK_FEATURE_SET -DBSTACK_FEATURE_ATOMIC -o $(TEST_SL_ATOMIC_BIN) $(TEST_SL_ATOMIC_OBJ) -L. -lbstack-alloc-set-atomic -lpthread + $(RUN)$(TEST_SL_ATOMIC_BIN) + $(TEST_CSL_OBJ): test_checked_slab.c bstack_alloc.h bstack.h $(CC) $(CFLAGS) -DBSTACK_FEATURE_SET -c -o $@ $< @@ -174,6 +191,13 @@ $(TEST_CSL_BIN): $(TEST_CSL_OBJ) $(LIB_ALLOC_SET) test-checked-slab: $(TEST_CSL_BIN) $(RUN)$(TEST_CSL_BIN) +# Same checked-slab test compiled against the SET+ATOMIC alloc lib, exercising +# the in-memory mutex and the try_discard / try_extend_zeros tail paths. +test-checked-slab-atomic: $(LIB_ALLOC_SET_ATOMIC) + $(CC) $(CFLAGS) -DBSTACK_FEATURE_SET -DBSTACK_FEATURE_ATOMIC -c -o $(TEST_CSL_ATOMIC_OBJ) test_checked_slab.c + $(CC) $(CFLAGS) -DBSTACK_FEATURE_SET -DBSTACK_FEATURE_ATOMIC -o $(TEST_CSL_ATOMIC_BIN) $(TEST_CSL_ATOMIC_OBJ) -L. -lbstack-alloc-set-atomic -lpthread + $(RUN)$(TEST_CSL_ATOMIC_BIN) + $(OBJ_BYTEVEC): bstack_bytevec.c bstack_bytevec.h bstack_alloc.h bstack.h $(CC) $(CFLAGS) -DBSTACK_FEATURE_SET -c -o $@ $< @@ -305,6 +329,8 @@ clean: test_first_fit-atomic.o test_first_fit-atomic$(EXE_EXT) \ $(TEST_GT_OBJ) $(TEST_GT_BIN) \ $(TEST_SL_OBJ) $(TEST_SL_BIN) \ - $(TEST_CSL_OBJ) $(TEST_CSL_BIN) \ + $(TEST_SL_ATOMIC_OBJ) $(TEST_SL_ATOMIC_BIN) \ + $(TEST_CSL_OBJ) $(TEST_CSL_BIN) \ + $(TEST_CSL_ATOMIC_OBJ) $(TEST_CSL_ATOMIC_BIN) \ $(OBJ_BYTEVEC) $(LIB_BYTEVEC_SET) $(TEST_BV_OBJ) $(TEST_BV_BIN) \ $(EXAMPLE_BINS) $(HASHMAP_BIN) $(ATOMIC_BIN) $(MOVE_COW_BIN) $(LINKED_LIST_BIN) $(CHECKSUMMED_CACHE_BIN) diff --git a/c/bstack_alloc.c b/c/bstack_alloc.c index 7c96893..9e5c7ab 100644 --- a/c/bstack_alloc.c +++ b/c/bstack_alloc.c @@ -2900,6 +2900,43 @@ bstack_t *ghost_tree_bstack_allocator_into_stack(ghost_tree_bstack_allocator_t * return bs; } +/* ========================================================================= + * Mutex helpers shared by slab and checked_slab allocators. + * Mirrors the pattern used by first_fit_bstack_allocator_t. + * Under BSTACK_FEATURE_ATOMIC these allocate/free the opaque lock field; + * without it they compile away to nothing. + * ====================================================================== */ + +#ifdef BSTACK_FEATURE_ATOMIC +static int bstack_alloc_lock_init(void **out) +{ +# ifdef _WIN32 + CRITICAL_SECTION *cs = malloc(sizeof *cs); + if (!cs) { errno = ENOMEM; return -1; } + InitializeCriticalSection(cs); + *out = cs; + return 0; +# else + pthread_mutex_t *m = malloc(sizeof *m); + if (!m) { errno = ENOMEM; return -1; } + if (pthread_mutex_init(m, NULL) != 0) { free(m); errno = EINVAL; return -1; } + *out = m; + return 0; +# endif +} + +static void bstack_alloc_lock_destroy(void *lock) +{ + if (!lock) return; +# ifdef _WIN32 + DeleteCriticalSection((CRITICAL_SECTION *)lock); +# else + pthread_mutex_destroy((pthread_mutex_t *)lock); +# endif + free(lock); +} +#endif /* BSTACK_FEATURE_ATOMIC */ + /* ========================================================================= * slab_bstack_allocator_t — fixed-block slab allocator * Requires -DBSTACK_FEATURE_SET (depends on bstack_set and bstack_zero). @@ -2915,7 +2952,7 @@ bstack_t *ghost_tree_bstack_allocator_into_stack(ghost_tree_bstack_allocator_t * #define SLAB_MIN_BLOCK_SIZE UINT64_C(8) #define SLAB_SENTINEL UINT64_C(0) -static const uint8_t alsl_magic[8] = {'A','L','S','L',0,1,0,0}; +static const uint8_t alsl_magic[8] = {'A','L','S','L',0,1,1,0}; static const uint8_t alsl_magic_prefix[6] = {'A','L','S','L',0,1}; /* ---- helpers ----------------------------------------------------------- */ @@ -3054,11 +3091,17 @@ static int slab_vtbl_alloc(bstack_allocator_t *base, uint64_t len, } if (len <= a->block_size) { - if (slab_pop_free_block(a->bs, a->block_size, &block) != 0) return -1; + int pop_r; + FF_LOCK(a); + pop_r = slab_pop_free_block(a->bs, a->block_size, &block); + FF_UNLOCK(a); + if (pop_r != 0) return -1; if (block != SLAB_SENTINEL) { out->allocator = base; out->offset = block; out->len = len; return 0; } + /* Free list empty: extend tail (no lock needed — bstack_extend is + * internally serialised and returns a distinct region per call). */ #if UINT64_MAX > SIZE_MAX if (a->block_size > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } #endif @@ -3082,7 +3125,7 @@ static int slab_vtbl_alloc(bstack_allocator_t *base, uint64_t len, static int slab_vtbl_dealloc(bstack_allocator_t *base, bstack_slice_t s) { slab_bstack_allocator_t *a = (slab_bstack_allocator_t *)base; - uint64_t n_blocks, backing_size, tail; + uint64_t n_blocks, backing_size; if (s.len == 0 && s.offset == SLAB_SENTINEL) return 0; @@ -3090,27 +3133,46 @@ static int slab_vtbl_dealloc(bstack_allocator_t *base, bstack_slice_t s) if (n_blocks > UINT64_MAX / a->block_size) { errno = EINVAL; return -1; } backing_size = n_blocks * a->block_size; - if (bstack_len(a->bs, &tail) != 0) return -1; - + /* Tail discard path: only for oversized allocations (> 1 block). + * try_discard atomically checks tail == slice_end and removes backing_size + * bytes under BStack's own write lock — no allocator lock needed. */ if (s.len > a->block_size) { if (s.offset > UINT64_MAX - backing_size) { errno = EINVAL; return -1; } - if (s.offset + backing_size == tail) { #if UINT64_MAX > SIZE_MAX - if (backing_size > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } + if (backing_size > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } #endif - return bstack_discard(a->bs, (size_t)backing_size); +#ifdef BSTACK_FEATURE_ATOMIC + { + int ok = 0; + if (bstack_try_discard(a->bs, s.offset + backing_size, + (size_t)backing_size, &ok) != 0) return -1; + if (ok) return 0; + } +#else + { + uint64_t tail; + if (bstack_len(a->bs, &tail) != 0) return -1; + if (s.offset + backing_size == tail) + return bstack_discard(a->bs, (size_t)backing_size); } +#endif } - return slab_push_free_blocks(a->bs, s.offset, n_blocks, a->block_size); + /* Not at tail (or single-block): push to the free list under the lock. */ + { + int r; + FF_LOCK(a); + r = slab_push_free_blocks(a->bs, s.offset, n_blocks, a->block_size); + FF_UNLOCK(a); + return r; + } } static int slab_vtbl_realloc(bstack_allocator_t *base, bstack_slice_t s, uint64_t new_len, bstack_slice_t *out) { slab_bstack_allocator_t *a = (slab_bstack_allocator_t *)base; - uint64_t old_n, new_n, old_backing, new_backing, tail; - int is_tail; + uint64_t old_n, new_n, old_backing, new_backing; if (s.len == 0 && s.offset == SLAB_SENTINEL) return slab_vtbl_alloc(base, new_len, out); @@ -3127,6 +3189,8 @@ static int slab_vtbl_realloc(bstack_allocator_t *base, bstack_slice_t s, new_n = slab_blocks_needed(new_len, a->block_size); if (old_n == new_n) { + /* Same backing blocks: zero newly-exposed bytes on grow (lock-free: + * zero stays within the caller's allocated region). */ if (new_len > s.len) { uint64_t to_zero = new_len - s.len; #if UINT64_MAX > SIZE_MAX @@ -3143,17 +3207,39 @@ static int slab_vtbl_realloc(bstack_allocator_t *base, bstack_slice_t s, old_backing = old_n * a->block_size; if (new_n > UINT64_MAX / a->block_size) { errno = EINVAL; return -1; } new_backing = new_n * a->block_size; - if (bstack_len(a->bs, &tail) != 0) return -1; if (s.offset > UINT64_MAX - old_backing) { errno = EINVAL; return -1; } - is_tail = (s.offset + old_backing == tail); - if (is_tail) { - if (new_n > old_n) { - uint64_t to_extend = new_backing - old_backing; + if (new_n > old_n) { + /* Grow path. + * With atomic: try_extend_zeros atomically checks tail == sentinel and + * appends the delta — no allocator lock needed. + * Without atomic: plain len() check then extend (single-threaded). */ + uint64_t to_extend = new_backing - old_backing; + uint64_t sentinel = s.offset + old_backing; + int tail_done = 0; #if UINT64_MAX > SIZE_MAX - if (to_extend > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } + if (to_extend > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } #endif - if (bstack_extend(a->bs, (size_t)to_extend, NULL) != 0) return -1; + +#ifdef BSTACK_FEATURE_ATOMIC + { + int ok = 0; + if (bstack_try_extend_zeros(a->bs, sentinel, + (size_t)to_extend, &ok) != 0) return -1; + if (ok) tail_done = 1; + } +#else + { + uint64_t tail; + if (bstack_len(a->bs, &tail) != 0) return -1; + if (sentinel == tail) { + if (bstack_extend(a->bs, (size_t)to_extend, NULL) != 0) return -1; + tail_done = 1; + } + } +#endif + + if (tail_done) { if (new_len > s.len) { uint64_t to_zero = new_len - s.len; #if UINT64_MAX > SIZE_MAX @@ -3162,56 +3248,95 @@ static int slab_vtbl_realloc(bstack_allocator_t *base, bstack_slice_t s, if (bstack_zero(a->bs, s.offset + s.len, (size_t)to_zero) != 0) return -1; } - } else { - uint64_t to_discard = old_backing - new_backing; + out->allocator = base; out->offset = s.offset; out->len = new_len; + return 0; + } + + /* Not at tail (or tail moved under atomic): grow non-tail. + * Read old data into a zeroed new_backing-sized buffer, push it as a + * single atomic write, then free the old blocks under the lock. */ + { + uint8_t *buf; + uint64_t push_offset; + int r; + #if UINT64_MAX > SIZE_MAX - if (to_discard > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } + if (new_backing > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } #endif - if (bstack_discard(a->bs, (size_t)to_discard) != 0) return -1; - } - out->allocator = base; out->offset = s.offset; out->len = new_len; - return 0; - } + buf = calloc(1, (size_t)new_backing); + if (!buf) return -1; - if (new_n < old_n) { - /* Shrink non-tail: return excess blocks to free list. */ - if (slab_push_free_blocks(a->bs, s.offset + new_n * a->block_size, - old_n - new_n, a->block_size) != 0) - return -1; - out->allocator = base; out->offset = s.offset; out->len = new_len; - return 0; + if (s.len > 0) { + if (bstack_get(a->bs, s.offset, s.offset + s.len, buf) != 0) { + free(buf); return -1; + } + } + + if (bstack_push(a->bs, buf, (size_t)new_backing, &push_offset) != 0) { + free(buf); return -1; + } + free(buf); + + FF_LOCK(a); + r = slab_push_free_blocks(a->bs, s.offset, old_n, a->block_size); + FF_UNLOCK(a); + if (r != 0) return -1; + + out->allocator = base; + out->offset = push_offset; + out->len = new_len; + return 0; + } } - /* Grow non-tail: read old data into a zeroed new_backing-sized buffer, - * push it as a single atomic write, then free the old blocks. */ + /* Shrink path (new_n < old_n). + * With atomic: try_discard atomically checks tail == sentinel and removes + * the excess — no lock needed. On failure the slice is not at the tail; + * fall through to shrink non-tail. + * Without atomic: plain len() check then discard (single-threaded). */ { - uint8_t *buf; - uint64_t push_offset; - + uint64_t to_discard = old_backing - new_backing; + uint64_t sentinel = s.offset + old_backing; + int tail_done = 0; #if UINT64_MAX > SIZE_MAX - if (new_backing > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } + if (to_discard > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } #endif - buf = calloc(1, (size_t)new_backing); - if (!buf) return -1; - if (s.len > 0) { - if (bstack_get(a->bs, s.offset, s.offset + s.len, buf) != 0) { - free(buf); return -1; +#ifdef BSTACK_FEATURE_ATOMIC + { + int ok = 0; + if (bstack_try_discard(a->bs, sentinel, + (size_t)to_discard, &ok) != 0) return -1; + if (ok) tail_done = 1; + } +#else + { + uint64_t tail; + if (bstack_len(a->bs, &tail) != 0) return -1; + if (sentinel == tail) { + if (bstack_discard(a->bs, (size_t)to_discard) != 0) return -1; + tail_done = 1; } } +#endif - if (bstack_push(a->bs, buf, (size_t)new_backing, &push_offset) != 0) { - free(buf); return -1; + if (tail_done) { + out->allocator = base; out->offset = s.offset; out->len = new_len; + return 0; } - free(buf); - if (slab_push_free_blocks(a->bs, s.offset, old_n, a->block_size) != 0) - return -1; - - out->allocator = base; - out->offset = push_offset; - out->len = new_len; - return 0; + /* Shrink non-tail: return excess blocks to free list under the lock. */ + { + int r; + FF_LOCK(a); + r = slab_push_free_blocks(a->bs, + s.offset + new_n * a->block_size, + old_n - new_n, a->block_size); + FF_UNLOCK(a); + if (r != 0) return -1; + out->allocator = base; out->offset = s.offset; out->len = new_len; + return 0; + } } } @@ -3242,12 +3367,21 @@ slab_bstack_allocator_t *slab_bstack_allocator_new(bstack_t *bs, a = malloc(sizeof *a); if (!a) { errno = ENOMEM; return NULL; } +#ifdef BSTACK_FEATURE_ATOMIC + if (bstack_alloc_lock_init(&a->lock) != 0) { free(a); return NULL; } +#endif + memset(hdr, 0, sizeof hdr); memcpy(hdr + (size_t)SLAB_OFFSET_SIZE, alsl_magic, 8); write_le64(hdr + (size_t)SLAB_BLOCK_SIZE_OFFSET, block_size); /* free_head at SLAB_FREE_HEAD_OFFSET stays 0 (SLAB_SENTINEL) */ - if (bstack_push(bs, hdr, sizeof hdr, NULL) != 0) { free(a); return NULL; } + if (bstack_push(bs, hdr, sizeof hdr, NULL) != 0) { +#ifdef BSTACK_FEATURE_ATOMIC + bstack_alloc_lock_destroy(a->lock); +#endif + free(a); return NULL; + } a->base.vtbl = &slab_allocator_vtbl; a->base.bulk_vtbl = NULL; @@ -3301,6 +3435,10 @@ slab_bstack_allocator_t *slab_bstack_allocator_open(bstack_t *bs) return NULL; } +#ifdef BSTACK_FEATURE_ATOMIC + if (bstack_alloc_lock_init(&a->lock) != 0) { free(a); return NULL; } +#endif + a->base.vtbl = &slab_allocator_vtbl; a->base.bulk_vtbl = NULL; a->bs = bs; @@ -3310,12 +3448,18 @@ slab_bstack_allocator_t *slab_bstack_allocator_open(bstack_t *bs) void slab_bstack_allocator_free(slab_bstack_allocator_t *alloc) { +#ifdef BSTACK_FEATURE_ATOMIC + bstack_alloc_lock_destroy(alloc->lock); +#endif free(alloc); } bstack_t *slab_bstack_allocator_into_stack(slab_bstack_allocator_t *alloc) { bstack_t *bs = alloc->bs; +#ifdef BSTACK_FEATURE_ATOMIC + bstack_alloc_lock_destroy(alloc->lock); +#endif free(alloc); return bs; } @@ -3345,7 +3489,7 @@ uint64_t slab_bstack_allocator_block_size(const slab_bstack_allocator_t *alloc) /* Maximum number of suspect blocks analysed in resync_tail before giving up. */ #define ALCK_MAX_RECOVER_REGION ((size_t)(1u << 26)) -static const uint8_t alck_magic[8] = {'A','L','C','K',0,1,0,0}; +static const uint8_t alck_magic[8] = {'A','L','C','K',0,1,1,0}; static const uint8_t alck_magic_prefix[6] = {'A','L','C','K',0,1}; /* ---- LE helpers (reuse read_le64 / write_le64 already defined above) --- */ @@ -3756,14 +3900,18 @@ int checked_slab_bstack_allocator_recover(checked_slab_bstack_allocator_t *alloc size_t i; int ret = 0; - if (bstack_len(bs, &stack_len) != 0) return -1; + FF_LOCK(alloc); + + if (bstack_len(bs, &stack_len) != 0) { FF_UNLOCK(alloc); return -1; } if (stack_len <= ALCK_ARENA_START) { if (out_unsure) *out_unsure = 0; - return 0; + FF_UNLOCK(alloc); return 0; } if (alck_scan_free_list(bs, stack_len, block_size, - &free_arr, &free_cnt, &free_corrupt) != 0) return -1; + &free_arr, &free_cnt, &free_corrupt) != 0) { + FF_UNLOCK(alloc); return -1; + } p = ALCK_ARENA_START; while (p < stack_len) { @@ -3860,6 +4008,7 @@ int checked_slab_bstack_allocator_recover(checked_slab_bstack_allocator_t *alloc done: free(free_arr); free(reclaim); + FF_UNLOCK(alloc); return ret; } @@ -3885,9 +4034,15 @@ static int alck_vt_alloc(bstack_allocator_t *base, uint64_t len, if (num_blocks == UINT64_MAX) { errno = EINVAL; return -1; } /* overflow */ if (num_blocks == 1) { + /* Lock is scoped to the free-list pop only; the tail-extend fallback + * below runs without the lock (bstack_extend is internally serialised + * and returns a distinct region to each concurrent caller). */ uint64_t block_start; - if (alck_pop_and_claim_block(a->bs, 1, a->block_size, &block_start) != 0) - return -1; + int pop_r; + FF_LOCK(a); + pop_r = alck_pop_and_claim_block(a->bs, 1, a->block_size, &block_start); + FF_UNLOCK(a); + if (pop_r != 0) return -1; if (block_start != ALCK_SENTINEL) { out->allocator = base; out->offset = block_start + ALCK_OVERHEAD; @@ -3926,13 +4081,15 @@ static int alck_vt_alloc(bstack_allocator_t *base, uint64_t len, static int alck_vt_dealloc(bstack_allocator_t *base, bstack_slice_t s) { checked_slab_bstack_allocator_t *a = (checked_slab_bstack_allocator_t *)base; - uint64_t block_start, overhead, num_blocks, backing, tail, slice_end; + uint64_t block_start, overhead, num_blocks, backing, slice_end; if (s.len == 0 && s.offset == 0) return 0; if (s.offset < ALCK_OVERHEAD) { errno = EINVAL; return -1; } block_start = s.offset - ALCK_OVERHEAD; + /* read_overhead is a single bstack read from a block owned by the caller; + * no lock required here. */ if (alck_read_overhead(a->bs, block_start, &overhead) != 0) return -1; if ((overhead & ALCK_IN_USE_BIT) == 0) { errno = EINVAL; return -1; } @@ -3942,18 +4099,39 @@ static int alck_vt_dealloc(bstack_allocator_t *base, bstack_slice_t s) if (num_blocks > UINT64_MAX / a->block_size) { errno = EINVAL; return -1; } backing = num_blocks * a->block_size; - if (bstack_len(a->bs, &tail) != 0) return -1; if (block_start > UINT64_MAX - backing) { errno = EINVAL; return -1; } slice_end = block_start + backing; - if (slice_end == tail) { #if UINT64_MAX > SIZE_MAX - if (backing > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } + if (backing > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } #endif - return bstack_discard(a->bs, (size_t)backing); + + /* Tail path: try_discard atomically checks tail == slice_end and removes + * backing bytes under bstack's own write lock — no allocator lock needed. */ +#ifdef BSTACK_FEATURE_ATOMIC + { + int ok = 0; + if (bstack_try_discard(a->bs, slice_end, (size_t)backing, &ok) != 0) + return -1; + if (ok) return 0; + } +#else + { + uint64_t tail; + if (bstack_len(a->bs, &tail) != 0) return -1; + if (slice_end == tail) + return bstack_discard(a->bs, (size_t)backing); } +#endif - return alck_push_free_blocks(a->bs, block_start, num_blocks, a->block_size); + /* Not at tail: push to the free list under the allocator lock. */ + { + int r; + FF_LOCK(a); + r = alck_push_free_blocks(a->bs, block_start, num_blocks, a->block_size); + FF_UNLOCK(a); + return r; + } } static int alck_vt_realloc(bstack_allocator_t *base, bstack_slice_t s, @@ -3961,8 +4139,7 @@ static int alck_vt_realloc(bstack_allocator_t *base, bstack_slice_t s, { checked_slab_bstack_allocator_t *a = (checked_slab_bstack_allocator_t *)base; uint64_t block_start, overhead, old_n, new_n, old_backing, new_backing; - uint64_t tail; - int is_tail; + /* tail and is_tail are now computed per-path in inner scopes */ if (s.len == 0 && s.offset == 0) return alck_vt_alloc(base, new_len, out); @@ -4005,85 +4182,166 @@ static int alck_vt_realloc(bstack_allocator_t *base, bstack_slice_t s, old_backing = old_n * a->block_size; if (new_n > UINT64_MAX / a->block_size) { errno = EINVAL; return -1; } new_backing = new_n * a->block_size; - - if (bstack_len(a->bs, &tail) != 0) return -1; if (block_start > UINT64_MAX - old_backing) { errno = EINVAL; return -1; } - is_tail = (block_start + old_backing == tail); - if (is_tail) { + /* Precompute the expected tail for this allocation (sentinel). */ + { + uint64_t sentinel = block_start + old_backing; + if (new_n > old_n) { - uint64_t delta = new_backing - old_backing; - uint64_t to_zero; + /* Grow path. + * With atomic: try_extend_zeros atomically checks tail == sentinel + * and appends the delta under bstack's write lock — no allocator + * lock needed. write_overhead then writes only to the exclusively- + * owned newly-extended region. If try_extend_zeros returns false + * the tail has moved and we are no longer the tail block — fall + * through to grow non-tail. + * Without atomic: plain len() check then extend (single-threaded). */ + uint64_t delta = new_backing - old_backing; + int tail_done = 0; #if UINT64_MAX > SIZE_MAX if (delta > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } #endif - if (bstack_extend(a->bs, (size_t)delta, NULL) != 0) return -1; - if (new_len > s.len) { - to_zero = new_len - s.len; + +#ifdef BSTACK_FEATURE_ATOMIC + { + int ok = 0; + if (bstack_try_extend_zeros(a->bs, sentinel, + (size_t)delta, &ok) != 0) return -1; + if (ok) tail_done = 1; + } +#else + { + uint64_t cur_tail; + if (bstack_len(a->bs, &cur_tail) != 0) return -1; + if (sentinel == cur_tail) { + if (bstack_extend(a->bs, (size_t)delta, NULL) != 0) return -1; + tail_done = 1; + } + } +#endif + + if (tail_done) { + if (new_len > s.len) { + uint64_t to_zero = new_len - s.len; #if UINT64_MAX > SIZE_MAX - if (to_zero > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } + if (to_zero > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } #endif - if (bstack_zero(a->bs, s.offset + s.len, (size_t)to_zero) != 0) - return -1; + if (bstack_zero(a->bs, s.offset + s.len, + (size_t)to_zero) != 0) return -1; + } + if (alck_write_overhead(a->bs, block_start, + ALCK_IN_USE_BIT | new_n) != 0) return -1; + out->allocator = base; out->offset = s.offset; out->len = new_len; + return 0; } - if (alck_write_overhead(a->bs, block_start, - ALCK_IN_USE_BIT | new_n) != 0) return -1; - } else { - /* Shrink: update overhead first, then discard (crash-safer) */ - uint64_t delta = old_backing - new_backing; - if (alck_write_overhead(a->bs, block_start, - ALCK_IN_USE_BIT | new_n) != 0) return -1; + + /* Not at tail (or tail moved under atomic): grow non-tail. + * alloc and dealloc each acquire the lock for their own free-list + * and tail operations independently. */ + { + bstack_slice_t new_s; + uint8_t *tmp; + uint64_t copy_len; + + if (alck_vt_alloc(base, new_len, &new_s) != 0) return -1; + + copy_len = s.len < new_len ? s.len : new_len; + if (copy_len > 0) { #if UINT64_MAX > SIZE_MAX - if (delta > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } + if (copy_len > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } #endif - if (bstack_discard(a->bs, (size_t)delta) != 0) return -1; - } - out->allocator = base; out->offset = s.offset; out->len = new_len; - return 0; - } - - if (new_n < old_n) { - /* Shrink non-tail: commit the new overhead first, then build free run. - * This is the commit point — before it the old view is fully intact. */ - uint64_t excess_start; - if (new_n > UINT64_MAX / a->block_size) { errno = EINVAL; return -1; } - excess_start = block_start + new_n * a->block_size; + tmp = malloc((size_t)copy_len); + if (!tmp) return -1; + if (bstack_get(a->bs, s.offset, s.offset + copy_len, tmp) != 0 + || bstack_set(a->bs, new_s.offset, tmp, + (size_t)copy_len) != 0) { + free(tmp); return -1; + } + free(tmp); + } - if (alck_write_overhead(a->bs, block_start, - ALCK_IN_USE_BIT | new_n) != 0) return -1; - if (alck_write_free_run(a->bs, excess_start, old_n - new_n, - a->block_size) != 0) return -1; - if (alck_write_free_head(a->bs, excess_start) != 0) return -1; + if (alck_vt_dealloc(base, s) != 0) return -1; + *out = new_s; + return 0; + } + } - out->allocator = base; out->offset = s.offset; out->len = new_len; - return 0; - } + /* Shrink path (new_n < old_n). The lock covers the overhead write, + * tail check, and non-tail free-list update so those steps are atomic + * w.r.t. other threads. */ + { + uint64_t delta = old_backing - new_backing; + uint64_t excess_start; + int r; +#if UINT64_MAX > SIZE_MAX + if (delta > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } +#endif - /* Grow non-tail: alloc a fresh region, copy data, release the old */ - { - bstack_slice_t new_s; - uint8_t *tmp; - uint64_t copy_len; + FF_LOCK(a); - if (alck_vt_alloc(base, new_len, &new_s) != 0) return -1; + /* Overhead is the commit point for both tail and non-tail paths: + * write it first so a crash after this point leaves an orphaned + * (but safely unreferenced) tail region or leaked blocks that + * recover() can reclaim, rather than an overhead that claims more + * blocks than the file contains. */ + if (alck_write_overhead(a->bs, block_start, + ALCK_IN_USE_BIT | new_n) != 0) { + FF_UNLOCK(a); return -1; + } - copy_len = s.len < new_len ? s.len : new_len; - if (copy_len > 0) { -#if UINT64_MAX > SIZE_MAX - if (copy_len > (uint64_t)SIZE_MAX) { errno = EINVAL; return -1; } + /* Tail shrink: try_discard atomically checks tail == sentinel and + * removes the excess under bstack's write lock, so no other thread + * can race between the check and the truncation. On failure the + * slice is not at the tail; fall through to recycle excess blocks. */ +#ifdef BSTACK_FEATURE_ATOMIC + { + int ok = 0; + if (bstack_try_discard(a->bs, sentinel, (size_t)delta, + &ok) != 0) { + FF_UNLOCK(a); return -1; + } + if (ok) { + FF_UNLOCK(a); + out->allocator = base; out->offset = s.offset; + out->len = new_len; + return 0; + } + } +#else + { + uint64_t cur_tail; + if (bstack_len(a->bs, &cur_tail) != 0) { + FF_UNLOCK(a); return -1; + } + if (sentinel == cur_tail) { + if (bstack_discard(a->bs, (size_t)delta) != 0) { + FF_UNLOCK(a); return -1; + } + FF_UNLOCK(a); + out->allocator = base; out->offset = s.offset; + out->len = new_len; + return 0; + } + } #endif - tmp = malloc((size_t)copy_len); - if (!tmp) return -1; - if (bstack_get(a->bs, s.offset, s.offset + copy_len, tmp) != 0 - || bstack_set(a->bs, new_s.offset, tmp, (size_t)copy_len) != 0) { - free(tmp); return -1; + + /* Shrink non-tail: overhead already written (commit point); write + * free-list metadata into the excess blocks and repoint free_head. */ + if (new_n > UINT64_MAX / a->block_size) { + FF_UNLOCK(a); errno = EINVAL; return -1; } - free(tmp); + excess_start = block_start + new_n * a->block_size; + if (alck_write_free_run(a->bs, excess_start, + old_n - new_n, a->block_size) != 0) { + FF_UNLOCK(a); return -1; + } + r = alck_write_free_head(a->bs, excess_start); + FF_UNLOCK(a); + if (r != 0) return -1; + out->allocator = base; out->offset = s.offset; out->len = new_len; + return 0; } - - if (alck_vt_dealloc(base, s) != 0) return -1; - *out = new_s; - return 0; } } @@ -4118,12 +4376,21 @@ checked_slab_bstack_allocator_t *checked_slab_bstack_allocator_new( a = malloc(sizeof *a); if (!a) { errno = ENOMEM; return NULL; } +#ifdef BSTACK_FEATURE_ATOMIC + if (bstack_alloc_lock_init(&a->lock) != 0) { free(a); return NULL; } +#endif + memset(hdr, 0, sizeof hdr); memcpy(hdr + (size_t)ALCK_OFFSET_SIZE, alck_magic, 8); write_le64(hdr + (size_t)ALCK_OFFSET_SIZE + 8, block_size); /* free_head at ALCK_FREE_HEAD_OFFSET stays 0 (ALCK_SENTINEL) */ - if (bstack_push(bs, hdr, sizeof hdr, NULL) != 0) { free(a); return NULL; } + if (bstack_push(bs, hdr, sizeof hdr, NULL) != 0) { +#ifdef BSTACK_FEATURE_ATOMIC + bstack_alloc_lock_destroy(a->lock); +#endif + free(a); return NULL; + } a->base.vtbl = &alck_allocator_vtbl; a->base.bulk_vtbl = NULL; @@ -4181,6 +4448,11 @@ checked_slab_bstack_allocator_t *checked_slab_bstack_allocator_open(bstack_t *bs a = malloc(sizeof *a); if (!a) { errno = ENOMEM; return NULL; } + +#ifdef BSTACK_FEATURE_ATOMIC + if (bstack_alloc_lock_init(&a->lock) != 0) { free(a); return NULL; } +#endif + a->base.vtbl = &alck_allocator_vtbl; a->base.bulk_vtbl = NULL; a->bs = bs; @@ -4188,6 +4460,9 @@ checked_slab_bstack_allocator_t *checked_slab_bstack_allocator_open(bstack_t *bs /* Auto-recover: reclaim leaked blocks and repair orphaned tails */ if (checked_slab_bstack_allocator_recover(a, NULL) != 0) { +#ifdef BSTACK_FEATURE_ATOMIC + bstack_alloc_lock_destroy(a->lock); +#endif free(a); return NULL; } return a; @@ -4195,6 +4470,9 @@ checked_slab_bstack_allocator_t *checked_slab_bstack_allocator_open(bstack_t *bs void checked_slab_bstack_allocator_free(checked_slab_bstack_allocator_t *alloc) { +#ifdef BSTACK_FEATURE_ATOMIC + bstack_alloc_lock_destroy(alloc->lock); +#endif free(alloc); } @@ -4202,6 +4480,9 @@ bstack_t *checked_slab_bstack_allocator_into_stack( checked_slab_bstack_allocator_t *alloc) { bstack_t *bs = alloc->bs; +#ifdef BSTACK_FEATURE_ATOMIC + bstack_alloc_lock_destroy(alloc->lock); +#endif free(alloc); return bs; } diff --git a/c/bstack_alloc.h b/c/bstack_alloc.h index 148beaa..058ddc3 100644 --- a/c/bstack_alloc.h +++ b/c/bstack_alloc.h @@ -696,7 +696,7 @@ bstack_t *ghost_tree_bstack_allocator_into_stack(ghost_tree_bstack_allocator_t * * * On-disk layout (all within the bstack payload): * [0..24) — reserved (OFFSET_SIZE; available for caller use) - * [24..32) — magic: "ALSL\x00\x01\x00\x00" + * [24..32) — magic: "ALSL\x00\x01\x01\x00" * [32..40) — block_size (8 B LE) * [40..48) — free_head (8 B LE) — payload offset of first free block, or 0 * [48..) — block arena @@ -716,6 +716,16 @@ bstack_t *ghost_tree_bstack_allocator_into_stack(ghost_tree_bstack_allocator_t * * next-pointer then update free_head). A crash between the two leaks the * block being added or removed but leaves the rest of the list intact. * + * Thread safety: without -DBSTACK_FEATURE_ATOMIC, an allocator handle must be + * used from one thread at a time — free-list mutations are a read then a write + * of free_head as separate bstack calls, a TOCTOU race under concurrent access + * that can result in two callers receiving the same block. With + * -DBSTACK_FEATURE_ATOMIC the handle owns an in-memory mutex (lock) that + * serialises all compound operations spanning multiple bstack calls: free-list + * pop/push and the tail-length checks preceding extend or discard. bstack + * extend / discard are internally serialised by bstack's own write lock; the + * allocator lock is not held during those calls. + * * Requires -DBSTACK_FEATURE_SET. * ====================================================================== */ @@ -723,6 +733,11 @@ typedef struct { bstack_allocator_t base; /* must be first — safe cast to bstack_allocator_t * */ bstack_t *bs; uint64_t block_size; +#ifdef BSTACK_FEATURE_ATOMIC + /* Opaque platform mutex; serialises free-list pop/push and tail operations + * so a single handle may be shared across threads. */ + void *lock; +#endif } slab_bstack_allocator_t; /* @@ -778,7 +793,7 @@ uint64_t slab_bstack_allocator_block_size(const slab_bstack_allocator_t *alloc); * On-disk layout (all within the bstack payload): * [0..24) — reserved (OFFSET_SIZE; available for caller use) * [24..48) — allocator header: magic[8] | block_size[8] | free_head[8] - * magic = "ALCK\x00\x01\x00\x00" + * magic = "ALCK\x00\x01\x01\x00" * [48..) — block arena * * Each block in the arena: @@ -803,6 +818,15 @@ uint64_t slab_bstack_allocator_block_size(const slab_bstack_allocator_t *alloc); * the rest of the list stays intact. checked_slab_bstack_allocator_recover * reclaims leaked blocks by a linear arena scan. * + * Thread safety: without -DBSTACK_FEATURE_ATOMIC, an allocator handle must be + * used from one thread at a time — free-list mutations read then write free_head + * as separate bstack calls, a TOCTOU race under concurrent access. With + * -DBSTACK_FEATURE_ATOMIC the handle owns an in-memory mutex (lock) that + * serialises all compound operations: free-list pop/push, the tail-length checks + * preceding extend or discard, and the recover scan. bstack_try_discard and + * bstack_try_extend_zeros are used for the tail paths — those check-and-act + * atomically under bstack's own write lock without the allocator lock. + * * Requires -DBSTACK_FEATURE_SET. * ====================================================================== */ @@ -810,6 +834,11 @@ typedef struct { bstack_allocator_t base; /* must be first — safe cast to bstack_allocator_t * */ bstack_t *bs; uint64_t block_size; +#ifdef BSTACK_FEATURE_ATOMIC + /* Opaque platform mutex; serialises free-list pop/push and tail operations + * so a single handle may be shared across threads. */ + void *lock; +#endif } checked_slab_bstack_allocator_t; /* diff --git a/c/test_checked_slab.c b/c/test_checked_slab.c index ddc41b2..cf73823 100644 --- a/c/test_checked_slab.c +++ b/c/test_checked_slab.c @@ -936,6 +936,168 @@ static int fuzz_persist_reopen(uint64_t data_size) static int test_fuzz_persist_8(void) { return fuzz_persist_reopen(8); } static int test_fuzz_persist_16(void) { return fuzz_persist_reopen(16); } +/* ========================================================================= + * Concurrent tests (requires -DBSTACK_FEATURE_ATOMIC) + * + * Without ATOMIC the allocator is not Sync: free-list mutations span two + * bstack calls with no lock, so sharing one allocator across threads is + * undefined and these tests would race on the on-disk free list with no + * protection. + * ====================================================================== */ + +#ifdef BSTACK_FEATURE_ATOMIC +#include + +#define CSL_THREADS 8 +#define CSL_ITERS 200 + +typedef struct { + bstack_allocator_t *a; + int tid; + int ok; +} csl_worker_arg_t; + +/* --- concurrent alloc / dealloc data integrity ------------------------ */ + +static void *csl_alloc_dealloc_worker(void *raw) +{ + csl_worker_arg_t *w = raw; + uint8_t pat = (uint8_t)w->tid; + w->ok = 1; + + for (int i = 0; i < CSL_ITERS; i++) { + bstack_slice_t s; + if (bstack_allocator_alloc(w->a, 8, &s) != 0) { w->ok = 0; return NULL; } + + uint8_t wbuf[8], rbuf[8]; + memset(wbuf, pat, 8); + if (bstack_slice_write(s, wbuf, 8) != 0 || + bstack_slice_read(s, rbuf) != 0 || + memcmp(wbuf, rbuf, 8) != 0) { + w->ok = 0; return NULL; + } + if (bstack_allocator_dealloc(w->a, s) != 0) { w->ok = 0; return NULL; } + } + return NULL; +} + +static int test_concurrent_alloc_dealloc_data_integrity(void) +{ + /* Verify that the free-list lock prevents two threads from receiving the + * same block. Each thread writes its pattern and reads it back; a + * duplicate-block race would clobber the pattern. The overhead bit also + * means a double-alloc would be caught as a double-free on dealloc. */ + char tmp[64]; make_tmp(tmp, sizeof tmp); + bstack_t *bs = bstack_open(tmp); CHECK(bs); + checked_slab_bstack_allocator_t *a = + checked_slab_bstack_allocator_new(bs, 8); + CHECK(a); + + pthread_t threads[CSL_THREADS]; + csl_worker_arg_t args[CSL_THREADS]; + for (int i = 0; i < CSL_THREADS; i++) { + args[i].a = (bstack_allocator_t *)a; + args[i].tid = i; + args[i].ok = 1; + pthread_create(&threads[i], NULL, csl_alloc_dealloc_worker, &args[i]); + } + for (int i = 0; i < CSL_THREADS; i++) pthread_join(threads[i], NULL); + for (int i = 0; i < CSL_THREADS; i++) CHECK(args[i].ok); + + bstack_close(checked_slab_bstack_allocator_into_stack(a)); + csl_unlink(tmp); return 0; +} + +/* --- concurrent realloc grow/shrink tail paths ------------------------ */ + +#define CSL_REALLOC_THREADS 6 +#define CSL_REALLOC_ITERS 150 + +typedef struct { + bstack_allocator_t *a; + int tid; + int ok; +} csl_realloc_arg_t; + +static void *csl_realloc_worker(void *raw) +{ + /* Each thread owns one allocation and repeatedly grows then shrinks it. + * Whichever allocation is at the tail exercises try_extend_zeros / + * try_discard under bstack's write lock; others hit copy-grow / + * block-recycle. data_size = 8, block_size = 16: + * alloc(8) → 1 block; alloc(32) → ceil((32+8)/16) = 3 blocks. */ + csl_realloc_arg_t *w = raw; + uint8_t pat = (uint8_t)(w->tid + 0x40); + w->ok = 1; + + bstack_slice_t s; + if (bstack_allocator_alloc(w->a, 8, &s) != 0) { w->ok = 0; return NULL; } + uint8_t init[8]; memset(init, pat, 8); + if (bstack_slice_write(s, init, 8) != 0) { w->ok = 0; return NULL; } + + for (int i = 0; i < CSL_REALLOC_ITERS; i++) { + bstack_slice_t s2, s3; + uint8_t rbuf[32]; + + /* Grow: tail → try_extend_zeros; non-tail → copy. */ + if (bstack_allocator_realloc(w->a, s, 32, &s2) != 0) { + w->ok = 0; return NULL; + } + if (bstack_slice_read(s2, rbuf) != 0) { w->ok = 0; return NULL; } + for (size_t j = 0; j < 8; j++) { + if (rbuf[j] != pat) { + fprintf(stderr, + " grow clobber tid=%d byte %zu got 0x%02x want 0x%02x\n", + w->tid, j, rbuf[j], pat); + w->ok = 0; return NULL; + } + } + + /* Shrink: tail → try_discard; non-tail → recycle excess blocks. */ + if (bstack_allocator_realloc(w->a, s2, 8, &s3) != 0) { + w->ok = 0; return NULL; + } + if (bstack_slice_read(s3, rbuf) != 0) { w->ok = 0; return NULL; } + for (size_t j = 0; j < 8; j++) { + if (rbuf[j] != pat) { + fprintf(stderr, + " shrink clobber tid=%d byte %zu got 0x%02x want 0x%02x\n", + w->tid, j, rbuf[j], pat); + w->ok = 0; return NULL; + } + } + s = s3; + } + + if (bstack_allocator_dealloc(w->a, s) != 0) { w->ok = 0; return NULL; } + return NULL; +} + +static int test_concurrent_realloc_tail_paths(void) +{ + char tmp[64]; make_tmp(tmp, sizeof tmp); + bstack_t *bs = bstack_open(tmp); CHECK(bs); + checked_slab_bstack_allocator_t *a = + checked_slab_bstack_allocator_new(bs, 8); + CHECK(a); + + pthread_t threads[CSL_REALLOC_THREADS]; + csl_realloc_arg_t args[CSL_REALLOC_THREADS]; + for (int i = 0; i < CSL_REALLOC_THREADS; i++) { + args[i].a = (bstack_allocator_t *)a; + args[i].tid = i; + args[i].ok = 1; + pthread_create(&threads[i], NULL, csl_realloc_worker, &args[i]); + } + for (int i = 0; i < CSL_REALLOC_THREADS; i++) pthread_join(threads[i], NULL); + for (int i = 0; i < CSL_REALLOC_THREADS; i++) CHECK(args[i].ok); + + bstack_close(checked_slab_bstack_allocator_into_stack(a)); + csl_unlink(tmp); return 0; +} + +#endif /* BSTACK_FEATURE_ATOMIC */ + /* ========================================================================= * main * ====================================================================== */ @@ -981,6 +1143,12 @@ int main(void) T(test_fuzz_persist_8); T(test_fuzz_persist_16); +#ifdef BSTACK_FEATURE_ATOMIC + /* ── concurrent ──────────────────────────────────────────────────── */ + T(test_concurrent_alloc_dealloc_data_integrity); + T(test_concurrent_realloc_tail_paths); +#endif + printf("\n%d / %d passed\n", g_passed, g_total); return g_passed == g_total ? 0 : 1; } diff --git a/c/test_slab.c b/c/test_slab.c index e9616e5..1fa7656 100644 --- a/c/test_slab.c +++ b/c/test_slab.c @@ -721,6 +721,165 @@ FUZZ_SUITE(8) FUZZ_SUITE(16) FUZZ_SUITE(64) +/* ========================================================================= + * Concurrent tests (requires -DBSTACK_FEATURE_ATOMIC) + * + * Without ATOMIC the allocator is not Sync: free-list mutations span two + * bstack calls with no lock, so sharing one allocator across threads is + * undefined and these tests would race on the on-disk free list with no + * protection. + * ====================================================================== */ + +#ifdef BSTACK_FEATURE_ATOMIC +#include + +#define SL_THREADS 8 +#define SL_ITERS 200 + +typedef struct { + bstack_allocator_t *a; + int tid; + int ok; +} sl_worker_arg_t; + +/* --- concurrent alloc / dealloc data integrity ------------------------ */ + +static void *sl_alloc_dealloc_worker(void *raw) +{ + sl_worker_arg_t *w = raw; + uint8_t pat = (uint8_t)w->tid; + w->ok = 1; + + for (int i = 0; i < SL_ITERS; i++) { + bstack_slice_t s; + if (bstack_allocator_alloc(w->a, 16, &s) != 0) { w->ok = 0; return NULL; } + + uint8_t wbuf[16], rbuf[16]; + memset(wbuf, pat, 16); + if (bstack_slice_write(s, wbuf, 16) != 0 || + bstack_slice_read(s, rbuf) != 0 || + memcmp(wbuf, rbuf, 16) != 0) { + w->ok = 0; return NULL; + } + if (bstack_allocator_dealloc(w->a, s) != 0) { w->ok = 0; return NULL; } + } + return NULL; +} + +static int test_concurrent_alloc_dealloc_data_integrity(void) +{ + /* Verify that the free-list lock prevents two threads from receiving the + * same block. Each thread writes its pattern after alloc and reads it + * back before dealloc; a duplicate-block race would produce a clobber. */ + char tmp[64]; make_tmp(tmp, sizeof tmp); + bstack_t *bs = bstack_open(tmp); CHECK(bs); + slab_bstack_allocator_t *a = slab_bstack_allocator_new(bs, 16); + CHECK(a); + + pthread_t threads[SL_THREADS]; + sl_worker_arg_t args[SL_THREADS]; + for (int i = 0; i < SL_THREADS; i++) { + args[i].a = (bstack_allocator_t *)a; + args[i].tid = i; + args[i].ok = 1; + pthread_create(&threads[i], NULL, sl_alloc_dealloc_worker, &args[i]); + } + for (int i = 0; i < SL_THREADS; i++) pthread_join(threads[i], NULL); + for (int i = 0; i < SL_THREADS; i++) CHECK(args[i].ok); + + bstack_close(slab_bstack_allocator_into_stack(a)); + sl_unlink(tmp); return 0; +} + +/* --- concurrent realloc grow/shrink tail paths ------------------------ */ + +#define SL_REALLOC_THREADS 6 +#define SL_REALLOC_ITERS 150 + +typedef struct { + bstack_allocator_t *a; + int tid; + int ok; +} sl_realloc_arg_t; + +static void *sl_realloc_worker(void *raw) +{ + /* Each thread owns one allocation and repeatedly grows then shrinks it. + * Whichever allocation is at the tail exercises try_extend_zeros / + * try_discard; others hit copy-grow / block-recycle paths. Both branches + * are exercised every round because threads race for the tail. + * block_size = 16: alloc(12) → 1 block; alloc(33) → 3 blocks. */ + sl_realloc_arg_t *w = raw; + uint8_t pat = (uint8_t)(w->tid + 0x40); + w->ok = 1; + + bstack_slice_t s; + if (bstack_allocator_alloc(w->a, 12, &s) != 0) { w->ok = 0; return NULL; } + uint8_t init[12]; memset(init, pat, 12); + if (bstack_slice_write(s, init, 12) != 0) { w->ok = 0; return NULL; } + + for (int i = 0; i < SL_REALLOC_ITERS; i++) { + bstack_slice_t s2, s3; + uint8_t rbuf[33]; + + /* Grow: tail → try_extend_zeros; non-tail → copy. */ + if (bstack_allocator_realloc(w->a, s, 33, &s2) != 0) { + w->ok = 0; return NULL; + } + if (bstack_slice_read(s2, rbuf) != 0) { w->ok = 0; return NULL; } + for (size_t j = 0; j < 12; j++) { + if (rbuf[j] != pat) { + fprintf(stderr, + " grow clobber tid=%d byte %zu got 0x%02x want 0x%02x\n", + w->tid, j, rbuf[j], pat); + w->ok = 0; return NULL; + } + } + + /* Shrink: tail → try_discard; non-tail → recycle excess blocks. */ + if (bstack_allocator_realloc(w->a, s2, 12, &s3) != 0) { + w->ok = 0; return NULL; + } + if (bstack_slice_read(s3, rbuf) != 0) { w->ok = 0; return NULL; } + for (size_t j = 0; j < 12; j++) { + if (rbuf[j] != pat) { + fprintf(stderr, + " shrink clobber tid=%d byte %zu got 0x%02x want 0x%02x\n", + w->tid, j, rbuf[j], pat); + w->ok = 0; return NULL; + } + } + s = s3; + } + + if (bstack_allocator_dealloc(w->a, s) != 0) { w->ok = 0; return NULL; } + return NULL; +} + +static int test_concurrent_realloc_tail_paths(void) +{ + char tmp[64]; make_tmp(tmp, sizeof tmp); + bstack_t *bs = bstack_open(tmp); CHECK(bs); + slab_bstack_allocator_t *a = slab_bstack_allocator_new(bs, 16); + CHECK(a); + + pthread_t threads[SL_REALLOC_THREADS]; + sl_realloc_arg_t args[SL_REALLOC_THREADS]; + for (int i = 0; i < SL_REALLOC_THREADS; i++) { + args[i].a = (bstack_allocator_t *)a; + args[i].tid = i; + args[i].ok = 1; + pthread_create(&threads[i], NULL, sl_realloc_worker, &args[i]); + } + for (int i = 0; i < SL_REALLOC_THREADS; i++) pthread_join(threads[i], NULL); + for (int i = 0; i < SL_REALLOC_THREADS; i++) CHECK(args[i].ok); + + bstack_close(slab_bstack_allocator_into_stack(a)); + sl_unlink(tmp); return 0; +} + +#endif /* BSTACK_FEATURE_ATOMIC */ + /* ========================================================================= * main * ====================================================================== */ @@ -751,6 +910,12 @@ int main(void) T(test_fuzz_alloc_realloc_dealloc_64); T(test_fuzz_reopen_64); +#ifdef BSTACK_FEATURE_ATOMIC + /* Concurrent */ + T(test_concurrent_alloc_dealloc_data_integrity); + T(test_concurrent_realloc_tail_paths); +#endif + printf("\n%d/%d passed\n", g_passed, g_total); return (g_passed == g_total) ? 0 : 1; } diff --git a/src/alloc/checked_slab.rs b/src/alloc/checked_slab.rs index d7b04ff..0dbadb6 100644 --- a/src/alloc/checked_slab.rs +++ b/src/alloc/checked_slab.rs @@ -9,11 +9,16 @@ use super::{BStackAllocator, BStackSlice}; use crate::BStack; -use core::{cell::Cell, marker::PhantomData}; +#[cfg(not(feature = "atomic"))] +use core::cell::Cell; +#[cfg(not(feature = "atomic"))] +use core::marker::PhantomData; +#[cfg(feature = "atomic")] +use std::sync::Mutex; use std::{collections::HashSet, fmt, io}; #[cfg(feature = "set")] -const ALCK_MAGIC: [u8; 8] = *b"ALCK\x00\x01\x00\x00"; +const ALCK_MAGIC: [u8; 8] = *b"ALCK\x00\x01\x01\x00"; /// Compatibility prefix checked on open: `ALCK` + major 0 + minor 1. /// Any file whose first 6 bytes match is considered compatible. @@ -89,10 +94,27 @@ const ALCK_MAGIC_PREFIX: [u8; 6] = *b"ALCK\x00\x01"; /// /// # Thread safety /// -/// Like [`SlabBStackAllocator`](super::SlabBStackAllocator), this allocator is -/// `Send` but not `Sync`: concurrent `&self` access must be externally -/// synchronized, because free-list mutation reads then writes `free_head` as -/// separate [`BStack`] calls. +/// `CheckedSlabBStackAllocator` is always **`Send`** — ownership can be +/// transferred to another thread. +/// +/// Without the `atomic` feature it is **not `Sync`**: free-list mutations read +/// then write `free_head` as separate [`BStack`] calls — a TOCTOU race under +/// concurrent `&self` access. +/// +/// With the `atomic` feature it **is `Sync`**. An internal [`Mutex`] serialises +/// compound allocator operations that span multiple [`BStack`] calls (free-list +/// pop/push and the public [`recover`](Self::recover) scan). Tail grow/shrink +/// paths use [`BStack::try_extend_zeros`] / [`BStack::try_discard`] to perform +/// check-and-act atomically under `BStack`'s write lock without holding the +/// allocator mutex. +/// +/// ``` +/// fn assert_send() {} +/// assert_send::(); +/// ``` +/// +/// Without `atomic` the type is `!Sync` (this fails to compile); with `atomic` +/// the internal `Mutex` makes it `Sync` (this compiles): /// /// # Method safety /// @@ -117,6 +139,12 @@ const ALCK_MAGIC_PREFIX: [u8; 6] = *b"ALCK\x00\x01"; /// *Partial* — crash keeps the free list consistent but may leak ≤ 1 block or batch; /// *N/A* — operation performs no I/O. /// +#[cfg_attr(not(feature = "atomic"), doc = "```compile_fail")] +#[cfg_attr(feature = "atomic", doc = "```")] +/// fn assert_sync() {} +/// assert_sync::(); +/// ``` +/// /// # Feature flags /// /// Requires both the `alloc` and `set` Cargo features: @@ -130,7 +158,11 @@ pub struct CheckedSlabBStackAllocator { /// Cached from the on-disk header; fixed for the lifetime of the allocator. /// Covers the full block including the 8-byte overhead; must be `≥ 16`. block_size: u64, - // Mark as !Sync to prevent concurrent access to the free list. + /// Serialises multi-step free-list and tail operations when `atomic` is + /// enabled, making the allocator `Sync`. + #[cfg(feature = "atomic")] + lock: Mutex<()>, + #[cfg(not(feature = "atomic"))] _not_sync: PhantomData>, } @@ -233,6 +265,9 @@ impl CheckedSlabBStackAllocator { Ok(Self { stack, block_size, + #[cfg(feature = "atomic")] + lock: Mutex::new(()), + #[cfg(not(feature = "atomic"))] _not_sync: PhantomData, }) } @@ -328,6 +363,9 @@ impl CheckedSlabBStackAllocator { let allocator = Self { stack, block_size: stored_block_size, + #[cfg(feature = "atomic")] + lock: Mutex::new(()), + #[cfg(not(feature = "atomic"))] _not_sync: PhantomData, }; // Reclaim leaks and repair a failed tail truncation left by an unclean @@ -386,6 +424,8 @@ impl CheckedSlabBStackAllocator { /// * Any [`io::Error`] propagated from the underlying [`BStack`] operations /// during the arena scan or while applying reclaim / tail-discard steps. pub fn recover(&self) -> io::Result { + #[cfg(feature = "atomic")] + let _guard = self.lock.lock().unwrap(); let stack_len = self.stack.len()?; if stack_len <= Self::ARENA_START { return Ok(0); @@ -744,8 +784,17 @@ impl BStackAllocator for CheckedSlabBStackAllocator { } let num_blocks = self.blocks_needed(len)?; + if num_blocks == 1 { - if let Some(block_start) = self.pop_and_claim_block(1)? { + // Lock is scoped to the free-list pop only; the tail-extend fallback + // below runs without the lock (see comment there). + let free_block = { + #[cfg(feature = "atomic")] + let _guard = self.lock.lock().unwrap(); + self.pop_and_claim_block(1)? + // guard dropped here + }; + if let Some(block_start) = free_block { // SAFETY: // 1. No overflow: `block_start` is a valid in-bounds arena offset; // `block_start + OVERHEAD + len ≤ block_start + block_size ≤ stack_len ≤ u64::MAX` @@ -759,21 +808,12 @@ impl BStackAllocator for CheckedSlabBStackAllocator { BStackSlice::from_raw_parts(self, block_start + Self::OVERHEAD, len) }); } - let block_start = self.stack.extend(self.block_size)?; - self.write_overhead(block_start, Self::IN_USE_BIT | 1)?; - // SAFETY: - // 1. No overflow: `extend` returns the previous tail, so - // `block_start + OVERHEAD + len ≤ block_start + block_size ≤ new stack_len ≤ u64::MAX` - // because `blocks_needed(len) == 1` implies `len ≤ data_size = block_size − OVERHEAD`. - // 2. In bounds: `stack.extend` just appended exactly `block_size` zeroed - // bytes, so the full range is within the stack payload. - // 3. Alloc origin: the slice covers the data region of this fresh - // allocation and may safely be passed to `dealloc`/`realloc`. - return Ok(unsafe { - BStackSlice::from_raw_parts(self, block_start + Self::OVERHEAD, len) - }); } + // Tail-extend path (single-block with no free block, and all multi-block + // allocations). No lock needed: BStack::extend is internally serialised by + // its write lock and returns a distinct region to each concurrent caller; + // write_overhead then writes only to that exclusively-owned region. let total = num_blocks.checked_mul(self.block_size).ok_or_else(|| { io::Error::new(io::ErrorKind::InvalidInput, "allocation size overflows u64") })?; @@ -835,6 +875,8 @@ impl BStackAllocator for CheckedSlabBStackAllocator { "slice start is below the overhead prefix; not a valid allocation", ) })?; + // read_overhead is a single BStack read from a block owned by the caller; + // no lock required here. let overhead = self.read_overhead(block_start)?; if overhead & Self::IN_USE_BIT == 0 { return Err(io::Error::new( @@ -856,7 +898,7 @@ impl BStackAllocator for CheckedSlabBStackAllocator { "deallocation size overflows u64", ) })?; - let current_tail = self.stack.len()?; + let slice_end = block_start.checked_add(backing).ok_or_else(|| { io::Error::new( io::ErrorKind::InvalidInput, @@ -864,10 +906,22 @@ impl BStackAllocator for CheckedSlabBStackAllocator { ) })?; - if slice_end == current_tail { + // Tail path: try_discard atomically checks tail == slice_end and removes + // backing bytes under BStack's own write lock — no allocator lock needed. + #[cfg(feature = "atomic")] + if self.stack.try_discard(slice_end, backing)? { + return Ok(()); + } + + // Non-atomic tail path. + #[cfg(not(feature = "atomic"))] + if slice_end == self.stack.len()? { return self.stack.discard(backing); } + // Not at tail: push to the free list under the allocator lock. + #[cfg(feature = "atomic")] + let _guard = self.lock.lock().unwrap(); self.push_free_blocks(block_start, num_blocks) } @@ -928,6 +982,9 @@ impl BStackAllocator for CheckedSlabBStackAllocator { return Ok(slice); } + // Overhead read, validation, and same-block-count handling are lock-free: + // read_overhead is a single BStack read from a caller-owned block, and + // stack.zero writes to caller-owned bytes — no shared state is touched. let block_start = slice.start().checked_sub(Self::OVERHEAD).ok_or_else(|| { io::Error::new( io::ErrorKind::InvalidInput, @@ -976,78 +1033,149 @@ impl BStackAllocator for CheckedSlabBStackAllocator { "new allocation size overflows u64", ) })?; - let current_tail = self.stack.len()?; - let is_tail = block_start.checked_add(old_backing).ok_or_else(|| { + // Precompute the expected tail for this allocation; used by both paths. + let sentinel = block_start.checked_add(old_backing).ok_or_else(|| { io::Error::new(io::ErrorKind::InvalidInput, "tail check overflows u64") - })? == current_tail; + })?; - if is_tail { - if new_n > old_n { - self.stack.extend(new_backing - old_backing)?; + if new_n > old_n { + // Grow path. + // + // With `atomic`: try_extend_zeros atomically checks tail == sentinel and + // appends the delta under BStack's own write lock, so no allocator lock + // is needed. write_overhead then writes only to the exclusively-owned + // newly-extended region. If try_extend_zeros returns false the tail has + // moved and we are no longer the tail block — fall through to grow non-tail. + // + // Without `atomic`: a plain len() check followed by extend is safe in a + // single-threaded context. + #[cfg(feature = "atomic")] + if self + .stack + .try_extend_zeros(sentinel, new_backing - old_backing)? + { if new_len > slice.len() { self.stack.zero(slice.end(), new_len - slice.len())?; } self.write_overhead(block_start, Self::IN_USE_BIT | new_n)?; - } else { - // Shrink the overhead first: a crash before the discard leaves an - // orphaned (but safely unreferenced) tail region rather than an - // overhead that claims more blocks than the file contains. + // SAFETY: + // 1. No overflow: slice.start() + new_len ≤ block_start + OVERHEAD + new_n * block_size − OVERHEAD ≤ u64::MAX + // because new_n * block_size ≤ new_backing ≤ stack_len. + // 2. In bounds: try_extend_zeros just extended the tail by new_backing − old_backing bytes. + // 3. Alloc origin: slice.start() is unchanged; overhead records new_n. + return Ok(unsafe { BStackSlice::from_raw_parts(self, slice.start(), new_len) }); + } + + #[cfg(not(feature = "atomic"))] + if sentinel == self.stack.len()? { + self.stack.extend(new_backing - old_backing)?; + if new_len > slice.len() { + self.stack.zero(slice.end(), new_len - slice.len())?; + } self.write_overhead(block_start, Self::IN_USE_BIT | new_n)?; - self.stack.discard(old_backing - new_backing)?; + // SAFETY: same invariants as the atomic path above. + return Ok(unsafe { BStackSlice::from_raw_parts(self, slice.start(), new_len) }); } + + // Not at tail (or tail moved under atomic): grow non-tail. + // alloc and dealloc each acquire the lock for their own free-list and + // tail operations independently. + let new_slice = self.alloc(new_len)?; + let data = slice.read()?; + new_slice.write(&data)?; + self.dealloc(slice)?; + return Ok(new_slice); + } + + // Shrink path (new_n < old_n). Lock covers the overhead write, tail + // check, and non-tail free-list update. + #[cfg(feature = "atomic")] + let _guard = self.lock.lock().unwrap(); + + // Overhead is the commit point for both tail and non-tail paths: write + // it first so a crash after this point leaves an orphaned (but safely + // unreferenced) tail region or leaked blocks that recover() can reclaim, + // rather than an overhead that claims more blocks than the file contains. + self.write_overhead(block_start, Self::IN_USE_BIT | new_n)?; + + // Tail shrink: try_discard atomically checks tail == sentinel and + // removes the excess under bstack's write lock, so no other thread can + // race between the check and the truncation. On failure the slice is + // not at the tail; fall through to recycle the excess blocks. + #[cfg(feature = "atomic")] + if self + .stack + .try_discard(sentinel, old_backing - new_backing)? + { // SAFETY: - // 1. No overflow: `slice.start() + new_len ≤ block_start + OVERHEAD + new_n * block_size − OVERHEAD ≤ u64::MAX` - // because `new_n * block_size ≤ new_backing ≤ stack_len`. - // 2. In bounds: the tail was just extended (grow) or discarded down to `new_backing` - // (shrink); the data region `[slice.start(), slice.start() + new_len)` is within payload. - // 3. Alloc origin: `slice.start()` is unchanged; the overhead now records `new_n`; - // the slice may safely be passed to `dealloc`/`realloc`. + // 1. No overflow: slice.start() + new_len ≤ block_start + OVERHEAD + new_n * block_size − OVERHEAD ≤ u64::MAX + // because new_n * block_size ≤ new_backing ≤ stack_len. + // 2. In bounds: tail discarded down to new_backing. + // 3. Alloc origin: slice.start() is unchanged; overhead records new_n. return Ok(unsafe { BStackSlice::from_raw_parts(self, slice.start(), new_len) }); } - if new_n < old_n { - // Shrink non-tail: recycle the excess blocks into the free list. - // - // Ordering matters, and the commit must come first. Shrinking the - // first block's count is the commit point: before it the old view - // (old_n blocks, original payload) is fully intact; after it the new - // view (new_n blocks) is in force. Only once committed do we write - // free-list metadata into the excess blocks (which clobbers their old - // payload) and repoint free_head. A crash before the commit leaves - // the original allocation untouched; a crash after it leaks the - // excess blocks but never corrupts a live allocation. Writing the - // free run first would shred the tail payload while the header still - // claims old_n, leaving a recovered allocation that is neither - // cleanly old nor cleanly new. - let excess_start = block_start - .checked_add(new_n.checked_mul(self.block_size).ok_or_else(|| { - io::Error::new( - io::ErrorKind::InvalidInput, - "free start multiplication overflows u64", - ) - })?) - .ok_or_else(|| { - io::Error::new(io::ErrorKind::InvalidInput, "free start overflows u64") - })?; - self.write_overhead(block_start, Self::IN_USE_BIT | new_n)?; - self.write_free_run(excess_start, old_n - new_n)?; - self.stack - .set(Self::FREE_HEAD_OFFSET, excess_start.to_le_bytes())?; + #[cfg(not(feature = "atomic"))] + if sentinel == self.stack.len()? { + self.stack.discard(old_backing - new_backing)?; // SAFETY: - // 1. No overflow: `slice.start() + new_len ≤ block_start + OVERHEAD + new_n * block_size − OVERHEAD ≤ u64::MAX` - // because `new_n * block_size ≤ old_backing ≤ stack_len`. - // 2. In bounds: the first `new_n` blocks are still live in the stack payload. - // 3. Alloc origin: `slice.start()` is unchanged; the overhead now records `new_n`; - // the slice may safely be passed to `dealloc`/`realloc`. + // 1. No overflow: slice.start() + new_len ≤ block_start + OVERHEAD + new_n * block_size − OVERHEAD ≤ u64::MAX + // because new_n * block_size ≤ new_backing ≤ stack_len. + // 2. In bounds: tail discarded down to new_backing. + // 3. Alloc origin: slice.start() is unchanged; overhead records new_n. return Ok(unsafe { BStackSlice::from_raw_parts(self, slice.start(), new_len) }); } - // Grow non-tail: allocate a fresh region, copy data, release the old. - let new_slice = self.alloc(new_len)?; - let data = slice.read()?; - new_slice.write(&data)?; - self.dealloc(slice)?; - Ok(new_slice) + // Shrink non-tail: recycle the excess blocks into the free list. + // + // Ordering matters, and the commit must come first. Shrinking the + // first block's count is the commit point: before it the old view + // (old_n blocks, original payload) is fully intact; after it the new + // view (new_n blocks) is in force. Only once committed do we write + // free-list metadata into the excess blocks (which clobbers their old + // payload) and repoint free_head. A crash before the commit leaves + // the original allocation untouched; a crash after it leaks the + // excess blocks but never corrupts a live allocation. Writing the + // free run first would shred the tail payload while the header still + // claims old_n, leaving a recovered allocation that is neither + // cleanly old nor cleanly new. + // + // Overhead was already written above (the commit point). + let excess_start = block_start + .checked_add(new_n.checked_mul(self.block_size).ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "free start multiplication overflows u64", + ) + })?) + .ok_or_else(|| { + io::Error::new(io::ErrorKind::InvalidInput, "free start overflows u64") + })?; + self.write_free_run(excess_start, old_n - new_n)?; + self.stack + .set(Self::FREE_HEAD_OFFSET, excess_start.to_le_bytes())?; + // SAFETY: + // 1. No overflow: slice.start() + new_len ≤ block_start + OVERHEAD + new_n * block_size − OVERHEAD ≤ u64::MAX + // because new_n * block_size ≤ old_backing ≤ stack_len. + // 2. In bounds: the first new_n blocks are still live in the stack payload. + // 3. Alloc origin: slice.start() is unchanged; overhead records new_n. + Ok(unsafe { BStackSlice::from_raw_parts(self, slice.start(), new_len) }) + } +} + +#[cfg(all(test, feature = "set"))] +mod _assertions { + use super::CheckedSlabBStackAllocator; + fn _send() + where + CheckedSlabBStackAllocator: Send, + { + } + #[cfg(feature = "atomic")] + fn _sync() + where + CheckedSlabBStackAllocator: Sync, + { } } @@ -1473,4 +1601,125 @@ mod tests { assert_eq!(alloc.recover().unwrap(), 0); assert_eq!(alloc.stack().len().unwrap(), len_after); } + + // ── concurrent (feature = "atomic") ─────────────────────────────────────── + + #[cfg(feature = "atomic")] + #[test] + fn concurrent_alloc_dealloc_no_live_duplicates() { + use std::collections::HashSet; + use std::sync::{Arc, Mutex}; + use std::thread; + + // Verify that concurrent alloc/dealloc calls never hand the same block + // to two callers simultaneously. Each thread claims a block, inserts + // its offset into a shared live-set (asserting uniqueness), writes its + // thread id, reads back and verifies the data, then removes the offset + // and deallocates. The free-list lock is what makes this safe; a bug + // there would produce a duplicate entry in the set. + const THREADS: usize = 8; + const ROUNDS: usize = 200; + + let (stack, path) = empty_stack(); + let _g = Guard(path); + let alloc = Arc::new(CheckedSlabBStackAllocator::new(stack, 8).unwrap()); + let live: Arc>> = Arc::new(Mutex::new(HashSet::new())); + + let handles: Vec<_> = (0..THREADS) + .map(|tid| { + let alloc = Arc::clone(&alloc); + let live = Arc::clone(&live); + thread::spawn(move || { + let a: &CheckedSlabBStackAllocator = &alloc; + for _ in 0..ROUNDS { + let slice = a.alloc(8).unwrap(); + let off = slice.start(); + { + let mut set = live.lock().unwrap(); + assert!(set.insert(off), "duplicate live offset {off}"); + } + slice.write(&[tid as u8; 8]).unwrap(); + let data = slice.read().unwrap(); + assert_eq!(data, vec![tid as u8; 8]); + { + let mut set = live.lock().unwrap(); + set.remove(&off); + } + a.dealloc(slice).unwrap(); + } + }) + }) + .collect(); + + for h in handles { + h.join().unwrap(); + } + + // All threads done: the allocator should be fully consistent. + assert_eq!(alloc.recover().unwrap(), 0); + } + + #[cfg(feature = "atomic")] + #[test] + fn concurrent_realloc_hammers_tail_paths() { + use std::sync::Arc; + use std::thread; + + // T threads each own one allocation and repeatedly grow then shrink it. + // Whichever allocation sits at the tail exercises try_extend_zeros / + // try_discard; the others hit the non-tail copy-grow / block-recycle + // paths. Because threads race, both branches are exercised on every + // round. Verify that each thread's data survives every round intact. + // + // With data_size = 8 (block_size = 16): + // SMALL = 8 → blocks_needed = ceil((8+8)/16) = 1 block + // LARGE = 32 → blocks_needed = ceil((32+8)/16) = 3 blocks + const THREADS: usize = 6; + const ROUNDS: usize = 150; + const SMALL: u64 = 8; + const LARGE: u64 = 32; + + let (stack, path) = empty_stack(); + let _g = Guard(path); + let alloc = Arc::new(CheckedSlabBStackAllocator::new(stack, 8).unwrap()); + + let handles: Vec<_> = (0..THREADS) + .map(|tid| { + let alloc = Arc::clone(&alloc); + thread::spawn(move || { + let a: &CheckedSlabBStackAllocator = &alloc; + let mut slice = a.alloc(SMALL).unwrap(); + slice.write(&[tid as u8; SMALL as usize]).unwrap(); + + for _ in 0..ROUNDS { + // Grow: tail → try_extend_zeros; non-tail → copy to new region. + slice = a.realloc(slice, LARGE).unwrap(); + let data = slice.read().unwrap(); + assert_eq!( + &data[..SMALL as usize], + &[tid as u8; SMALL as usize], + "data corrupted after grow (tid {tid})", + ); + + // Shrink: tail → try_discard; non-tail → recycle excess blocks. + slice = a.realloc(slice, SMALL).unwrap(); + let data = slice.read().unwrap(); + assert_eq!( + data, + vec![tid as u8; SMALL as usize], + "data corrupted after shrink (tid {tid})", + ); + } + + a.dealloc(slice).unwrap(); + }) + }) + .collect(); + + for h in handles { + h.join().unwrap(); + } + + assert_eq!(alloc.recover().unwrap(), 0); + } } diff --git a/src/alloc/slab.rs b/src/alloc/slab.rs index f97924c..a0e5fd8 100644 --- a/src/alloc/slab.rs +++ b/src/alloc/slab.rs @@ -6,11 +6,17 @@ use super::{BStackAllocator, BStackSlice}; use crate::BStack; -use core::{cell::Cell, marker::PhantomData, num::NonZeroU64}; +#[cfg(not(feature = "atomic"))] +use core::cell::Cell; +#[cfg(not(feature = "atomic"))] +use core::marker::PhantomData; +use core::num::NonZeroU64; +#[cfg(feature = "atomic")] +use std::sync::Mutex; use std::{fmt, io}; #[cfg(feature = "set")] -const ALSL_MAGIC: [u8; 8] = *b"ALSL\x00\x01\x00\x00"; +const ALSL_MAGIC: [u8; 8] = *b"ALSL\x00\x01\x01\x00"; /// Compatibility prefix checked on open: `ALSL` + major 0 + minor 1. /// Any file whose first 6 bytes match is considered compatible. @@ -83,12 +89,33 @@ const ALSL_MAGIC_PREFIX: [u8; 6] = *b"ALSL\x00\x01"; /// /// # Thread safety /// -/// `SlabBStackAllocator` is only `Send` but not `Sync`: concurrent access to -/// the same allocator must be externally synchronized. Free-list mutations +/// `SlabBStackAllocator` is always **`Send`** — ownership can be transferred +/// to another thread. +/// +/// Without the `atomic` feature it is **not `Sync`**: free-list mutations /// require a read then a write of `free_head` as separate `BStack` calls — a /// TOCTOU race under concurrent `&self` access that can result in two callers /// receiving the same block. /// +/// With the `atomic` feature it **is `Sync`**. An internal [`Mutex`] serialises +/// free-list pop/push operations that require multiple [`BStack`] calls. +/// Tail grow/shrink paths use [`BStack::try_extend_zeros`] / [`BStack::try_discard`] +/// to perform check-and-act atomically under `BStack`'s write lock without holding +/// the allocator mutex. +/// ``` +/// fn assert_send() {} +/// assert_send::(); +/// ``` +/// +/// Without `atomic` the type is `!Sync` (this fails to compile); with `atomic` +/// the internal `Mutex` makes it `Sync` (this compiles): +/// +#[cfg_attr(not(feature = "atomic"), doc = "```compile_fail")] +#[cfg_attr(feature = "atomic", doc = "```")] +/// fn assert_sync() {} +/// assert_sync::(); +/// ``` +/// /// # Feature flags /// /// Requires both the `alloc` and `set` Cargo features: @@ -101,7 +128,11 @@ pub struct SlabBStackAllocator { stack: BStack, /// Cached from the on-disk header; fixed for the lifetime of the allocator. block_size: u64, - // Mark as !Sync to prevent concurrent access to the free list. + /// Serialises multi-step free-list and tail operations when `atomic` is + /// enabled, making the allocator `Sync`. + #[cfg(feature = "atomic")] + lock: Mutex<()>, + #[cfg(not(feature = "atomic"))] _not_sync: PhantomData>, } @@ -162,6 +193,9 @@ impl SlabBStackAllocator { Ok(Self { stack, block_size, + #[cfg(feature = "atomic")] + lock: Mutex::new(()), + #[cfg(not(feature = "atomic"))] _not_sync: PhantomData, }) } @@ -244,6 +278,9 @@ impl SlabBStackAllocator { Ok(Self { stack, block_size: stored_block_size, + #[cfg(feature = "atomic")] + lock: Mutex::new(()), + #[cfg(not(feature = "atomic"))] _not_sync: PhantomData, }) } @@ -416,6 +453,8 @@ impl BStackAllocator for SlabBStackAllocator { } if len <= self.block_size { + #[cfg(feature = "atomic")] + let _guard = self.lock.lock().unwrap(); if let Some(block) = self.pop_free_block()? { // SAFETY: block is a valid block_size region from pop_free_block return Ok(unsafe { BStackSlice::from_raw_parts(self, block.into(), len) }); @@ -457,7 +496,6 @@ impl BStackAllocator for SlabBStackAllocator { "deallocation size overflows u64", ) })?; - let current_tail = self.stack.len()?; let slice_end = slice.start().checked_add(backing_size).ok_or_else(|| { io::Error::new( io::ErrorKind::InvalidInput, @@ -465,10 +503,22 @@ impl BStackAllocator for SlabBStackAllocator { ) })?; - if slice.len() > self.block_size && slice_end == current_tail { + // Tail discard path: only for oversized allocations (> 1 block). + // try_discard atomically checks tail == slice_end and removes backing_size + // bytes under BStack's own write lock — no allocator lock needed. + #[cfg(feature = "atomic")] + if slice.len() > self.block_size && self.stack.try_discard(slice_end, backing_size)? { + return Ok(()); + } + + #[cfg(not(feature = "atomic"))] + if slice.len() > self.block_size && slice_end == self.stack.len()? { return self.stack.discard(backing_size); } + // Not at tail (or single-block): push to the free list under the lock. + #[cfg(feature = "atomic")] + let _guard = self.lock.lock().unwrap(); self.push_free_blocks(slice.start(), n_blocks) } @@ -524,62 +574,117 @@ impl BStackAllocator for SlabBStackAllocator { "new allocation size overflows u64", ) })?; - let current_tail = self.stack.len()?; - let is_tail = slice.start().checked_add(old_backing).ok_or_else(|| { + + let checked_len = slice.start().checked_add(old_backing).ok_or_else(|| { io::Error::new(io::ErrorKind::InvalidInput, "tail check overflows u64") - })? == current_tail; + })?; - if is_tail { - if new_n > old_n { + if new_n > old_n { + // Grow path. + // With `atomic`: try_extend_zeros atomically checks tail == checked_len + // and appends the delta — no allocator lock needed. + // Without `atomic`: plain len() check then extend (single-threaded). + #[cfg(feature = "atomic")] + if self + .stack + .try_extend_zeros(checked_len, new_backing - old_backing)? + { + if new_len > slice.len() { + self.stack.zero(slice.end(), new_len - slice.len())?; + } + // SAFETY: slice extended in place at the tail + return Ok(unsafe { BStackSlice::from_raw_parts(self, slice.start(), new_len) }); + } + + #[cfg(not(feature = "atomic"))] + if checked_len == self.stack.len()? { self.stack.extend(new_backing - old_backing)?; if new_len > slice.len() { self.stack.zero(slice.end(), new_len - slice.len())?; } - } else { - self.stack.discard(old_backing - new_backing)?; + // SAFETY: slice extended in place at the tail + return Ok(unsafe { BStackSlice::from_raw_parts(self, slice.start(), new_len) }); } - // SAFETY: slice extended or shrunk in place at the tail + + // Grow non-tail: copy data into a fresh region, then free the old blocks. + // get_into and push need no lock; push_free_blocks mutates the free list. + let buf_len = usize::try_from(new_backing).map_err(|_| { + io::Error::new( + io::ErrorKind::InvalidInput, + "reallocation too large for this platform", + ) + })?; + let mut data_buf = vec![0u8; buf_len]; + let old_visible_len = usize::try_from(slice.len()).map_err(|_| { + io::Error::new( + io::ErrorKind::InvalidInput, + "existing allocation too large for this platform", + ) + })?; + self.stack + .get_into(slice.start(), &mut data_buf[..old_visible_len])?; + let new_ptr = self.stack.push(data_buf)?; + #[cfg(feature = "atomic")] + let _guard = self.lock.lock().unwrap(); + self.push_free_blocks(slice.start(), old_n)?; + // SAFETY: new_len fits within the new_n blocks of the newly pushed region + return Ok(unsafe { BStackSlice::from_raw_parts(self, new_ptr, new_len) }); + } + + // Shrink path (new_n < old_n). + // With `atomic`: try_discard atomically checks tail == checked_len and removes + // the excess — no lock needed. On failure the slice is not at the tail; + // fall through to shrink non-tail. + // Without `atomic`: plain len() check then discard (single-threaded). + #[cfg(feature = "atomic")] + if self + .stack + .try_discard(checked_len, old_backing - new_backing)? + { + // SAFETY: slice shrunk in place at the tail return Ok(unsafe { BStackSlice::from_raw_parts(self, slice.start(), new_len) }); } - if new_n < old_n { - // Shrink non-tail: recycle excess blocks into the free list. - let free_start = slice - .start() - .checked_add(new_n.checked_mul(self.block_size).ok_or_else(|| { - io::Error::new( - io::ErrorKind::InvalidInput, - "free start multiplication overflows u64", - ) - })?) - .ok_or_else(|| { - io::Error::new(io::ErrorKind::InvalidInput, "free start overflows u64") - })?; - self.push_free_blocks(free_start, old_n - new_n)?; - // SAFETY: new_len fits within the first new_n retained blocks + #[cfg(not(feature = "atomic"))] + if checked_len == self.stack.len()? { + self.stack.discard(old_backing - new_backing)?; + // SAFETY: slice shrunk in place at the tail return Ok(unsafe { BStackSlice::from_raw_parts(self, slice.start(), new_len) }); } - // Grow non-tail: copy data, allocate a new region, release old. - let buf_len = usize::try_from(new_backing).map_err(|_| { - io::Error::new( - io::ErrorKind::InvalidInput, - "reallocation too large for this platform", - ) - })?; - let mut data_buf = vec![0u8; buf_len]; - let old_visible_len = usize::try_from(slice.len()).map_err(|_| { - io::Error::new( - io::ErrorKind::InvalidInput, - "existing allocation too large for this platform", - ) - })?; - self.stack - .get_into(slice.start(), &mut data_buf[..old_visible_len])?; - let new_ptr = self.stack.push(data_buf)?; - self.push_free_blocks(slice.start(), old_n)?; - // SAFETY: new_len fits within the new_n blocks of the newly pushed region - Ok(unsafe { BStackSlice::from_raw_parts(self, new_ptr, new_len) }) + // Shrink non-tail: recycle excess blocks into the free list. + let free_start = slice + .start() + .checked_add(new_n.checked_mul(self.block_size).ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "free start multiplication overflows u64", + ) + })?) + .ok_or_else(|| { + io::Error::new(io::ErrorKind::InvalidInput, "free start overflows u64") + })?; + #[cfg(feature = "atomic")] + let _guard = self.lock.lock().unwrap(); + self.push_free_blocks(free_start, old_n - new_n)?; + // SAFETY: new_len fits within the first new_n retained blocks + Ok(unsafe { BStackSlice::from_raw_parts(self, slice.start(), new_len) }) + } +} + +#[cfg(all(test, feature = "set"))] +mod _assertions { + use super::SlabBStackAllocator; + fn _send() + where + SlabBStackAllocator: Send, + { + } + #[cfg(feature = "atomic")] + fn _sync() + where + SlabBStackAllocator: Sync, + { } } @@ -796,4 +901,120 @@ mod tests { let s2 = unsafe { crate::alloc::BStackSlice::from_raw_parts(&alloc2, offset, 5) }; assert_eq!(s2.read().unwrap(), b"hello"); } + + // ── concurrent (feature = "atomic") ─────────────────────────────────────── + + #[cfg(feature = "atomic")] + #[test] + fn concurrent_alloc_dealloc_no_live_duplicates() { + use std::collections::HashSet; + use std::sync::{Arc, Mutex}; + use std::thread; + + // Verify that concurrent alloc/dealloc never hands the same block to + // two callers simultaneously. Each thread claims a block, inserts its + // offset into a shared live-set (asserting uniqueness), writes and + // reads back its thread id, then removes the offset and deallocates. + // Slab has no per-block overhead, so a free-list race would silently + // produce a duplicate offset rather than an error; the HashSet catches + // that. + const THREADS: usize = 8; + const ROUNDS: usize = 200; + + let (stack, path) = empty_stack(); + let _g = Guard(path); + let alloc = Arc::new(SlabBStackAllocator::new(stack, 16).unwrap()); + let live: Arc>> = Arc::new(Mutex::new(HashSet::new())); + + let handles: Vec<_> = (0..THREADS) + .map(|tid| { + let alloc = Arc::clone(&alloc); + let live = Arc::clone(&live); + thread::spawn(move || { + let a: &SlabBStackAllocator = &alloc; + for _ in 0..ROUNDS { + let slice = a.alloc(16).unwrap(); + let off = slice.start(); + { + let mut set = live.lock().unwrap(); + assert!(set.insert(off), "duplicate live offset {off}"); + } + slice.write(&[tid as u8; 16]).unwrap(); + let data = slice.read().unwrap(); + assert_eq!(data, vec![tid as u8; 16]); + { + let mut set = live.lock().unwrap(); + set.remove(&off); + } + a.dealloc(slice).unwrap(); + } + }) + }) + .collect(); + + for h in handles { + h.join().unwrap(); + } + } + + #[cfg(feature = "atomic")] + #[test] + fn concurrent_realloc_hammers_tail_paths() { + use std::sync::Arc; + use std::thread; + + // T threads each own one allocation and repeatedly grow then shrink it. + // Whichever allocation sits at the tail exercises try_extend_zeros / + // try_discard; the others hit the non-tail copy-grow / block-recycle + // paths. Both branches are exercised on every round because threads + // race for the tail. Verify each thread's data survives every round. + // + // With block_size = 16: + // alloc(12) → 1 block; alloc(17) → 2 blocks; alloc(33) → 3 blocks. + const THREADS: usize = 6; + const ROUNDS: usize = 150; + const SMALL: u64 = 12; // fits in 1 block (block_size = 16) + const LARGE: u64 = 33; // needs 3 blocks + + let (stack, path) = empty_stack(); + let _g = Guard(path); + let alloc = Arc::new(SlabBStackAllocator::new(stack, 16).unwrap()); + + let handles: Vec<_> = (0..THREADS) + .map(|tid| { + let alloc = Arc::clone(&alloc); + thread::spawn(move || { + let a: &SlabBStackAllocator = &alloc; + let mut slice = a.alloc(SMALL).unwrap(); + slice.write(&[tid as u8; SMALL as usize]).unwrap(); + + for _ in 0..ROUNDS { + // Grow: tail → try_extend_zeros; non-tail → copy to new region. + slice = a.realloc(slice, LARGE).unwrap(); + let data = slice.read().unwrap(); + assert_eq!( + &data[..SMALL as usize], + &[tid as u8; SMALL as usize], + "data corrupted after grow (tid {tid})", + ); + + // Shrink: tail → try_discard; non-tail → recycle excess blocks. + slice = a.realloc(slice, SMALL).unwrap(); + let data = slice.read().unwrap(); + assert_eq!( + data, + vec![tid as u8; SMALL as usize], + "data corrupted after shrink (tid {tid})", + ); + } + + a.dealloc(slice).unwrap(); + }) + }) + .collect(); + + for h in handles { + h.join().unwrap(); + } + } } diff --git a/src/test.rs b/src/test.rs index 489e7ab..130108f 100644 --- a/src/test.rs +++ b/src/test.rs @@ -3162,11 +3162,6 @@ mod first_fit_tests { for h in handles { h.join().unwrap(); } - - // After every alloc has been paired with a dealloc, the arena should - // be empty: the tail-coalesce path eventually discards everything - // back to the 48-byte header. - assert_eq!(alloc.len().unwrap(), ALFF_HDR_OFFSET); } #[cfg(feature = "atomic")]