From 7d35a29b0ffa902a74047f1e9d2b4d146d287ba0 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Mon, 15 Jun 2026 08:15:21 -0300 Subject: [PATCH 01/12] feat(hashtable): dual-mode (arena or host_malloc), hashtable_destroy - hashtable_create(NULL) allocates via host_malloc; hashtable_create(arena) unchanged. - hashtable_destroy frees nodes + key copies + table struct in host_malloc mode; no-op for arena-backed (arena handles it). - hashtable_remove / hashtable_clear free their nodes in host_malloc mode (otherwise leak). - 6 new tests for the host_malloc path; existing arena tests untouched. - Foundation step for migrating primitives off arena one at a time. --- hashtable.c | 81 +++++++++++++++++++++++++++++++++++------ hashtable.h | 41 +++++++++++++-------- test_hashtable.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 190 insertions(+), 25 deletions(-) diff --git a/hashtable.c b/hashtable.c index 1a47088..b117f07 100644 --- a/hashtable.c +++ b/hashtable.c @@ -1,4 +1,5 @@ #include "hashtable.h" +#include "host.h" #include "string.h" const char *hashtable_insert_result_name(HashTableInsertResult r) { @@ -20,25 +21,66 @@ typedef struct HashTableNode { } HashTableNode; HashTable *hashtable_create(Arena *arena) { + if (arena == NULL) { + HashTable *table = host_malloc(sizeof(HashTable)); + if (!table) { + return NULL; + } + + table->arena = NULL; + table->head = NULL; + return table; + } + HashTable *table = arena_alloc(arena, sizeof(HashTable)); - if (!table) + if (!table) { return NULL; + } table->arena = arena; + table->head = NULL; return table; } -HashTableInsertResult _hashtable_insert(HashTable *table, const void *key, - size_t key_len, void *value) { - HashTableNode *node = arena_alloc(table->arena, sizeof(HashTableNode)); - if (!node) { - return HASHTABLE_ERR_OOM; +void hashtable_destroy(HashTable *table) { + if (table != NULL && table->arena == NULL) { + HashTableNode *n = table->head; + while (n) { + HashTableNode *next = n->next; + host_free(n->key); + host_free(n); + n = next; + } + host_free(table); } +} + +static HashTableInsertResult _hashtable_insert(HashTable *table, + const void *key, size_t key_len, + void *value) { + HashTableNode *node = NULL; + if (table->arena == NULL) { + node = host_malloc(sizeof(HashTableNode)); + if (!node) { + return HASHTABLE_ERR_OOM; + } - node->key = arena_alloc(table->arena, key_len); - if (!node->key) { - return HASHTABLE_ERR_OOM; + node->key = host_malloc(key_len); + if (!node->key) { + host_free(node); + return HASHTABLE_ERR_OOM; + } + } else { + node = arena_alloc(table->arena, sizeof(HashTableNode)); + if (!node) { + return HASHTABLE_ERR_OOM; + } + + node->key = arena_alloc(table->arena, key_len); + if (!node->key) { + return HASHTABLE_ERR_OOM; + } } memcpy(node->key, key, key_len); @@ -88,6 +130,12 @@ HashTableRemoveResult hashtable_remove(HashTable *table, const void *key, } else { table->head = n->next; } + + if (table->arena == NULL) { + host_free(n->key); + host_free(n); + } + return HASHTABLE_REMOVE_OK; } } @@ -100,7 +148,6 @@ HashTableUpdateResult hashtable_update(HashTable *table, const void *key, for (HashTableNode *n = table->head; n; n = n->next) { if (n->key_len == key_len && memcmp(n->key, key, key_len) == 0) { n->value = value; - n->key_len = key_len; return HASHTABLE_UPDATE_OK; } } @@ -123,7 +170,19 @@ HashTableUpsertResult hashtable_upsert(HashTable *table, const void *key, } } -void hashtable_clear(HashTable *table) { table->head = NULL; } +void hashtable_clear(HashTable *table) { + if (table->arena == NULL) { + HashTableNode *n = table->head; + while (n) { + HashTableNode *next = n->next; + host_free(n->key); + host_free(n); + n = next; + } + } + + table->head = NULL; +} HashTableIter hashtable_iter(HashTable *table) { HashTableIter it = {0}; diff --git a/hashtable.h b/hashtable.h index 5e3c99c..8aa2dac 100644 --- a/hashtable.h +++ b/hashtable.h @@ -1,24 +1,30 @@ #ifndef _CRDT_HASHTABLE_H #define _CRDT_HASHTABLE_H +// Allocation backing — two modes: +// 1. `hashtable_create(arena)` — node structs + key-byte copies allocated +// in the supplied Arena. Bulk lifetime: arena_destroy frees everything. +// No-op `hashtable_destroy` for this mode. +// 2. `hashtable_create(NULL)` — node structs + key-byte copies allocated +// via `host_malloc`. Caller MUST call `hashtable_destroy(table)` when +// done to release everything. +// // Ownership: -// Keys — table copies key_len bytes into its arena when a new entry is -// inserted (insert, and the insert path of upsert). Keys are raw -// bytes: embedded NULs and the length are significant — they are not -// NUL-terminated strings. Caller's `key` pointer may be transient -// (stack, freed after the call). Keys returned by -// `hashtable_iter_next` are table-owned; valid as long as the arena -// lives. Caller must not free them. +// Keys — table copies key_len bytes when a new entry is inserted (insert, +// and the insert path of upsert). Keys are raw bytes: embedded +// NULs and the length are significant — they are not NUL-terminated +// strings. Caller's `key` pointer may be transient (stack, freed +// after the call). Keys returned by `hashtable_iter_next` are +// table-owned; valid as long as the table lives. Caller must not +// free them. // Values — stored as opaque `void *`; table does NOT copy. Caller owns the -// pointed-to memory (typically arena-allocated). Lifetime must -// outlive any get/iter that reads the slot. +// pointed-to memory. Lifetime must outlive any get/iter that +// reads the slot. // -// Lifetime: HashTable must not outlive its arena. Resetting the arena -// invalidates every key, value, and the table itself. +// Lifetime: For arena-backed tables, must not outlive the arena. For +// host_malloc-backed tables, must outlive any pointer returned by get/iter. // // Iteration: do NOT insert into or remove from the table while iterating it. -// Mutation can leave the iterator's cursor pointing at an unlinked entry or -// cause entries to be skipped. Finish iterating first, then mutate. #include "arena.h" #include @@ -27,12 +33,19 @@ typedef struct HashTableNode HashTableNode; typedef struct HashTable { - Arena *arena; + Arena *arena; // NULL when host_malloc-backed HashTableNode *head; } HashTable; +// `arena` may be NULL — in that case, the table allocates via host_malloc +// and the caller must release with hashtable_destroy. HashTable *hashtable_create(Arena *arena); +// Release a host_malloc-backed table (frees nodes, key copies, the table +// struct itself). No-op for arena-backed tables (their arena owns the +// memory). Safe to call regardless of backing. +void hashtable_destroy(HashTable *table); + typedef enum { HASHTABLE_OK, HASHTABLE_ERR_OOM, diff --git a/test_hashtable.c b/test_hashtable.c index 3903887..4212bb7 100644 --- a/test_hashtable.c +++ b/test_hashtable.c @@ -349,6 +349,91 @@ TEST(clear_empties_table) { ASSERT(out == &vc); } +// --- host_malloc-backed mode (NULL arena) --- +// +// All existing semantics must hold when the table is allocated via +// host_malloc instead of an arena. The caller releases via +// hashtable_destroy. Probes a handful of representative operations rather +// than mirroring every test above — semantics should be identical. + +TEST(host_malloc_create_and_destroy_empty) { + HashTable *t = hashtable_create(NULL); + ASSERT(t != NULL); + void *out; + ASSERT(hashtable_get(t, SK("missing"), &out) == false); + hashtable_destroy(t); +} + +TEST(host_malloc_insert_then_get) { + HashTable *t = hashtable_create(NULL); + int v = 42; + ASSERT_EQ(hashtable_insert(t, SK("k"), &v), HASHTABLE_OK); + void *out; + ASSERT(hashtable_get(t, SK("k"), &out) == true); + ASSERT(out == &v); + hashtable_destroy(t); +} + +// Key bytes must be copied in host_malloc mode too — mutating the caller's +// buffer after insert must not affect the stored key. +TEST(host_malloc_key_is_copied) { + HashTable *t = hashtable_create(NULL); + int v = 1; + char buf[8]; + memcpy(buf, "key", 3); + ASSERT_EQ(hashtable_insert(t, buf, 3, &v), HASHTABLE_OK); + memset(buf, 'X', sizeof buf); + void *out; + ASSERT(hashtable_get(t, "key", 3, &out) == true); + ASSERT(out == &v); + hashtable_destroy(t); +} + +TEST(host_malloc_remove_and_reinsert) { + HashTable *t = hashtable_create(NULL); + int v1 = 1, v2 = 2; + hashtable_insert(t, SK("k"), &v1); + ASSERT_EQ(hashtable_remove(t, SK("k")), HASHTABLE_REMOVE_OK); + void *out; + ASSERT(hashtable_get(t, SK("k"), &out) == false); + ASSERT_EQ(hashtable_insert(t, SK("k"), &v2), HASHTABLE_OK); + ASSERT(hashtable_get(t, SK("k"), &out) == true); + ASSERT(out == &v2); + hashtable_destroy(t); +} + +TEST(host_malloc_iter_visits_all) { + HashTable *t = hashtable_create(NULL); + int a = 1, b = 2, c = 3; + hashtable_insert(t, SK("a"), &a); + hashtable_insert(t, SK("b"), &b); + hashtable_insert(t, SK("c"), &c); + + HashTableIter it = hashtable_iter(t); + const void *k = NULL; + size_t klen = 0; + void *v = NULL; + int count = 0; + while (hashtable_iter_next(&it, &k, &klen, &v)) + count++; + ASSERT_EQ(count, 3); + hashtable_destroy(t); +} + +// `hashtable_destroy` on an arena-backed table is a no-op — must not crash +// and must not double-free the arena's contents. +TEST(arena_backed_destroy_is_noop) { + Arena *a = arena_create(); + HashTable *t = hashtable_create(a); + int v = 1; + hashtable_insert(t, SK("k"), &v); + hashtable_destroy(t); // safe no-op + // Table still usable until arena_destroy releases it. + void *out; + ASSERT(hashtable_get(t, SK("k"), &out) == true); + arena_destroy(a); +} + int main(void) { RUN(create_empty); RUN(insert_then_get); @@ -370,5 +455,13 @@ int main(void) { RUN(iter_empty_yields_nothing); RUN(iter_visits_all_once); RUN(clear_empties_table); + + RUN(host_malloc_create_and_destroy_empty); + RUN(host_malloc_insert_then_get); + RUN(host_malloc_key_is_copied); + RUN(host_malloc_remove_and_reinsert); + RUN(host_malloc_iter_visits_all); + RUN(arena_backed_destroy_is_noop); + TEST_SUMMARY(); } From 8587944e639fdbd533e836b1e27623895a278af8 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Mon, 15 Jun 2026 08:24:05 -0300 Subject: [PATCH 02/12] feat(scalar): scalar_clone supports host_malloc backing; add scalar_free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - scalar_clone(NULL, value) allocates string bytes via host_malloc. - scalar_clone(arena, value) unchanged. - Empty-string clone is a value passthrough — no allocation, no leak. - scalar_free releases host_malloc'd string bytes; no-op for non-string kinds and empty strings. - OOM message now reflects which allocator failed. - 8 new tests for the host_malloc path including empty-string + embedded NUL + survives-source-free. --- scalar.c | 21 +++++++++++-- scalar.h | 15 ++++++++++ test_scalar.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 3 deletions(-) diff --git a/scalar.c b/scalar.c index c19c5c6..5586570 100644 --- a/scalar.c +++ b/scalar.c @@ -60,11 +60,20 @@ bool scalar_eq(Scalar a, Scalar b) { Scalar scalar_clone(Arena *arena, Scalar value) { switch (value.kind) { case SCALAR_STRING: { - uint8_t *bytes_copy = arena_alloc(arena, value.as.s.len); + // Empty string: no bytes to copy. Pass the value through unchanged. + // Avoids portability fragility around malloc(0) / arena_alloc(0) + // (implementation-defined return) and the matching free-leak that + // scalar_free would otherwise miss. + if (value.as.s.len == 0) { + return value; + } + uint8_t *bytes_copy = arena == NULL + ? host_malloc(value.as.s.len) + : arena_alloc(arena, value.as.s.len); if (!bytes_copy) { - host_abortf("scalar_clone: arena OOM (requested %zu bytes for " + host_abortf("scalar_clone: %s OOM (requested %zu bytes for " "string value)", - value.as.s.len); + arena ? "arena" : "host_malloc", value.as.s.len); } memcpy(bytes_copy, value.as.s.bytes, value.as.s.len); Scalar copy = {0}; @@ -80,3 +89,9 @@ Scalar scalar_clone(Arena *arena, Scalar value) { return value; } } + +void scalar_free(Scalar value) { + if (value.kind == SCALAR_STRING && value.as.s.len > 0) { + host_free((void *)value.as.s.bytes); + } +} diff --git a/scalar.h b/scalar.h index 72f226f..6921e41 100644 --- a/scalar.h +++ b/scalar.h @@ -51,6 +51,21 @@ Scalar scalar_string(const uint8_t *bytes, size_t len); bool scalar_eq(Scalar a, Scalar b); +// Clone a Scalar into owned storage. Two modes: +// 1. `scalar_clone(arena, value)` — string bytes (if any) allocated in +// the supplied Arena. Bulk lifetime: arena_destroy frees them. Do +// NOT call scalar_free on the result. +// 2. `scalar_clone(NULL, value)` — string bytes (if any) allocated via +// host_malloc. Caller MUST release with scalar_free when done. +// +// For non-string kinds (NULL / BOOL / INT) cloning is a value-copy +// regardless of mode — nothing to allocate. Scalar scalar_clone(Arena *arena, Scalar value); +// Release a host_malloc-backed Scalar's string bytes. No-op for non-string +// kinds and for empty strings (no allocation to release). MUST only be +// called on Scalars produced by `scalar_clone(NULL, ...)`. Calling on an +// arena-backed Scalar is undefined. +void scalar_free(Scalar value); + #endif // _CRDT_SCALAR_H diff --git a/test_scalar.c b/test_scalar.c index 73804f1..787f12e 100644 --- a/test_scalar.c +++ b/test_scalar.c @@ -207,6 +207,78 @@ TEST(clone_string_with_embedded_nul) { arena_destroy(a); } +// --- host_malloc-backed clone (NULL arena) --- +// +// scalar_clone(NULL, ...) allocates string bytes via host_malloc; caller +// releases with scalar_free. Non-string kinds pass through as value copies +// and do not need scalar_free (it's safe to call but a no-op). + +TEST(host_clone_null_passes_through) { + Scalar c = scalar_clone(NULL, scalar_null()); + ASSERT(scalar_eq(c, scalar_null()) == true); + scalar_free(c); // no-op, safe +} + +TEST(host_clone_bool_passes_through) { + Scalar t = scalar_clone(NULL, scalar_bool(true)); + Scalar f = scalar_clone(NULL, scalar_bool(false)); + ASSERT(scalar_eq(t, scalar_bool(true)) == true); + ASSERT(scalar_eq(f, scalar_bool(false)) == true); + scalar_free(t); + scalar_free(f); +} + +TEST(host_clone_int_passes_through) { + Scalar c = scalar_clone(NULL, scalar_int(42)); + ASSERT(scalar_eq(c, scalar_int(42)) == true); + scalar_free(c); +} + +// Source buffer mutated after clone — clone bytes must be independent. +TEST(host_clone_string_owns_bytes) { + uint8_t src[5]; + memcpy(src, "hello", 5); + Scalar c = scalar_clone(NULL, scalar_string(src, 5)); + src[0] = 'X'; + src[1] = 'X'; + ASSERT(scalar_eq(c, scalar_string((const uint8_t *)"hello", 5)) == true); + scalar_free(c); +} + +// Source buffer freed entirely — clone still readable. +TEST(host_clone_string_survives_source_free) { + uint8_t *src = (uint8_t *)malloc(5); + memcpy(src, "hello", 5); + Scalar c = scalar_clone(NULL, scalar_string(src, 5)); + free(src); + ASSERT(scalar_eq(c, scalar_string((const uint8_t *)"hello", 5)) == true); + scalar_free(c); +} + +TEST(host_clone_empty_string_handled) { + Scalar c = scalar_clone(NULL, scalar_string((const uint8_t *)"", 0)); + ASSERT(scalar_eq(c, scalar_string((const uint8_t *)"", 0)) == true); + scalar_free(c); // safe on empty (no allocation made) +} + +TEST(host_clone_string_with_embedded_nul) { + uint8_t src[4] = {0x01, 0x00, 0x02, 0x03}; + Scalar c = scalar_clone(NULL, scalar_string(src, sizeof src)); + ASSERT(scalar_eq(c, scalar_string(src, sizeof src)) == true); + scalar_free(c); +} + +// scalar_free is safe on a default-constructed scalar (e.g. a stack Scalar +// the caller created via scalar_int / scalar_bool / scalar_null) — no-op +// for non-string kinds. +TEST(scalar_free_noop_on_non_string_kinds) { + scalar_free(scalar_null()); + scalar_free(scalar_bool(true)); + scalar_free(scalar_int(99)); + // If we got here without crashing, pass. + ASSERT(true); +} + int main(void) { RUN(null_has_null_kind); RUN(bool_true_kind_and_value); @@ -243,5 +315,14 @@ int main(void) { RUN(clone_empty_string_handled); RUN(clone_string_with_embedded_nul); + RUN(host_clone_null_passes_through); + RUN(host_clone_bool_passes_through); + RUN(host_clone_int_passes_through); + RUN(host_clone_string_owns_bytes); + RUN(host_clone_string_survives_source_free); + RUN(host_clone_empty_string_handled); + RUN(host_clone_string_with_embedded_nul); + RUN(scalar_free_noop_on_non_string_kinds); + TEST_SUMMARY(); } From daabad7c52d891947a744c5105faa07084971d73 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sat, 27 Jun 2026 21:13:32 -0300 Subject: [PATCH 03/12] refactor(counter): refcounted host_malloc lifecycle + displace signal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop arena backing. counter_create allocates via host_malloc with refcount=1; counter_acquire/release manage lifetime, freeing per-client entries, hashtable, and struct at refcount 0. Add counter_displace / counter_is_displaced — a per-instance signal (independent of refcount) for the Map slot path to mark an LWW-evicted handle. counter_clone returns a fresh refcount=1, never-displaced copy. --- counter.c | 67 ++++++++++++++++------- counter.h | 40 +++++++++++--- test_counter.c | 145 ++++++++++++++++++++++++++++++++++--------------- 3 files changed, 183 insertions(+), 69 deletions(-) diff --git a/counter.c b/counter.c index 2306f05..7611ad0 100644 --- a/counter.c +++ b/counter.c @@ -1,12 +1,13 @@ #include "counter.h" -#include "arena.h" #include "hashtable.h" #include "host.h" struct Counter { ElementId id; - Arena *arena; HashTable *entries; // ClientId -> CounterEntry + + size_t refcount; + bool displaced; }; static inline uint32_t max_u32(uint32_t a, uint32_t b) { @@ -16,20 +17,22 @@ static inline uint32_t max_u32(uint32_t a, uint32_t b) { return b; } -Counter *counter_create(Arena *arena, ElementId id) { - Counter *counter = arena_alloc(arena, sizeof(Counter)); +Counter *counter_create(ElementId id) { + Counter *counter = host_malloc(sizeof(Counter)); if (!counter) { host_abortf( - "counter_create: arena OOM (requested %zu bytes for Counter)", + "counter_create: host_malloc OOM (requested %zu bytes for Counter)", sizeof(Counter)); } counter->id = id; - counter->arena = arena; - counter->entries = hashtable_create(arena); + counter->entries = hashtable_create(NULL); if (!counter->entries) { + host_free(counter); host_abort("counter_create: hashtable_create OOM (per-client tallies " "table)"); } + counter->refcount = 1; + counter->displaced = false; return counter; } @@ -43,10 +46,11 @@ static CounterEntry *counter_entry_for(Counter *counter, ClientId client_id) { &entry_ptr)) { return entry_ptr; } - CounterEntry *entry = arena_alloc(counter->arena, sizeof *entry); + CounterEntry *entry = host_malloc(sizeof *entry); if (!entry) { - host_abortf("counter: arena OOM (requested %zu bytes for CounterEntry)", - sizeof *entry); + host_abortf( + "counter: host_malloc OOM (requested %zu bytes for CounterEntry)", + sizeof *entry); } entry->client_id = client_id; entry->inc = 0; @@ -87,10 +91,10 @@ void counter_merge(Counter *dst, const Counter *src) { dst_entry->inc = max_u32(dst_entry->inc, src_entry->inc); dst_entry->dec = max_u32(dst_entry->dec, src_entry->dec); } else { - CounterEntry *copy = arena_alloc(dst->arena, sizeof *copy); + CounterEntry *copy = host_malloc(sizeof *copy); if (!copy) { - host_abortf("counter_merge: arena OOM (requested %zu bytes for " - "CounterEntry)", + host_abortf("counter_merge: host_malloc OOM (requested %zu " + "bytes for CounterEntry)", sizeof *copy); } *copy = *src_entry; @@ -112,19 +116,18 @@ void counter_dec(Counter *counter, ClientId client_id, uint32_t amount) { counter_entry_for(counter, client_id)->dec += amount; } -Counter *counter_clone(Arena *arena, const Counter *counter) { - Counter *clone = counter_create(arena, counter->id); +Counter *counter_clone(const Counter *counter) { + Counter *clone = counter_create(counter->id); HashTableIter it = hashtable_iter(counter->entries); const void *key; size_t key_len; void *value; while (hashtable_iter_next(&it, &key, &key_len, &value)) { CounterEntry *entry = value; - CounterEntry *entry_copy = - arena_alloc(clone->arena, sizeof *entry_copy); + CounterEntry *entry_copy = host_malloc(sizeof *entry_copy); if (!entry_copy) { - host_abortf("counter_clone: arena OOM (requested %zu bytes for " - "CounterEntry)", + host_abortf("counter_clone: host_malloc OOM (requested %zu bytes " + "for CounterEntry)", sizeof *entry_copy); } *entry_copy = *entry; @@ -137,3 +140,29 @@ Counter *counter_clone(Arena *arena, const Counter *counter) { } return clone; } + +void counter_acquire(Counter *counter) { counter->refcount++; } + +void counter_release(Counter *counter) { + if (counter->refcount == 0) { + host_abort("counter_release: refcount already zero"); + } + counter->refcount--; + if (counter->refcount == 0) { + // Per-client entries were host_malloc'd individually; free each before + // tearing down the table. + HashTableIter it = hashtable_iter(counter->entries); + const void *key; + size_t key_len; + void *value; + while (hashtable_iter_next(&it, &key, &key_len, &value)) { + host_free(value); + } + hashtable_destroy(counter->entries); + host_free(counter); + } +} + +void counter_displace(Counter *counter) { counter->displaced = true; } + +bool counter_is_displaced(const Counter *counter) { return counter->displaced; } diff --git a/counter.h b/counter.h index 8f21fbc..fdefa24 100644 --- a/counter.h +++ b/counter.h @@ -22,15 +22,24 @@ // - Increments and decrements use uint32_t to keep per-direction max // well-defined; counter_read widens to int64_t for the signed total. // -// Ownership: -// - Per-client entries live in the Counter's arena. -// -// Lifetime: Counter must not outlive its arena. +// Lifetime — refcounted: +// - counter_create returns a Counter with refcount = 1; the creator owns +// that ref. +// - counter_acquire bumps the refcount; counter_release drops it. +// - When refcount hits 0, the Counter is freed (per-client entries, +// hashtable, struct). Standard C ownership semantics. +// - The displacement signal is independent of refcount: a Counter +// marked via counter_displace (called by the Map slot path when an +// LWW displacement happens) stays alive as long as some holder still +// has a ref. Mutations on a displaced Counter still mutate local +// state but the Doc layer (when wired) skips op emission. Holders +// can check counter_is_displaced to know their handle is no longer +// installed in any slot. -#include "arena.h" #include "clientid.h" #include "elementid.h" #include "hashtable.h" +#include #include typedef struct CounterEntry { @@ -41,7 +50,8 @@ typedef struct CounterEntry { typedef struct Counter Counter; -Counter *counter_create(Arena *arena, ElementId id); +// Allocate a Counter via host_malloc with refcount=1. Caller owns one ref. +Counter *counter_create(ElementId id); ElementId counter_id(const Counter *counter); @@ -53,6 +63,22 @@ void counter_inc(Counter *counter, ClientId client_id, uint32_t amount); void counter_dec(Counter *counter, ClientId client_id, uint32_t amount); -Counter *counter_clone(Arena *arena, const Counter *counter); +// Deep-copy the source Counter into a fresh allocation. Returned clone has +// refcount=1 and is NOT marked displaced (regardless of src's displaced +// flag — displacement is a per-instance signal, not part of the value). +Counter *counter_clone(const Counter *counter); + +// Reference counting. Acquire bumps the refcount; release drops it. On +// reaching zero, the Counter is freed (per-client entries, hashtable, +// struct). +void counter_acquire(Counter *counter); +void counter_release(Counter *counter); + +// Displacement signal — see lifetime notes above. counter_displace marks +// the Counter as no-longer-in-a-slot (Map slot path calls this when it +// LWW-displaces the slot's previous value). counter_is_displaced reads +// the flag. +void counter_displace(Counter *counter); +bool counter_is_displaced(const Counter *counter); #endif // _CRDT_COUNTER_H diff --git a/test_counter.c b/test_counter.c index 15bcef9..453efc0 100644 --- a/test_counter.c +++ b/test_counter.c @@ -1,4 +1,3 @@ -#include "arena.h" #include "clientid.h" #include "counter.h" #include "elementid.h" @@ -24,17 +23,13 @@ static ElementId eid(uint64_t hi, uint64_t lo) { // Default id for tests that don't care about identity. static ElementId default_id(void) { return eid(0xFF, 0); } -static Counter *fresh(void) { - Arena *arena = arena_create(); - return counter_create(arena, default_id()); -} +static Counter *fresh(void) { return counter_create(default_id()); } TEST(counter_create_stores_id) { - Arena *a = arena_create(); ElementId id = eid(7, 42); - Counter *c = counter_create(a, id); + Counter *c = counter_create(id); ASSERT(elementid_eq(counter_id(c), id) == true); - arena_destroy(a); + counter_release(c); } // --- local operations (single replica) --- @@ -262,71 +257,128 @@ TEST(local_inc_after_merge_accumulates) { // --- counter_clone: deep copy into a target arena --- TEST(clone_empty_counter_reads_zero) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Counter *src = counter_create(as, default_id()); - Counter *clone = counter_clone(ad, src); + Counter *src = counter_create(default_id()); + Counter *clone = counter_clone(src); ASSERT(clone != NULL); ASSERT(clone != src); ASSERT_EQ(counter_read(clone), 0); - arena_destroy(as); - arena_destroy(ad); + counter_release(src); + counter_release(clone); } // Clone preserves the source's id. Cloned element represents the same -// logical element, just materialized in a different arena. +// logical element, just an independent host_malloc'd copy. TEST(clone_preserves_id) { - Arena *as = arena_create(); - Arena *ad = arena_create(); ElementId id = eid(7, 42); - Counter *src = counter_create(as, id); - Counter *clone = counter_clone(ad, src); + Counter *src = counter_create(id); + Counter *clone = counter_clone(src); ASSERT(elementid_eq(counter_id(clone), id) == true); - arena_destroy(as); - arena_destroy(ad); + counter_release(src); + counter_release(clone); } TEST(clone_preserves_per_client_tallies) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Counter *src = counter_create(as, default_id()); + Counter *src = counter_create(default_id()); counter_inc(src, cid(1), 5); counter_inc(src, cid(2), 3); counter_dec(src, cid(1), 2); - Counter *clone = counter_clone(ad, src); + Counter *clone = counter_clone(src); ASSERT_EQ(counter_read(clone), counter_read(src)); ASSERT_EQ(counter_read(clone), 6); // (5-2) + 3 - arena_destroy(as); - arena_destroy(ad); + counter_release(src); + counter_release(clone); } -// Clone owns its tallies in dst arena — destroying the source arena must -// leave the clone intact. -TEST(clone_survives_src_arena_destroy) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Counter *src = counter_create(as, default_id()); +// Clone owns its own data — releasing the source must leave the clone +// intact. +TEST(clone_survives_src_release) { + Counter *src = counter_create(default_id()); counter_inc(src, cid(1), 5); counter_inc(src, cid(2), 3); - Counter *clone = counter_clone(ad, src); - arena_destroy(as); + Counter *clone = counter_clone(src); + counter_release(src); ASSERT_EQ(counter_read(clone), 8); - arena_destroy(ad); + counter_release(clone); } // Mutating src after clone must not affect the clone, and vice versa. TEST(clone_independent_of_src) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Counter *src = counter_create(as, default_id()); + Counter *src = counter_create(default_id()); counter_inc(src, cid(1), 5); - Counter *clone = counter_clone(ad, src); + Counter *clone = counter_clone(src); counter_inc(src, cid(1), 100); // src now 105 counter_inc(clone, cid(2), 7); // clone now 12 (5 + 7) ASSERT_EQ(counter_read(src), 105); ASSERT_EQ(counter_read(clone), 12); - arena_destroy(as); - arena_destroy(ad); + counter_release(src); + counter_release(clone); +} + +// --- refcount + displacement --- +// +// counter_create returns refcount=1; release on a fresh handle frees. +// acquire/release accounting balances correctly across multiple holders. +// The displaced flag is independent of refcount: marking displaced does +// not free the Counter; only refcount reaching zero does. + +TEST(create_starts_not_displaced) { + Counter *c = counter_create(default_id()); + ASSERT(counter_is_displaced(c) == false); + counter_release(c); +} + +TEST(displace_sets_flag) { + Counter *c = counter_create(default_id()); + counter_displace(c); + ASSERT(counter_is_displaced(c) == true); + counter_release(c); +} + +TEST(displaced_counter_still_mutable_locally) { + Counter *c = counter_create(default_id()); + counter_inc(c, cid(1), 5); + counter_displace(c); + // Zombie writes still mutate the local Counter — Doc layer is + // responsible for skipping op emission. The primitive itself doesn't + // refuse mutations. + counter_inc(c, cid(1), 3); + ASSERT_EQ(counter_read(c), 8); + counter_release(c); +} + +// acquire balances release: two acquires, three releases would free the +// Counter (refcount: 1 -> 2 -> 3 -> 2 -> 1 -> 0). Test the balanced case +// of one extra acquire + one extra release. +TEST(acquire_release_balanced_keeps_alive) { + Counter *c = counter_create(default_id()); + counter_acquire(c); // refcount = 2 + counter_inc(c, cid(1), 5); + counter_release(c); // refcount = 1 + // Still alive, still readable. + ASSERT_EQ(counter_read(c), 5); + counter_release(c); // refcount = 0 → freed +} + +// Clone is created with refcount=1 (independent of source's refcount). +// Releasing source while the clone is held leaves clone alive. +TEST(clone_has_independent_refcount) { + Counter *src = counter_create(default_id()); + counter_inc(src, cid(1), 5); + Counter *clone = counter_clone(src); + counter_release(src); // src frees; clone untouched + ASSERT_EQ(counter_read(clone), 5); + counter_release(clone); +} + +// Clone of a displaced Counter is itself not displaced — displacement is +// per-instance state, not part of the value. +TEST(clone_of_displaced_counter_is_not_displaced) { + Counter *src = counter_create(default_id()); + counter_displace(src); + Counter *clone = counter_clone(src); + ASSERT(counter_is_displaced(clone) == false); + counter_release(src); + counter_release(clone); } int main(void) { @@ -351,8 +403,15 @@ int main(void) { RUN(clone_empty_counter_reads_zero); RUN(clone_preserves_id); RUN(clone_preserves_per_client_tallies); - RUN(clone_survives_src_arena_destroy); + RUN(clone_survives_src_release); RUN(clone_independent_of_src); + RUN(create_starts_not_displaced); + RUN(displace_sets_flag); + RUN(displaced_counter_still_mutable_locally); + RUN(acquire_release_balanced_keeps_alive); + RUN(clone_has_independent_refcount); + RUN(clone_of_displaced_counter_is_not_displaced); + TEST_SUMMARY(); } From 52368c2ce4571a5ed08f772a1ac38663e2b42aae Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sat, 27 Jun 2026 21:13:32 -0300 Subject: [PATCH 04/12] refactor(register): refcounted host_malloc lifecycle + displace signal Mirror the Counter conversion. register_create allocates via host_malloc with refcount=1; acquire/release free the value's string bytes (scalar_free) and struct at refcount 0. Values deep-copied via scalar_clone(NULL, ...); set and winning merge now scalar_free the old value before storing the new one, closing the per-overwrite string leak the arena model masked. Add register_displace / register_is_displaced. Refresh header ownership/lifetime docs; drop the stale arena.h include. --- register.c | 59 +++++++---- register.h | 49 ++++++++-- test_register.c | 253 ++++++++++++++++++++++++++++++++---------------- 3 files changed, 247 insertions(+), 114 deletions(-) diff --git a/register.c b/register.c index 5899666..0bb4dc3 100644 --- a/register.c +++ b/register.c @@ -1,28 +1,29 @@ #include "register.h" -#include "arena.h" #include "host.h" #include "scalar.h" #include struct Register { ElementId id; - Arena *arena; Scalar value; Stamp stamp; + + size_t refcount; + bool displaced; }; -Register *register_create(Arena *arena, ElementId id, Scalar value, - Stamp stamp) { - Register *reg = arena_alloc(arena, sizeof(Register)); +Register *register_create(ElementId id, Scalar value, Stamp stamp) { + Register *reg = host_malloc(sizeof(Register)); if (!reg) { - host_abortf( - "register_create: arena OOM (requested %zu bytes for Register)", - sizeof(Register)); + host_abortf("register_create: host_malloc OOM (requested %zu bytes for " + "Register)", + sizeof(Register)); } reg->id = id; - reg->arena = arena; - reg->value = scalar_clone(arena, value); + reg->value = scalar_clone(NULL, value); reg->stamp = stamp; + reg->refcount = 1; + reg->displaced = false; return reg; } @@ -32,28 +33,48 @@ Scalar register_read(const Register *reg) { return reg->value; } void register_set(Register *reg, Scalar value, Stamp stamp) { if (stamp_gt(stamp, reg->stamp)) { - reg->value = scalar_clone(reg->arena, value); + scalar_free(reg->value); + reg->value = scalar_clone(NULL, value); reg->stamp = stamp; } } void register_merge(Register *dst, const Register *src) { if (stamp_gt(src->stamp, dst->stamp)) { - dst->value = scalar_clone(dst->arena, src->value); + scalar_free(dst->value); + dst->value = scalar_clone(NULL, src->value); dst->stamp = src->stamp; } } -Register *register_clone(Arena *arena, const Register *reg) { - Register *clone = arena_alloc(arena, sizeof(Register)); +Register *register_clone(const Register *reg) { + Register *clone = host_malloc(sizeof(Register)); if (!clone) { - host_abortf( - "register_clone: arena OOM (requested %zu bytes for Register)", - sizeof(Register)); + host_abortf("register_clone: host_malloc OOM (requested %zu bytes for " + "Register)", + sizeof(Register)); } clone->id = reg->id; - clone->arena = arena; - clone->value = scalar_clone(arena, reg->value); + clone->value = scalar_clone(NULL, reg->value); clone->stamp = reg->stamp; + clone->refcount = 1; + clone->displaced = false; return clone; } + +void register_acquire(Register *reg) { reg->refcount++; } + +void register_release(Register *reg) { + if (reg->refcount == 0) { + host_abort("register_release: refcount already zero"); + } + reg->refcount--; + if (reg->refcount == 0) { + scalar_free(reg->value); + host_free(reg); + } +} + +void register_displace(Register *reg) { reg->displaced = true; } + +bool register_is_displaced(const Register *reg) { return reg->displaced; } diff --git a/register.h b/register.h index e7def10..a1d4008 100644 --- a/register.h +++ b/register.h @@ -24,16 +24,28 @@ // - Scalar values are passed by value (~24 bytes). For SCALAR_NULL / _BOOL // / _INT the payload is in the struct. // - For SCALAR_STRING, the Scalar carries a borrowed (bytes, len) view. -// Register dups those bytes into its arena on every accepted write -// (create, set, winning merge). Old bytes leak in the arena (bump -// allocator can't free) — fine for arena lifetime. +// Register owns its value: on every accepted write (create, set, winning +// merge) it deep-copies the bytes via scalar_clone(NULL, ...) into a +// host_malloc allocation, and frees the previous value's bytes. No leak +// across overwrites. // - register_read returns a Scalar by value; for SCALAR_STRING the bytes -// pointer is a borrowed view into the register's arena. Valid as long as -// the arena lives. Caller must not free or mutate. +// pointer is a borrowed view into the Register's own storage. Valid until +// the next accepted write or until the Register is freed (refcount 0). +// Caller must not free or mutate. // -// Lifetime: Register must not outlive its arena. +// Lifetime — refcounted: +// - register_create returns a Register with refcount = 1; the creator owns +// that ref. register_acquire bumps it; register_release drops it. +// - When refcount hits 0, the Register is freed: the value's string bytes +// (scalar_free) then the struct. Standard C ownership semantics. +// - The displacement signal is independent of refcount: a Register marked +// via register_displace (called by the Map slot path on an LWW +// displacement) stays alive as long as some holder still has a ref. +// Mutations on a displaced Register still mutate local state, but the Doc +// layer (when wired) skips op emission. Holders can check +// register_is_displaced to know their handle is no longer installed in a +// slot. -#include "arena.h" #include "elementid.h" #include "scalar.h" #include "stamp.h" @@ -42,17 +54,34 @@ typedef struct Register Register; -Register *register_create(Arena *arena, ElementId id, Scalar value, - Stamp stamp); +// Allocate a Register via host_malloc with refcount=1, seeded with `value` +// (deep-copied) and `stamp`. Caller owns one ref. +Register *register_create(ElementId id, Scalar value, Stamp stamp); ElementId register_id(const Register *reg); +// Borrowed view of the current value — see Ownership notes on validity. Scalar register_read(const Register *reg); void register_set(Register *reg, Scalar value, Stamp stamp); void register_merge(Register *dst, const Register *src); -Register *register_clone(Arena *arena, const Register *reg); +// Deep-copy the source Register into a fresh allocation. Returned clone has +// refcount=1 and is NOT marked displaced (regardless of src's displaced flag +// — displacement is a per-instance signal, not part of the value). +Register *register_clone(const Register *reg); + +// Reference counting. Acquire bumps the refcount; release drops it. On +// reaching zero, the Register is freed (value bytes via scalar_free, then the +// struct). +void register_acquire(Register *reg); +void register_release(Register *reg); + +// Displacement signal — see lifetime notes above. register_displace marks the +// Register as no-longer-in-a-slot (Map slot path calls this when it +// LWW-displaces the slot's previous value). register_is_displaced reads it. +void register_displace(Register *reg); +bool register_is_displaced(const Register *reg); #endif // _CRDT_REGISTER_H diff --git a/test_register.c b/test_register.c index 6205e3d..724389b 100644 --- a/test_register.c +++ b/test_register.c @@ -1,4 +1,3 @@ -#include "arena.h" #include "clientid.h" #include "elementid.h" #include "register.h" @@ -7,6 +6,16 @@ #include "string.h" #include "test_util.h" +// NOTE: these tests target the refcounted lifecycle contract (host_malloc +// backing, no arena), mirroring test_counter.c. They will not link until +// register.h / register.c are converted: +// Register *register_create(ElementId id, Scalar value, Stamp stamp); // rc=1 +// Register *register_clone(const Register *reg); // rc=1 +// void register_acquire(Register *); +// void register_release(Register *); // frees at rc 0; scalar_free's value +// void register_displace(Register *); +// bool register_is_displaced(const Register *); + // Build a ClientId fixture from a single byte (rest zero). static ClientId cid(uint8_t first_byte) { uint8_t b[16] = {0}; @@ -31,115 +40,101 @@ static Stamp stmp(uint64_t lamport, uint8_t client_first_byte) { } static Register *fresh(Scalar value, Stamp stamp) { - Arena *arena = arena_create(); - return register_create(arena, default_id(), value, stamp); + return register_create(default_id(), value, stamp); } TEST(register_create_stores_id) { - Arena *a = arena_create(); ElementId id = eid(7, 42); - Register *r = register_create(a, id, scalar_int(0), stmp(1, 1)); + Register *r = register_create(id, scalar_int(0), stmp(1, 1)); ASSERT(elementid_eq(register_id(r), id) == true); - arena_destroy(a); -} - -TEST(register_clone_preserves_id) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - ElementId id = eid(7, 42); - Register *src = register_create(as, id, scalar_int(42), stmp(1, 1)); - Register *clone = register_clone(ad, src); - ASSERT(elementid_eq(register_id(clone), id) == true); - arena_destroy(as); - arena_destroy(ad); + register_release(r); } // --- create / read --- TEST(create_seeds_value) { - Register *r = fresh(scalar_int(42), stmp(1, 1)); ASSERT(scalar_eq(register_read(r), scalar_int(42))); + register_release(r); } TEST(create_with_string) { - Register *r = fresh(scalar_string((const uint8_t *)"hello", 5), stmp(1, 1)); ASSERT(scalar_eq(register_read(r), scalar_string((const uint8_t *)"hello", 5))); + register_release(r); } TEST(create_with_null) { - Register *r = fresh(scalar_null(), stmp(1, 1)); ASSERT(scalar_eq(register_read(r), scalar_null())); + register_release(r); } TEST(create_with_bool) { - Register *r = fresh(scalar_bool(true), stmp(1, 1)); ASSERT(scalar_eq(register_read(r), scalar_bool(true))); + register_release(r); } // --- LWW: local set --- TEST(higher_lamport_wins) { - Register *r = fresh(scalar_int(10), stmp(1, 1)); register_set(r, scalar_int(20), stmp(2, 1)); ASSERT(scalar_eq(register_read(r), scalar_int(20))); + register_release(r); } TEST(lower_lamport_ignored) { - Register *r = fresh(scalar_int(20), stmp(5, 1)); register_set(r, scalar_int(10), stmp(3, 1)); // older lamport — ignored ASSERT(scalar_eq(register_read(r), scalar_int(20))); + register_release(r); } TEST(equal_lamport_higher_client_wins) { - Register *r = fresh(scalar_int(10), stmp(5, 1)); register_set(r, scalar_int(20), stmp(5, 2)); // same lamport, higher client ASSERT(scalar_eq(register_read(r), scalar_int(20))); + register_release(r); } TEST(equal_lamport_lower_client_ignored) { - Register *r = fresh(scalar_int(20), stmp(5, 2)); register_set(r, scalar_int(10), stmp(5, 1)); // same lamport, lower client ASSERT(scalar_eq(register_read(r), scalar_int(20))); + register_release(r); } TEST(set_same_stamp_idempotent) { - Register *r = fresh(scalar_int(42), stmp(5, 1)); register_set(r, scalar_int(42), stmp(5, 1)); ASSERT(scalar_eq(register_read(r), scalar_int(42))); + register_release(r); } // Order of application does not matter: newer-then-older converges to newer. TEST(out_of_order_sets_converge) { - Register *r = fresh(scalar_int(1), stmp(1, 1)); register_set(r, scalar_int(99), stmp(10, 1)); // newer register_set(r, scalar_int(2), stmp(2, 1)); // older — ignored ASSERT(scalar_eq(register_read(r), scalar_int(99))); + register_release(r); } // A newer write can change the Scalar kind. TEST(kind_changes_on_newer_write) { - Register *r = fresh(scalar_int(42), stmp(1, 1)); register_set(r, scalar_string((const uint8_t *)"hi", 2), stmp(2, 1)); ASSERT( scalar_eq(register_read(r), scalar_string((const uint8_t *)"hi", 2))); + register_release(r); } -// String bytes must be copied into the arena: mutating the caller's buffer +// String bytes must be copied into owned storage: mutating the caller's buffer // after set/create must not affect what register_read returns. TEST(string_bytes_are_copied) { - uint8_t key[8]; memcpy(key, "hello", 5); Register *r = fresh(scalar_string(key, 5), stmp(1, 1)); @@ -149,35 +144,50 @@ TEST(string_bytes_are_copied) { ASSERT(scalar_eq(register_read(r), scalar_string((const uint8_t *)"hello", 5))); + register_release(r); +} + +// A newer string write must free the previously-held string and own the new +// one. (Behaviorally observable only as "no leak"; the read assertion guards +// correctness, ASan/leak-checking guards the free.) +TEST(string_replaced_by_newer_string) { + Register *r = fresh(scalar_string((const uint8_t *)"first", 5), stmp(1, 1)); + register_set(r, scalar_string((const uint8_t *)"second", 6), stmp(2, 1)); + ASSERT(scalar_eq(register_read(r), + scalar_string((const uint8_t *)"second", 6))); + register_release(r); } // --- merge (two replicas) --- TEST(merge_src_newer_wins) { - Register *a = fresh(scalar_int(10), stmp(1, 1)); Register *b = fresh(scalar_int(20), stmp(2, 2)); // newer register_merge(a, b); ASSERT(scalar_eq(register_read(a), scalar_int(20))); + register_release(a); + register_release(b); } TEST(merge_src_older_ignored) { - Register *a = fresh(scalar_int(20), stmp(5, 1)); // newer Register *b = fresh(scalar_int(10), stmp(2, 2)); register_merge(a, b); ASSERT(scalar_eq(register_read(a), scalar_int(20))); + register_release(a); + register_release(b); } TEST(merge_equal_lamport_client_tiebreak) { - Register *a = fresh(scalar_int(10), stmp(5, 1)); Register *b = fresh(scalar_int(20), stmp(5, 2)); // same lamport, > cid register_merge(a, b); ASSERT(scalar_eq(register_read(a), scalar_int(20))); + register_release(a); + register_release(b); } // Concurrent writes converge to the same winner regardless of merge direction. @@ -192,10 +202,13 @@ TEST(merge_commutative) { ASSERT(scalar_eq(register_read(a1), register_read(b2))); ASSERT(scalar_eq(register_read(a1), scalar_int(20))); + register_release(a1); + register_release(b1); + register_release(a2); + register_release(b2); } TEST(merge_idempotent) { - Register *a = fresh(scalar_int(10), stmp(1, 1)); Register *b = fresh(scalar_int(20), stmp(2, 1)); @@ -206,6 +219,8 @@ TEST(merge_idempotent) { ASSERT(scalar_eq(once, twice)); ASSERT(scalar_eq(twice, scalar_int(20))); + register_release(a); + register_release(b); } TEST(merge_associative) { @@ -225,94 +240,154 @@ TEST(merge_associative) { ASSERT(scalar_eq(register_read(a), register_read(a2))); ASSERT(scalar_eq(register_read(a), scalar_int(30))); + register_release(a); + register_release(b); + register_release(c); + register_release(a2); + register_release(b2); + register_release(c2); } TEST(merge_does_not_mutate_src) { - Register *a = fresh(scalar_int(99), stmp(10, 1)); // a newer Register *b = fresh(scalar_int(7), stmp(1, 1)); register_merge(a, b); ASSERT(scalar_eq(register_read(b), scalar_int(7))); // b unchanged + register_release(a); + register_release(b); } // When merge takes src's winning string value, dst must own its own copy. -// Mutating src's value bytes after merge must not affect dst's read. -TEST(merge_copies_string_into_dst_arena) { - - uint8_t src_bytes[8]; - memcpy(src_bytes, "hello", 5); - +// Releasing src after the merge must leave dst's string intact. +TEST(merge_string_survives_src_release) { Register *a = fresh(scalar_int(0), stmp(1, 1)); - Register *b = fresh(scalar_string(src_bytes, 5), stmp(5, 1)); - - register_merge(a, b); // a takes b's string + Register *b = fresh(scalar_string((const uint8_t *)"hello", 5), stmp(5, 1)); - // Scribble src's buffer. - src_bytes[0] = 'X'; - src_bytes[1] = 'X'; + register_merge(a, b); // a takes b's string (deep copy) + register_release(b); // b frees; a must be unaffected ASSERT(scalar_eq(register_read(a), scalar_string((const uint8_t *)"hello", 5))); + register_release(a); } -// --- register_clone: deep copy into a target arena --- +// --- register_clone: deep copy, fresh refcount --- + +TEST(register_clone_preserves_id) { + ElementId id = eid(7, 42); + Register *src = register_create(id, scalar_int(42), stmp(1, 1)); + Register *clone = register_clone(src); + ASSERT(elementid_eq(register_id(clone), id) == true); + register_release(src); + register_release(clone); +} TEST(clone_preserves_value) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Register *src = - register_create(as, default_id(), scalar_int(42), stmp(5, 1)); - Register *clone = register_clone(ad, src); + Register *src = register_create(default_id(), scalar_int(42), stmp(5, 1)); + Register *clone = register_clone(src); ASSERT(clone != NULL); ASSERT(clone != src); ASSERT(scalar_eq(register_read(clone), scalar_int(42))); - arena_destroy(as); - arena_destroy(ad); -} - -// Clone must own its string bytes in dst arena — destroying src arena -// must leave the clone intact. -TEST(clone_string_survives_src_arena_destroy) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Register *src = - register_create(as, default_id(), - scalar_string((const uint8_t *)"hello", 5), stmp(1, 1)); - Register *clone = register_clone(ad, src); - arena_destroy(as); + register_release(src); + register_release(clone); +} + +// Clone must own its own string copy — releasing src must leave clone intact. +TEST(clone_string_survives_src_release) { + Register *src = register_create( + default_id(), scalar_string((const uint8_t *)"hello", 5), stmp(1, 1)); + Register *clone = register_clone(src); + register_release(src); ASSERT(scalar_eq(register_read(clone), scalar_string((const uint8_t *)"hello", 5))); - arena_destroy(ad); + register_release(clone); } // Mutating src after clone must not affect the clone, and vice versa. TEST(clone_independent_of_src) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Register *src = - register_create(as, default_id(), scalar_int(1), stmp(1, 1)); - Register *clone = register_clone(ad, src); + Register *src = register_create(default_id(), scalar_int(1), stmp(1, 1)); + Register *clone = register_clone(src); register_set(src, scalar_int(99), stmp(10, 1)); register_set(clone, scalar_int(7), stmp(10, 1)); ASSERT(scalar_eq(register_read(src), scalar_int(99))); ASSERT(scalar_eq(register_read(clone), scalar_int(7))); - arena_destroy(as); - arena_destroy(ad); + register_release(src); + register_release(clone); } -// Clone preserves the stamp — subsequent set with a stamp ≤ the source's +// Clone preserves the stamp — a subsequent set with a stamp ≤ the source's // original stamp must lose LWW on the clone. TEST(clone_preserves_stamp) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Register *src = - register_create(as, default_id(), scalar_int(10), stmp(5, 1)); - Register *clone = register_clone(ad, src); + Register *src = register_create(default_id(), scalar_int(10), stmp(5, 1)); + Register *clone = register_clone(src); register_set(clone, scalar_int(99), stmp(3, 1)); // older, must lose ASSERT(scalar_eq(register_read(clone), scalar_int(10))); - arena_destroy(as); - arena_destroy(ad); + register_release(src); + register_release(clone); +} + +// --- refcount + displacement --- +// +// register_create returns refcount=1; release on a fresh handle frees. +// acquire/release accounting balances correctly across multiple holders. +// The displaced flag is independent of refcount: marking displaced does not +// free the Register; only refcount reaching zero does. + +TEST(create_starts_not_displaced) { + Register *r = fresh(scalar_int(0), stmp(1, 1)); + ASSERT(register_is_displaced(r) == false); + register_release(r); +} + +TEST(displace_sets_flag) { + Register *r = fresh(scalar_int(0), stmp(1, 1)); + register_displace(r); + ASSERT(register_is_displaced(r) == true); + register_release(r); +} + +TEST(displaced_register_still_mutable_locally) { + Register *r = fresh(scalar_int(1), stmp(1, 1)); + register_displace(r); + // Zombie writes still mutate the local Register — the Doc layer is + // responsible for skipping op emission. The primitive does not refuse + // mutations. + register_set(r, scalar_int(2), stmp(2, 1)); + ASSERT(scalar_eq(register_read(r), scalar_int(2))); + register_release(r); +} + +// acquire balances release: one extra acquire + one extra release keeps the +// Register alive and readable (refcount: 1 -> 2 -> 1). +TEST(acquire_release_balanced_keeps_alive) { + Register *r = fresh(scalar_int(5), stmp(1, 1)); + register_acquire(r); // refcount = 2 + register_set(r, scalar_int(9), stmp(2, 1)); + register_release(r); // refcount = 1 + ASSERT(scalar_eq(register_read(r), scalar_int(9))); + register_release(r); // refcount = 0 → freed +} + +// Clone is created with refcount=1 (independent of source's refcount). +// Releasing source while the clone is held leaves clone alive. +TEST(clone_has_independent_refcount) { + Register *src = register_create(default_id(), scalar_int(5), stmp(1, 1)); + Register *clone = register_clone(src); + register_release(src); // src frees; clone untouched + ASSERT(scalar_eq(register_read(clone), scalar_int(5))); + register_release(clone); +} + +// Clone of a displaced Register is itself not displaced — displacement is +// per-instance state, not part of the value. +TEST(clone_of_displaced_register_is_not_displaced) { + Register *src = register_create(default_id(), scalar_int(0), stmp(1, 1)); + register_displace(src); + Register *clone = register_clone(src); + ASSERT(register_is_displaced(clone) == false); + register_release(src); + register_release(clone); } int main(void) { @@ -330,6 +405,7 @@ int main(void) { RUN(out_of_order_sets_converge); RUN(kind_changes_on_newer_write); RUN(string_bytes_are_copied); + RUN(string_replaced_by_newer_string); RUN(merge_src_newer_wins); RUN(merge_src_older_ignored); @@ -338,13 +414,20 @@ int main(void) { RUN(merge_idempotent); RUN(merge_associative); RUN(merge_does_not_mutate_src); - RUN(merge_copies_string_into_dst_arena); + RUN(merge_string_survives_src_release); RUN(register_clone_preserves_id); RUN(clone_preserves_value); - RUN(clone_string_survives_src_arena_destroy); + RUN(clone_string_survives_src_release); RUN(clone_independent_of_src); RUN(clone_preserves_stamp); + RUN(create_starts_not_displaced); + RUN(displace_sets_flag); + RUN(displaced_register_still_mutable_locally); + RUN(acquire_release_balanced_keeps_alive); + RUN(clone_has_independent_refcount); + RUN(clone_of_displaced_register_is_not_displaced); + TEST_SUMMARY(); } From 674a8a67719fac68974200a7d85878ba9acf2bc7 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sat, 27 Jun 2026 22:10:02 -0300 Subject: [PATCH 05/12] refactor(element,map): refcounted lifecycle with Share slot semantics Convert Element and Map off arena ownership onto the refcount/displace model (completing the lifecycle refactor alongside Counter and Register). Element forwards lifecycle to its composite: element_acquire / _release / _displace / _is_displaced dispatch by kind (SCALAR is a no-op for acquire/displace and scalar_free on release); element_clone drops the arena param and produces refcount=1 children. Map slots hold refcounted composites under Share semantics: an accepted map_set acquires the slot's own ref, so callers always retain and release their own handle regardless of the LWW outcome. Eviction (winning set, delete, merge LWW-replace) displaces then releases the loser. map_get and the helper install path return borrows; the helper detached path (losing LWW) returns an owned, born-displaced handle. map_clone deep-copies with refcount=1; map_release recursively drops slot refs. Tests rewritten for the new contract (test_element +35, test_map +76), including displacement, borrow/retain, and detached-handle coverage. Verified green under ASan (no use-after-free / double-free); note macOS ASan ships no LeakSanitizer, so the leak class needs Linux CI. --- element.c | 74 ++++- element.h | 8 +- map.c | 90 ++++-- map.h | 73 +++-- test_element.c | 305 ++++++++++++-------- test_map.c | 769 +++++++++++++++++++++++++++++-------------------- 6 files changed, 839 insertions(+), 480 deletions(-) diff --git a/element.c b/element.c index c1e0939..4bf43e4 100644 --- a/element.c +++ b/element.c @@ -76,28 +76,92 @@ void element_merge(Element dst, Element src) { } } -Element element_clone(Arena *arena, Element e) { +Element element_clone(Element e) { Element result; switch (e.kind) { case ELEMENT_SCALAR: { - Scalar cloned = scalar_clone(arena, e.as.scalar); + Scalar cloned = scalar_clone(NULL, e.as.scalar); result = element_scalar(cloned); } break; case ELEMENT_REGISTER: { - Register *reg = register_clone(arena, e.as.reg); + Register *reg = register_clone(e.as.reg); result = element_register(reg); } break; case ELEMENT_COUNTER: { - Counter *counter = counter_clone(arena, e.as.counter); + Counter *counter = counter_clone(e.as.counter); result = element_counter(counter); } break; case ELEMENT_MAP: { - Map *map = map_clone(arena, e.as.map); + Map *map = map_clone(e.as.map); result = element_map(map); } break; } return result; } + +void element_acquire(Element e) { + switch (e.kind) { + case ELEMENT_SCALAR: + // No-op: scalar elements have no refcount. + break; + case ELEMENT_REGISTER: + register_acquire(e.as.reg); + break; + case ELEMENT_COUNTER: + counter_acquire(e.as.counter); + break; + case ELEMENT_MAP: + map_acquire(e.as.map); + break; + } +} + +void element_release(Element e) { + switch (e.kind) { + case ELEMENT_SCALAR: + // No-op: scalar elements have no refcount. + break; + case ELEMENT_REGISTER: + register_release(e.as.reg); + break; + case ELEMENT_COUNTER: + counter_release(e.as.counter); + break; + case ELEMENT_MAP: + map_release(e.as.map); + break; + } +} + +void element_displace(Element e) { + switch (e.kind) { + case ELEMENT_SCALAR: + // No-op: scalar elements are never displaced. + break; + case ELEMENT_REGISTER: + register_displace(e.as.reg); + break; + case ELEMENT_COUNTER: + counter_displace(e.as.counter); + break; + case ELEMENT_MAP: + map_displace(e.as.map); + break; + } +} + +bool element_is_displaced(Element e) { + switch (e.kind) { + case ELEMENT_SCALAR: + return false; // scalar elements are never displaced + case ELEMENT_REGISTER: + return register_is_displaced(e.as.reg); + case ELEMENT_COUNTER: + return counter_is_displaced(e.as.counter); + case ELEMENT_MAP: + return map_is_displaced(e.as.map); + } +} diff --git a/element.h b/element.h index 5ed46fa..5aae155 100644 --- a/element.h +++ b/element.h @@ -56,6 +56,12 @@ Element element_map(Map *m); ElementKind element_kind(Element e); const char *element_kind_name(ElementKind k); void element_merge(Element dst, Element src); -Element element_clone(Arena *arena, Element e); +Element element_clone(Element e); + +void element_acquire(Element e); +void element_release(Element e); + +void element_displace(Element e); +bool element_is_displaced(Element e); #endif // _CRDT_ELEMENT_H diff --git a/map.c b/map.c index 0ad45fa..7ee7e73 100644 --- a/map.c +++ b/map.c @@ -12,19 +12,22 @@ typedef struct MapEntry { struct Map { ElementId id; - Arena *arena; HashTable *entries; + + size_t refcount; + bool displaced; }; -Map *map_create(Arena *arena, ElementId id) { - Map *map = arena_alloc(arena, sizeof(Map)); +Map *map_create(ElementId id) { + Map *map = host_malloc(sizeof(Map)); if (!map) { host_abortf("map_create: arena OOM (requested %zu bytes for Map)", sizeof(Map)); } map->id = id; - map->arena = arena; - map->entries = hashtable_create(arena); + map->entries = hashtable_create(NULL); + map->refcount = 1; + map->displaced = false; if (!map->entries) { host_abort("map_create: hashtable_create OOM (slot table)"); } @@ -55,8 +58,9 @@ void map_set(Map *map, const void *key, size_t key_len, Element value, bool present = hashtable_get(map->entries, key, key_len, (void **)&entry); bool update = !present || stamp_gt(stamp, entry->stamp); if (update) { + element_acquire(value); if (!present) { - entry = arena_alloc(map->arena, sizeof(Entry)); + entry = host_malloc(sizeof(Entry)); if (!entry) { host_abortf( "map_set: arena OOM (requested %zu bytes for Entry)", @@ -72,7 +76,7 @@ void map_set(Map *map, const void *key, size_t key_len, Element value, switch (value.kind) { case ELEMENT_SCALAR: { - Scalar copy = scalar_clone(map->arena, value.as.scalar); + Scalar copy = scalar_clone(NULL, value.as.scalar); value.as.scalar = copy; break; } @@ -84,6 +88,8 @@ void map_set(Map *map, const void *key, size_t key_len, Element value, break; } + element_displace(entry->value); + element_release(entry->value); entry->value = value; entry->stamp = stamp; entry->is_tombstone = false; @@ -97,11 +103,13 @@ void map_delete(Map *map, const void *key, size_t key_len, Stamp stamp) { if (stamp_gt(stamp, entry->stamp)) { entry->stamp = stamp; entry->is_tombstone = true; + element_displace(entry->value); + element_release(entry->value); } } else { // Install a tombstone for the absent key, so that future merges can // compare stamps and know that the delete wins over older sets. - entry = arena_alloc(map->arena, sizeof(Entry)); + entry = host_malloc(sizeof(Entry)); if (!entry) { host_abortf("map_delete: arena OOM (requested %zu bytes for Entry)", sizeof(Entry)); @@ -140,7 +148,7 @@ void map_merge(Map *dst, const Map *src) { // slot; if src wins, replace dst's composite with a // clone of src's. Loser is orphaned. if (stamp_gt(se->stamp, de->stamp)) { - de->value = element_clone(dst->arena, se->value); + de->value = element_clone(se->value); de->stamp = se->stamp; } continue; @@ -168,7 +176,7 @@ void map_merge(Map *dst, const Map *src) { continue; } - Element cloned = element_clone(dst->arena, se->value); + Element cloned = element_clone(se->value); map_set(dst, k, klen, cloned, se->stamp); } } @@ -199,12 +207,18 @@ Counter *map_counter(Map *map, const void *key, size_t key_len, Stamp stamp) { } ElementId id = elementid_derive(map->id, key, key_len, ELEMENT_COUNTER); - Counter *fresh = counter_create(map->arena, id); + Counter *fresh = counter_create(id); // create-ref: rc = 1 if (!present || stamp_gt(stamp, existing->stamp)) { + // Installed: map_set acquires the slot's ref (rc = 2). Drop our + // create-ref so the slot is the sole owner and the returned handle is + // a borrow (rc = 1). map_set(map, key, key_len, element_counter(fresh), stamp); + counter_release(fresh); + } else { + // Detached when LWW lost: never installed, so born displaced. The + // caller owns this rc = 1 handle and must release it. + counter_displace(fresh); } - // Detached when LWW lost: returned anyway so caller always gets a usable - // handle. return fresh; } @@ -219,9 +233,17 @@ Register *map_register(Map *map, const void *key, size_t key_len, Scalar seed, } ElementId id = elementid_derive(map->id, key, key_len, ELEMENT_REGISTER); - Register *fresh = register_create(map->arena, id, seed, stamp); + Register *fresh = register_create(id, seed, stamp); // create-ref: rc = 1 if (!present || stamp_gt(stamp, existing->stamp)) { + // Installed: map_set acquires the slot's ref (rc = 2). Drop our + // create-ref so the slot is the sole owner and the returned handle is + // a borrow (rc = 1). map_set(map, key, key_len, element_register(fresh), stamp); + register_release(fresh); + } else { + // Detached when LWW lost: never installed, so born displaced. The + // caller owns this rc = 1 handle and must release it. + register_displace(fresh); } return fresh; } @@ -236,22 +258,30 @@ Map *map_map(Map *map, const void *key, size_t key_len, Stamp stamp) { } ElementId id = elementid_derive(map->id, key, key_len, ELEMENT_MAP); - Map *fresh = map_create(map->arena, id); + Map *fresh = map_create(id); // create-ref: rc = 1 if (!present || stamp_gt(stamp, existing->stamp)) { + // Installed: map_set acquires the slot's ref (rc = 2). Drop our + // create-ref so the slot is the sole owner and the returned handle is + // a borrow (rc = 1). map_set(map, key, key_len, element_map(fresh), stamp); + map_release(fresh); + } else { + // Detached when LWW lost: never installed, so born displaced. The + // caller owns this rc = 1 handle and must release it. + map_displace(fresh); } return fresh; } -Map *map_clone(Arena *arena, const Map *map) { - Map *clone = map_create(arena, map->id); +Map *map_clone(const Map *map) { + Map *clone = map_create(map->id); HashTableIter it = hashtable_iter(map->entries); const void *k; size_t klen; void *v; while (hashtable_iter_next(&it, &k, &klen, &v)) { Entry *entry = v; - Entry *copy = arena_alloc(clone->arena, sizeof(Entry)); + Entry *copy = host_malloc(sizeof(Entry)); if (!copy) { host_abortf("map_clone: arena OOM (requested %zu bytes for Entry)", sizeof(Entry)); @@ -260,7 +290,7 @@ Map *map_clone(Arena *arena, const Map *map) { copy->stamp = entry->stamp; copy->is_tombstone = entry->is_tombstone; if (!entry->is_tombstone) { - copy->value = element_clone(clone->arena, entry->value); + copy->value = element_clone(entry->value); } HashTableInsertResult r = hashtable_insert(clone->entries, k, klen, copy); @@ -271,3 +301,25 @@ Map *map_clone(Arena *arena, const Map *map) { } return clone; } + +void map_acquire(Map *map) { map->refcount++; } +void map_release(Map *map) { + if (--map->refcount == 0) { + HashTableIter it = hashtable_iter(map->entries); + const void *k; + size_t klen; + void *v; + while (hashtable_iter_next(&it, &k, &klen, &v)) { + Entry *entry = v; + if (!entry->is_tombstone) { + element_release(entry->value); + } + host_free(entry); + } + hashtable_destroy(map->entries); + host_free(map); + } +} + +void map_displace(Map *map) { map->displaced = true; } +bool map_is_displaced(Map *map) { return map->displaced; } diff --git a/map.h b/map.h index 0ce7cc1..5cc73a1 100644 --- a/map.h +++ b/map.h @@ -24,27 +24,30 @@ // - Both alive AND same composite kind BUT mismatched ids → distinct // logical elements that happen to share the slot. LWW on slot stamp; // if src wins, dst's composite is replaced with a deep clone of src's -// into dst's arena. Loser is orphaned. -// - Otherwise → LWW on slot stamp. Scalar winners are scalar_clone'd -// into dst's arena. Composite winners are deep-cloned via element_clone -// into dst's arena, so dst owns its slot fully and survives src arena -// destroy. +// (element_clone, refcount=1). The loser is displaced + released. +// - Otherwise → LWW on slot stamp. Scalar winners are deep-copied; composite +// winners are deep-cloned via element_clone (refcount=1) so dst owns its +// slot fully and is independent of src. // -// Ownership: -// - SCALAR_STRING values are cloned into the Map's arena on every accepted -// write (set, winning merge). When map_get fills *out with a SCALAR -// Element, the string bytes are a borrowed view into that arena; valid -// as long as the arena lives. Caller must not free or mutate. -// - Composite slots (REGISTER / COUNTER / MAP) are stored as pointers. -// map_set does NOT clone composites — the pointed-to object must live -// in the same arena as the Map. map_merge's LWW path clones via -// element_clone, so the cross-arena hazard does not surface there. -// - Map lives in its arena; arena_destroy cleans up everything (no -// separate map_destroy needed). +// Ownership (Share semantics, refcounted — no arena): +// - SCALAR_STRING values are deep-copied into Map-owned storage on every +// accepted write (set, winning merge). When map_get fills *out with a +// SCALAR Element, the bytes are a borrowed view valid until the slot's +// next accepted write or until the Map is freed. Caller must not free or +// mutate. +// - Composite slots (REGISTER / COUNTER / MAP) are stored as refcounted +// pointers. An accepted map_set element_acquires the slot's own ref, so +// the caller always retains and releases the handle it passed, regardless +// of the LWW outcome. Eviction (winning set/delete, merge LWW-replace) +// displaces then releases the slot's ref on the loser. +// - map_get and the helper install path return BORROWS (the slot owns the +// ref); acquire to keep one valid past the next eviction. // -// Lifetime: Map must not outlive its arena. +// Lifetime — refcounted: map_create returns refcount=1; map_acquire / +// map_release manage it. At refcount 0 the Map releases each live slot +// composite (recursive for nested maps), then frees itself. map_displace marks +// the Map as evicted from a parent slot (it is itself a composite kind). -#include "arena.h" #include "element.h" #include "elementid.h" #include "scalar.h" @@ -54,7 +57,7 @@ typedef struct Map Map; -Map *map_create(Arena *arena, ElementId id); +Map *map_create(ElementId id); ElementId map_id(const Map *map); @@ -73,14 +76,15 @@ void map_merge(Map *dst, const Map *src); size_t map_size(const Map *map); // Get-or-create helpers. Behaviour per call: -// 1. Live slot with matching kind at `key` → return the existing pointer -// (stamp + seed value ignored). +// 1. Live slot with matching kind at `key` → return the existing pointer as +// a borrow (stamp + seed value ignored). // 2. Else (empty, tombstone, scalar, or different-kind composite) AND -// `stamp` wins LWW against any existing entry → create a fresh -// composite in the Map's arena, install in the slot, return it. -// 3. Else → return a DETACHED composite: created in the Map's arena but -// not installed in the slot. Caller always gets a usable handle; the -// slot is left untouched. +// `stamp` wins LWW against any existing entry → create and install a +// fresh composite, returning a BORROW (the slot owns the ref). +// 3. Else → return a DETACHED composite: created but not installed, born +// displaced and OWNED by the caller (refcount=1, must release). The slot +// is left untouched. +// In all cases the caller may acquire a borrow to keep it past eviction. Counter *map_counter(Map *map, const void *key, size_t key_len, Stamp stamp); Register *map_register(Map *map, const void *key, size_t key_len, Scalar seed, @@ -88,6 +92,21 @@ Register *map_register(Map *map, const void *key, size_t key_len, Scalar seed, Map *map_map(Map *map, const void *key, size_t key_len, Stamp stamp); -Map *map_clone(Arena *arena, const Map *map); +// Deep recursive copy into a fresh allocation with refcount=1 (composite slots +// cloned via element_clone). The clone is NOT displaced and is independent of +// the source. +Map *map_clone(const Map *map); + +// Reference counting. Acquire bumps the refcount; release drops it. On reaching +// zero, the Map releases each live slot composite (recursive) then frees +// itself. +void map_acquire(Map *map); +void map_release(Map *map); + +// Displacement signal — marks the Map as no-longer-in-a-slot (a parent Map's +// slot path calls this when it LWW-displaces this Map). Independent of +// refcount; see the lifetime notes above. +void map_displace(Map *map); +bool map_is_displaced(Map *map); #endif // _CRDT_MAP_H diff --git a/test_element.c b/test_element.c index 87bff3e..e83434c 100644 --- a/test_element.c +++ b/test_element.c @@ -1,4 +1,3 @@ -#include "arena.h" #include "clientid.h" #include "counter.h" #include "element.h" @@ -10,6 +9,20 @@ #include "string.h" #include "test_util.h" +// NOTE: targets the refcounted lifecycle contract (Option A — Element forwards +// lifecycle to the composite). Will not link until BOTH element.c and map.c +// are converted, since Element's MAP arm calls into Map. Expected surface: +// - element_clone(Element): drops arena, makes refcount=1 children. +// - element_acquire(Element): SCALAR no-op; composite -> *_acquire. +// - element_release(Element): SCALAR scalar_free; composite -> *_release. +// - element_displace(Element): SCALAR no-op; composite -> *_displace. +// - element_is_displaced(Element): SCALAR false; composite -> *_is_displaced. +// +// Ownership rule mirrored from scalar_free: element_release on a SCALAR frees +// the scalar's bytes, so it is valid ONLY on owned scalars (those produced by +// element_clone, i.e. scalar_clone(NULL, ...) provenance) — never on a +// borrowed-buffer element_scalar(...) passed transiently into map_set. + // Helpers. static ClientId cid(uint8_t first_byte) { @@ -42,30 +55,27 @@ TEST(scalar_constructor_sets_kind_and_value) { } TEST(register_constructor_sets_kind_and_pointer) { - Arena *a = arena_create(); - Register *r = register_create(a, default_id(), scalar_int(1), stmp(1, 1)); + Register *r = register_create(default_id(), scalar_int(1), stmp(1, 1)); Element e = element_register(r); ASSERT_EQ(element_kind(e), ELEMENT_REGISTER); ASSERT(e.as.reg == r); - arena_destroy(a); + element_release(e); } TEST(counter_constructor_sets_kind_and_pointer) { - Arena *a = arena_create(); - Counter *c = counter_create(a, default_id()); + Counter *c = counter_create(default_id()); Element e = element_counter(c); ASSERT_EQ(element_kind(e), ELEMENT_COUNTER); ASSERT(e.as.counter == c); - arena_destroy(a); + element_release(e); } TEST(map_constructor_sets_kind_and_pointer) { - Arena *a = arena_create(); - Map *m = map_create(a, default_id()); + Map *m = map_create(default_id()); Element e = element_map(m); ASSERT_EQ(element_kind(e), ELEMENT_MAP); ASSERT(e.as.map == m); - arena_destroy(a); + element_release(e); } // --- kind name (for diagnostics) --- @@ -86,43 +96,58 @@ TEST(kind_name_map) { ASSERT(strcmp(element_kind_name(ELEMENT_MAP), "MAP") == 0); } +// --- element_id reads the composite's id --- + +TEST(id_register) { + ElementId id = eid(7, 42); + Register *r = register_create(id, scalar_int(1), stmp(1, 1)); + ASSERT(elementid_eq(element_id(element_register(r)), id) == true); + register_release(r); +} + +TEST(id_counter) { + ElementId id = eid(7, 42); + Counter *c = counter_create(id); + ASSERT(elementid_eq(element_id(element_counter(c)), id) == true); + counter_release(c); +} + +TEST(id_map) { + ElementId id = eid(7, 42); + Map *m = map_create(id); + ASSERT(elementid_eq(element_id(element_map(m)), id) == true); + map_release(m); +} + // --- merge dispatches by kind to the underlying _merge --- TEST(merge_register_takes_newer_value) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - Register *dst = - register_create(ad, default_id(), scalar_int(10), stmp(1, 1)); - Register *src = - register_create(as, default_id(), scalar_int(20), stmp(5, 1)); + Register *dst = register_create(default_id(), scalar_int(10), stmp(1, 1)); + Register *src = register_create(default_id(), scalar_int(20), stmp(5, 1)); element_merge(element_register(dst), element_register(src)); ASSERT(scalar_eq(register_read(dst), scalar_int(20))); - arena_destroy(ad); - arena_destroy(as); + register_release(dst); + register_release(src); } TEST(merge_counter_unions_clients) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - Counter *dst = counter_create(ad, default_id()); - Counter *src = counter_create(as, default_id()); + Counter *dst = counter_create(default_id()); + Counter *src = counter_create(default_id()); counter_inc(dst, cid(1), 5); counter_inc(src, cid(2), 3); element_merge(element_counter(dst), element_counter(src)); ASSERT_EQ(counter_read(dst), 8); - arena_destroy(ad); - arena_destroy(as); + counter_release(dst); + counter_release(src); } TEST(merge_map_takes_newer_slot) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - Map *dst = map_create(ad, default_id()); - Map *src = map_create(as, default_id()); + Map *dst = map_create(default_id()); + Map *src = map_create(default_id()); const uint8_t *k = (const uint8_t *)"k"; size_t klen = 1; @@ -135,44 +160,36 @@ TEST(merge_map_takes_newer_slot) { ASSERT(map_get(dst, k, klen, &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_SCALAR); ASSERT(scalar_eq(out.as.scalar, scalar_int(20))); - arena_destroy(ad); - arena_destroy(as); + map_release(dst); + map_release(src); } TEST(merge_register_does_not_mutate_src) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - Register *dst = - register_create(ad, default_id(), scalar_int(99), stmp(10, 1)); - Register *src = - register_create(as, default_id(), scalar_int(7), stmp(1, 1)); + Register *dst = register_create(default_id(), scalar_int(99), stmp(10, 1)); + Register *src = register_create(default_id(), scalar_int(7), stmp(1, 1)); element_merge(element_register(dst), element_register(src)); ASSERT(scalar_eq(register_read(src), scalar_int(7))); - arena_destroy(ad); - arena_destroy(as); + register_release(dst); + register_release(src); } TEST(merge_counter_does_not_mutate_src) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - Counter *dst = counter_create(ad, default_id()); - Counter *src = counter_create(as, default_id()); + Counter *dst = counter_create(default_id()); + Counter *src = counter_create(default_id()); counter_inc(src, cid(1), 3); element_merge(element_counter(dst), element_counter(src)); ASSERT_EQ(counter_read(src), 3); - arena_destroy(ad); - arena_destroy(as); + counter_release(dst); + counter_release(src); } TEST(merge_map_does_not_mutate_src) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - Map *dst = map_create(ad, default_id()); - Map *src = map_create(as, default_id()); + Map *dst = map_create(default_id()); + Map *src = map_create(default_id()); const uint8_t *k = (const uint8_t *)"k"; map_set(src, k, 1, element_scalar(scalar_int(7)), stmp(1, 1)); @@ -182,86 +199,75 @@ TEST(merge_map_does_not_mutate_src) { ASSERT(map_get(src, k, 1, &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_SCALAR); ASSERT(scalar_eq(out.as.scalar, scalar_int(7))); - arena_destroy(ad); - arena_destroy(as); + map_release(dst); + map_release(src); } TEST(round_trip_via_kind_and_payload) { - Arena *a = arena_create(); - Counter *c = counter_create(a, default_id()); + Counter *c = counter_create(default_id()); Element e = element_counter(c); ASSERT_EQ(element_kind(e), ELEMENT_COUNTER); ASSERT(e.as.counter == c); - arena_destroy(a); + element_release(e); } -// --- element_clone: deep copy into a target arena --- +// --- element_clone: deep copy, drops arena, refcount=1 children --- // -// Used by map_merge when an LWW winner is a composite from a foreign arena. -// The clone must own all its memory in the destination arena, so the source -// arena can be destroyed independently. Mutating the source after clone must -// NOT affect the clone. +// Used by map_merge when an LWW winner is a composite from a foreign replica. +// The clone owns all its memory; releasing the source must leave the clone +// intact. Mutating the source after clone must NOT affect the clone. TEST(clone_scalar_int_preserves_value) { - Arena *dst = arena_create(); - Element src = element_scalar(scalar_int(42)); - Element clone = element_clone(dst, src); + Element clone = element_clone(element_scalar(scalar_int(42))); ASSERT_EQ(element_kind(clone), ELEMENT_SCALAR); ASSERT(scalar_eq(clone.as.scalar, scalar_int(42))); - arena_destroy(dst); + element_release(clone); // owned scalar (int) — scalar_free is a no-op } -TEST(clone_scalar_string_owns_bytes_in_dst_arena) { - Arena *src_arena = arena_create(); - Arena *dst = arena_create(); - Scalar src_scalar = - scalar_clone(src_arena, scalar_string((const uint8_t *)"hello", 5)); - Element src = element_scalar(src_scalar); - Element clone = element_clone(dst, src); - arena_destroy(src_arena); // src dies; clone must survive +// Clone owns its string bytes: scribbling the source buffer after clone must +// not change what the clone reads. +TEST(clone_scalar_string_owns_bytes) { + uint8_t buf[8]; + memcpy(buf, "hello", 5); + Element clone = element_clone(element_scalar(scalar_string(buf, 5))); + buf[0] = 'X'; + buf[1] = 'X'; ASSERT_EQ(element_kind(clone), ELEMENT_SCALAR); ASSERT( scalar_eq(clone.as.scalar, scalar_string((const uint8_t *)"hello", 5))); - arena_destroy(dst); + element_release(clone); // owned string — frees the host_malloc'd copy } TEST(clone_register_deep_copies_value) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Register *src = - register_create(as, default_id(), scalar_int(42), stmp(5, 1)); - Element clone = element_clone(ad, element_register(src)); - arena_destroy(as); + Register *src = register_create(default_id(), scalar_int(42), stmp(5, 1)); + Element clone = element_clone(element_register(src)); + register_release(src); // src frees; clone must survive ASSERT_EQ(element_kind(clone), ELEMENT_REGISTER); ASSERT(clone.as.reg != src); ASSERT(scalar_eq(register_read(clone.as.reg), scalar_int(42))); - arena_destroy(ad); + element_release(clone); } TEST(clone_counter_deep_copies_per_client_tallies) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Counter *src = counter_create(as, default_id()); + Counter *src = counter_create(default_id()); counter_inc(src, cid(1), 5); counter_inc(src, cid(2), 3); - Element clone = element_clone(ad, element_counter(src)); - arena_destroy(as); + Element clone = element_clone(element_counter(src)); + counter_release(src); ASSERT_EQ(element_kind(clone), ELEMENT_COUNTER); ASSERT(clone.as.counter != src); ASSERT_EQ(counter_read(clone.as.counter), 8); - arena_destroy(ad); + element_release(clone); } TEST(clone_map_deep_copies_recursively) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Map *src = map_create(as, default_id()); + Map *src = map_create(default_id()); map_set(src, (const void *)"a", 1, element_scalar(scalar_int(1)), stmp(1, 1)); map_set(src, (const void *)"b", 1, element_scalar(scalar_int(2)), stmp(1, 1)); - Element clone = element_clone(ad, element_map(src)); - arena_destroy(as); + Element clone = element_clone(element_map(src)); + map_release(src); ASSERT_EQ(element_kind(clone), ELEMENT_MAP); ASSERT(clone.as.map != src); Element a_out, b_out; @@ -269,62 +275,114 @@ TEST(clone_map_deep_copies_recursively) { ASSERT(map_get(clone.as.map, (const void *)"b", 1, &b_out) == true); ASSERT(scalar_eq(a_out.as.scalar, scalar_int(1))); ASSERT(scalar_eq(b_out.as.scalar, scalar_int(2))); - arena_destroy(ad); + element_release(clone); } // Mutating src after clone must not affect the clone. TEST(clone_counter_independent_of_src) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Counter *src = counter_create(as, default_id()); + Counter *src = counter_create(default_id()); counter_inc(src, cid(1), 5); - Element clone = element_clone(ad, element_counter(src)); + Element clone = element_clone(element_counter(src)); counter_inc(src, cid(1), 100); ASSERT_EQ(counter_read(src), 105); ASSERT_EQ(counter_read(clone.as.counter), 5); - arena_destroy(as); - arena_destroy(ad); + counter_release(src); + element_release(clone); } // --- element_clone preserves the source's id --- -// -// Cloned elements represent the same logical element (just materialized -// in a different arena), so ids carry over unchanged. TEST(clone_register_preserves_id) { - Arena *as = arena_create(); - Arena *ad = arena_create(); ElementId id = eid(7, 42); - Register *src = register_create(as, id, scalar_int(1), stmp(1, 1)); - Element clone = element_clone(ad, element_register(src)); + Register *src = register_create(id, scalar_int(1), stmp(1, 1)); + Element clone = element_clone(element_register(src)); ASSERT_EQ(element_kind(clone), ELEMENT_REGISTER); ASSERT(elementid_eq(register_id(clone.as.reg), id) == true); - arena_destroy(as); - arena_destroy(ad); + register_release(src); + element_release(clone); } TEST(clone_counter_preserves_id) { - Arena *as = arena_create(); - Arena *ad = arena_create(); ElementId id = eid(7, 42); - Counter *src = counter_create(as, id); - Element clone = element_clone(ad, element_counter(src)); + Counter *src = counter_create(id); + Element clone = element_clone(element_counter(src)); ASSERT_EQ(element_kind(clone), ELEMENT_COUNTER); ASSERT(elementid_eq(counter_id(clone.as.counter), id) == true); - arena_destroy(as); - arena_destroy(ad); + counter_release(src); + element_release(clone); } TEST(clone_map_preserves_id) { - Arena *as = arena_create(); - Arena *ad = arena_create(); ElementId id = eid(7, 42); - Map *src = map_create(as, id); - Element clone = element_clone(ad, element_map(src)); + Map *src = map_create(id); + Element clone = element_clone(element_map(src)); ASSERT_EQ(element_kind(clone), ELEMENT_MAP); ASSERT(elementid_eq(map_id(clone.as.map), id) == true); - arena_destroy(as); - arena_destroy(ad); + map_release(src); + element_release(clone); +} + +// --- lifecycle forwarding: acquire / release / displace / is_displaced --- +// +// Element forwards each call to the underlying composite. For SCALAR: +// acquire/displace are no-ops, is_displaced is always false, release frees +// owned bytes. + +// element_acquire bumps the composite refcount; the matching element_release +// drops it. One extra acquire + one extra release keeps the composite alive. +TEST(acquire_release_balanced_keeps_composite_alive) { + Counter *c = counter_create(default_id()); + counter_inc(c, cid(1), 5); + Element e = element_counter(c); + element_acquire(e); // refcount 1 -> 2 + element_release(e); // refcount 2 -> 1, still alive + ASSERT_EQ(counter_read(c), 5); + element_release(e); // refcount 1 -> 0, freed +} + +// element_displace forwards to the composite's displace flag. +TEST(displace_forwards_to_composite) { + Counter *c = counter_create(default_id()); + Element e = element_counter(c); + ASSERT(element_is_displaced(e) == false); + element_displace(e); + ASSERT(element_is_displaced(e) == true); + ASSERT(counter_is_displaced(c) == true); // forwarded to the real flag + element_release(e); +} + +TEST(displace_forwards_to_register) { + Register *r = register_create(default_id(), scalar_int(1), stmp(1, 1)); + Element e = element_register(r); + element_displace(e); + ASSERT(register_is_displaced(r) == true); + element_release(e); +} + +// SCALAR has no composite: displace is a no-op, is_displaced is always false. +TEST(scalar_displace_is_noop) { + Element e = element_scalar(scalar_int(7)); + element_displace(e); // must not crash + ASSERT(element_is_displaced(e) == false); + // No element_release: a borrowed-int scalar owns nothing. +} + +// element_acquire on a SCALAR is a no-op (nothing to refcount); must not crash. +TEST(scalar_acquire_is_noop) { + Element e = element_scalar(scalar_int(7)); + element_acquire(e); + ASSERT(element_is_displaced(e) == false); +} + +// Clone of a displaced composite is itself not displaced — displacement is a +// per-instance signal, reset on clone (verified at the primitive level too). +TEST(clone_of_displaced_composite_is_not_displaced) { + Counter *src = counter_create(default_id()); + element_displace(element_counter(src)); + Element clone = element_clone(element_counter(src)); + ASSERT(element_is_displaced(clone) == false); + counter_release(src); + element_release(clone); } int main(void) { @@ -338,6 +396,10 @@ int main(void) { RUN(kind_name_counter); RUN(kind_name_map); + RUN(id_register); + RUN(id_counter); + RUN(id_map); + RUN(merge_register_takes_newer_value); RUN(merge_counter_unions_clients); RUN(merge_map_takes_newer_slot); @@ -349,7 +411,7 @@ int main(void) { RUN(round_trip_via_kind_and_payload); RUN(clone_scalar_int_preserves_value); - RUN(clone_scalar_string_owns_bytes_in_dst_arena); + RUN(clone_scalar_string_owns_bytes); RUN(clone_register_deep_copies_value); RUN(clone_counter_deep_copies_per_client_tallies); RUN(clone_map_deep_copies_recursively); @@ -359,5 +421,12 @@ int main(void) { RUN(clone_counter_preserves_id); RUN(clone_map_preserves_id); + RUN(acquire_release_balanced_keeps_composite_alive); + RUN(displace_forwards_to_composite); + RUN(displace_forwards_to_register); + RUN(scalar_displace_is_noop); + RUN(scalar_acquire_is_noop); + RUN(clone_of_displaced_composite_is_not_displaced); + TEST_SUMMARY(); } diff --git a/test_map.c b/test_map.c index e9cfdf3..55dbcb1 100644 --- a/test_map.c +++ b/test_map.c @@ -1,4 +1,3 @@ -#include "arena.h" #include "clientid.h" #include "counter.h" #include "element.h" @@ -11,6 +10,26 @@ #include "test_util.h" #include +// NOTE: targets the refcounted "Share" lifecycle contract (no arena). Will not +// link until map.c (and element.c) are converted. Expected Map surface: +// Map *map_create(ElementId id); // refcount = 1 +// void map_acquire(Map *); +// void map_release(Map *); // drops slot composite refs (recursive), frees +// void map_displace(Map *); +// bool map_is_displaced(const Map *); +// Map *map_clone(const Map *); // refcount = 1, deep copy +// +// Share semantics, exercised throughout: +// - map_set of a composite: if the write is ACCEPTED (LWW wins), the Map +// element_acquires its own ref on the composite. If REJECTED, no-op. Either +// way the caller still owns the handle it passed and must release it. +// - map_get and the helper INSTALL path return BORROWS (slot keeps owning the +// ref). To keep a borrowed handle valid past the next eviction, acquire it. +// - Eviction (winning set/delete over a live composite, or merge LWW-replace) +// displaces + releases the slot's ref on the loser. +// - Helper DETACHED path (stamp loses LWW) returns an OWNED rc=1 handle that +// is born displaced; the caller must release it. + static ClientId cid(uint8_t first_byte) { uint8_t b[16] = {0}; b[0] = first_byte; @@ -40,17 +59,13 @@ static Stamp stmp(uint64_t lamport, uint8_t client_first_byte) { #define EI(n) element_scalar(scalar_int(n)) #define ES(p, n) element_scalar(scalar_string((const uint8_t *)(p), (n))) -static Map *fresh(void) { - Arena *arena = arena_create(); - return map_create(arena, default_id()); -} +static Map *fresh(void) { return map_create(default_id()); } TEST(map_create_stores_id) { - Arena *a = arena_create(); ElementId id = eid(7, 42); - Map *m = map_create(a, id); + Map *m = map_create(id); ASSERT(elementid_eq(map_id(m), id) == true); - arena_destroy(a); + map_release(m); } #define ASSERT_SCALAR_EQ(out, expected) \ @@ -65,6 +80,7 @@ TEST(empty_get_returns_false) { Map *m = fresh(); Element out; ASSERT(map_get(m, SK("missing"), &out) == false); + map_release(m); } TEST(set_then_get) { @@ -73,6 +89,7 @@ TEST(set_then_get) { Element out; ASSERT(map_get(m, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(42)); + map_release(m); } TEST(set_overwrites_with_newer_stamp) { @@ -82,6 +99,7 @@ TEST(set_overwrites_with_newer_stamp) { Element out; ASSERT(map_get(m, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(20)); + map_release(m); } TEST(set_lower_stamp_ignored) { @@ -91,6 +109,7 @@ TEST(set_lower_stamp_ignored) { Element out; ASSERT(map_get(m, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(20)); + map_release(m); } TEST(set_equal_lamport_higher_client_wins) { @@ -100,6 +119,7 @@ TEST(set_equal_lamport_higher_client_wins) { Element out; ASSERT(map_get(m, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(20)); + map_release(m); } TEST(set_equal_lamport_lower_client_ignored) { @@ -109,6 +129,7 @@ TEST(set_equal_lamport_lower_client_ignored) { Element out; ASSERT(map_get(m, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(20)); + map_release(m); } TEST(set_same_stamp_idempotent) { @@ -118,6 +139,7 @@ TEST(set_same_stamp_idempotent) { Element out; ASSERT(map_get(m, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(42)); + map_release(m); } TEST(set_can_change_value_kind) { @@ -127,6 +149,7 @@ TEST(set_can_change_value_kind) { Element out; ASSERT(map_get(m, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_string((const uint8_t *)"hi", 2)); + map_release(m); } TEST(distinct_keys_are_independent) { @@ -138,6 +161,7 @@ TEST(distinct_keys_are_independent) { ASSERT(map_get(m, SK("b"), &b) == true); ASSERT_SCALAR_EQ(a, scalar_int(1)); ASSERT_SCALAR_EQ(b, scalar_int(2)); + map_release(m); } TEST(keys_with_embedded_nul_are_distinct) { @@ -151,6 +175,7 @@ TEST(keys_with_embedded_nul_are_distinct) { ASSERT(map_get(m, k2, sizeof k2, &v2) == true); ASSERT_SCALAR_EQ(v1, scalar_int(1)); ASSERT_SCALAR_EQ(v2, scalar_int(2)); + map_release(m); } // --- delete / tombstones --- @@ -161,6 +186,7 @@ TEST(delete_makes_get_return_false) { map_delete(m, SK("k"), stmp(2, 1)); Element out; ASSERT(map_get(m, SK("k"), &out) == false); + map_release(m); } TEST(delete_with_lower_stamp_ignored) { @@ -170,6 +196,7 @@ TEST(delete_with_lower_stamp_ignored) { Element out; ASSERT(map_get(m, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(42)); + map_release(m); } TEST(set_after_delete_with_higher_stamp_resurrects) { @@ -180,6 +207,7 @@ TEST(set_after_delete_with_higher_stamp_resurrects) { Element out; ASSERT(map_get(m, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(20)); + map_release(m); } TEST(set_after_delete_with_lower_stamp_ignored) { @@ -189,6 +217,7 @@ TEST(set_after_delete_with_lower_stamp_ignored) { map_set(m, SK("k"), EI(20), stmp(3, 1)); Element out; ASSERT(map_get(m, SK("k"), &out) == false); + map_release(m); } TEST(set_vs_delete_higher_stamp_wins_delete) { @@ -197,6 +226,7 @@ TEST(set_vs_delete_higher_stamp_wins_delete) { map_delete(m, SK("k"), stmp(5, 1)); Element out; ASSERT(map_get(m, SK("k"), &out) == false); + map_release(m); } TEST(delete_idempotent_same_stamp) { @@ -206,6 +236,7 @@ TEST(delete_idempotent_same_stamp) { map_delete(m, SK("k"), stmp(5, 1)); Element out; ASSERT(map_get(m, SK("k"), &out) == false); + map_release(m); } TEST(delete_absent_key_still_installs_tombstone) { @@ -214,6 +245,7 @@ TEST(delete_absent_key_still_installs_tombstone) { map_set(m, SK("ghost"), EI(1), stmp(5, 1)); Element out; ASSERT(map_get(m, SK("ghost"), &out) == false); + map_release(m); } // --- map_size --- @@ -221,6 +253,7 @@ TEST(delete_absent_key_still_installs_tombstone) { TEST(size_zero_initially) { Map *m = fresh(); ASSERT_EQ(map_size(m), 0); + map_release(m); } TEST(size_counts_live_entries) { @@ -229,6 +262,7 @@ TEST(size_counts_live_entries) { map_set(m, SK("b"), EI(2), stmp(1, 1)); map_set(m, SK("c"), EI(3), stmp(1, 1)); ASSERT_EQ(map_size(m), 3); + map_release(m); } TEST(size_excludes_tombstones) { @@ -237,6 +271,7 @@ TEST(size_excludes_tombstones) { map_set(m, SK("b"), EI(2), stmp(1, 1)); map_delete(m, SK("b"), stmp(2, 1)); ASSERT_EQ(map_size(m), 1); + map_release(m); } TEST(size_recovers_on_resurrect) { @@ -246,43 +281,49 @@ TEST(size_recovers_on_resurrect) { ASSERT_EQ(map_size(m), 0); map_set(m, SK("k"), EI(2), stmp(3, 1)); ASSERT_EQ(map_size(m), 1); + map_release(m); } // --- composite slot reads --- +// +// Pattern: create the composite (rc=1), map_set installs it (Map acquires its +// own ref, rc=2), then the caller releases its handle (rc=1, Map owns). The +// pointer stays valid because the Map still holds a ref; map_release frees it. TEST(set_counter_then_get_returns_element_counter) { - Arena *ar = arena_create(); - Map *m = map_create(ar, default_id()); - Counter *c = counter_create(ar, default_id()); + Map *m = fresh(); + Counter *c = counter_create(default_id()); counter_inc(c, cid(1), 5); map_set(m, SK("votes"), element_counter(c), stmp(1, 1)); + counter_release(c); // Map now owns the sole ref Element out; ASSERT(map_get(m, SK("votes"), &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_COUNTER); + ASSERT(out.as.counter == c); ASSERT_EQ(counter_read(out.as.counter), 5); - arena_destroy(ar); + map_release(m); } TEST(set_register_then_get_returns_element_register) { - Arena *ar = arena_create(); - Map *m = map_create(ar, default_id()); - Register *r = register_create(ar, default_id(), scalar_int(7), stmp(1, 1)); + Map *m = fresh(); + Register *r = register_create(default_id(), scalar_int(7), stmp(1, 1)); map_set(m, SK("title"), element_register(r), stmp(1, 1)); + register_release(r); Element out; ASSERT(map_get(m, SK("title"), &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_REGISTER); ASSERT(scalar_eq(register_read(out.as.reg), scalar_int(7))); - arena_destroy(ar); + map_release(m); } TEST(set_nested_map_then_get_returns_element_map) { - Arena *ar = arena_create(); - Map *outer = map_create(ar, default_id()); - Map *inner = map_create(ar, default_id()); + Map *outer = fresh(); + Map *inner = map_create(default_id()); map_set(inner, SK("a"), EI(1), stmp(1, 1)); map_set(outer, SK("child"), element_map(inner), stmp(1, 1)); + map_release(inner); // outer owns the sole ref Element out; ASSERT(map_get(outer, SK("child"), &out) == true); @@ -290,14 +331,14 @@ TEST(set_nested_map_then_get_returns_element_map) { Element inner_out; ASSERT(map_get(out.as.map, SK("a"), &inner_out) == true); ASSERT_SCALAR_EQ(inner_out, scalar_int(1)); - arena_destroy(ar); + map_release(outer); // recursively releases inner } // --- merge (two replicas, scalar slots) --- TEST(merge_disjoint_keys_unions) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); + Map *a = fresh(); + Map *b = fresh(); map_set(a, SK("x"), EI(1), stmp(1, 1)); map_set(b, SK("y"), EI(2), stmp(1, 2)); @@ -308,11 +349,13 @@ TEST(merge_disjoint_keys_unions) { ASSERT_SCALAR_EQ(x, scalar_int(1)); ASSERT_SCALAR_EQ(y, scalar_int(2)); ASSERT_EQ(map_size(a), 2); + map_release(a); + map_release(b); } TEST(merge_same_key_newer_wins) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); + Map *a = fresh(); + Map *b = fresh(); map_set(a, SK("k"), EI(10), stmp(1, 1)); map_set(b, SK("k"), EI(20), stmp(2, 2)); @@ -320,11 +363,13 @@ TEST(merge_same_key_newer_wins) { Element out; ASSERT(map_get(a, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(20)); + map_release(a); + map_release(b); } TEST(merge_src_older_loses) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); + Map *a = fresh(); + Map *b = fresh(); map_set(a, SK("k"), EI(20), stmp(5, 1)); map_set(b, SK("k"), EI(10), stmp(2, 2)); @@ -332,22 +377,26 @@ TEST(merge_src_older_loses) { Element out; ASSERT(map_get(a, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(20)); + map_release(a); + map_release(b); } TEST(merge_delete_beats_older_set) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); + Map *a = fresh(); + Map *b = fresh(); map_set(a, SK("k"), EI(10), stmp(1, 1)); map_delete(b, SK("k"), stmp(5, 1)); map_merge(a, b); Element out; ASSERT(map_get(a, SK("k"), &out) == false); + map_release(a); + map_release(b); } TEST(merge_set_beats_older_delete) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); + Map *a = fresh(); + Map *b = fresh(); map_delete(a, SK("k"), stmp(1, 1)); map_set(b, SK("k"), EI(42), stmp(5, 1)); @@ -355,17 +404,19 @@ TEST(merge_set_beats_older_delete) { Element out; ASSERT(map_get(a, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(42)); + map_release(a); + map_release(b); } TEST(merge_commutative) { - Map *a1 = map_create(arena_create(), default_id()); - Map *b1 = map_create(arena_create(), default_id()); + Map *a1 = fresh(); + Map *b1 = fresh(); map_set(a1, SK("k"), EI(10), stmp(5, 1)); map_set(b1, SK("k"), EI(20), stmp(5, 2)); map_merge(a1, b1); - Map *a2 = map_create(arena_create(), default_id()); - Map *b2 = map_create(arena_create(), default_id()); + Map *a2 = fresh(); + Map *b2 = fresh(); map_set(a2, SK("k"), EI(10), stmp(5, 1)); map_set(b2, SK("k"), EI(20), stmp(5, 2)); map_merge(b2, a2); @@ -377,11 +428,15 @@ TEST(merge_commutative) { ASSERT_EQ(element_kind(v2), ELEMENT_SCALAR); ASSERT(scalar_eq(v1.as.scalar, v2.as.scalar)); ASSERT(scalar_eq(v1.as.scalar, scalar_int(20))); + map_release(a1); + map_release(b1); + map_release(a2); + map_release(b2); } TEST(merge_idempotent) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); + Map *a = fresh(); + Map *b = fresh(); map_set(a, SK("k"), EI(10), stmp(1, 1)); map_set(b, SK("k"), EI(20), stmp(2, 1)); @@ -395,21 +450,23 @@ TEST(merge_idempotent) { ASSERT_EQ(element_kind(twice), ELEMENT_SCALAR); ASSERT(scalar_eq(once.as.scalar, twice.as.scalar)); ASSERT(scalar_eq(twice.as.scalar, scalar_int(20))); + map_release(a); + map_release(b); } TEST(merge_associative) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); - Map *c = map_create(arena_create(), default_id()); + Map *a = fresh(); + Map *b = fresh(); + Map *c = fresh(); map_set(a, SK("k"), EI(10), stmp(1, 1)); map_set(b, SK("k"), EI(20), stmp(2, 1)); map_set(c, SK("k"), EI(30), stmp(3, 1)); map_merge(a, b); map_merge(a, c); - Map *a2 = map_create(arena_create(), default_id()); - Map *b2 = map_create(arena_create(), default_id()); - Map *c2 = map_create(arena_create(), default_id()); + Map *a2 = fresh(); + Map *b2 = fresh(); + Map *c2 = fresh(); map_set(a2, SK("k"), EI(10), stmp(1, 1)); map_set(b2, SK("k"), EI(20), stmp(2, 1)); map_set(c2, SK("k"), EI(30), stmp(3, 1)); @@ -423,11 +480,17 @@ TEST(merge_associative) { ASSERT_EQ(element_kind(v2), ELEMENT_SCALAR); ASSERT(scalar_eq(v1.as.scalar, v2.as.scalar)); ASSERT(scalar_eq(v1.as.scalar, scalar_int(30))); + map_release(a); + map_release(b); + map_release(c); + map_release(a2); + map_release(b2); + map_release(c2); } TEST(merge_does_not_mutate_src) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); + Map *a = fresh(); + Map *b = fresh(); map_set(a, SK("k"), EI(99), stmp(10, 1)); map_set(b, SK("k"), EI(7), stmp(1, 1)); @@ -435,11 +498,15 @@ TEST(merge_does_not_mutate_src) { Element out; ASSERT(map_get(b, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(7)); + map_release(a); + map_release(b); } -TEST(merge_copies_string_into_dst_arena) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); +// Dst must own its own copy of a winning string value: scribbling the source +// buffer after merge must not change what dst reads. +TEST(merge_copies_string_into_dst) { + Map *a = fresh(); + Map *b = fresh(); uint8_t src_bytes[8]; memcpy(src_bytes, "hello", 5); @@ -455,17 +522,21 @@ TEST(merge_copies_string_into_dst_arena) { Element out; ASSERT(map_get(a, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_string((const uint8_t *)"hello", 5)); + map_release(a); + map_release(b); } TEST(merge_preserves_tombstone_against_older_set) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); + Map *a = fresh(); + Map *b = fresh(); map_delete(a, SK("k"), stmp(5, 1)); map_set(b, SK("k"), EI(10), stmp(2, 1)); map_merge(a, b); Element out; ASSERT(map_get(a, SK("k"), &out) == false); + map_release(a); + map_release(b); } // --- recursive merge: same kind at same key recurses regardless of stamp --- @@ -475,18 +546,17 @@ TEST(merge_preserves_tombstone_against_older_set) { // element_merge. Slot stamp advances to max(dst, src). TEST(merge_same_kind_counter_recurses) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - - Map *dst = map_create(ad, default_id()); - Counter *dc = counter_create(ad, default_id()); + Map *dst = fresh(); + Counter *dc = counter_create(default_id()); counter_inc(dc, cid(1), 5); map_set(dst, SK("votes"), element_counter(dc), stmp(1, 1)); + counter_release(dc); - Map *src = map_create(as, default_id()); - Counter *sc = counter_create(as, default_id()); + Map *src = fresh(); + Counter *sc = counter_create(default_id()); counter_inc(sc, cid(2), 3); map_set(src, SK("votes"), element_counter(sc), stmp(10, 1)); + counter_release(sc); map_merge(dst, src); @@ -495,23 +565,20 @@ TEST(merge_same_kind_counter_recurses) { ASSERT_EQ(element_kind(out), ELEMENT_COUNTER); ASSERT(out.as.counter == dc); // dst kept its own pointer ASSERT_EQ(counter_read(out.as.counter), 8); - arena_destroy(ad); - arena_destroy(as); + map_release(dst); + map_release(src); } TEST(merge_same_kind_register_recurses) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - - Map *dst = map_create(ad, default_id()); - Register *dr = - register_create(ad, default_id(), scalar_int(10), stmp(1, 1)); + Map *dst = fresh(); + Register *dr = register_create(default_id(), scalar_int(10), stmp(1, 1)); map_set(dst, SK("title"), element_register(dr), stmp(1, 1)); + register_release(dr); - Map *src = map_create(as, default_id()); - Register *sr = - register_create(as, default_id(), scalar_int(20), stmp(5, 1)); + Map *src = fresh(); + Register *sr = register_create(default_id(), scalar_int(20), stmp(5, 1)); map_set(src, SK("title"), element_register(sr), stmp(1, 1)); + register_release(sr); map_merge(dst, src); @@ -520,23 +587,22 @@ TEST(merge_same_kind_register_recurses) { ASSERT_EQ(element_kind(out), ELEMENT_REGISTER); ASSERT(out.as.reg == dr); ASSERT(scalar_eq(register_read(dr), scalar_int(20))); - arena_destroy(ad); - arena_destroy(as); + map_release(dst); + map_release(src); } TEST(merge_same_kind_nested_map_recurses) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - - Map *dst = map_create(ad, default_id()); - Map *di = map_create(ad, default_id()); + Map *dst = fresh(); + Map *di = map_create(default_id()); map_set(di, SK("a"), EI(1), stmp(1, 1)); map_set(dst, SK("child"), element_map(di), stmp(1, 1)); + map_release(di); - Map *src = map_create(as, default_id()); - Map *si = map_create(as, default_id()); + Map *src = fresh(); + Map *si = map_create(default_id()); map_set(si, SK("b"), EI(2), stmp(1, 2)); map_set(src, SK("child"), element_map(si), stmp(1, 1)); + map_release(si); map_merge(dst, src); @@ -549,29 +615,28 @@ TEST(merge_same_kind_nested_map_recurses) { ASSERT(map_get(di, SK("b"), &b_out) == true); ASSERT_SCALAR_EQ(a_out, scalar_int(1)); ASSERT_SCALAR_EQ(b_out, scalar_int(2)); - arena_destroy(ad); - arena_destroy(as); + map_release(dst); + map_release(src); } TEST(merge_same_kind_counter_does_not_mutate_src) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - - Map *dst = map_create(ad, default_id()); - Counter *dc = counter_create(ad, default_id()); + Map *dst = fresh(); + Counter *dc = counter_create(default_id()); counter_inc(dc, cid(1), 5); map_set(dst, SK("votes"), element_counter(dc), stmp(1, 1)); + counter_release(dc); - Map *src = map_create(as, default_id()); - Counter *sc = counter_create(as, default_id()); + Map *src = fresh(); + Counter *sc = counter_create(default_id()); counter_inc(sc, cid(2), 3); map_set(src, SK("votes"), element_counter(sc), stmp(1, 1)); map_merge(dst, src); ASSERT_EQ(counter_read(sc), 3); - arena_destroy(ad); - arena_destroy(as); + counter_release(sc); + map_release(dst); + map_release(src); } // Recursive merge must advance the slot stamp to max(dst, src). Otherwise @@ -579,18 +644,17 @@ TEST(merge_same_kind_counter_does_not_mutate_src) { // a subsequent set with a stamp above dst's old slot stamp but below src's // must be rejected. TEST(merge_same_kind_counter_advances_slot_stamp) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - - Map *dst = map_create(ad, default_id()); - Counter *dc = counter_create(ad, default_id()); + Map *dst = fresh(); + Counter *dc = counter_create(default_id()); counter_inc(dc, cid(1), 5); map_set(dst, SK("votes"), element_counter(dc), stmp(1, 1)); + counter_release(dc); - Map *src = map_create(as, default_id()); - Counter *sc = counter_create(as, default_id()); + Map *src = fresh(); + Counter *sc = counter_create(default_id()); counter_inc(sc, cid(2), 3); map_set(src, SK("votes"), element_counter(sc), stmp(10, 1)); + counter_release(sc); map_merge(dst, src); @@ -603,71 +667,106 @@ TEST(merge_same_kind_counter_advances_slot_stamp) { ASSERT(map_get(dst, SK("votes"), &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_COUNTER); ASSERT_EQ(counter_read(out.as.counter), 8); - arena_destroy(ad); - arena_destroy(as); + map_release(dst); + map_release(src); } -// --- type-flip via LWW --- +// --- type-flip via LWW: loser composite is displaced + released --- // -// Composites at a key can flip kind. The newer-stamped write wins, the -// old object is orphaned (still alive in the arena but unreachable from -// the slot). +// A newer-stamped write of a different kind wins the slot. The old composite +// is displaced (its handle marked) and the Map drops its ref. If no other +// holder exists, the loser is freed; tests that want to observe it first +// acquire a ref. TEST(set_composite_displaces_scalar_at_lww) { - Arena *ar = arena_create(); - Map *m = map_create(ar, default_id()); - + Map *m = fresh(); map_set(m, SK("score"), EI(42), stmp(1, 1)); // scalar first - Counter *c = counter_create(ar, default_id()); + Counter *c = counter_create(default_id()); map_set(m, SK("score"), element_counter(c), stmp(5, 1)); // newer composite + counter_release(c); Element out; ASSERT(map_get(m, SK("score"), &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_COUNTER); ASSERT(out.as.counter == c); - arena_destroy(ar); + map_release(m); } TEST(set_scalar_displaces_composite_at_lww) { - Arena *ar = arena_create(); - Map *m = map_create(ar, default_id()); - - Counter *c = counter_create(ar, default_id()); + Map *m = fresh(); + Counter *c = counter_create(default_id()); map_set(m, SK("score"), element_counter(c), stmp(1, 1)); + counter_release(c); // Map owns the sole ref; the scalar set below frees it map_set(m, SK("score"), EI(42), stmp(5, 1)); Element out; ASSERT(map_get(m, SK("score"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(42)); - arena_destroy(ar); + map_release(m); } TEST(set_different_kind_composite_displaces_at_lww) { - Arena *ar = arena_create(); - Map *m = map_create(ar, default_id()); - - Counter *c = counter_create(ar, default_id()); + Map *m = fresh(); + Counter *c = counter_create(default_id()); map_set(m, SK("score"), element_counter(c), stmp(1, 1)); - Register *r = register_create(ar, default_id(), scalar_int(42), stmp(5, 1)); + counter_release(c); + Register *r = register_create(default_id(), scalar_int(42), stmp(5, 1)); map_set(m, SK("score"), element_register(r), stmp(5, 1)); + register_release(r); Element out; ASSERT(map_get(m, SK("score"), &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_REGISTER); ASSERT(out.as.reg == r); - arena_destroy(ar); + map_release(m); } -// --- cross-arena composite LWW: clone winner into dst's arena --- +// A holder that acquired its own ref before eviction keeps the displaced +// composite alive and observes the displaced flag. +TEST(evicted_composite_is_displaced_and_outlives_via_held_ref) { + Map *m = fresh(); + Counter *c = counter_create(default_id()); + counter_inc(c, cid(1), 5); + map_set(m, SK("score"), element_counter(c), stmp(1, 1)); + // Caller keeps its ref (does NOT release) — it wants to observe the + // eviction. After set the refcount is 2 (caller + Map). + + map_set(m, SK("score"), EI(42), stmp(5, 1)); // evicts the Counter + + // Map dropped its ref; the caller's ref keeps c alive, now displaced. + ASSERT(counter_is_displaced(c) == true); + ASSERT_EQ(counter_read(c), 5); // still readable, just orphaned + + counter_release(c); // caller's ref → freed + map_release(m); +} + +// Deleting a live composite slot displaces + releases it, same as an +// overwriting set. +TEST(delete_composite_displaces_it) { + Map *m = fresh(); + Counter *c = counter_create(default_id()); + map_set(m, SK("score"), element_counter(c), stmp(1, 1)); + // keep caller ref to observe + + map_delete(m, SK("score"), stmp(5, 1)); + + Element out; + ASSERT(map_get(m, SK("score"), &out) == false); // tombstoned + ASSERT(counter_is_displaced(c) == true); + counter_release(c); + map_release(m); +} + +// --- cross-replica composite LWW: clone winner, displace+release loser --- TEST(merge_composite_src_wins_into_empty_slot_clones) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - Map *dst = map_create(ad, default_id()); - Map *src = map_create(as, default_id()); - Counter *sc = counter_create(as, default_id()); + Map *dst = fresh(); + Map *src = fresh(); + Counter *sc = counter_create(default_id()); counter_inc(sc, cid(1), 5); map_set(src, SK("votes"), element_counter(sc), stmp(5, 1)); + counter_release(sc); map_merge(dst, src); @@ -676,63 +775,60 @@ TEST(merge_composite_src_wins_into_empty_slot_clones) { ASSERT_EQ(element_kind(out), ELEMENT_COUNTER); ASSERT(out.as.counter != sc); // dst owns a clone - arena_destroy(as); // src dies; dst clone must survive + map_release(src); // src dies; dst clone must survive Element out2; ASSERT(map_get(dst, SK("votes"), &out2) == true); ASSERT_EQ(counter_read(out2.as.counter), 5); - arena_destroy(ad); + map_release(dst); } -// When src would LOSE the LWW comparison, map_merge must NOT element_clone -// src's value into dst's arena — that clone would be unreachable garbage. -// Probe via arena_used: a losing merge must not grow dst's arena beyond -// trivial bookkeeping. +// When src LOSES the LWW comparison, map_merge must NOT clone src's value into +// dst — that clone would be unreachable garbage (a leak in the refcount model). +// +// NOTE: the original arena-based test asserted this via arena_used byte cost. +// There is no refcount equivalent without a host alloc-counter seam, so the +// perf-probe half is dropped; this keeps the functional guarantee (dst keeps +// its winning scalar, and is independent of src after src is released). A true +// "no wasteful clone" check now needs ASan/LeakSanitizer. TEST(merge_does_not_clone_when_src_loses_lww) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - Map *dst = map_create(ad, default_id()); - Map *src = map_create(as, default_id()); + Map *dst = fresh(); + Map *src = fresh(); // dst has newer scalar at "k". map_set(dst, SK("k"), EI(42), stmp(10, 1)); - // src has big nested Counter at "k" with OLDER stamp — must lose LWW. - Counter *sc = counter_create(as, default_id()); + // src has a nested Counter at "k" with OLDER stamp — must lose LWW. + Counter *sc = counter_create(default_id()); for (int i = 0; i < 50; i++) { counter_inc(sc, cid((uint8_t)(i + 1)), 1); } map_set(src, SK("k"), element_counter(sc), stmp(1, 1)); + counter_release(sc); - size_t before = arena_used(ad); map_merge(dst, src); - size_t after = arena_used(ad); Element out; ASSERT(map_get(dst, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(42)); - // Without the fix, the Counter (~50 entries) gets cloned into ad even - // though src lost LWW. Cost should be near-zero on the honest path. - size_t cost = after - before; - ASSERT(cost < 256); - - arena_destroy(ad); - arena_destroy(as); + map_release(src); // dst must be unaffected + Element out2; + ASSERT(map_get(dst, SK("k"), &out2) == true); + ASSERT_SCALAR_EQ(out2, scalar_int(42)); + map_release(dst); } TEST(merge_kind_mismatch_clones_winner_into_dst) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - - Map *dst = map_create(ad, default_id()); - Counter *dc = counter_create(ad, default_id()); + Map *dst = fresh(); + Counter *dc = counter_create(default_id()); counter_inc(dc, cid(1), 5); map_set(dst, SK("x"), element_counter(dc), stmp(1, 1)); + counter_release(dc); - Map *src = map_create(as, default_id()); - Register *sr = - register_create(as, default_id(), scalar_int(42), stmp(10, 1)); + Map *src = fresh(); + Register *sr = register_create(default_id(), scalar_int(42), stmp(10, 1)); map_set(src, SK("x"), element_register(sr), stmp(10, 1)); + register_release(sr); map_merge(dst, src); @@ -741,8 +837,8 @@ TEST(merge_kind_mismatch_clones_winner_into_dst) { ASSERT_EQ(element_kind(out), ELEMENT_REGISTER); ASSERT(out.as.reg != sr); // clone, not src's pointer ASSERT(scalar_eq(register_read(out.as.reg), scalar_int(42))); - arena_destroy(ad); - arena_destroy(as); + map_release(dst); + map_release(src); } // Two replicas hold a Counter of the same kind at the same slot but with @@ -752,20 +848,20 @@ TEST(merge_kind_mismatch_clones_winner_into_dst) { // (which would silently union their tallies); it must take the LWW path // and orphan one side. TEST(merge_same_kind_different_id_uses_lww_not_recurse) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - Map *dst = map_create(ad, default_id()); - Map *src = map_create(as, default_id()); + Map *dst = fresh(); + Map *src = fresh(); // dst: distinct id, 5 increments under cid 1, older slot stamp. - Counter *dc = counter_create(ad, eid(7, 1)); + Counter *dc = counter_create(eid(7, 1)); counter_inc(dc, cid(1), 5); map_set(dst, SK("votes"), element_counter(dc), stmp(1, 1)); + counter_release(dc); // src: DIFFERENT id, 3 increments under cid 2, newer slot stamp. - Counter *sc = counter_create(as, eid(7, 2)); + Counter *sc = counter_create(eid(7, 2)); counter_inc(sc, cid(2), 3); map_set(src, SK("votes"), element_counter(sc), stmp(5, 1)); + counter_release(sc); map_merge(dst, src); @@ -780,24 +876,24 @@ TEST(merge_same_kind_different_id_uses_lww_not_recurse) { // Clone's id is src's id, not dst's old id (dst's Counter is orphaned). ASSERT(elementid_eq(counter_id(out.as.counter), eid(7, 2)) == true); - // dst owns the clone in its arena — not src's pointer. + // dst owns the clone — not src's pointer. ASSERT(out.as.counter != sc); - arena_destroy(ad); - arena_destroy(as); + map_release(dst); + map_release(src); } // --- get-or-create helpers --- // -// map_counter / map_register / map_map: install a composite at the given -// key if the slot is empty or has a different kind (and the stamp wins -// LWW). If the slot already has a matching kind, return the existing -// pointer (stamp + value seed ignored). If the stamp loses LWW, return -// NULL. +// map_counter / map_register / map_map install a composite at the given key if +// the slot is empty or has a different kind (and the stamp wins LWW). The +// INSTALL path returns a BORROW (the slot owns the ref). If the slot already +// has a matching kind, the existing pointer is returned (stamp + seed ignored). +// If the stamp LOSES LWW, a DETACHED owned handle is returned (born displaced, +// rc=1) and the caller must release it. TEST(map_counter_creates_and_installs_at_key) { - Arena *ar = arena_create(); - Map *m = map_create(ar, default_id()); + Map *m = fresh(); Counter *c = map_counter(m, SK("votes"), stmp(1, 1)); ASSERT(c != NULL); @@ -806,22 +902,20 @@ TEST(map_counter_creates_and_installs_at_key) { ASSERT(map_get(m, SK("votes"), &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_COUNTER); ASSERT(out.as.counter == c); - arena_destroy(ar); + map_release(m); // installed borrow is owned by the slot } TEST(map_counter_returns_same_pointer_on_repeat) { - Arena *ar = arena_create(); - Map *m = map_create(ar, default_id()); + Map *m = fresh(); Counter *first = map_counter(m, SK("votes"), stmp(1, 1)); Counter *second = map_counter(m, SK("votes"), stmp(2, 1)); ASSERT(first == second); - arena_destroy(ar); + map_release(m); } TEST(map_register_creates_and_installs_at_key) { - Arena *ar = arena_create(); - Map *m = map_create(ar, default_id()); + Map *m = fresh(); Register *r = map_register(m, SK("title"), scalar_int(42), stmp(1, 1)); ASSERT(r != NULL); @@ -831,12 +925,11 @@ TEST(map_register_creates_and_installs_at_key) { ASSERT(map_get(m, SK("title"), &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_REGISTER); ASSERT(out.as.reg == r); - arena_destroy(ar); + map_release(m); } TEST(map_register_returns_same_pointer_on_repeat) { - Arena *ar = arena_create(); - Map *m = map_create(ar, default_id()); + Map *m = fresh(); Register *first = map_register(m, SK("title"), scalar_int(1), stmp(1, 1)); // Second call's seed value is ignored — slot already exists. @@ -844,12 +937,11 @@ TEST(map_register_returns_same_pointer_on_repeat) { map_register(m, SK("title"), scalar_int(999), stmp(2, 1)); ASSERT(first == second); ASSERT(scalar_eq(register_read(first), scalar_int(1))); - arena_destroy(ar); + map_release(m); } TEST(map_map_creates_and_installs_at_key) { - Arena *ar = arena_create(); - Map *outer = map_create(ar, default_id()); + Map *outer = fresh(); Map *child = map_map(outer, SK("child"), stmp(1, 1)); ASSERT(child != NULL); @@ -858,27 +950,27 @@ TEST(map_map_creates_and_installs_at_key) { ASSERT(map_get(outer, SK("child"), &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_MAP); ASSERT(out.as.map == child); - arena_destroy(ar); + map_release(outer); } TEST(map_map_returns_same_pointer_on_repeat) { - Arena *ar = arena_create(); - Map *outer = map_create(ar, default_id()); + Map *outer = fresh(); Map *first = map_map(outer, SK("child"), stmp(1, 1)); Map *second = map_map(outer, SK("child"), stmp(2, 1)); ASSERT(first == second); - arena_destroy(ar); + map_release(outer); } -// Helper called over a different-kind slot with a winning stamp must flip -// the kind via LWW and return a fresh composite. +// Helper called over a different-kind slot with a winning stamp flips the kind +// via LWW and returns a fresh installed composite. The displaced Counter is +// released by the Map; a caller that wants to observe it must hold its own ref. TEST(map_register_after_map_counter_flips_kind_via_lww) { - Arena *ar = arena_create(); - Map *m = map_create(ar, default_id()); + Map *m = fresh(); Counter *c = map_counter(m, SK("score"), stmp(1, 1)); ASSERT(c != NULL); + counter_acquire(c); // retain past the imminent eviction Register *r = map_register(m, SK("score"), scalar_int(42), stmp(5, 1)); ASSERT(r != NULL); @@ -888,19 +980,19 @@ TEST(map_register_after_map_counter_flips_kind_via_lww) { ASSERT_EQ(element_kind(out), ELEMENT_REGISTER); ASSERT(out.as.reg == r); - // The displaced Counter is still alive for direct use, just unreachable - // from the slot. + // The displaced Counter is still alive via the retained ref, just + // unreachable from the slot. + ASSERT(counter_is_displaced(c) == true); ASSERT_EQ(counter_read(c), 0); - arena_destroy(ar); + counter_release(c); + map_release(m); } -// Helper called with a stamp that LOSES LWW returns a DETACHED composite — -// the caller always gets a usable handle, but the slot keeps its existing -// content. Detached composite lives in the arena and supports direct use, -// just isn't reachable from the slot. +// Helper called with a stamp that LOSES LWW returns a DETACHED composite: an +// owned, born-displaced handle. The slot keeps its existing content, and the +// caller must release the detached handle. TEST(map_helper_losing_stamp_returns_detached_and_keeps_slot) { - Arena *ar = arena_create(); - Map *m = map_create(ar, default_id()); + Map *m = fresh(); Counter *c = map_counter(m, SK("score"), stmp(10, 1)); ASSERT(c != NULL); @@ -908,23 +1000,73 @@ TEST(map_helper_losing_stamp_returns_detached_and_keeps_slot) { Register *r = map_register(m, SK("score"), scalar_int(7), stmp(5, 1)); ASSERT(r != NULL); // detached, but still returned ASSERT(scalar_eq(register_read(r), scalar_int(7))); + ASSERT(register_is_displaced(r) == true); // born displaced - // Slot kept its Counter — detached Register did not displace. + // Slot kept its Counter — detached Register did not displace it. Element out; ASSERT(map_get(m, SK("score"), &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_COUNTER); ASSERT(out.as.counter == c); - arena_destroy(ar); + + register_release(r); // caller owns the detached handle + map_release(m); +} + +// map_counter losing LWW returns a detached, born-displaced Counter; the slot +// keeps its existing (different-kind) content. +TEST(map_counter_losing_stamp_returns_detached_displaced) { + Map *m = fresh(); + map_register(m, SK("score"), scalar_int(1), stmp(10, 1)); // slot-owned + + Counter *c = map_counter(m, SK("score"), stmp(5, 1)); // loses LWW + ASSERT(c != NULL); + ASSERT(counter_is_displaced(c) == true); + + Element out; + ASSERT(map_get(m, SK("score"), &out) == true); + ASSERT_EQ(element_kind(out), ELEMENT_REGISTER); // slot unchanged + + counter_release(c); // caller owns the detached handle + map_release(m); } -// Cross-replica: two replicas each call map_counter on the same key. They -// get separate Counter pointers (own arenas), but merge takes the -// recursive path because (key, kind) matches. +// map_map losing LWW returns a detached, born-displaced Map. +TEST(map_map_losing_stamp_returns_detached_displaced) { + Map *m = fresh(); + map_register(m, SK("child"), scalar_int(1), stmp(10, 1)); + + Map *child = map_map(m, SK("child"), stmp(5, 1)); // loses LWW + ASSERT(child != NULL); + ASSERT(map_is_displaced(child) == true); + + Element out; + ASSERT(map_get(m, SK("child"), &out) == true); + ASSERT_EQ(element_kind(out), ELEMENT_REGISTER); + + map_release(child); // caller owns the detached handle + map_release(m); +} + +// A helper INSTALL returns a borrow: the slot owns the sole ref. A caller that +// wants to outlive the Map must acquire its own ref; map_release then drops +// only the slot's ref, not the caller's. (Also guards the helper's release +// pairing under ASan — an over-release would surface here as UAF.) +TEST(map_counter_installed_handle_is_a_borrow) { + Map *m = fresh(); + Counter *c = map_counter(m, SK("votes"), stmp(1, 1)); // borrow, slot owns + counter_acquire(c); // caller co-owns + counter_inc(c, cid(1), 4); + map_release(m); // slot drops its ref; c stays alive on the caller's ref + ASSERT_EQ(counter_read(c), 4); + counter_release(c); // last ref → freed +} + +// Cross-replica: two replicas each call map_counter on the same key. They get +// separate Counter pointers, but merge takes the recursive path because +// (key, kind, id) matches. TEST(map_counter_cross_replica_merge_recurses) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - Map *dst = map_create(ad, default_id()); - Map *src = map_create(as, default_id()); + Map *dst = fresh(); + Map *src = fresh(); Counter *dc = map_counter(dst, SK("votes"), stmp(1, 1)); Counter *sc = map_counter(src, SK("votes"), stmp(1, 2)); @@ -937,143 +1079,155 @@ TEST(map_counter_cross_replica_merge_recurses) { ASSERT(map_get(dst, SK("votes"), &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_COUNTER); ASSERT_EQ(counter_read(out.as.counter), 8); - arena_destroy(ad); - arena_destroy(as); + map_release(dst); + map_release(src); } // --- helper id derivation --- // -// Helpers must derive ids deterministically from (parent_id, key, kind) -// so two replicas independently calling the same helper land on the same -// id. The composite's id_field is the only authoritative source of truth -// for "are these the same logical thing?" +// Helpers must derive ids deterministically from (parent_id, key, kind) so two +// replicas independently calling the same helper land on the same id. TEST(map_counter_derives_id_from_parent_key_kind) { - Arena *ar = arena_create(); ElementId parent_id = eid(7, 42); - Map *m = map_create(ar, parent_id); + Map *m = map_create(parent_id); Counter *c = map_counter(m, SK("votes"), stmp(1, 1)); ElementId expected = elementid_derive(parent_id, SK("votes"), (uint8_t)ELEMENT_COUNTER); ASSERT(elementid_eq(counter_id(c), expected) == true); - arena_destroy(ar); + map_release(m); } TEST(map_register_derives_id_from_parent_key_kind) { - Arena *ar = arena_create(); ElementId parent_id = eid(7, 42); - Map *m = map_create(ar, parent_id); + Map *m = map_create(parent_id); Register *r = map_register(m, SK("title"), scalar_int(0), stmp(1, 1)); ElementId expected = elementid_derive(parent_id, SK("title"), (uint8_t)ELEMENT_REGISTER); ASSERT(elementid_eq(register_id(r), expected) == true); - arena_destroy(ar); + map_release(m); } TEST(map_map_derives_id_from_parent_key_kind) { - Arena *ar = arena_create(); ElementId parent_id = eid(7, 42); - Map *m = map_create(ar, parent_id); + Map *m = map_create(parent_id); Map *child = map_map(m, SK("child"), stmp(1, 1)); ElementId expected = elementid_derive(parent_id, SK("child"), (uint8_t)ELEMENT_MAP); ASSERT(elementid_eq(map_id(child), expected) == true); - arena_destroy(ar); + map_release(m); } -// Two replicas with the same parent_id calling the same helper at the -// same key land on identical ids — the convergent-creation guarantee. +// Two replicas with the same parent_id calling the same helper at the same key +// land on identical ids — the convergent-creation guarantee. TEST(helpers_converge_across_replicas) { - Arena *aa = arena_create(); - Arena *ab = arena_create(); ElementId shared_parent = eid(7, 42); - Map *map_a = map_create(aa, shared_parent); - Map *map_b = map_create(ab, shared_parent); + Map *map_a = map_create(shared_parent); + Map *map_b = map_create(shared_parent); Counter *ca = map_counter(map_a, SK("votes"), stmp(1, 1)); Counter *cb = map_counter(map_b, SK("votes"), stmp(1, 2)); ASSERT(elementid_eq(counter_id(ca), counter_id(cb)) == true); - arena_destroy(aa); - arena_destroy(ab); + map_release(map_a); + map_release(map_b); } -// Different kinds at the same key derive DIFFERENT ids — that's how -// recursive merge distinguishes Counter@"x" from Register@"x" as -// independent logical elements. +// Different kinds at the same key derive DIFFERENT ids — that's how recursive +// merge distinguishes Counter@"x" from Register@"x" as independent elements. TEST(helpers_at_same_key_different_kind_have_distinct_ids) { - Arena *ar = arena_create(); - Map *m = map_create(ar, eid(7, 42)); + Map *m = map_create(eid(7, 42)); Counter *c = map_counter(m, SK("x"), stmp(1, 1)); - // Counter is installed in slot. map_register loses the LWW slot here - // (same stamp), so returns a DETACHED Register. Its id should still - // be derived from (parent_id, "x", REGISTER), distinct from c's id. + // Counter is installed. map_register loses the LWW slot here (same stamp), + // so returns a DETACHED Register. Its id should still be derived from + // (parent_id, "x", REGISTER), distinct from c's id. Register *r = map_register(m, SK("x"), scalar_int(0), stmp(1, 1)); ASSERT(elementid_eq(counter_id(c), register_id(r)) == false); - arena_destroy(ar); + register_release(r); // detached owned handle + map_release(m); } -// --- map_clone: deep recursive copy into a target arena --- +// --- map lifecycle: release / displace --- + +// map_release drops the Map's ref on each live slot composite (recursively for +// nested maps). A composite the caller also holds a ref on survives until the +// caller releases too. +TEST(map_release_drops_slot_refs_but_held_ref_survives) { + Map *m = fresh(); + Counter *c = counter_create(default_id()); + counter_inc(c, cid(1), 7); + map_set(m, SK("votes"), element_counter(c), stmp(1, 1)); + // Caller keeps its ref (refcount = 2: caller + Map). + + map_release(m); // drops Map's ref → refcount = 1, c still alive + + ASSERT_EQ(counter_read(c), 7); + counter_release(c); // last ref → freed +} + +// map_displace forwards to the Map's own displaced flag (Map is itself a +// composite kind that can be displaced from a parent slot). +TEST(map_displace_sets_flag) { + Map *m = fresh(); + ASSERT(map_is_displaced(m) == false); + map_displace(m); + ASSERT(map_is_displaced(m) == true); + map_release(m); +} + +// --- map_clone: deep recursive copy, refcount=1 --- TEST(clone_empty_map_is_empty) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Map *src = map_create(as, default_id()); - Map *clone = map_clone(ad, src); + Map *src = fresh(); + Map *clone = map_clone(src); ASSERT(clone != NULL); ASSERT(clone != src); ASSERT_EQ(map_size(clone), 0); - arena_destroy(as); - arena_destroy(ad); + map_release(src); + map_release(clone); } TEST(clone_preserves_scalar_slots) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Map *src = map_create(as, default_id()); + Map *src = fresh(); map_set(src, SK("a"), EI(1), stmp(1, 1)); map_set(src, SK("b"), ES("hi", 2), stmp(1, 1)); - Map *clone = map_clone(ad, src); + Map *clone = map_clone(src); ASSERT_EQ(map_size(clone), 2); Element a_out, b_out; ASSERT(map_get(clone, SK("a"), &a_out) == true); ASSERT(map_get(clone, SK("b"), &b_out) == true); ASSERT_SCALAR_EQ(a_out, scalar_int(1)); ASSERT_SCALAR_EQ(b_out, scalar_string((const uint8_t *)"hi", 2)); - arena_destroy(as); - arena_destroy(ad); + map_release(src); + map_release(clone); } -// Clone owns all its data — destroying the source arena leaves the clone -// fully usable. -TEST(clone_survives_src_arena_destroy) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Map *src = map_create(as, default_id()); +// Clone owns all its data — releasing the source leaves the clone fully usable. +TEST(clone_survives_src_release) { + Map *src = fresh(); map_set(src, SK("k"), ES("hello", 5), stmp(1, 1)); - Map *clone = map_clone(ad, src); - arena_destroy(as); + Map *clone = map_clone(src); + map_release(src); Element out; ASSERT(map_get(clone, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_string((const uint8_t *)"hello", 5)); - arena_destroy(ad); + map_release(clone); } -// Composite slots are recursively cloned — the clone's nested composites -// are independent objects in dst's arena. +// Composite slots are recursively cloned — the clone's nested composites are +// independent objects with their own refcounts. TEST(clone_recurses_into_composite_slots) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Map *src = map_create(as, default_id()); - Counter *sc = counter_create(as, default_id()); + Map *src = fresh(); + Counter *sc = counter_create(default_id()); counter_inc(sc, cid(1), 5); map_set(src, SK("votes"), element_counter(sc), stmp(1, 1)); + counter_release(sc); - Map *clone = map_clone(ad, src); + Map *clone = map_clone(src); Element out; ASSERT(map_get(clone, SK("votes"), &out) == true); @@ -1081,38 +1235,34 @@ TEST(clone_recurses_into_composite_slots) { ASSERT(out.as.counter != sc); // recursive clone, independent object ASSERT_EQ(counter_read(out.as.counter), 5); - arena_destroy(as); + map_release(src); Element out2; ASSERT(map_get(clone, SK("votes"), &out2) == true); ASSERT_EQ(counter_read(out2.as.counter), 5); - arena_destroy(ad); + map_release(clone); } // Tombstones must round-trip through clone so deletion semantics survive. TEST(clone_preserves_tombstones) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Map *src = map_create(as, default_id()); + Map *src = fresh(); map_set(src, SK("k"), EI(1), stmp(1, 1)); map_delete(src, SK("k"), stmp(5, 1)); - Map *clone = map_clone(ad, src); + Map *clone = map_clone(src); // Tombstone present at stamp(5,1) — older set must lose LWW. map_set(clone, SK("k"), EI(99), stmp(3, 1)); Element out; ASSERT(map_get(clone, SK("k"), &out) == false); - arena_destroy(as); - arena_destroy(ad); + map_release(src); + map_release(clone); } // Mutating src after clone must not affect the clone. TEST(clone_independent_of_src) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Map *src = map_create(as, default_id()); + Map *src = fresh(); map_set(src, SK("k"), EI(1), stmp(1, 1)); - Map *clone = map_clone(ad, src); + Map *clone = map_clone(src); map_set(src, SK("k"), EI(99), stmp(5, 1)); map_set(src, SK("new"), EI(7), stmp(1, 1)); @@ -1120,51 +1270,42 @@ TEST(clone_independent_of_src) { ASSERT(map_get(clone, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(1)); ASSERT(map_get(clone, SK("new"), &out) == false); - arena_destroy(as); - arena_destroy(ad); + map_release(src); + map_release(clone); } -// Tombstone entries carry a stale (or uninit) value field. map_clone must -// NOT recursively clone that stale value into the destination arena — -// doing so wastes memory and reads possibly-undefined bytes. +// Tombstone entries carry a stale value field. map_clone must NOT recursively +// clone that stale value into the destination. // -// Probe: build a sizeable subtree under a key, delete the slot (the Entry -// keeps the composite pointer in its `value` field), clone the Map, and -// check that the clone's arena did not absorb the full subtree. +// NOTE: the original arena-based test measured this via arena_used byte cost. +// Without an alloc-counter seam there is no refcount equivalent, so the +// perf-probe half is dropped; this keeps the functional guarantee (tombstone +// survives the clone, clone is independent). A real "did not recurse the stale +// value" check now needs ASan/LeakSanitizer. TEST(clone_tombstone_does_not_recurse_into_stale_value) { - Arena *as = arena_create(); - Arena *ad = arena_create(); - Map *src = map_create(as, default_id()); + Map *src = fresh(); - // Big inner map under the to-be-tombstoned key. - Map *inner = map_create(as, default_id()); - for (int i = 0; i < 50; i++) { + // Inner map under the to-be-tombstoned key. + Map *inner = map_create(default_id()); + for (int i = 0; i < 10; i++) { char k[16]; int n = snprintf(k, sizeof k, "k%d", i); map_set(inner, k, (size_t)n, EI(i), stmp(1, 1)); } map_set(src, SK("child"), element_map(inner), stmp(1, 1)); - // Delete — Entry stays in src's hashtable with is_tombstone=true but - // the `value` field still points at `inner`. + map_release(inner); + // Delete — Entry stays in src with is_tombstone=true but its value field + // may still reference the (now released-by-map) inner subtree. map_delete(src, SK("child"), stmp(5, 1)); - size_t before = arena_used(ad); - Map *clone = map_clone(ad, src); - size_t after = arena_used(ad); + Map *clone = map_clone(src); // Tombstone semantics survive. Element out; ASSERT(map_get(clone, SK("child"), &out) == false); - // Bug surfaces as massive over-allocation in dst: the bogus - // element_clone on the tombstone recursively clones the 50-entry - // inner Map into ad. The honest clone only allocates the outer Map, - // its hashtable, and one tombstone Entry — well under 1 KB. - size_t cost = after - before; - ASSERT(cost < 1024); - - arena_destroy(as); - arena_destroy(ad); + map_release(src); + map_release(clone); } int main(void) { @@ -1206,7 +1347,7 @@ int main(void) { RUN(merge_idempotent); RUN(merge_associative); RUN(merge_does_not_mutate_src); - RUN(merge_copies_string_into_dst_arena); + RUN(merge_copies_string_into_dst); RUN(merge_preserves_tombstone_against_older_set); RUN(merge_same_kind_counter_recurses); @@ -1218,6 +1359,8 @@ int main(void) { RUN(set_composite_displaces_scalar_at_lww); RUN(set_scalar_displaces_composite_at_lww); RUN(set_different_kind_composite_displaces_at_lww); + RUN(evicted_composite_is_displaced_and_outlives_via_held_ref); + RUN(delete_composite_displaces_it); RUN(merge_composite_src_wins_into_empty_slot_clones); RUN(merge_does_not_clone_when_src_loses_lww); @@ -1232,6 +1375,9 @@ int main(void) { RUN(map_map_returns_same_pointer_on_repeat); RUN(map_register_after_map_counter_flips_kind_via_lww); RUN(map_helper_losing_stamp_returns_detached_and_keeps_slot); + RUN(map_counter_losing_stamp_returns_detached_displaced); + RUN(map_map_losing_stamp_returns_detached_displaced); + RUN(map_counter_installed_handle_is_a_borrow); RUN(map_counter_cross_replica_merge_recurses); RUN(map_counter_derives_id_from_parent_key_kind); @@ -1240,9 +1386,12 @@ int main(void) { RUN(helpers_converge_across_replicas); RUN(helpers_at_same_key_different_kind_have_distinct_ids); + RUN(map_release_drops_slot_refs_but_held_ref_survives); + RUN(map_displace_sets_flag); + RUN(clone_empty_map_is_empty); RUN(clone_preserves_scalar_slots); - RUN(clone_survives_src_arena_destroy); + RUN(clone_survives_src_release); RUN(clone_recurses_into_composite_slots); RUN(clone_preserves_tombstones); RUN(clone_independent_of_src); From 81d184ad568fc49b31e69d1931e80d30d778caaf Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sat, 27 Jun 2026 22:25:48 -0300 Subject: [PATCH 06/12] refactor: drop arena allocator, host_malloc everywhere The CRDT core moved fully to refcount/host_malloc ownership, leaving the arena allocator and its dual-mode seams with no production consumer. Strip the arena branch from hashtable and scalar: hashtable_create() and scalar_clone(value) lose the Arena* param and allocate via host_malloc only. Drop the arena.h includes, the HashTable.arena field, and the arena-mode test groups (now redundant with the host_malloc path). Delete arena.c / arena.h / test_arena.c and the test-arena make target. Full suite green; hashtable / scalar / map clean under ASan. --- .gitignore | 1 - Makefile | 31 ++-- arena.c | 159 ------------------ arena.h | 38 ----- counter.c | 2 +- element.c | 2 +- element.h | 9 +- hashtable.c | 85 ++++------ hashtable.h | 25 +-- host.h | 4 +- map.c | 22 +-- map.h | 2 +- register.c | 8 +- scalar.c | 19 +-- scalar.h | 26 ++- stamp.h | 2 +- test_arena.c | 420 ----------------------------------------------- test_counter.c | 2 +- test_element.c | 19 +-- test_hashtable.c | 103 +++--------- test_map.c | 31 +--- test_register.c | 10 -- test_scalar.c | 118 +++---------- 23 files changed, 151 insertions(+), 987 deletions(-) delete mode 100644 arena.c delete mode 100644 arena.h delete mode 100644 test_arena.c diff --git a/.gitignore b/.gitignore index 0783945..295203e 100644 --- a/.gitignore +++ b/.gitignore @@ -15,7 +15,6 @@ a.out *.dSYM/ # Test binaries -/test_arena /test_hashtable /test_string /test_counter diff --git a/Makefile b/Makefile index 44c562e..fc43358 100644 --- a/Makefile +++ b/Makefile @@ -21,14 +21,9 @@ fmt: $(CLANG_FORMAT) fmt-check: $(CLANG_FORMAT) $(CLANG_FORMAT) --dry-run --Werror $(SRC) -.PHONY: test-arena -test-arena: arena.c string.c host_posix.c test_arena.c test_util.h - $(CC) $(CFLAGS) -o test_arena arena.c string.c host_posix.c test_arena.c - ./test_arena - .PHONY: test-hashtable -test-hashtable: arena.c string.c host_posix.c hashtable.c test_hashtable.c test_util.h - $(CC) $(CFLAGS) -o test_hashtable arena.c string.c host_posix.c hashtable.c test_hashtable.c +test-hashtable: string.c host_posix.c hashtable.c test_hashtable.c test_util.h + $(CC) $(CFLAGS) -o test_hashtable string.c host_posix.c hashtable.c test_hashtable.c ./test_hashtable .PHONY: test-string @@ -37,28 +32,28 @@ test-string: string.c test_string.c test_util.h ./test_string .PHONY: test-counter -test-counter: arena.c string.c hashtable.c clientid.c elementid.c uuid.c sha1.c host_posix.c counter.c test_counter.c test_util.h - $(CC) $(CFLAGS) -o test_counter arena.c string.c hashtable.c clientid.c elementid.c uuid.c sha1.c host_posix.c counter.c test_counter.c +test-counter: string.c hashtable.c clientid.c elementid.c uuid.c sha1.c host_posix.c counter.c test_counter.c test_util.h + $(CC) $(CFLAGS) -o test_counter string.c hashtable.c clientid.c elementid.c uuid.c sha1.c host_posix.c counter.c test_counter.c ./test_counter .PHONY: test-scalar -test-scalar: arena.c string.c host_posix.c scalar.c test_scalar.c test_util.h - $(CC) $(CFLAGS) -o test_scalar arena.c string.c host_posix.c scalar.c test_scalar.c +test-scalar: string.c host_posix.c scalar.c test_scalar.c test_util.h + $(CC) $(CFLAGS) -o test_scalar string.c host_posix.c scalar.c test_scalar.c ./test_scalar .PHONY: test-register -test-register: arena.c string.c clientid.c elementid.c uuid.c sha1.c host_posix.c stamp.c scalar.c register.c test_register.c test_util.h - $(CC) $(CFLAGS) -o test_register arena.c string.c clientid.c elementid.c uuid.c sha1.c host_posix.c stamp.c scalar.c register.c test_register.c +test-register: string.c clientid.c elementid.c uuid.c sha1.c host_posix.c stamp.c scalar.c register.c test_register.c test_util.h + $(CC) $(CFLAGS) -o test_register string.c clientid.c elementid.c uuid.c sha1.c host_posix.c stamp.c scalar.c register.c test_register.c ./test_register .PHONY: test-map -test-map: arena.c string.c hashtable.c clientid.c elementid.c uuid.c sha1.c host_posix.c stamp.c scalar.c register.c counter.c element.c map.c test_map.c test_util.h - $(CC) $(CFLAGS) -o test_map arena.c string.c hashtable.c clientid.c elementid.c uuid.c sha1.c host_posix.c stamp.c scalar.c register.c counter.c element.c map.c test_map.c +test-map: string.c hashtable.c clientid.c elementid.c uuid.c sha1.c host_posix.c stamp.c scalar.c register.c counter.c element.c map.c test_map.c test_util.h + $(CC) $(CFLAGS) -o test_map string.c hashtable.c clientid.c elementid.c uuid.c sha1.c host_posix.c stamp.c scalar.c register.c counter.c element.c map.c test_map.c ./test_map .PHONY: test-element -test-element: arena.c string.c hashtable.c clientid.c elementid.c uuid.c sha1.c host_posix.c stamp.c scalar.c register.c counter.c map.c element.c test_element.c test_util.h - $(CC) $(CFLAGS) -o test_element arena.c string.c hashtable.c clientid.c elementid.c uuid.c sha1.c host_posix.c stamp.c scalar.c register.c counter.c map.c element.c test_element.c +test-element: string.c hashtable.c clientid.c elementid.c uuid.c sha1.c host_posix.c stamp.c scalar.c register.c counter.c map.c element.c test_element.c test_util.h + $(CC) $(CFLAGS) -o test_element string.c hashtable.c clientid.c elementid.c uuid.c sha1.c host_posix.c stamp.c scalar.c register.c counter.c map.c element.c test_element.c ./test_element .PHONY: test-elementid @@ -96,4 +91,4 @@ test-stamp: string.c clientid.c host_posix.c stamp.c test_stamp.c test_util.h ./test_stamp .PHONY: test -test: test-arena test-hashtable test-string test-counter test-scalar test-register test-clientid test-stamp test-map test-sha1 test-sha1-runtime-endian test-uuid test-elementid test-element +test: test-hashtable test-string test-counter test-scalar test-register test-clientid test-stamp test-map test-sha1 test-sha1-runtime-endian test-uuid test-elementid test-element diff --git a/arena.c b/arena.c deleted file mode 100644 index f52a415..0000000 --- a/arena.c +++ /dev/null @@ -1,159 +0,0 @@ -#include "arena.h" -#include "host.h" -#include "string.h" - -#include - -struct ArenaPage { - struct ArenaPage *next; - size_t size; // payload capacity - size_t offset; // bytes used in this page - uint8_t *data; // aligned payload start -}; - -struct Arena { - ArenaPage *first; - ArenaPage *current; - size_t total_used; - size_t total_capacity; - size_t peak; -}; - -static size_t align_up(size_t value, size_t align) { - return (value + align - 1) & ~(align - 1); -} - -static uintptr_t align_up_ptr(uintptr_t value, size_t align) { - return (value + align - 1) & ~(uintptr_t)(align - 1); -} - -// Allocate one chained page. The single allocation holds the ArenaPage struct -// followed by alignment padding and the payload. Returns NULL on host_malloc -// failure. -static ArenaPage *new_page(size_t payload_size) { - size_t align = _Alignof(max_align_t); - // Reserve room for the struct + worst-case alignment slack + the payload. - size_t alloc_size = sizeof(ArenaPage) + (align - 1) + payload_size; - uint8_t *raw = host_malloc(alloc_size); - if (!raw) { - return NULL; - } - ArenaPage *page = (ArenaPage *)raw; - page->next = NULL; - uintptr_t data_at = - align_up_ptr((uintptr_t)(raw + sizeof(ArenaPage)), align); - page->data = (uint8_t *)data_at; - page->size = (size_t)((uintptr_t)(raw + alloc_size) - data_at); - page->offset = 0; - return page; -} - -Arena *arena_create(void) { - Arena *arena = host_malloc(sizeof(Arena)); - if (!arena) { - return NULL; - } - ArenaPage *page = new_page(ARENA_DEFAULT_PAGE); - if (!page) { - host_free(arena); - return NULL; - } - arena->first = page; - arena->current = page; - arena->total_used = 0; - arena->total_capacity = page->size; - arena->peak = 0; - return arena; -} - -void arena_destroy(Arena *arena) { - if (!arena) { - return; - } - ArenaPage *page = arena->first; - while (page) { - ArenaPage *next = page->next; - host_free(page); - page = next; - } - host_free(arena); -} - -void *arena_alloc(Arena *arena, size_t size) { - size_t align = _Alignof(max_align_t); - size_t aligned_size = align_up(size, align); - - ArenaPage *page = arena->current; - if (page->offset + aligned_size > page->size) { - // Grow: allocate a new page sized to fit the request (at least the - // default page size). - size_t new_payload = aligned_size > ARENA_DEFAULT_PAGE - ? aligned_size - : ARENA_DEFAULT_PAGE; - ArenaPage *grown = new_page(new_payload); - if (!grown) { - return NULL; - } - page->next = grown; - arena->current = grown; - arena->total_capacity += grown->size; - page = grown; - } - - void *ptr = page->data + page->offset; - page->offset += aligned_size; - arena->total_used += aligned_size; - if (arena->total_used > arena->peak) { - arena->peak = arena->total_used; - } - memset(ptr, 0, size); - return ptr; -} - -ArenaMark arena_mark(const Arena *arena) { - ArenaMark mark = {arena->current, arena->current->offset}; - return mark; -} - -void arena_restore(Arena *arena, ArenaMark mark) { - // Walk the chain to validate `mark.page` and compute the cumulative byte - // count up to that page. - ArenaPage *page = arena->first; - size_t cumulative = 0; - while (page && page != mark.page) { - cumulative += page->offset; - page = page->next; - } - if (!page) { - host_abort("arena_restore: mark page is not in this arena"); - } - if (mark.offset > page->offset) { - host_abortf( - "arena_restore: mark offset %zu exceeds page offset %zu (would " - "advance, not rewind)", - mark.offset, page->offset); - } - - page->offset = mark.offset; - ArenaPage *tail = page->next; - page->next = NULL; - arena->current = page; - while (tail) { - ArenaPage *next = tail->next; - arena->total_capacity -= tail->size; - host_free(tail); - tail = next; - } - - arena->total_used = cumulative + mark.offset; - // Peak deliberately untouched — it's a watermark. -} - -void arena_reset(Arena *arena) { - ArenaMark zero = {arena->first, 0}; - arena_restore(arena, zero); -} - -size_t arena_used(const Arena *arena) { return arena->total_used; } -size_t arena_capacity(const Arena *arena) { return arena->total_capacity; } -size_t arena_peak(const Arena *arena) { return arena->peak; } diff --git a/arena.h b/arena.h deleted file mode 100644 index 79b3766..0000000 --- a/arena.h +++ /dev/null @@ -1,38 +0,0 @@ -#ifndef _CRDT_ARENA_H -#define _CRDT_ARENA_H - -#include -#include - -// Opaque handles. Bodies live in arena.c. -typedef struct Arena Arena; -typedef struct ArenaPage ArenaPage; - -// Position into an arena's allocation timeline. Returned by arena_mark and -// passed back to arena_restore. Pass-by-value; the page pointer it carries -// is owned by the arena and must not be freed by the caller. -typedef struct ArenaMark { - ArenaPage *page; - size_t offset; -} ArenaMark; - -// Default size used for the first page and for subsequently grown pages when -// the requested allocation fits. Allocations larger than this get a dedicated -// page sized for the request. -#define ARENA_DEFAULT_PAGE 4096 - -Arena *arena_create(void); -void arena_destroy(Arena *arena); - -void *arena_alloc(Arena *arena, size_t size); - -ArenaMark arena_mark(const Arena *arena); -void arena_restore(Arena *arena, ArenaMark mark); -void arena_reset(Arena *arena); - -// Stats. -size_t arena_used(const Arena *arena); // bytes allocated right now -size_t arena_capacity(const Arena *arena); // bytes available across all pages -size_t arena_peak(const Arena *arena); // peak `used` watermark since create - -#endif // _CRDT_ARENA_H diff --git a/counter.c b/counter.c index 7611ad0..8793015 100644 --- a/counter.c +++ b/counter.c @@ -25,7 +25,7 @@ Counter *counter_create(ElementId id) { sizeof(Counter)); } counter->id = id; - counter->entries = hashtable_create(NULL); + counter->entries = hashtable_create(); if (!counter->entries) { host_free(counter); host_abort("counter_create: hashtable_create OOM (per-client tallies " diff --git a/element.c b/element.c index 4bf43e4..9021f33 100644 --- a/element.c +++ b/element.c @@ -81,7 +81,7 @@ Element element_clone(Element e) { switch (e.kind) { case ELEMENT_SCALAR: { - Scalar cloned = scalar_clone(NULL, e.as.scalar); + Scalar cloned = scalar_clone(e.as.scalar); result = element_scalar(cloned); } break; diff --git a/element.h b/element.h index 5aae155..4d88433 100644 --- a/element.h +++ b/element.h @@ -16,10 +16,11 @@ // lives at the slot level (in Map). Reaching this branch // is a programmer error. // -// Ownership: composites are referenced by pointer; element_merge mutates -// dst's composite in place and never touches src's. Callers are -// responsible for keeping pointed-to composites alive (typically by -// putting them in the same arena as the containing Map). +// Ownership: composites are referenced by refcounted pointer; element_merge +// mutates dst's composite in place and never touches src's. element_acquire / +// _release / _displace / _is_displaced forward to the underlying composite +// (SCALAR is a no-op for acquire/displace and scalar_free on release). Callers +// are responsible for keeping pointed-to composites alive via the refcount. #include "counter.h" #include "elementid.h" diff --git a/hashtable.c b/hashtable.c index b117f07..ac84741 100644 --- a/hashtable.c +++ b/hashtable.c @@ -20,67 +20,42 @@ typedef struct HashTableNode { HashTableNode *next; } HashTableNode; -HashTable *hashtable_create(Arena *arena) { - if (arena == NULL) { - HashTable *table = host_malloc(sizeof(HashTable)); - if (!table) { - return NULL; - } - - table->arena = NULL; - table->head = NULL; - return table; - } - - HashTable *table = arena_alloc(arena, sizeof(HashTable)); +HashTable *hashtable_create(void) { + HashTable *table = host_malloc(sizeof(HashTable)); if (!table) { return NULL; } - table->arena = arena; table->head = NULL; - return table; } void hashtable_destroy(HashTable *table) { - if (table != NULL && table->arena == NULL) { - HashTableNode *n = table->head; - while (n) { - HashTableNode *next = n->next; - host_free(n->key); - host_free(n); - n = next; - } - host_free(table); + if (table == NULL) { + return; + } + HashTableNode *n = table->head; + while (n) { + HashTableNode *next = n->next; + host_free(n->key); + host_free(n); + n = next; } + host_free(table); } static HashTableInsertResult _hashtable_insert(HashTable *table, const void *key, size_t key_len, void *value) { - HashTableNode *node = NULL; - if (table->arena == NULL) { - node = host_malloc(sizeof(HashTableNode)); - if (!node) { - return HASHTABLE_ERR_OOM; - } - - node->key = host_malloc(key_len); - if (!node->key) { - host_free(node); - return HASHTABLE_ERR_OOM; - } - } else { - node = arena_alloc(table->arena, sizeof(HashTableNode)); - if (!node) { - return HASHTABLE_ERR_OOM; - } + HashTableNode *node = host_malloc(sizeof(HashTableNode)); + if (!node) { + return HASHTABLE_ERR_OOM; + } - node->key = arena_alloc(table->arena, key_len); - if (!node->key) { - return HASHTABLE_ERR_OOM; - } + node->key = host_malloc(key_len); + if (!node->key) { + host_free(node); + return HASHTABLE_ERR_OOM; } memcpy(node->key, key, key_len); @@ -131,10 +106,8 @@ HashTableRemoveResult hashtable_remove(HashTable *table, const void *key, table->head = n->next; } - if (table->arena == NULL) { - host_free(n->key); - host_free(n); - } + host_free(n->key); + host_free(n); return HASHTABLE_REMOVE_OK; } @@ -171,14 +144,12 @@ HashTableUpsertResult hashtable_upsert(HashTable *table, const void *key, } void hashtable_clear(HashTable *table) { - if (table->arena == NULL) { - HashTableNode *n = table->head; - while (n) { - HashTableNode *next = n->next; - host_free(n->key); - host_free(n); - n = next; - } + HashTableNode *n = table->head; + while (n) { + HashTableNode *next = n->next; + host_free(n->key); + host_free(n); + n = next; } table->head = NULL; diff --git a/hashtable.h b/hashtable.h index 8aa2dac..efcb9e5 100644 --- a/hashtable.h +++ b/hashtable.h @@ -1,13 +1,8 @@ #ifndef _CRDT_HASHTABLE_H #define _CRDT_HASHTABLE_H -// Allocation backing — two modes: -// 1. `hashtable_create(arena)` — node structs + key-byte copies allocated -// in the supplied Arena. Bulk lifetime: arena_destroy frees everything. -// No-op `hashtable_destroy` for this mode. -// 2. `hashtable_create(NULL)` — node structs + key-byte copies allocated -// via `host_malloc`. Caller MUST call `hashtable_destroy(table)` when -// done to release everything. +// Allocation: node structs + key-byte copies are allocated via host_malloc. +// The caller MUST call hashtable_destroy(table) when done to release them. // // Ownership: // Keys — table copies key_len bytes when a new entry is inserted (insert, @@ -21,29 +16,25 @@ // pointed-to memory. Lifetime must outlive any get/iter that // reads the slot. // -// Lifetime: For arena-backed tables, must not outlive the arena. For -// host_malloc-backed tables, must outlive any pointer returned by get/iter. +// Lifetime: the table must outlive any pointer returned by get/iter. // // Iteration: do NOT insert into or remove from the table while iterating it. -#include "arena.h" #include #include typedef struct HashTableNode HashTableNode; typedef struct HashTable { - Arena *arena; // NULL when host_malloc-backed HashTableNode *head; } HashTable; -// `arena` may be NULL — in that case, the table allocates via host_malloc -// and the caller must release with hashtable_destroy. -HashTable *hashtable_create(Arena *arena); +// Allocate an empty table via host_malloc. The caller must release it with +// hashtable_destroy. +HashTable *hashtable_create(void); -// Release a host_malloc-backed table (frees nodes, key copies, the table -// struct itself). No-op for arena-backed tables (their arena owns the -// memory). Safe to call regardless of backing. +// Release the table: frees nodes, key copies, and the table struct itself. +// Safe to call on NULL. void hashtable_destroy(HashTable *table); typedef enum { diff --git a/host.h b/host.h index 0ec3fac..355d1ab 100644 --- a/host.h +++ b/host.h @@ -26,8 +26,8 @@ uint64_t host_now_ms(void); void host_fill_entropy(uint8_t *buf, size_t n); // Abort the process — programmer-error escape hatch for invariant violations -// (e.g. passing an out-of-range mark to arena_restore). `reason` is a static -// string suitable for logging. Never returns. +// (e.g. a refcount underflow or a hashtable insert collision). `reason` is a +// static string suitable for logging. Never returns. _Noreturn void host_abort(const char *reason); // printf-style variant. Per-target impls must support at minimum %s, %d, %u diff --git a/map.c b/map.c index 7ee7e73..c8451a1 100644 --- a/map.c +++ b/map.c @@ -21,11 +21,11 @@ struct Map { Map *map_create(ElementId id) { Map *map = host_malloc(sizeof(Map)); if (!map) { - host_abortf("map_create: arena OOM (requested %zu bytes for Map)", + host_abortf("map_create: host_malloc OOM (requested %zu bytes for Map)", sizeof(Map)); } map->id = id; - map->entries = hashtable_create(NULL); + map->entries = hashtable_create(); map->refcount = 1; map->displaced = false; if (!map->entries) { @@ -63,7 +63,7 @@ void map_set(Map *map, const void *key, size_t key_len, Element value, entry = host_malloc(sizeof(Entry)); if (!entry) { host_abortf( - "map_set: arena OOM (requested %zu bytes for Entry)", + "map_set: host_malloc OOM (requested %zu bytes for Entry)", sizeof(Entry)); } HashTableInsertResult r = @@ -76,7 +76,7 @@ void map_set(Map *map, const void *key, size_t key_len, Element value, switch (value.kind) { case ELEMENT_SCALAR: { - Scalar copy = scalar_clone(NULL, value.as.scalar); + Scalar copy = scalar_clone(value.as.scalar); value.as.scalar = copy; break; } @@ -111,8 +111,9 @@ void map_delete(Map *map, const void *key, size_t key_len, Stamp stamp) { // compare stamps and know that the delete wins over older sets. entry = host_malloc(sizeof(Entry)); if (!entry) { - host_abortf("map_delete: arena OOM (requested %zu bytes for Entry)", - sizeof(Entry)); + host_abortf( + "map_delete: host_malloc OOM (requested %zu bytes for Entry)", + sizeof(Entry)); } entry->stamp = stamp; entry->is_tombstone = true; @@ -170,8 +171,8 @@ void map_merge(Map *dst, const Map *src) { // Skip the clone+set if src would lose LWW — map_set would do the // same stamp check internally and discard the work, but element_clone - // on a composite is deep recursive and leaks into dst's arena even - // when the value is never installed. + // on a composite is deep recursive and would leak a never-installed + // refcounted copy when the value never wins. if (dst_has && !stamp_gt(se->stamp, de->stamp)) { continue; } @@ -283,8 +284,9 @@ Map *map_clone(const Map *map) { Entry *entry = v; Entry *copy = host_malloc(sizeof(Entry)); if (!copy) { - host_abortf("map_clone: arena OOM (requested %zu bytes for Entry)", - sizeof(Entry)); + host_abortf( + "map_clone: host_malloc OOM (requested %zu bytes for Entry)", + sizeof(Entry)); } copy->stamp = entry->stamp; diff --git a/map.h b/map.h index 5cc73a1..62bd7f1 100644 --- a/map.h +++ b/map.h @@ -29,7 +29,7 @@ // winners are deep-cloned via element_clone (refcount=1) so dst owns its // slot fully and is independent of src. // -// Ownership (Share semantics, refcounted — no arena): +// Ownership (Share semantics, refcounted): // - SCALAR_STRING values are deep-copied into Map-owned storage on every // accepted write (set, winning merge). When map_get fills *out with a // SCALAR Element, the bytes are a borrowed view valid until the slot's diff --git a/register.c b/register.c index 0bb4dc3..5f7ab24 100644 --- a/register.c +++ b/register.c @@ -20,7 +20,7 @@ Register *register_create(ElementId id, Scalar value, Stamp stamp) { sizeof(Register)); } reg->id = id; - reg->value = scalar_clone(NULL, value); + reg->value = scalar_clone(value); reg->stamp = stamp; reg->refcount = 1; reg->displaced = false; @@ -34,7 +34,7 @@ Scalar register_read(const Register *reg) { return reg->value; } void register_set(Register *reg, Scalar value, Stamp stamp) { if (stamp_gt(stamp, reg->stamp)) { scalar_free(reg->value); - reg->value = scalar_clone(NULL, value); + reg->value = scalar_clone(value); reg->stamp = stamp; } } @@ -42,7 +42,7 @@ void register_set(Register *reg, Scalar value, Stamp stamp) { void register_merge(Register *dst, const Register *src) { if (stamp_gt(src->stamp, dst->stamp)) { scalar_free(dst->value); - dst->value = scalar_clone(NULL, src->value); + dst->value = scalar_clone(src->value); dst->stamp = src->stamp; } } @@ -55,7 +55,7 @@ Register *register_clone(const Register *reg) { sizeof(Register)); } clone->id = reg->id; - clone->value = scalar_clone(NULL, reg->value); + clone->value = scalar_clone(reg->value); clone->stamp = reg->stamp; clone->refcount = 1; clone->displaced = false; diff --git a/scalar.c b/scalar.c index 5586570..c20106e 100644 --- a/scalar.c +++ b/scalar.c @@ -1,5 +1,4 @@ #include "scalar.h" -#include "arena.h" #include "host.h" #include #include @@ -57,23 +56,21 @@ bool scalar_eq(Scalar a, Scalar b) { } } -Scalar scalar_clone(Arena *arena, Scalar value) { +Scalar scalar_clone(Scalar value) { switch (value.kind) { case SCALAR_STRING: { // Empty string: no bytes to copy. Pass the value through unchanged. - // Avoids portability fragility around malloc(0) / arena_alloc(0) - // (implementation-defined return) and the matching free-leak that - // scalar_free would otherwise miss. + // Avoids portability fragility around malloc(0) (implementation-defined + // return) and the matching free-leak that scalar_free would otherwise + // miss. if (value.as.s.len == 0) { return value; } - uint8_t *bytes_copy = arena == NULL - ? host_malloc(value.as.s.len) - : arena_alloc(arena, value.as.s.len); + uint8_t *bytes_copy = host_malloc(value.as.s.len); if (!bytes_copy) { - host_abortf("scalar_clone: %s OOM (requested %zu bytes for " - "string value)", - arena ? "arena" : "host_malloc", value.as.s.len); + host_abortf("scalar_clone: host_malloc OOM (requested %zu bytes " + "for string value)", + value.as.s.len); } memcpy(bytes_copy, value.as.s.bytes, value.as.s.len); Scalar copy = {0}; diff --git a/scalar.h b/scalar.h index 6921e41..b67eec7 100644 --- a/scalar.h +++ b/scalar.h @@ -10,13 +10,13 @@ // Ownership: passed by value (~24 bytes). For SCALAR_STRING the struct // carries a BORROWED (bytes, len) view; the caller owns the underlying // memory at the API boundary. Anything that needs to store a Scalar past -// the call (Register, Map, ...) must clone the bytes into its own arena. +// the call (Register, Map, ...) must clone the bytes via scalar_clone and +// release them with scalar_free. // // scalar_eq is kind-strict: cross-kind comparison is always false, even // for "obvious" coincidences (scalar_int(0) != scalar_bool(false)). For // SCALAR_STRING, equality is bytes+len memcmp (binary-safe). -#include "arena.h" #include #include #include @@ -51,21 +51,15 @@ Scalar scalar_string(const uint8_t *bytes, size_t len); bool scalar_eq(Scalar a, Scalar b); -// Clone a Scalar into owned storage. Two modes: -// 1. `scalar_clone(arena, value)` — string bytes (if any) allocated in -// the supplied Arena. Bulk lifetime: arena_destroy frees them. Do -// NOT call scalar_free on the result. -// 2. `scalar_clone(NULL, value)` — string bytes (if any) allocated via -// host_malloc. Caller MUST release with scalar_free when done. -// -// For non-string kinds (NULL / BOOL / INT) cloning is a value-copy -// regardless of mode — nothing to allocate. -Scalar scalar_clone(Arena *arena, Scalar value); +// Clone a Scalar into owned storage: string bytes (if any) are allocated via +// host_malloc and the caller MUST release them with scalar_free when done. For +// non-string kinds (NULL / BOOL / INT) cloning is a value-copy — nothing to +// allocate, and scalar_free is a harmless no-op. +Scalar scalar_clone(Scalar value); -// Release a host_malloc-backed Scalar's string bytes. No-op for non-string -// kinds and for empty strings (no allocation to release). MUST only be -// called on Scalars produced by `scalar_clone(NULL, ...)`. Calling on an -// arena-backed Scalar is undefined. +// Release a cloned Scalar's string bytes. No-op for non-string kinds and for +// empty strings (no allocation to release). MUST only be called on Scalars +// produced by scalar_clone. void scalar_free(Scalar value); #endif // _CRDT_SCALAR_H diff --git a/stamp.h b/stamp.h index 5c517de..fd1fbed 100644 --- a/stamp.h +++ b/stamp.h @@ -17,7 +17,7 @@ // - transitive — stamp_gt(a, b) and stamp_gt(b, c) implies stamp_gt(a, c) // - trichotomous — for any a, b: exactly one of (a > b, b > a, equal) // -// Ownership: pass-by-value. No allocation, no arena. +// Ownership: pass-by-value. No allocation. #include "clientid.h" #include diff --git a/test_arena.c b/test_arena.c deleted file mode 100644 index 31af873..0000000 --- a/test_arena.c +++ /dev/null @@ -1,420 +0,0 @@ -#include "arena.h" -#include "test_util.h" - -#include -#include - -// All returned pointers must satisfy this alignment. -#define ARENA_ALIGN _Alignof(max_align_t) - -// --- create / destroy / basic alloc --- - -TEST(create_returns_non_null) { - Arena *a = arena_create(); - ASSERT(a != NULL); - arena_destroy(a); -} - -TEST(destroy_after_allocs_does_not_crash) { - Arena *a = arena_create(); - arena_alloc(a, 64); - arena_alloc(a, 4096); // probably grows - arena_destroy(a); -} - -TEST(alloc_returns_non_null) { - Arena *a = arena_create(); - ASSERT(arena_alloc(a, 32) != NULL); - arena_destroy(a); -} - -TEST(alloc_returns_distinct_pointers) { - Arena *a = arena_create(); - void *p1 = arena_alloc(a, 32); - void *p2 = arena_alloc(a, 32); - ASSERT(p1 != p2); - arena_destroy(a); -} - -TEST(alloc_zeroes_memory) { - Arena *a = arena_create(); - uint8_t *p = arena_alloc(a, 64); - for (int i = 0; i < 64; i++) { - ASSERT_EQ(p[i], 0); - } - // Dirty the memory and ensure a *fresh* alloc still returns zeros. - for (int i = 0; i < 64; i++) { - p[i] = 0xAB; - } - uint8_t *q = arena_alloc(a, 64); - for (int i = 0; i < 64; i++) { - ASSERT_EQ(q[i], 0); - } - arena_destroy(a); -} - -// --- alignment of returned pointers --- - -TEST(alloc_pointers_are_aligned) { - Arena *a = arena_create(); - for (int i = 0; i < 8; i++) { - void *p = arena_alloc(a, 32); - ASSERT_EQ((uintptr_t)p % ARENA_ALIGN, 0); - } - arena_destroy(a); -} - -TEST(alloc_aligned_after_odd_sizes) { - Arena *a = arena_create(); - size_t odd_sizes[] = {1, 3, 5, 7, 9, 11, 13, 15, 17, 33}; - for (size_t i = 0; i < sizeof(odd_sizes) / sizeof(odd_sizes[0]); i++) { - void *p = arena_alloc(a, odd_sizes[i]); - ASSERT(p != NULL); - ASSERT_EQ((uintptr_t)p % ARENA_ALIGN, 0); - } - arena_destroy(a); -} - -// A 1-byte alloc consumes more than 1 byte because the next allocation must -// land on the alignment boundary. -TEST(odd_size_alloc_advances_to_next_boundary) { - Arena *a = arena_create(); - uint8_t *p1 = arena_alloc(a, 1); - uint8_t *p2 = arena_alloc(a, 1); - ASSERT(p1 != NULL && p2 != NULL); - // p1 and p2 are guaranteed to be on the same first page (no growth yet). - ASSERT((size_t)(p2 - p1) >= ARENA_ALIGN); - arena_destroy(a); -} - -// --- reset --- - -TEST(reset_allows_full_reuse) { - Arena *a = arena_create(); - void *p1 = arena_alloc(a, 128); - ASSERT(p1 != NULL); - arena_reset(a); - void *p2 = arena_alloc(a, 128); - ASSERT(p2 != NULL); - ASSERT(p2 == p1); // reset rewound to the start of the first page - arena_destroy(a); -} - -TEST(reset_zeroes_memory_on_next_alloc) { - Arena *a = arena_create(); - uint8_t *p = arena_alloc(a, 64); - for (int i = 0; i < 64; i++) { - p[i] = 0xFF; - } - arena_reset(a); - uint8_t *p2 = arena_alloc(a, 64); - for (int i = 0; i < 64; i++) { - ASSERT_EQ(p2[i], 0); - } - arena_destroy(a); -} - -// --- mark / restore --- - -TEST(restore_to_mark_undoes_intervening_allocs) { - Arena *a = arena_create(); - void *p1 = arena_alloc(a, 128); - ASSERT(p1 != NULL); - - ArenaMark mark = arena_mark(a); - - void *p2 = arena_alloc(a, 128); - ASSERT(p2 != NULL); - - arena_restore(a, mark); - - void *p3 = arena_alloc(a, 128); - ASSERT(p3 == p2); // same address as the rolled-back alloc - arena_destroy(a); -} - -// Memory allocated BEFORE the mark must remain valid after restore. -TEST(memory_before_mark_remains_valid) { - Arena *a = arena_create(); - uint8_t *p1 = arena_alloc(a, 64); - for (int i = 0; i < 64; i++) { - p1[i] = (uint8_t)(0x80 | i); - } - ArenaMark mark = arena_mark(a); - arena_alloc(a, 256); - arena_restore(a, mark); - - for (int i = 0; i < 64; i++) { - ASSERT_EQ(p1[i], (uint8_t)(0x80 | i)); - } - arena_destroy(a); -} - -TEST(restore_to_zero_is_equivalent_to_reset) { - Arena *a = arena_create(); - ArenaMark initial = arena_mark(a); - void *p1 = arena_alloc(a, 128); - ASSERT(p1 != NULL); - arena_alloc(a, 64); - arena_alloc(a, 64); - arena_restore(a, initial); - - void *p2 = arena_alloc(a, 128); - ASSERT(p2 == p1); - arena_destroy(a); -} - -TEST(nested_marks_restore_in_lifo_order) { - Arena *a = arena_create(); - void *p_outer = arena_alloc(a, 128); - ASSERT(p_outer != NULL); - - ArenaMark outer_mark = arena_mark(a); - void *p_inner = arena_alloc(a, 128); - ASSERT(p_inner != NULL); - - ArenaMark inner_mark = arena_mark(a); - arena_alloc(a, 64); - arena_alloc(a, 64); - - arena_restore(a, inner_mark); - void *p_after_inner = arena_alloc(a, 64); - ASSERT(p_after_inner != NULL); - ASSERT(p_after_inner > p_inner); - - arena_restore(a, outer_mark); - void *p_after_outer = arena_alloc(a, 128); - ASSERT(p_after_outer == p_inner); - arena_destroy(a); -} - -// After restore, the next alloc must return zeroed memory — scratch data from -// the rolled-back allocs must not leak. -TEST(restore_then_alloc_returns_zeroed_memory) { - Arena *a = arena_create(); - ArenaMark mark = arena_mark(a); - - uint8_t *scratch = arena_alloc(a, 128); - for (int i = 0; i < 128; i++) { - scratch[i] = 0xCC; - } - - arena_restore(a, mark); - - uint8_t *fresh_p = arena_alloc(a, 128); - for (int i = 0; i < 128; i++) { - ASSERT_EQ(fresh_p[i], 0); - } - arena_destroy(a); -} - -// --- growth (chained pages backed by host_malloc) --- - -// A request larger than the first page's capacity must succeed — the arena -// allocates a new page. -TEST(alloc_beyond_first_page_grows_capacity) { - Arena *a = arena_create(); - size_t cap_before = arena_capacity(a); - void *p = arena_alloc(a, cap_before * 4); - ASSERT(p != NULL); - ASSERT(arena_capacity(a) > cap_before); - arena_destroy(a); -} - -// A request larger than ARENA_DEFAULT_PAGE must still succeed; the arena -// should allocate a dedicated page sized for the request. -TEST(alloc_larger_than_default_page_grows_to_fit) { - Arena *a = arena_create(); - void *p = arena_alloc(a, ARENA_DEFAULT_PAGE * 4); - ASSERT(p != NULL); - ASSERT_EQ((uintptr_t)p % ARENA_ALIGN, 0); - arena_destroy(a); -} - -// Earlier allocations must not move or get clobbered when later allocations -// trigger growth. -TEST(earlier_allocs_survive_growth) { - Arena *a = arena_create(); - uint8_t *first = arena_alloc(a, 64); - ASSERT(first != NULL); - for (int i = 0; i < 64; i++) { - first[i] = (uint8_t)(0x80 | i); - } - for (int i = 0; i < 50; i++) { - ASSERT(arena_alloc(a, 256) != NULL); - } - for (int i = 0; i < 64; i++) { - ASSERT_EQ(first[i], (uint8_t)(0x80 | i)); - } - arena_destroy(a); -} - -TEST(allocs_on_secondary_pages_are_aligned) { - Arena *a = arena_create(); - for (int i = 0; i < 100; i++) { - void *p = arena_alloc(a, 33); - ASSERT(p != NULL); - ASSERT_EQ((uintptr_t)p % ARENA_ALIGN, 0); - } - arena_destroy(a); -} - -// arena_reset must free secondary pages, returning capacity to the first -// page's payload size. -TEST(reset_frees_secondary_pages) { - Arena *a = arena_create(); - size_t cap_initial = arena_capacity(a); - for (int i = 0; i < 100; i++) { - ASSERT(arena_alloc(a, 256) != NULL); - } - ASSERT(arena_capacity(a) > cap_initial); - - arena_reset(a); - ASSERT_EQ(arena_capacity(a), cap_initial); - arena_destroy(a); -} - -// arena_restore must free any pages allocated past the mark. -TEST(restore_frees_secondary_pages_past_mark) { - Arena *a = arena_create(); - size_t cap_initial = arena_capacity(a); - ArenaMark mark = arena_mark(a); - - for (int i = 0; i < 100; i++) { - ASSERT(arena_alloc(a, 256) != NULL); - } - ASSERT(arena_capacity(a) > cap_initial); - - arena_restore(a, mark); - ASSERT_EQ(arena_capacity(a), cap_initial); - arena_destroy(a); -} - -// --- stats: used / capacity / peak --- - -TEST(used_starts_at_zero) { - Arena *a = arena_create(); - ASSERT_EQ(arena_used(a), 0); - arena_destroy(a); -} - -TEST(used_advances_by_aligned_size) { - Arena *a = arena_create(); - size_t before = arena_used(a); - arena_alloc(a, 7); - size_t after_one = arena_used(a); - ASSERT(after_one >= before + 7); - ASSERT(after_one - before <= 7 + ARENA_ALIGN); - - arena_alloc(a, 7); - size_t after_two = arena_used(a); - ASSERT(after_two - after_one >= 7); - arena_destroy(a); -} - -TEST(used_returns_to_zero_after_reset) { - Arena *a = arena_create(); - arena_alloc(a, 200); - arena_alloc(a, 200); - ASSERT(arena_used(a) > 0); - arena_reset(a); - ASSERT_EQ(arena_used(a), 0); - arena_destroy(a); -} - -TEST(restore_rewinds_used_to_mark_time) { - Arena *a = arena_create(); - arena_alloc(a, 100); - size_t used_at_mark = arena_used(a); - ArenaMark mark = arena_mark(a); - arena_alloc(a, 100); - arena_alloc(a, 100); - - arena_restore(a, mark); - ASSERT_EQ(arena_used(a), used_at_mark); - arena_destroy(a); -} - -TEST(capacity_at_least_covers_used) { - Arena *a = arena_create(); - arena_alloc(a, 200); - ASSERT(arena_capacity(a) >= arena_used(a)); - arena_destroy(a); -} - -TEST(peak_starts_at_zero) { - Arena *a = arena_create(); - ASSERT_EQ(arena_peak(a), 0); - arena_destroy(a); -} - -TEST(peak_tracks_high_water_mark) { - Arena *a = arena_create(); - arena_alloc(a, 300); - arena_alloc(a, 300); - size_t high = arena_used(a); - ASSERT_EQ(arena_peak(a), high); - - arena_reset(a); - ASSERT_EQ(arena_used(a), 0); - // Peak does NOT decrease — it's a watermark. - ASSERT_EQ(arena_peak(a), high); - arena_destroy(a); -} - -TEST(peak_only_grows) { - Arena *a = arena_create(); - arena_alloc(a, 500); - size_t peak_after_first = arena_peak(a); - - ArenaMark mark = arena_mark(a); - arena_alloc(a, 500); - size_t peak_after_two = arena_peak(a); - ASSERT(peak_after_two >= peak_after_first); - - arena_restore(a, mark); - ASSERT_EQ(arena_peak(a), peak_after_two); - - arena_alloc(a, 100); - ASSERT_EQ(arena_peak(a), peak_after_two); - arena_destroy(a); -} - -int main(void) { - RUN(create_returns_non_null); - RUN(destroy_after_allocs_does_not_crash); - RUN(alloc_returns_non_null); - RUN(alloc_returns_distinct_pointers); - RUN(alloc_zeroes_memory); - - RUN(alloc_pointers_are_aligned); - RUN(alloc_aligned_after_odd_sizes); - RUN(odd_size_alloc_advances_to_next_boundary); - - RUN(reset_allows_full_reuse); - RUN(reset_zeroes_memory_on_next_alloc); - - RUN(restore_to_mark_undoes_intervening_allocs); - RUN(memory_before_mark_remains_valid); - RUN(restore_to_zero_is_equivalent_to_reset); - RUN(nested_marks_restore_in_lifo_order); - RUN(restore_then_alloc_returns_zeroed_memory); - - RUN(alloc_beyond_first_page_grows_capacity); - RUN(alloc_larger_than_default_page_grows_to_fit); - RUN(earlier_allocs_survive_growth); - RUN(allocs_on_secondary_pages_are_aligned); - RUN(reset_frees_secondary_pages); - RUN(restore_frees_secondary_pages_past_mark); - - RUN(used_starts_at_zero); - RUN(used_advances_by_aligned_size); - RUN(used_returns_to_zero_after_reset); - RUN(restore_rewinds_used_to_mark_time); - RUN(capacity_at_least_covers_used); - RUN(peak_starts_at_zero); - RUN(peak_tracks_high_water_mark); - RUN(peak_only_grows); - - TEST_SUMMARY(); -} diff --git a/test_counter.c b/test_counter.c index 453efc0..f518404 100644 --- a/test_counter.c +++ b/test_counter.c @@ -254,7 +254,7 @@ TEST(local_inc_after_merge_accumulates) { ASSERT_EQ(counter_read(a), 7); } -// --- counter_clone: deep copy into a target arena --- +// --- counter_clone: deep copy into a fresh refcount=1 allocation --- TEST(clone_empty_counter_reads_zero) { Counter *src = counter_create(default_id()); diff --git a/test_element.c b/test_element.c index e83434c..4316c9b 100644 --- a/test_element.c +++ b/test_element.c @@ -9,19 +9,10 @@ #include "string.h" #include "test_util.h" -// NOTE: targets the refcounted lifecycle contract (Option A — Element forwards -// lifecycle to the composite). Will not link until BOTH element.c and map.c -// are converted, since Element's MAP arm calls into Map. Expected surface: -// - element_clone(Element): drops arena, makes refcount=1 children. -// - element_acquire(Element): SCALAR no-op; composite -> *_acquire. -// - element_release(Element): SCALAR scalar_free; composite -> *_release. -// - element_displace(Element): SCALAR no-op; composite -> *_displace. -// - element_is_displaced(Element): SCALAR false; composite -> *_is_displaced. -// -// Ownership rule mirrored from scalar_free: element_release on a SCALAR frees -// the scalar's bytes, so it is valid ONLY on owned scalars (those produced by -// element_clone, i.e. scalar_clone(NULL, ...) provenance) — never on a -// borrowed-buffer element_scalar(...) passed transiently into map_set. +// element_release on a SCALAR frees the scalar's bytes, so it is valid ONLY on +// owned scalars (those produced by element_clone, i.e. scalar_clone provenance) +// — never on a borrowed-buffer element_scalar(...) passed transiently into +// map_set. // Helpers. @@ -211,7 +202,7 @@ TEST(round_trip_via_kind_and_payload) { element_release(e); } -// --- element_clone: deep copy, drops arena, refcount=1 children --- +// --- element_clone: deep copy, refcount=1 children --- // // Used by map_merge when an LWW winner is a composite from a foreign replica. // The clone owns all its memory; releasing the source must leave the clone diff --git a/test_hashtable.c b/test_hashtable.c index 4212bb7..df266df 100644 --- a/test_hashtable.c +++ b/test_hashtable.c @@ -2,7 +2,6 @@ #include #include -#include "arena.h" #include "hashtable.h" #include "test_util.h" @@ -17,15 +16,15 @@ // const void *key, size_t key_len, void *value); bool // hashtable_iter_next(HashTableIter*, const void **key, size_t *key_len, void // **value); -// Keys are copied (key_len bytes) into the arena. Binary-safe: embedded NUL -// bytes are part of the key, and length is significant. +// Keys are copied (key_len bytes) into the table's own host_malloc storage. +// Binary-safe: embedded NUL bytes are part of the key, and length is +// significant. // String-key shorthand: expands to (bytes, length) without the NUL terminator. #define SK(s) (s), strlen(s) static HashTable *fresh(void) { - Arena *arena = arena_create(); - HashTable *table = hashtable_create(arena); + HashTable *table = hashtable_create(); assert(table != NULL); return table; } @@ -349,89 +348,35 @@ TEST(clear_empties_table) { ASSERT(out == &vc); } -// --- host_malloc-backed mode (NULL arena) --- +// --- hashtable_destroy --- // -// All existing semantics must hold when the table is allocated via -// host_malloc instead of an arena. The caller releases via -// hashtable_destroy. Probes a handful of representative operations rather -// than mirroring every test above — semantics should be identical. +// Most tests above leak their table (no teardown hook in this harness); these +// exercise the destroy path explicitly, on an empty and a populated table. +// Run under ASan/LSan to confirm nodes + key copies are freed. -TEST(host_malloc_create_and_destroy_empty) { - HashTable *t = hashtable_create(NULL); +TEST(destroy_empty_table) { + HashTable *t = hashtable_create(); ASSERT(t != NULL); void *out; ASSERT(hashtable_get(t, SK("missing"), &out) == false); hashtable_destroy(t); } -TEST(host_malloc_insert_then_get) { - HashTable *t = hashtable_create(NULL); - int v = 42; - ASSERT_EQ(hashtable_insert(t, SK("k"), &v), HASHTABLE_OK); - void *out; - ASSERT(hashtable_get(t, SK("k"), &out) == true); - ASSERT(out == &v); - hashtable_destroy(t); -} - -// Key bytes must be copied in host_malloc mode too — mutating the caller's -// buffer after insert must not affect the stored key. -TEST(host_malloc_key_is_copied) { - HashTable *t = hashtable_create(NULL); - int v = 1; +TEST(destroy_populated_table_frees_nodes_and_keys) { + HashTable *t = hashtable_create(); + int va = 1, vb = 2, vc = 3; char buf[8]; memcpy(buf, "key", 3); - ASSERT_EQ(hashtable_insert(t, buf, 3, &v), HASHTABLE_OK); - memset(buf, 'X', sizeof buf); - void *out; - ASSERT(hashtable_get(t, "key", 3, &out) == true); - ASSERT(out == &v); - hashtable_destroy(t); -} + ASSERT_EQ(hashtable_insert(t, buf, 3, &va), HASHTABLE_OK); + memset(buf, 'X', sizeof buf); // key must have been copied, not borrowed + hashtable_insert(t, SK("b"), &vb); + hashtable_insert(t, SK("c"), &vc); -TEST(host_malloc_remove_and_reinsert) { - HashTable *t = hashtable_create(NULL); - int v1 = 1, v2 = 2; - hashtable_insert(t, SK("k"), &v1); - ASSERT_EQ(hashtable_remove(t, SK("k")), HASHTABLE_REMOVE_OK); void *out; - ASSERT(hashtable_get(t, SK("k"), &out) == false); - ASSERT_EQ(hashtable_insert(t, SK("k"), &v2), HASHTABLE_OK); - ASSERT(hashtable_get(t, SK("k"), &out) == true); - ASSERT(out == &v2); - hashtable_destroy(t); -} - -TEST(host_malloc_iter_visits_all) { - HashTable *t = hashtable_create(NULL); - int a = 1, b = 2, c = 3; - hashtable_insert(t, SK("a"), &a); - hashtable_insert(t, SK("b"), &b); - hashtable_insert(t, SK("c"), &c); - - HashTableIter it = hashtable_iter(t); - const void *k = NULL; - size_t klen = 0; - void *v = NULL; - int count = 0; - while (hashtable_iter_next(&it, &k, &klen, &v)) - count++; - ASSERT_EQ(count, 3); - hashtable_destroy(t); -} + ASSERT(hashtable_get(t, "key", 3, &out) == true); + ASSERT(out == &va); -// `hashtable_destroy` on an arena-backed table is a no-op — must not crash -// and must not double-free the arena's contents. -TEST(arena_backed_destroy_is_noop) { - Arena *a = arena_create(); - HashTable *t = hashtable_create(a); - int v = 1; - hashtable_insert(t, SK("k"), &v); - hashtable_destroy(t); // safe no-op - // Table still usable until arena_destroy releases it. - void *out; - ASSERT(hashtable_get(t, SK("k"), &out) == true); - arena_destroy(a); + hashtable_destroy(t); // frees the three nodes + their key copies } int main(void) { @@ -456,12 +401,8 @@ int main(void) { RUN(iter_visits_all_once); RUN(clear_empties_table); - RUN(host_malloc_create_and_destroy_empty); - RUN(host_malloc_insert_then_get); - RUN(host_malloc_key_is_copied); - RUN(host_malloc_remove_and_reinsert); - RUN(host_malloc_iter_visits_all); - RUN(arena_backed_destroy_is_noop); + RUN(destroy_empty_table); + RUN(destroy_populated_table_frees_nodes_and_keys); TEST_SUMMARY(); } diff --git a/test_map.c b/test_map.c index 55dbcb1..f7ba8e2 100644 --- a/test_map.c +++ b/test_map.c @@ -10,16 +10,7 @@ #include "test_util.h" #include -// NOTE: targets the refcounted "Share" lifecycle contract (no arena). Will not -// link until map.c (and element.c) are converted. Expected Map surface: -// Map *map_create(ElementId id); // refcount = 1 -// void map_acquire(Map *); -// void map_release(Map *); // drops slot composite refs (recursive), frees -// void map_displace(Map *); -// bool map_is_displaced(const Map *); -// Map *map_clone(const Map *); // refcount = 1, deep copy -// -// Share semantics, exercised throughout: +// Share lifecycle semantics, exercised throughout: // - map_set of a composite: if the write is ACCEPTED (LWW wins), the Map // element_acquires its own ref on the composite. If REJECTED, no-op. Either // way the caller still owns the handle it passed and must release it. @@ -783,13 +774,9 @@ TEST(merge_composite_src_wins_into_empty_slot_clones) { } // When src LOSES the LWW comparison, map_merge must NOT clone src's value into -// dst — that clone would be unreachable garbage (a leak in the refcount model). -// -// NOTE: the original arena-based test asserted this via arena_used byte cost. -// There is no refcount equivalent without a host alloc-counter seam, so the -// perf-probe half is dropped; this keeps the functional guarantee (dst keeps -// its winning scalar, and is independent of src after src is released). A true -// "no wasteful clone" check now needs ASan/LeakSanitizer. +// dst — that clone would be an unreachable leak. This asserts the functional +// guarantee (dst keeps its winning scalar, independent of src after release); +// proving no wasteful clone allocation needs ASan/LeakSanitizer. TEST(merge_does_not_clone_when_src_loses_lww) { Map *dst = fresh(); Map *src = fresh(); @@ -1275,13 +1262,9 @@ TEST(clone_independent_of_src) { } // Tombstone entries carry a stale value field. map_clone must NOT recursively -// clone that stale value into the destination. -// -// NOTE: the original arena-based test measured this via arena_used byte cost. -// Without an alloc-counter seam there is no refcount equivalent, so the -// perf-probe half is dropped; this keeps the functional guarantee (tombstone -// survives the clone, clone is independent). A real "did not recurse the stale -// value" check now needs ASan/LeakSanitizer. +// clone that stale value into the destination. This asserts the functional +// guarantee (tombstone survives the clone); proving no stale-value recursion +// allocation needs ASan/LeakSanitizer. TEST(clone_tombstone_does_not_recurse_into_stale_value) { Map *src = fresh(); diff --git a/test_register.c b/test_register.c index 724389b..271ef45 100644 --- a/test_register.c +++ b/test_register.c @@ -6,16 +6,6 @@ #include "string.h" #include "test_util.h" -// NOTE: these tests target the refcounted lifecycle contract (host_malloc -// backing, no arena), mirroring test_counter.c. They will not link until -// register.h / register.c are converted: -// Register *register_create(ElementId id, Scalar value, Stamp stamp); // rc=1 -// Register *register_clone(const Register *reg); // rc=1 -// void register_acquire(Register *); -// void register_release(Register *); // frees at rc 0; scalar_free's value -// void register_displace(Register *); -// bool register_is_displaced(const Register *); - // Build a ClientId fixture from a single byte (rest zero). static ClientId cid(uint8_t first_byte) { uint8_t b[16] = {0}; diff --git a/test_scalar.c b/test_scalar.c index 787f12e..8895ebb 100644 --- a/test_scalar.c +++ b/test_scalar.c @@ -1,4 +1,3 @@ -#include "arena.h" #include "scalar.h" #include "string.h" #include "test_util.h" @@ -141,104 +140,38 @@ TEST(string_neq_null) { ASSERT(scalar_eq(scalar_string(NULL, 0), scalar_null()) == false); } -// --- scalar_clone: deep copy into a target arena --- - -TEST(clone_null_passes_through) { - Arena *a = arena_create(); - Scalar c = scalar_clone(a, scalar_null()); - ASSERT(scalar_eq(c, scalar_null()) == true); - arena_destroy(a); -} - -TEST(clone_bool_passes_through) { - Arena *a = arena_create(); - Scalar t = scalar_clone(a, scalar_bool(true)); - Scalar f = scalar_clone(a, scalar_bool(false)); - ASSERT(scalar_eq(t, scalar_bool(true)) == true); - ASSERT(scalar_eq(f, scalar_bool(false)) == true); - arena_destroy(a); -} - -TEST(clone_int_passes_through) { - Arena *a = arena_create(); - Scalar c = scalar_clone(a, scalar_int(42)); - ASSERT(scalar_eq(c, scalar_int(42)) == true); - arena_destroy(a); -} - -// String bytes must be deep-copied into dst arena, not borrowed from caller. -// After clone, mutating the source bytes must NOT affect the clone. -TEST(clone_string_owns_bytes_in_dst_arena) { - Arena *a = arena_create(); - uint8_t src[5]; - memcpy(src, "hello", 5); - Scalar c = scalar_clone(a, scalar_string(src, 5)); - src[0] = 'X'; - src[1] = 'X'; - ASSERT(scalar_eq(c, scalar_string((const uint8_t *)"hello", 5)) == true); - arena_destroy(a); -} - -// Clone survives destruction of the original source: the clone's bytes -// must live in the dst arena, not anywhere tied to the caller's buffer. -TEST(clone_string_survives_after_source_buffer_freed) { - Arena *a = arena_create(); - uint8_t *src = (uint8_t *)malloc(5); - memcpy(src, "hello", 5); - Scalar c = scalar_clone(a, scalar_string(src, 5)); - free(src); - ASSERT(scalar_eq(c, scalar_string((const uint8_t *)"hello", 5)) == true); - arena_destroy(a); -} - -TEST(clone_empty_string_handled) { - Arena *a = arena_create(); - Scalar c = scalar_clone(a, scalar_string((const uint8_t *)"", 0)); - ASSERT(scalar_eq(c, scalar_string((const uint8_t *)"", 0)) == true); - arena_destroy(a); -} - -// Bytes with embedded NUL must round-trip byte-for-byte. -TEST(clone_string_with_embedded_nul) { - Arena *a = arena_create(); - uint8_t src[4] = {0x01, 0x00, 0x02, 0x03}; - Scalar c = scalar_clone(a, scalar_string(src, sizeof src)); - ASSERT(scalar_eq(c, scalar_string(src, sizeof src)) == true); - arena_destroy(a); -} - -// --- host_malloc-backed clone (NULL arena) --- +// --- scalar_clone: deep copy into host_malloc-backed owned storage --- // -// scalar_clone(NULL, ...) allocates string bytes via host_malloc; caller -// releases with scalar_free. Non-string kinds pass through as value copies -// and do not need scalar_free (it's safe to call but a no-op). +// scalar_clone allocates string bytes via host_malloc; the caller releases +// with scalar_free. Non-string kinds pass through as value copies and do not +// need scalar_free (it's safe to call but a no-op). -TEST(host_clone_null_passes_through) { - Scalar c = scalar_clone(NULL, scalar_null()); +TEST(clone_null_passes_through) { + Scalar c = scalar_clone(scalar_null()); ASSERT(scalar_eq(c, scalar_null()) == true); scalar_free(c); // no-op, safe } -TEST(host_clone_bool_passes_through) { - Scalar t = scalar_clone(NULL, scalar_bool(true)); - Scalar f = scalar_clone(NULL, scalar_bool(false)); +TEST(clone_bool_passes_through) { + Scalar t = scalar_clone(scalar_bool(true)); + Scalar f = scalar_clone(scalar_bool(false)); ASSERT(scalar_eq(t, scalar_bool(true)) == true); ASSERT(scalar_eq(f, scalar_bool(false)) == true); scalar_free(t); scalar_free(f); } -TEST(host_clone_int_passes_through) { - Scalar c = scalar_clone(NULL, scalar_int(42)); +TEST(clone_int_passes_through) { + Scalar c = scalar_clone(scalar_int(42)); ASSERT(scalar_eq(c, scalar_int(42)) == true); scalar_free(c); } // Source buffer mutated after clone — clone bytes must be independent. -TEST(host_clone_string_owns_bytes) { +TEST(clone_string_owns_bytes) { uint8_t src[5]; memcpy(src, "hello", 5); - Scalar c = scalar_clone(NULL, scalar_string(src, 5)); + Scalar c = scalar_clone(scalar_string(src, 5)); src[0] = 'X'; src[1] = 'X'; ASSERT(scalar_eq(c, scalar_string((const uint8_t *)"hello", 5)) == true); @@ -246,24 +179,25 @@ TEST(host_clone_string_owns_bytes) { } // Source buffer freed entirely — clone still readable. -TEST(host_clone_string_survives_source_free) { +TEST(clone_string_survives_source_free) { uint8_t *src = (uint8_t *)malloc(5); memcpy(src, "hello", 5); - Scalar c = scalar_clone(NULL, scalar_string(src, 5)); + Scalar c = scalar_clone(scalar_string(src, 5)); free(src); ASSERT(scalar_eq(c, scalar_string((const uint8_t *)"hello", 5)) == true); scalar_free(c); } -TEST(host_clone_empty_string_handled) { - Scalar c = scalar_clone(NULL, scalar_string((const uint8_t *)"", 0)); +TEST(clone_empty_string_handled) { + Scalar c = scalar_clone(scalar_string((const uint8_t *)"", 0)); ASSERT(scalar_eq(c, scalar_string((const uint8_t *)"", 0)) == true); scalar_free(c); // safe on empty (no allocation made) } -TEST(host_clone_string_with_embedded_nul) { +// Bytes with embedded NUL must round-trip byte-for-byte. +TEST(clone_string_with_embedded_nul) { uint8_t src[4] = {0x01, 0x00, 0x02, 0x03}; - Scalar c = scalar_clone(NULL, scalar_string(src, sizeof src)); + Scalar c = scalar_clone(scalar_string(src, sizeof src)); ASSERT(scalar_eq(c, scalar_string(src, sizeof src)) == true); scalar_free(c); } @@ -310,18 +244,10 @@ int main(void) { RUN(clone_null_passes_through); RUN(clone_bool_passes_through); RUN(clone_int_passes_through); - RUN(clone_string_owns_bytes_in_dst_arena); - RUN(clone_string_survives_after_source_buffer_freed); + RUN(clone_string_owns_bytes); + RUN(clone_string_survives_source_free); RUN(clone_empty_string_handled); RUN(clone_string_with_embedded_nul); - - RUN(host_clone_null_passes_through); - RUN(host_clone_bool_passes_through); - RUN(host_clone_int_passes_through); - RUN(host_clone_string_owns_bytes); - RUN(host_clone_string_survives_source_free); - RUN(host_clone_empty_string_handled); - RUN(host_clone_string_with_embedded_nul); RUN(scalar_free_noop_on_non_string_kinds); TEST_SUMMARY(); From 22f26aa71aebfb51103eef4d5d110f2dab9dd1c1 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sat, 27 Jun 2026 22:44:47 -0300 Subject: [PATCH 07/12] fix(map): don't release uninitialized slot values map_set released entry->value before it was initialized for brand-new entries (and tombstones), and map_clone left a cloned tombstone's value uninitialized. With glibc's chunk recycling a fresh Entry can carry bytes that look like a live ELEMENT_COUNTER whose refcount reads 0, so the spurious release aborted ("counter_release: refcount already zero") on Linux while staying silent on macOS. Release a displaced value only when overwriting an existing live slot (present && !is_tombstone), and seed every tombstone value with a null-scalar sentinel so no path ever releases an uninitialized handle. --- map.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/map.c b/map.c index c8451a1..c96b08b 100644 --- a/map.c +++ b/map.c @@ -88,8 +88,13 @@ void map_set(Map *map, const void *key, size_t key_len, Element value, break; } - element_displace(entry->value); - element_release(entry->value); + // Drop the displaced value only when overwriting an existing live + // slot. A brand-new entry has no prior value, and a tombstone's value + // field is not a live handle — neither holds a ref to release. + if (present && !entry->is_tombstone) { + element_displace(entry->value); + element_release(entry->value); + } entry->value = value; entry->stamp = stamp; entry->is_tombstone = false; @@ -102,9 +107,13 @@ void map_delete(Map *map, const void *key, size_t key_len, Stamp stamp) { if (present) { if (stamp_gt(stamp, entry->stamp)) { entry->stamp = stamp; + // Drop the live composite (if any) before tombstoning. A + // re-delete over an existing tombstone has no live handle. + if (!entry->is_tombstone) { + element_displace(entry->value); + element_release(entry->value); + } entry->is_tombstone = true; - element_displace(entry->value); - element_release(entry->value); } } else { // Install a tombstone for the absent key, so that future merges can @@ -117,6 +126,9 @@ void map_delete(Map *map, const void *key, size_t key_len, Stamp stamp) { } entry->stamp = stamp; entry->is_tombstone = true; + // A tombstone holds no live composite; give value a safe sentinel so + // nothing ever releases an uninitialized handle. + entry->value = element_scalar(scalar_null()); HashTableInsertResult r = hashtable_insert(map->entries, key, key_len, entry); if (r != HASHTABLE_OK) { @@ -293,6 +305,8 @@ Map *map_clone(const Map *map) { copy->is_tombstone = entry->is_tombstone; if (!entry->is_tombstone) { copy->value = element_clone(entry->value); + } else { + copy->value = element_scalar(scalar_null()); } HashTableInsertResult r = hashtable_insert(clone->entries, k, klen, copy); From 05c62b4318aec497b4ba71bdbc65d7c9aec6794b Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sun, 28 Jun 2026 18:16:31 -0300 Subject: [PATCH 08/12] fix(element,map): plug refcount and scalar-string leaks element_release was a no-op for SCALAR, so every scalar string stored in a Map slot (via scalar_clone) leaked when the slot was overwritten or the Map freed. Make it scalar_free the value (valid because slots only ever hold owned copies). In map_merge, the mismatched-id LWW path overwrote dst's composite without releasing the loser, and the clone+set fallthrough never dropped the element_clone create-ref that map_set's own acquire duplicates. Both now release. Suite is leak-free under macOS `leaks`. --- element.c | 5 ++++- map.c | 9 +++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/element.c b/element.c index 9021f33..c906d44 100644 --- a/element.c +++ b/element.c @@ -122,7 +122,10 @@ void element_acquire(Element e) { void element_release(Element e) { switch (e.kind) { case ELEMENT_SCALAR: - // No-op: scalar elements have no refcount. + // Scalars have no refcount, but an owned (scalar_clone'd) string holds + // host_malloc'd bytes that must be freed. Valid only on owned scalars + // — slots always store owned copies. + scalar_free(e.as.scalar); break; case ELEMENT_REGISTER: register_release(e.as.reg); diff --git a/map.c b/map.c index c96b08b..517a65a 100644 --- a/map.c +++ b/map.c @@ -158,9 +158,11 @@ void map_merge(Map *dst, const Map *src) { de->value.kind != ELEMENT_SCALAR) { if (!elementid_eq(element_id(de->value), element_id(se->value))) { // Distinct logical elements at the same slot. LWW the - // slot; if src wins, replace dst's composite with a - // clone of src's. Loser is orphaned. + // slot; if src wins, replace dst's composite with a clone of + // src's and displace + release the loser. if (stamp_gt(se->stamp, de->stamp)) { + element_displace(de->value); + element_release(de->value); de->value = element_clone(se->value); de->stamp = se->stamp; } @@ -189,8 +191,11 @@ void map_merge(Map *dst, const Map *src) { continue; } + // map_set acquires the slot's own ref (and deep-copies a scalar), so + // drop this clone's create-ref afterward — the slot is the sole owner. Element cloned = element_clone(se->value); map_set(dst, k, klen, cloned, se->stamp); + element_release(cloned); } } From 42d6b841328b01f6480d321a276479d64546fb7f Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sun, 28 Jun 2026 18:16:31 -0300 Subject: [PATCH 09/12] test: release/destroy handles so counter and hashtable suites are leak-free The early counter tests and the hashtable tests created handles via the fresh() helpers without tearing them down. Add counter_release / hashtable_destroy at the end of each so the whole suite reports zero leaks (gating-ready for a Linux ASan+LSan CI job). --- test_counter.c | 49 ++++++++++++++++++++++++++++++------------------ test_hashtable.c | 20 ++++++++++++++++++++ 2 files changed, 51 insertions(+), 18 deletions(-) diff --git a/test_counter.c b/test_counter.c index f518404..c7513b0 100644 --- a/test_counter.c +++ b/test_counter.c @@ -35,55 +35,54 @@ TEST(counter_create_stores_id) { // --- local operations (single replica) --- TEST(empty_reads_zero) { - Counter *c = fresh(); ASSERT_EQ(counter_read(c), 0); + counter_release(c); } TEST(single_inc) { - Counter *c = fresh(); counter_inc(c, cid(1), 5); ASSERT_EQ(counter_read(c), 5); + counter_release(c); } TEST(inc_then_dec_nets) { - Counter *c = fresh(); counter_inc(c, cid(1), 5); counter_dec(c, cid(1), 2); ASSERT_EQ(counter_read(c), 3); + counter_release(c); } // Local repeated ops on the same client accumulate (this is NOT max). TEST(local_inc_accumulates) { - Counter *c = fresh(); counter_inc(c, cid(1), 5); counter_inc(c, cid(1), 2); ASSERT_EQ(counter_read(c), 7); + counter_release(c); } TEST(read_can_go_negative) { - Counter *c = fresh(); counter_dec(c, cid(1), 3); ASSERT_EQ(counter_read(c), -3); + counter_release(c); } TEST(two_clients_sum_in_one_replica) { - Counter *c = fresh(); counter_inc(c, cid(1), 5); counter_inc(c, cid(2), 3); counter_dec(c, cid(2), 1); ASSERT_EQ(counter_read(c), 7); // (5-0) + (3-1) + counter_release(c); } // Two clients are distinguished by the FULL 16-byte ClientId, not the first // byte (proves the hashtable key uses sizeof(ClientId) and not just a prefix). TEST(client_ids_distinguished_by_full_bytes) { - Counter *c = fresh(); uint8_t a_bytes[16] = {1, 2, 3, 4, 5, 6, 7, 8, @@ -96,12 +95,12 @@ TEST(client_ids_distinguished_by_full_bytes) { counter_inc(c, a, 5); counter_inc(c, b, 3); ASSERT_EQ(counter_read(c), 8); // distinct clients -> two entries + counter_release(c); } // --- merge (two replicas) --- TEST(merge_disjoint_clients_unions) { - Counter *a = fresh(); Counter *b = fresh(); @@ -110,12 +109,13 @@ TEST(merge_disjoint_clients_unions) { counter_merge(a, b); ASSERT_EQ(counter_read(a), 8); + counter_release(a); + counter_release(b); } // Classic CRDT result: concurrent increments on different clients converge, // both replicas read the same value after exchanging state. TEST(concurrent_inc_converges) { - Counter *a = fresh(); Counter *b = fresh(); @@ -127,12 +127,13 @@ TEST(concurrent_inc_converges) { ASSERT_EQ(counter_read(a), 8); ASSERT_EQ(counter_read(b), 8); + counter_release(a); + counter_release(b); } // Same client seen with different counts on two replicas: merge takes the MAX, // not the sum (the lower replica was simply behind). TEST(merge_same_client_takes_max_not_sum) { - Counter *a = fresh(); Counter *b = fresh(); @@ -141,10 +142,11 @@ TEST(merge_same_client_takes_max_not_sum) { counter_merge(a, b); ASSERT_EQ(counter_read(a), 5); // max(5,3), NOT 8 + counter_release(a); + counter_release(b); } TEST(merge_same_client_max_on_both_directions) { - Counter *a = fresh(); Counter *b = fresh(); @@ -158,10 +160,11 @@ TEST(merge_same_client_max_on_both_directions) { counter_merge(a, b); // max(inc)=10, max(dec)=6 -> 10 - 6 = 4 ASSERT_EQ(counter_read(a), 4); + counter_release(a); + counter_release(b); } TEST(merge_idempotent) { - Counter *a = fresh(); Counter *b = fresh(); @@ -175,11 +178,12 @@ TEST(merge_idempotent) { ASSERT_EQ(once, twice); ASSERT_EQ(twice, 8); + counter_release(a); + counter_release(b); } TEST(merge_commutative) { // (a <- b) - Counter *a1 = fresh(); Counter *b1 = fresh(); counter_inc(a1, cid(1), 5); @@ -188,7 +192,6 @@ TEST(merge_commutative) { counter_merge(a1, b1); // (b <- a) - Counter *a2 = fresh(); Counter *b2 = fresh(); counter_inc(a2, cid(1), 5); @@ -197,10 +200,13 @@ TEST(merge_commutative) { counter_merge(b2, a2); ASSERT_EQ(counter_read(a1), counter_read(b2)); + counter_release(a1); + counter_release(b1); + counter_release(a2); + counter_release(b2); } TEST(merge_associative) { - Counter *a = fresh(); Counter *b = fresh(); Counter *c = fresh(); @@ -213,7 +219,6 @@ TEST(merge_associative) { counter_merge(a, c); // a <- (b <- c) (rebuild on a fresh accumulator) - Counter *a2 = fresh(); Counter *b2 = fresh(); Counter *c2 = fresh(); @@ -225,11 +230,16 @@ TEST(merge_associative) { ASSERT_EQ(counter_read(a), counter_read(a2)); ASSERT_EQ(counter_read(a), 10); + counter_release(a); + counter_release(b); + counter_release(c); + counter_release(a2); + counter_release(b2); + counter_release(c2); } // Merge leaves the source untouched. TEST(merge_does_not_mutate_src) { - Counter *a = fresh(); Counter *b = fresh(); @@ -238,12 +248,13 @@ TEST(merge_does_not_mutate_src) { counter_merge(a, b); ASSERT_EQ(counter_read(b), 3); // b unchanged + counter_release(a); + counter_release(b); } // After merge, a subsequent local inc on a newly-learned client accumulates // from the merged-in value (not from zero). TEST(local_inc_after_merge_accumulates) { - Counter *a = fresh(); Counter *b = fresh(); @@ -252,6 +263,8 @@ TEST(local_inc_after_merge_accumulates) { counter_inc(a, cid(2), 4); // a now also acting as client 2: accumulate to 7 ASSERT_EQ(counter_read(a), 7); + counter_release(a); + counter_release(b); } // --- counter_clone: deep copy into a fresh refcount=1 allocation --- diff --git a/test_hashtable.c b/test_hashtable.c index df266df..e11952d 100644 --- a/test_hashtable.c +++ b/test_hashtable.c @@ -37,6 +37,7 @@ TEST(create_empty) { ASSERT(hashtable_get(t, &k, sizeof k, &out) == false); // out must be untouched on miss. ASSERT(out == (void *)0xdead); + hashtable_destroy(t); } // uint32 key, fetched via a separate variable holding the same value — proves @@ -53,6 +54,7 @@ TEST(insert_then_get) { ASSERT(hashtable_get(t, &k2, sizeof k2, &out) == true); ASSERT(out == &v); ASSERT_EQ(*(int *)out, 99); + hashtable_destroy(t); } TEST(insert_duplicate_rejected) { @@ -65,6 +67,7 @@ TEST(insert_duplicate_rejected) { void *out = NULL; ASSERT(hashtable_get(t, SK("k"), &out) == true); ASSERT(out == &a); + hashtable_destroy(t); } TEST(get_missing_returns_false) { @@ -75,6 +78,7 @@ TEST(get_missing_returns_false) { void *out = NULL; ASSERT(hashtable_get(t, SK("absent"), &out) == false); + hashtable_destroy(t); } // NULL is a storable value, distinguishable from "not found". @@ -89,6 +93,7 @@ TEST(stored_null_is_distinguishable) { out = (void *)0xbeef; ASSERT(hashtable_get(t, SK("other"), &out) == false); + hashtable_destroy(t); } TEST(update_existing) { @@ -101,6 +106,7 @@ TEST(update_existing) { void *out = NULL; ASSERT(hashtable_get(t, SK("k"), &out) == true); ASSERT(out == &b); + hashtable_destroy(t); } TEST(update_missing_rejected) { @@ -109,6 +115,7 @@ TEST(update_missing_rejected) { int b = 2; ASSERT_EQ(hashtable_update(t, SK("ghost"), &b), HASHTABLE_UPDATE_ERR_NOT_FOUND); + hashtable_destroy(t); } TEST(upsert_inserts_when_absent) { @@ -120,6 +127,7 @@ TEST(upsert_inserts_when_absent) { void *out = NULL; ASSERT(hashtable_get(t, SK("k"), &out) == true); ASSERT(out == &v); + hashtable_destroy(t); } TEST(upsert_updates_when_present) { @@ -132,6 +140,7 @@ TEST(upsert_updates_when_present) { void *out = NULL; ASSERT(hashtable_get(t, SK("k"), &out) == true); ASSERT(out == &b); + hashtable_destroy(t); } TEST(remove_existing) { @@ -143,12 +152,14 @@ TEST(remove_existing) { void *out = NULL; ASSERT(hashtable_get(t, SK("k"), &out) == false); + hashtable_destroy(t); } TEST(remove_missing_rejected) { HashTable *t = fresh(); ASSERT_EQ(hashtable_remove(t, SK("nope")), HASHTABLE_REMOVE_ERR_NOT_FOUND); + hashtable_destroy(t); } TEST(remove_then_reinsert) { @@ -162,6 +173,7 @@ TEST(remove_then_reinsert) { void *out = NULL; ASSERT(hashtable_get(t, SK("k"), &out) == true); ASSERT(out == &b); + hashtable_destroy(t); } // Table must copy the key bytes: mutating the caller's buffer after insert @@ -185,6 +197,7 @@ TEST(key_is_copied_not_borrowed) { uint8_t mutated[4] = {9, 9, 3, 4}; out = NULL; ASSERT(hashtable_get(t, mutated, sizeof mutated, &out) == false); + hashtable_destroy(t); } // The headline reason for byte keys: keys with embedded NUL bytes must be @@ -209,6 +222,7 @@ TEST(embedded_nul_keys_distinct) { ASSERT(out == &vb); ASSERT(hashtable_get(t, k3, sizeof k3, &out) == true); ASSERT(out == &vc); + hashtable_destroy(t); } // Same prefix, different length: must be distinct keys. @@ -227,6 +241,7 @@ TEST(length_distinguishes_keys) { ASSERT(out == &va); ASSERT(hashtable_get(t, b, sizeof b, &out) == true); ASSERT(out == &vb); + hashtable_destroy(t); } TEST(collisions_resolve) { @@ -247,6 +262,7 @@ TEST(collisions_resolve) { ASSERT(out == &vc); ASSERT(hashtable_get(t, SK("delta"), &out) == true); ASSERT(out == &vd); + hashtable_destroy(t); } // Insert many more than initial size to force at least one grow/rehash. @@ -269,6 +285,7 @@ TEST(grow_preserves_entries) { ASSERT(hashtable_get(t, &keys[i], sizeof keys[i], &out) == true); ASSERT(out == &vals[i]); } + hashtable_destroy(t); } TEST(iter_empty_yields_nothing) { @@ -279,6 +296,7 @@ TEST(iter_empty_yields_nothing) { size_t klen = 0; void *v = NULL; ASSERT(hashtable_iter_next(&it, &k, &klen, &v) == false); + hashtable_destroy(t); } // Iteration must visit every entry exactly once (order unspecified), yielding @@ -321,6 +339,7 @@ TEST(iter_visits_all_once) { ASSERT_EQ(seen1, 1); ASSERT_EQ(seen2, 1); ASSERT_EQ(seen3, 1); + hashtable_destroy(t); } TEST(clear_empties_table) { @@ -346,6 +365,7 @@ TEST(clear_empties_table) { ASSERT_EQ(hashtable_insert(t, SK("c"), &vc), HASHTABLE_OK); ASSERT(hashtable_get(t, SK("c"), &out) == true); ASSERT(out == &vc); + hashtable_destroy(t); } // --- hashtable_destroy --- From f83a23bea66745ce41f2d255756ff6b01f586ac8 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sun, 28 Jun 2026 18:19:25 -0300 Subject: [PATCH 10/12] ci: add Linux ASan + UBSan + LSan job Second job builds the suite with -fsanitize=address,undefined and runs it; LeakSanitizer (bundled into ASan on Linux) gates memory leaks, which macOS ASan cannot catch. -fno-sanitize-recover=all makes UBSan findings fail the build. --- .github/workflows/ci.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e8664c9..bc2cbed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,3 +19,20 @@ jobs: - name: Build and run tests run: make test + + sanitize: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + + - name: Install toolchain + run: sudo apt-get update && sudo apt-get install -y clang make + + # AddressSanitizer + UndefinedBehaviorSanitizer, with LeakSanitizer + # (bundled into ASan on Linux, on by default) gating memory leaks. + # -fno-sanitize-recover=all makes UBSan findings fail the build. + - name: Build and run tests (ASan + UBSan + LSan) + run: make test CFLAGS="-Wall -Wextra -g -O1 -fno-omit-frame-pointer -fsanitize=address,undefined -fno-sanitize-recover=all" + env: + ASAN_OPTIONS: detect_leaks=1:abort_on_error=1 + UBSAN_OPTIONS: print_stacktrace=1 From c5a006efafc9e7ea1513d3c7b6187a18d023d93f Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sun, 28 Jun 2026 18:24:30 -0300 Subject: [PATCH 11/12] =?UTF-8?q?fix(map):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20release=20underflow=20guard,=20const,=20doc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - map_release aborts on refcount-zero before decrementing (mirrors counter_release/register_release) instead of wrapping size_t to SIZE_MAX - map_is_displaced takes const Map * for consistency with the other *_is_displaced and to allow const callers - register.h: drop stale scalar_clone(NULL, ...) wording (now scalar_clone) --- map.c | 5 ++++- map.h | 2 +- register.h | 6 +++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/map.c b/map.c index 517a65a..c580f89 100644 --- a/map.c +++ b/map.c @@ -325,6 +325,9 @@ Map *map_clone(const Map *map) { void map_acquire(Map *map) { map->refcount++; } void map_release(Map *map) { + if (map->refcount == 0) { + host_abort("map_release: refcount already zero"); + } if (--map->refcount == 0) { HashTableIter it = hashtable_iter(map->entries); const void *k; @@ -343,4 +346,4 @@ void map_release(Map *map) { } void map_displace(Map *map) { map->displaced = true; } -bool map_is_displaced(Map *map) { return map->displaced; } +bool map_is_displaced(const Map *map) { return map->displaced; } diff --git a/map.h b/map.h index 62bd7f1..09e2821 100644 --- a/map.h +++ b/map.h @@ -107,6 +107,6 @@ void map_release(Map *map); // slot path calls this when it LWW-displaces this Map). Independent of // refcount; see the lifetime notes above. void map_displace(Map *map); -bool map_is_displaced(Map *map); +bool map_is_displaced(const Map *map); #endif // _CRDT_MAP_H diff --git a/register.h b/register.h index a1d4008..41658ef 100644 --- a/register.h +++ b/register.h @@ -25,9 +25,9 @@ // / _INT the payload is in the struct. // - For SCALAR_STRING, the Scalar carries a borrowed (bytes, len) view. // Register owns its value: on every accepted write (create, set, winning -// merge) it deep-copies the bytes via scalar_clone(NULL, ...) into a -// host_malloc allocation, and frees the previous value's bytes. No leak -// across overwrites. +// merge) it deep-copies the bytes via scalar_clone into a host_malloc +// allocation, and frees the previous value's bytes. No leak across +// overwrites. // - register_read returns a Scalar by value; for SCALAR_STRING the bytes // pointer is a borrowed view into the Register's own storage. Valid until // the next accepted write or until the Register is freed (refcount 0). From ec20fe55957e8639faf3edcd00e4161bc59637e6 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Sun, 28 Jun 2026 18:44:32 -0300 Subject: [PATCH 12/12] fix(map): don't displace a re-set of the same composite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit map_set unconditionally displaced+released the prior slot value on an accepted overwrite. When the incoming value is the exact composite already installed, that flagged a still-installed handle as displaced (and churned its refcount), which would make the Doc layer treat a live handle as orphaned. Skip acquire/displace/release for the same-pointer case and just advance the stamp. Also document in element.h that element_release on a SCALAR frees the string bytes, so it is valid only on owned scalars — never on a borrowed scalar or a SCALAR returned by map_get. --- element.h | 7 +++++++ map.c | 11 +++++++++++ test_map.c | 21 +++++++++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/element.h b/element.h index 4d88433..2ab8ccf 100644 --- a/element.h +++ b/element.h @@ -21,6 +21,13 @@ // _release / _displace / _is_displaced forward to the underlying composite // (SCALAR is a no-op for acquire/displace and scalar_free on release). Callers // are responsible for keeping pointed-to composites alive via the refcount. +// +// Sharp edge: element_release on a SCALAR frees the value's string bytes +// (scalar_free), so it is valid ONLY on an OWNED scalar — one produced by +// element_clone, or stored in a container that owns its copy (e.g. a Map slot, +// which clones on set). Do NOT call it on a borrowed-buffer scalar such as +// element_scalar(scalar_string(...)) or on a SCALAR Element returned by +// map_get — that would free memory still owned by the caller or the Map. #include "counter.h" #include "elementid.h" diff --git a/map.c b/map.c index c580f89..7905768 100644 --- a/map.c +++ b/map.c @@ -58,6 +58,17 @@ void map_set(Map *map, const void *key, size_t key_len, Element value, bool present = hashtable_get(map->entries, key, key_len, (void **)&entry); bool update = !present || stamp_gt(stamp, entry->stamp); if (update) { + // Re-setting the exact composite already installed in the slot: just + // advance the stamp. Acquiring then displacing + releasing it would + // spuriously flag a still-installed handle as displaced (and churn its + // refcount). The union aliases all composite pointers, so comparing + // as.counter compares the stored pointer regardless of kind. + if (present && !entry->is_tombstone && value.kind != ELEMENT_SCALAR && + value.kind == entry->value.kind && + value.as.counter == entry->value.as.counter) { + entry->stamp = stamp; + return; + } element_acquire(value); if (!present) { entry = host_malloc(sizeof(Entry)); diff --git a/test_map.c b/test_map.c index f7ba8e2..5a89528 100644 --- a/test_map.c +++ b/test_map.c @@ -749,6 +749,26 @@ TEST(delete_composite_displaces_it) { map_release(m); } +// Re-setting the exact composite already in the slot (newer stamp) must NOT +// mark it displaced — it is still installed — and must not churn its refcount. +TEST(set_same_composite_newer_stamp_keeps_it_live) { + Map *m = fresh(); + Counter *c = counter_create(default_id()); + counter_inc(c, cid(1), 5); + map_set(m, SK("votes"), element_counter(c), stmp(1, 1)); + counter_release(c); // slot owns the sole ref + + // Same pointer, higher stamp. + map_set(m, SK("votes"), element_counter(c), stmp(5, 1)); + + ASSERT(counter_is_displaced(c) == false); // still installed, not orphaned + Element out; + ASSERT(map_get(m, SK("votes"), &out) == true); + ASSERT(out.as.counter == c); + ASSERT_EQ(counter_read(out.as.counter), 5); + map_release(m); // frees c exactly once (refcount stayed 1) +} + // --- cross-replica composite LWW: clone winner, displace+release loser --- TEST(merge_composite_src_wins_into_empty_slot_clones) { @@ -1344,6 +1364,7 @@ int main(void) { RUN(set_different_kind_composite_displaces_at_lww); RUN(evicted_composite_is_displaced_and_outlives_via_held_ref); RUN(delete_composite_displaces_it); + RUN(set_same_composite_newer_stamp_keeps_it_live); RUN(merge_composite_src_wins_into_empty_slot_clones); RUN(merge_does_not_clone_when_src_loses_lww);