From 462c7d0af1c75b11acd3b277d4a8a00aaaa838d6 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Wed, 3 Jun 2026 20:45:12 -0300 Subject: [PATCH 1/3] refactor: drop ElementId; identity = position in tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eliminates the ElementId field on Counter / Register / Map. Identity for recursive merge comes from the slot's (key, kind) tuple in the containing Map. Removes the elementid module entirely and the dedicated death test binary that exercised the cross-arena composite abort. - map_merge's recursive guard simplifies to (same kind, both alive) — no id comparison. - map_merge's LWW composite path now element_clones the winner into the dst arena instead of host_abort'ing. Cross-arena composite displacement is supported and observable. - map_set tolerates type-flip at a key via LWW (composite displaces scalar, scalar displaces composite, different-kind composite displaces). - New get-or-create helpers: map_counter / map_register / map_map. Each derives the composite at the given key (same kind → return existing pointer; different kind / empty / tombstone → install fresh if stamp wins LWW; stamp loses → return detached composite). - Deep-copy clones land for all primitives: scalar_clone (renamed from scalar_dup), register_clone, counter_clone, map_clone, and the element_clone dispatcher. - map_clone skips element_clone on tombstones — stale value field would otherwise read as undefined; covered by a new regression that probes arena_used after cloning a deleted big-subtree slot. - map_clone now checks hashtable_insert; was unchecked. - counter_inc / counter_dec factored through a counter_entry_for helper removing ~50 lines of duplication. - scalar_eq's string branch uses memcmp instead of a hand-rolled loop. Drops: elementid.c/h, test_elementid.c, test_map_abort.c, counter_id / register_id / map_id accessors, ElementId arg from all *_create. --- .gitignore | 2 - Makefile | 30 +-- counter.c | 92 ++++--- counter.h | 7 +- element.c | 29 ++ element.h | 1 + elementid.c | 31 --- elementid.h | 31 --- map.c | 138 ++++++---- map.h | 17 +- register.c | 45 ++-- register.h | 8 +- scalar.c | 14 +- scalar.h | 4 +- test_counter.c | 82 ++++-- test_element.c | 154 ++++++++--- test_elementid.c | 178 ------------- test_map.c | 679 ++++++++++++++++++++++++++++++++++++----------- test_map_abort.c | 45 ---- test_register.c | 78 ++++-- test_scalar.c | 77 ++++++ 21 files changed, 1063 insertions(+), 679 deletions(-) delete mode 100644 elementid.c delete mode 100644 elementid.h delete mode 100644 test_elementid.c delete mode 100644 test_map_abort.c diff --git a/.gitignore b/.gitignore index 756daed..28c2d7c 100644 --- a/.gitignore +++ b/.gitignore @@ -24,8 +24,6 @@ a.out /test_clientid /test_stamp /test_map -/test_map_abort -/test_elementid /test_element # Tooling diff --git a/Makefile b/Makefile index f0ba96b..9970446 100644 --- a/Makefile +++ b/Makefile @@ -37,8 +37,8 @@ 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 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 host_posix.c counter.c test_counter.c +test-counter: arena.c string.c hashtable.c clientid.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 host_posix.c counter.c test_counter.c ./test_counter .PHONY: test-scalar @@ -47,25 +47,18 @@ test-scalar: arena.c string.c host_posix.c scalar.c test_scalar.c test_util.h ./test_scalar .PHONY: test-register -test-register: arena.c string.c clientid.c elementid.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 host_posix.c stamp.c scalar.c register.c test_register.c +test-register: arena.c string.c clientid.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 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 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 host_posix.c stamp.c scalar.c register.c counter.c element.c map.c test_map.c +test-map: arena.c string.c hashtable.c clientid.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 host_posix.c stamp.c scalar.c register.c counter.c element.c map.c test_map.c ./test_map -# Death test: binary must abort. Exit status is inverted so success means -# the process died via host_abort. -.PHONY: test-map-abort -test-map-abort: arena.c string.c hashtable.c clientid.c elementid.c host_posix.c stamp.c scalar.c register.c counter.c element.c map.c test_map_abort.c - $(CC) $(CFLAGS) -o test_map_abort arena.c string.c hashtable.c clientid.c elementid.c host_posix.c stamp.c scalar.c register.c counter.c element.c map.c test_map_abort.c - ! ./test_map_abort 2>/dev/null - .PHONY: test-element -test-element: arena.c string.c hashtable.c clientid.c elementid.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 host_posix.c stamp.c scalar.c register.c counter.c map.c element.c test_element.c +test-element: arena.c string.c hashtable.c clientid.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 host_posix.c stamp.c scalar.c register.c counter.c map.c element.c test_element.c ./test_element .PHONY: test-clientid @@ -78,10 +71,5 @@ test-stamp: string.c clientid.c host_posix.c stamp.c test_stamp.c test_util.h $(CC) $(CFLAGS) -o test_stamp string.c clientid.c host_posix.c stamp.c test_stamp.c ./test_stamp -.PHONY: test-elementid -test-elementid: string.c clientid.c host_posix.c elementid.c test_elementid.c test_util.h - $(CC) $(CFLAGS) -o test_elementid string.c clientid.c host_posix.c elementid.c test_elementid.c - ./test_elementid - .PHONY: test -test: test-arena test-hashtable test-string test-counter test-scalar test-register test-clientid test-stamp test-map test-map-abort test-elementid test-element +test: test-arena test-hashtable test-string test-counter test-scalar test-register test-clientid test-stamp test-map test-element diff --git a/counter.c b/counter.c index a6eafea..21829bb 100644 --- a/counter.c +++ b/counter.c @@ -4,7 +4,6 @@ #include "host.h" struct Counter { - ElementId id; Arena *arena; HashTable *entries; // client_id (uint32_t) -> CounterEntry }; @@ -17,23 +16,46 @@ static inline uint32_t max_u32(uint32_t a, uint32_t b) { return b; } -Counter *counter_create(Arena *arena, ElementId id) { +Counter *counter_create(Arena *arena) { Counter *counter = arena_alloc(arena, sizeof(Counter)); if (!counter) { host_abortf( "counter_create: arena OOM (requested %zu bytes for Counter)", sizeof(Counter)); } - counter->id = id; counter->arena = arena; counter->entries = hashtable_create(arena); if (!counter->entries) { - host_abort("counter_create: hashtable_create OOM"); + host_abort("counter_create: hashtable_create OOM (per-client tallies " + "table)"); } return counter; } -ElementId counter_id(const Counter *counter) { return counter->id; } +// Get-or-create the per-client CounterEntry for `client_id`. Initializes a +// fresh entry to {inc=0, dec=0} on first call for a given client. +static CounterEntry *counter_entry_for(Counter *counter, ClientId client_id) { + void *entry_ptr; + if (hashtable_get(counter->entries, &client_id, sizeof(client_id), + &entry_ptr)) { + return entry_ptr; + } + CounterEntry *entry = arena_alloc(counter->arena, sizeof *entry); + if (!entry) { + host_abortf("counter: arena OOM (requested %zu bytes for CounterEntry)", + sizeof *entry); + } + entry->client_id = client_id; + entry->inc = 0; + entry->dec = 0; + HashTableInsertResult r = hashtable_insert(counter->entries, &client_id, + sizeof(client_id), entry); + if (r != HASHTABLE_OK) { + host_abortf("counter: hashtable_insert -> %s", + hashtable_insert_result_name(r)); + } + return entry; +} int64_t counter_read(const Counter *counter) { int64_t total = 0; @@ -80,51 +102,35 @@ void counter_merge(Counter *dst, const Counter *src) { } void counter_inc(Counter *counter, ClientId client_id, uint32_t amount) { - void *entry_ptr; - if (hashtable_get(counter->entries, &client_id, sizeof(client_id), - &entry_ptr)) { - CounterEntry *entry = entry_ptr; - entry->inc += amount; - } else { - CounterEntry *entry = arena_alloc(counter->arena, sizeof *entry); - if (!entry) { - host_abortf("counter_inc: arena OOM (requested %zu bytes for " - "CounterEntry)", - sizeof *entry); - } - entry->client_id = client_id; - entry->inc = amount; - entry->dec = 0; - HashTableInsertResult r = hashtable_insert(counter->entries, &client_id, - sizeof(client_id), entry); - if (r != HASHTABLE_OK) { - host_abortf("counter_inc: hashtable_insert -> %s", - hashtable_insert_result_name(r)); - } - } + counter_entry_for(counter, client_id)->inc += amount; } void counter_dec(Counter *counter, ClientId client_id, uint32_t amount) { - void *entry_ptr; - if (hashtable_get(counter->entries, &client_id, sizeof(client_id), - &entry_ptr)) { - CounterEntry *entry = entry_ptr; - entry->dec += amount; - } else { - CounterEntry *entry = arena_alloc(counter->arena, sizeof *entry); - if (!entry) { - host_abortf("counter_dec: arena OOM (requested %zu bytes for " + counter_entry_for(counter, client_id)->dec += amount; +} + +Counter *counter_clone(Arena *arena, const Counter *counter) { + Counter *clone = counter_create(arena); + 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); + if (!entry_copy) { + host_abortf("counter_clone: arena OOM (requested %zu bytes for " "CounterEntry)", - sizeof *entry); + sizeof *entry_copy); } - entry->client_id = client_id; - entry->inc = 0; - entry->dec = amount; - HashTableInsertResult r = hashtable_insert(counter->entries, &client_id, - sizeof(client_id), entry); + *entry_copy = *entry; + HashTableInsertResult r = + hashtable_insert(clone->entries, key, key_len, entry_copy); if (r != HASHTABLE_OK) { - host_abortf("counter_dec: hashtable_insert -> %s", + host_abortf("counter_clone: hashtable_insert -> %s", hashtable_insert_result_name(r)); } } + return clone; } diff --git a/counter.h b/counter.h index 9a70504..a7968b5 100644 --- a/counter.h +++ b/counter.h @@ -25,7 +25,6 @@ #include "arena.h" #include "clientid.h" -#include "elementid.h" #include "hashtable.h" #include @@ -37,9 +36,7 @@ typedef struct CounterEntry { typedef struct Counter Counter; -Counter *counter_create(Arena *arena, ElementId id); - -ElementId counter_id(const Counter *counter); +Counter *counter_create(Arena *arena); int64_t counter_read(const Counter *counter); @@ -49,4 +46,6 @@ 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); + #endif // _CRDT_COUNTER_H diff --git a/element.c b/element.c index 22853ff..e181c4f 100644 --- a/element.c +++ b/element.c @@ -1,6 +1,9 @@ #include "element.h" +#include "counter.h" #include "host.h" #include "map.h" +#include "register.h" +#include "scalar.h" Element element_scalar(Scalar s) { Element e = {.kind = ELEMENT_SCALAR, .as.scalar = s}; @@ -58,3 +61,29 @@ void element_merge(Element dst, Element src) { break; } } + +Element element_clone(Arena *arena, Element e) { + Element result; + + switch (e.kind) { + case ELEMENT_SCALAR: { + Scalar cloned = scalar_clone(arena, e.as.scalar); + + result = element_scalar(cloned); + } break; + case ELEMENT_REGISTER: { + Register *reg = register_clone(arena, e.as.reg); + result = element_register(reg); + } break; + case ELEMENT_COUNTER: { + Counter *counter = counter_clone(arena, e.as.counter); + result = element_counter(counter); + } break; + case ELEMENT_MAP: { + Map *map = map_clone(arena, e.as.map); + result = element_map(map); + } break; + } + + return result; +} diff --git a/element.h b/element.h index 7207639..551c56b 100644 --- a/element.h +++ b/element.h @@ -54,5 +54,6 @@ 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); #endif // _CRDT_ELEMENT_H diff --git a/elementid.c b/elementid.c deleted file mode 100644 index 5498747..0000000 --- a/elementid.c +++ /dev/null @@ -1,31 +0,0 @@ -#include "elementid.h" - -ElementId elementid_new(ClientId origin, uint64_t seq) { - ElementId id; - id.origin = origin; - id.seq = seq; - return id; -} - -ElementId elementid_root(void) { - return elementid_new(clientid_from_bytes((uint8_t[16]){0}), 0); -} - -bool elementid_eq(ElementId a, ElementId b) { - return clientid_eq(a.origin, b.origin) && a.seq == b.seq; -} - -int elementid_cmp(ElementId a, ElementId b) { - int client_cmp = clientid_cmp(a.origin, b.origin); - if (client_cmp != 0) { - return client_cmp; - } - - if (a.seq < b.seq) { - return -1; - } else if (a.seq > b.seq) { - return 1; - } else { - return 0; - } -} diff --git a/elementid.h b/elementid.h deleted file mode 100644 index 81a3031..0000000 --- a/elementid.h +++ /dev/null @@ -1,31 +0,0 @@ -#ifndef _CRDT_ELEMENTID_H -#define _CRDT_ELEMENTID_H - -// ElementId: identity of a composite element (Register / Counter / Map), -// shared across replicas. Two replicas creating "the same logical element" -// must give it the same ElementId; that's the hook map_merge uses to know -// "these two slots are the same object, recurse" vs "these are different -// objects, LWW the slot". -// -// Shape: { ClientId origin, uint64 seq }. Pass by value (~24 bytes), like -// Stamp / ClientId / Scalar. Fields are public. -// -// elementid_new builds one from (origin, seq). elementid_root is a fixed -// sentinel for the top-level Map of a document; it does not collide with -// any id derived from a real ClientId. elementid_eq is the equality used -// by map_merge's recursive path; elementid_cmp gives a total order -// (origin first via clientid_cmp, then seq). - -#include "clientid.h" - -typedef struct ElementId { - ClientId origin; - uint64_t seq; -} ElementId; - -ElementId elementid_new(ClientId origin, uint64_t seq); -ElementId elementid_root(void); -bool elementid_eq(ElementId a, ElementId b); -int elementid_cmp(ElementId a, ElementId b); - -#endif // _CRDT_ELEMENTID_H diff --git a/map.c b/map.c index 28e1482..e9d38f8 100644 --- a/map.c +++ b/map.c @@ -11,28 +11,24 @@ typedef struct MapEntry { } Entry; struct Map { - ElementId id; Arena *arena; HashTable *entries; }; -Map *map_create(Arena *arena, ElementId id) { +Map *map_create(Arena *arena) { Map *map = arena_alloc(arena, 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); if (!map->entries) { - host_abort("map_create: hashtable_create OOM"); + host_abort("map_create: hashtable_create OOM (slot table)"); } return map; } -ElementId map_id(const Map *map) { return map->id; } - bool map_get(const Map *map, const void *key, size_t key_len, Element *out) { void *entry; bool present = hashtable_get(map->entries, key, key_len, &entry); @@ -72,7 +68,7 @@ void map_set(Map *map, const void *key, size_t key_len, Element value, switch (value.kind) { case ELEMENT_SCALAR: { - Scalar copy = scalar_dup(map->arena, value.as.scalar); + Scalar copy = scalar_clone(map->arena, value.as.scalar); value.as.scalar = copy; break; } @@ -133,34 +129,13 @@ void map_merge(Map *dst, const Map *src) { if (dst_has && !de->is_tombstone && !se->is_tombstone && de->value.kind == se->value.kind && de->value.kind != ELEMENT_SCALAR) { - ElementId did, sid; - switch (de->value.kind) { - case ELEMENT_SCALAR: - did = sid = elementid_root(); // unused, won't compare equal, - // assign to silence warning - break; - case ELEMENT_REGISTER: - did = register_id(de->value.as.reg); - sid = register_id(se->value.as.reg); - break; - case ELEMENT_COUNTER: - did = counter_id(de->value.as.counter); - sid = counter_id(se->value.as.counter); - break; - case ELEMENT_MAP: - did = map_id(de->value.as.map); - sid = map_id(se->value.as.map); - break; - } - if (elementid_eq(did, sid)) { - element_merge(de->value, se->value); - // Advance slot stamp to max(dst, src) so future slot-level - // ops on this key are LWW-deterministic across replicas. - if (stamp_gt(se->stamp, de->stamp)) { - de->stamp = se->stamp; - } - continue; + element_merge(de->value, se->value); + // Advance slot stamp to max(dst, src) so future slot-level + // ops on this key are LWW-deterministic across replicas. + if (stamp_gt(se->stamp, de->stamp)) { + de->stamp = se->stamp; } + continue; } // LWW fallthrough. @@ -169,19 +144,8 @@ void map_merge(Map *dst, const Map *src) { continue; } - // Reaching the LWW path with a composite means id derivation is - // broken or types diverged at this key — programmer error - // regardless of which side's stamp wins. Abort unconditionally so - // the failure is not stamp-dependent (which would let the same - // broken state crash one replica and silently pass on another). - if (se->value.kind != ELEMENT_SCALAR) { - host_abortf("map_merge: composite at LWW path — " - "src kind %s, dst id != src id (or dst empty / " - "tombstoned / different kind). " - "Use deterministic id derivation for composite slots.", - element_kind_name(se->value.kind)); - } - map_set(dst, k, klen, se->value, se->stamp); + Element cloned = element_clone(dst->arena, se->value); + map_set(dst, k, klen, cloned, se->stamp); } } @@ -200,3 +164,83 @@ size_t map_size(const Map *map) { return count; } + +Counter *map_counter(Map *map, const void *key, size_t key_len, Stamp stamp) { + Entry *existing; + bool present = + hashtable_get(map->entries, key, key_len, (void **)&existing); + if (present && !existing->is_tombstone && + existing->value.kind == ELEMENT_COUNTER) { + return existing->value.as.counter; + } + + Counter *fresh = counter_create(map->arena); + if (!present || stamp_gt(stamp, existing->stamp)) { + map_set(map, key, key_len, element_counter(fresh), stamp); + } + // Detached when LWW lost: returned anyway so caller always gets a usable + // handle. + return fresh; +} + +Register *map_register(Map *map, const void *key, size_t key_len, Scalar seed, + Stamp stamp) { + Entry *existing; + bool present = + hashtable_get(map->entries, key, key_len, (void **)&existing); + if (present && !existing->is_tombstone && + existing->value.kind == ELEMENT_REGISTER) { + return existing->value.as.reg; + } + + Register *fresh = register_create(map->arena, seed, stamp); + if (!present || stamp_gt(stamp, existing->stamp)) { + map_set(map, key, key_len, element_register(fresh), stamp); + } + return fresh; +} + +Map *map_map(Map *map, const void *key, size_t key_len, Stamp stamp) { + Entry *existing; + bool present = + hashtable_get(map->entries, key, key_len, (void **)&existing); + if (present && !existing->is_tombstone && + existing->value.kind == ELEMENT_MAP) { + return existing->value.as.map; + } + + Map *fresh = map_create(map->arena); + if (!present || stamp_gt(stamp, existing->stamp)) { + map_set(map, key, key_len, element_map(fresh), stamp); + } + return fresh; +} + +Map *map_clone(Arena *arena, const Map *map) { + Map *clone = map_create(arena); + 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)); + if (!copy) { + host_abortf("map_clone: arena OOM (requested %zu bytes for Entry)", + sizeof(Entry)); + } + + copy->stamp = entry->stamp; + copy->is_tombstone = entry->is_tombstone; + if (!entry->is_tombstone) { + copy->value = element_clone(clone->arena, entry->value); + } + HashTableInsertResult r = + hashtable_insert(clone->entries, k, klen, copy); + if (r != HASHTABLE_OK) { + host_abortf("map_clone: hashtable_insert -> %s", + hashtable_insert_result_name(r)); + } + } + return clone; +} diff --git a/map.h b/map.h index a31bc11..ff34baa 100644 --- a/map.h +++ b/map.h @@ -38,7 +38,6 @@ #include "arena.h" #include "element.h" -#include "elementid.h" #include "scalar.h" #include "stamp.h" #include @@ -46,9 +45,7 @@ typedef struct Map Map; -Map *map_create(Arena *arena, ElementId id); - -ElementId map_id(const Map *map); +Map *map_create(Arena *arena); // Returns true if the key has a live (non-tombstone) entry, in which case // *out is set. Returns false otherwise; *out is untouched. @@ -64,4 +61,16 @@ void map_merge(Map *dst, const Map *src); // Count of live (non-tombstone) entries. size_t map_size(const Map *map); +// Helper for "get or create" pattern: returns the Counter at key if present; +Counter *map_counter(Map *map, const void *key, size_t key_len, Stamp stamp); + +// Helper for "get or create" pattern: returns the Register at key if present; +Register *map_register(Map *map, const void *key, size_t key_len, Scalar seed, + Stamp stamp); + +// Helper for "get or create" pattern: returns the Map at key if present; +Map *map_map(Map *map, const void *key, size_t key_len, Stamp stamp); + +Map *map_clone(Arena *arena, const Map *map); + #endif // _CRDT_MAP_H diff --git a/register.c b/register.c index 388d40c..3fd19eb 100644 --- a/register.c +++ b/register.c @@ -6,60 +6,49 @@ #include struct Register { - ElementId id; Arena *arena; Scalar value; Stamp stamp; }; -Scalar accept_value(Arena *arena, Scalar value) { - switch (value.kind) { - case SCALAR_STRING: { - // Copy string bytes into arena. - uint8_t *dst = arena_alloc(arena, value.as.s.len); - if (!dst && value.as.s.len > 0) { - host_abortf("register: arena OOM copying %zu string bytes", - value.as.s.len); - } - memcpy(dst, value.as.s.bytes, value.as.s.len); - return scalar_string(dst, value.as.s.len); - } - case SCALAR_NULL: - case SCALAR_BOOL: - case SCALAR_INT: - return value; - } -} - -Register *register_create(Arena *arena, ElementId id, Scalar value, - Stamp stamp) { +Register *register_create(Arena *arena, Scalar value, Stamp stamp) { Register *reg = arena_alloc(arena, sizeof(Register)); if (!reg) { host_abortf( "register_create: arena OOM (requested %zu bytes for Register)", sizeof(Register)); } - reg->id = id; reg->arena = arena; - reg->value = accept_value(arena, value); + reg->value = scalar_clone(arena, value); reg->stamp = stamp; return reg; } -ElementId register_id(const Register *reg) { return reg->id; } - 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 = accept_value(reg->arena, value); + reg->value = scalar_clone(reg->arena, value); reg->stamp = stamp; } } void register_merge(Register *dst, const Register *src) { if (stamp_gt(src->stamp, dst->stamp)) { - dst->value = accept_value(dst->arena, src->value); + dst->value = scalar_clone(dst->arena, src->value); dst->stamp = src->stamp; } } + +Register *register_clone(Arena *arena, const Register *reg) { + Register *clone = arena_alloc(arena, sizeof(Register)); + if (!clone) { + host_abortf( + "register_clone: arena OOM (requested %zu bytes for Register)", + sizeof(Register)); + } + clone->arena = arena; + clone->value = scalar_clone(arena, reg->value); + clone->stamp = reg->stamp; + return clone; +} diff --git a/register.h b/register.h index e3b96ea..e8fd7eb 100644 --- a/register.h +++ b/register.h @@ -31,7 +31,6 @@ // Lifetime: Register must not outlive its arena. #include "arena.h" -#include "elementid.h" #include "scalar.h" #include "stamp.h" #include @@ -39,10 +38,7 @@ typedef struct Register Register; -Register *register_create(Arena *arena, ElementId id, Scalar value, - Stamp stamp); - -ElementId register_id(const Register *reg); +Register *register_create(Arena *arena, Scalar value, Stamp stamp); Scalar register_read(const Register *reg); @@ -50,4 +46,6 @@ 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); + #endif // _CRDT_REGISTER_H diff --git a/scalar.c b/scalar.c index c354947..8da2014 100644 --- a/scalar.c +++ b/scalar.c @@ -2,7 +2,6 @@ #include "arena.h" #include "host.h" #include -#include #include Scalar scalar_null(void) { @@ -49,21 +48,16 @@ bool scalar_eq(Scalar a, Scalar b) { if (a.as.s.len != b.as.s.len) { return false; } - for (size_t i = 0; i < a.as.s.len; i++) { - if (a.as.s.bytes[i] != b.as.s.bytes[i]) { - return false; - } - } - return true; + return memcmp(a.as.s.bytes, b.as.s.bytes, a.as.s.len) == 0; } } -Scalar scalar_dup(Arena *arena, Scalar value) { +Scalar scalar_clone(Arena *arena, Scalar value) { switch (value.kind) { case SCALAR_STRING: { uint8_t *bytes_copy = arena_alloc(arena, value.as.s.len); if (!bytes_copy) { - host_abortf("scalar_dup: arena OOM (requested %zu bytes for " + host_abortf("scalar_clone: arena OOM (requested %zu bytes for " "string value)", value.as.s.len); } @@ -77,7 +71,7 @@ Scalar scalar_dup(Arena *arena, Scalar value) { case SCALAR_INT: case SCALAR_BOOL: case SCALAR_NULL: - // No heap data to dup, just copy the struct. + // No heap data to clone, just copy the struct. return value; } } diff --git a/scalar.h b/scalar.h index 67b2787..72f226f 100644 --- a/scalar.h +++ b/scalar.h @@ -10,7 +10,7 @@ // 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 dup the bytes into its own arena. +// the call (Register, Map, ...) must clone the bytes into its own arena. // // scalar_eq is kind-strict: cross-kind comparison is always false, even // for "obvious" coincidences (scalar_int(0) != scalar_bool(false)). For @@ -51,6 +51,6 @@ Scalar scalar_string(const uint8_t *bytes, size_t len); bool scalar_eq(Scalar a, Scalar b); -Scalar scalar_dup(Arena *arena, Scalar value); +Scalar scalar_clone(Arena *arena, Scalar value); #endif // _CRDT_SCALAR_H diff --git a/test_counter.c b/test_counter.c index 08f5b0b..f8a5e88 100644 --- a/test_counter.c +++ b/test_counter.c @@ -1,7 +1,6 @@ #include "arena.h" #include "clientid.h" #include "counter.h" -#include "elementid.h" #include "test_util.h" // Build a ClientId fixture from a single byte (rest zero). Keeps tests @@ -12,24 +11,9 @@ static ClientId cid(uint8_t first_byte) { return clientid_from_bytes(b); } -static ElementId eid(uint8_t origin_byte, uint64_t seq) { - return elementid_new(cid(origin_byte), seq); -} - -// 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()); -} - -TEST(counter_create_stores_id) { - Arena *a = arena_create(); - ElementId id = eid(7, 42); - Counter *c = counter_create(a, id); - ASSERT(elementid_eq(counter_id(c), id) == true); - arena_destroy(a); + return counter_create(arena); } // --- local operations (single replica) --- @@ -254,8 +238,64 @@ TEST(local_inc_after_merge_accumulates) { ASSERT_EQ(counter_read(a), 7); } +// --- 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); + Counter *clone = counter_clone(ad, src); + ASSERT(clone != NULL); + ASSERT(clone != src); + ASSERT_EQ(counter_read(clone), 0); + arena_destroy(as); + arena_destroy(ad); +} + +TEST(clone_preserves_per_client_tallies) { + Arena *as = arena_create(); + Arena *ad = arena_create(); + Counter *src = counter_create(as); + counter_inc(src, cid(1), 5); + counter_inc(src, cid(2), 3); + counter_dec(src, cid(1), 2); + Counter *clone = counter_clone(ad, src); + ASSERT_EQ(counter_read(clone), counter_read(src)); + ASSERT_EQ(counter_read(clone), 6); // (5-2) + 3 + arena_destroy(as); + arena_destroy(ad); +} + +// 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); + counter_inc(src, cid(1), 5); + counter_inc(src, cid(2), 3); + Counter *clone = counter_clone(ad, src); + arena_destroy(as); + ASSERT_EQ(counter_read(clone), 8); + arena_destroy(ad); +} + +// 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); + counter_inc(src, cid(1), 5); + Counter *clone = counter_clone(ad, 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); +} + int main(void) { - RUN(counter_create_stores_id); RUN(empty_reads_zero); RUN(single_inc); RUN(inc_then_dec_nets); @@ -272,5 +312,11 @@ int main(void) { RUN(merge_associative); RUN(merge_does_not_mutate_src); RUN(local_inc_after_merge_accumulates); + + RUN(clone_empty_counter_reads_zero); + RUN(clone_preserves_per_client_tallies); + RUN(clone_survives_src_arena_destroy); + RUN(clone_independent_of_src); + TEST_SUMMARY(); } diff --git a/test_element.c b/test_element.c index d73b59a..a9129b5 100644 --- a/test_element.c +++ b/test_element.c @@ -2,7 +2,6 @@ #include "clientid.h" #include "counter.h" #include "element.h" -#include "elementid.h" #include "map.h" #include "register.h" #include "scalar.h" @@ -18,12 +17,6 @@ static ClientId cid(uint8_t first_byte) { return clientid_from_bytes(b); } -static ElementId eid(uint8_t origin_byte, uint64_t seq) { - return elementid_new(cid(origin_byte), seq); -} - -static ElementId default_id(void) { return eid(0xFF, 0); } - static Stamp stmp(uint64_t lamport, uint8_t client_first_byte) { return (Stamp){.lamport = lamport, .client_id = cid(client_first_byte)}; } @@ -38,7 +31,7 @@ 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(a, scalar_int(1), stmp(1, 1)); Element e = element_register(r); ASSERT_EQ(element_kind(e), ELEMENT_REGISTER); ASSERT(e.as.reg == r); @@ -47,7 +40,7 @@ TEST(register_constructor_sets_kind_and_pointer) { TEST(counter_constructor_sets_kind_and_pointer) { Arena *a = arena_create(); - Counter *c = counter_create(a, default_id()); + Counter *c = counter_create(a); Element e = element_counter(c); ASSERT_EQ(element_kind(e), ELEMENT_COUNTER); ASSERT(e.as.counter == c); @@ -56,7 +49,7 @@ TEST(counter_constructor_sets_kind_and_pointer) { TEST(map_constructor_sets_kind_and_pointer) { Arena *a = arena_create(); - Map *m = map_create(a, default_id()); + Map *m = map_create(a); Element e = element_map(m); ASSERT_EQ(element_kind(e), ELEMENT_MAP); ASSERT(e.as.map == m); @@ -83,15 +76,11 @@ TEST(kind_name_map) { // --- merge dispatches by kind to the underlying _merge --- -// REGISTER: element_merge must call register_merge — dst takes src's value when -// src has the higher Stamp. 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)); // newer + Register *dst = register_create(ad, scalar_int(10), stmp(1, 1)); + Register *src = register_create(as, scalar_int(20), stmp(5, 1)); element_merge(element_register(dst), element_register(src)); @@ -100,34 +89,31 @@ TEST(merge_register_takes_newer_value) { arena_destroy(as); } -// COUNTER: element_merge must call counter_merge — per-client max on inc/dec. 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(ad); + Counter *src = counter_create(as); 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); // 5 from client 1 + 3 from client 2 + ASSERT_EQ(counter_read(dst), 8); arena_destroy(ad); arena_destroy(as); } -// MAP: element_merge must call map_merge — dst takes src's slots when src has -// the higher Stamp. 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(ad); + Map *src = map_create(as); const uint8_t *k = (const uint8_t *)"k"; size_t klen = 1; map_set(dst, k, klen, element_scalar(scalar_int(10)), stmp(1, 1)); - map_set(src, k, klen, element_scalar(scalar_int(20)), stmp(5, 1)); // newer + map_set(src, k, klen, element_scalar(scalar_int(20)), stmp(5, 1)); element_merge(element_map(dst), element_map(src)); @@ -139,14 +125,11 @@ TEST(merge_map_takes_newer_slot) { arena_destroy(as); } -// REGISTER: merge does not mutate 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)); // newer - Register *src = - register_create(as, default_id(), scalar_int(7), stmp(1, 1)); + Register *dst = register_create(ad, scalar_int(99), stmp(10, 1)); + Register *src = register_create(as, scalar_int(7), stmp(1, 1)); element_merge(element_register(dst), element_register(src)); @@ -155,12 +138,11 @@ TEST(merge_register_does_not_mutate_src) { arena_destroy(as); } -// COUNTER: merge does not mutate 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(ad); + Counter *src = counter_create(as); counter_inc(src, cid(1), 3); element_merge(element_counter(dst), element_counter(src)); @@ -170,12 +152,11 @@ TEST(merge_counter_does_not_mutate_src) { arena_destroy(as); } -// MAP: merge does not mutate 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(ad); + Map *src = map_create(as); const uint8_t *k = (const uint8_t *)"k"; map_set(src, k, 1, element_scalar(scalar_int(7)), stmp(1, 1)); @@ -189,17 +170,105 @@ TEST(merge_map_does_not_mutate_src) { arena_destroy(as); } -// Mixed-kind helper: an Element of any composite kind round-trips through the -// constructor / accessor pair without losing the payload. TEST(round_trip_via_kind_and_payload) { Arena *a = arena_create(); - Counter *c = counter_create(a, default_id()); + Counter *c = counter_create(a); Element e = element_counter(c); ASSERT_EQ(element_kind(e), ELEMENT_COUNTER); ASSERT(e.as.counter == c); arena_destroy(a); } +// --- element_clone: deep copy into a target arena --- +// +// 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. + +TEST(clone_scalar_int_preserves_value) { + Arena *dst = arena_create(); + Element src = element_scalar(scalar_int(42)); + Element clone = element_clone(dst, src); + ASSERT_EQ(element_kind(clone), ELEMENT_SCALAR); + ASSERT(scalar_eq(clone.as.scalar, scalar_int(42))); + arena_destroy(dst); +} + +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 + ASSERT_EQ(element_kind(clone), ELEMENT_SCALAR); + ASSERT( + scalar_eq(clone.as.scalar, scalar_string((const uint8_t *)"hello", 5))); + arena_destroy(dst); +} + +TEST(clone_register_deep_copies_value) { + Arena *as = arena_create(); + Arena *ad = arena_create(); + Register *src = register_create(as, scalar_int(42), stmp(5, 1)); + Element clone = element_clone(ad, element_register(src)); + arena_destroy(as); + 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); +} + +TEST(clone_counter_deep_copies_per_client_tallies) { + Arena *as = arena_create(); + Arena *ad = arena_create(); + Counter *src = counter_create(as); + counter_inc(src, cid(1), 5); + counter_inc(src, cid(2), 3); + Element clone = element_clone(ad, element_counter(src)); + arena_destroy(as); + ASSERT_EQ(element_kind(clone), ELEMENT_COUNTER); + ASSERT(clone.as.counter != src); + ASSERT_EQ(counter_read(clone.as.counter), 8); + arena_destroy(ad); +} + +TEST(clone_map_deep_copies_recursively) { + Arena *as = arena_create(); + Arena *ad = arena_create(); + Map *src = map_create(as); + 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); + ASSERT_EQ(element_kind(clone), ELEMENT_MAP); + ASSERT(clone.as.map != src); + Element a_out, b_out; + ASSERT(map_get(clone.as.map, (const void *)"a", 1, &a_out) == true); + 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); +} + +// 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); + counter_inc(src, cid(1), 5); + Element clone = element_clone(ad, 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); +} + int main(void) { RUN(scalar_constructor_sets_kind_and_value); RUN(register_constructor_sets_kind_and_pointer); @@ -221,5 +290,12 @@ 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_register_deep_copies_value); + RUN(clone_counter_deep_copies_per_client_tallies); + RUN(clone_map_deep_copies_recursively); + RUN(clone_counter_independent_of_src); + TEST_SUMMARY(); } diff --git a/test_elementid.c b/test_elementid.c deleted file mode 100644 index b476343..0000000 --- a/test_elementid.c +++ /dev/null @@ -1,178 +0,0 @@ -#include "clientid.h" -#include "elementid.h" -#include "test_util.h" -#include - -// Helpers. - -static ClientId cid(uint8_t first_byte) { - uint8_t b[16] = {0}; - b[0] = first_byte; - return clientid_from_bytes(b); -} - -static ElementId eid(uint8_t origin_byte, uint64_t seq) { - return elementid_new(cid(origin_byte), seq); -} - -// --- construction --- - -TEST(new_sets_origin_and_seq) { - ClientId origin = cid(7); - ElementId id = elementid_new(origin, 42); - ASSERT(clientid_eq(id.origin, origin) == true); - ASSERT_EQ(id.seq, 42); -} - -TEST(seq_zero_is_valid) { - ElementId id = eid(1, 0); - ASSERT_EQ(id.seq, 0); -} - -TEST(seq_max_is_valid) { - ElementId id = eid(1, UINT64_MAX); - ASSERT_EQ(id.seq, UINT64_MAX); -} - -// --- root sentinel --- - -// root() must be deterministic — every call returns the same value. -TEST(root_is_stable) { - ElementId r1 = elementid_root(); - ElementId r2 = elementid_root(); - ASSERT(elementid_eq(r1, r2) == true); -} - -// root must not collide with any "regular" id constructed from a non-zero -// client or seq. Two regular ids that happen to match root would break the -// recursive merge dispatcher. -TEST(root_distinct_from_regular_ids) { - ElementId r = elementid_root(); - ASSERT(elementid_eq(r, eid(1, 0)) == false); - ASSERT(elementid_eq(r, eid(0, 1)) == false); -} - -// --- equality --- - -TEST(eq_same) { - ElementId a = eid(5, 100); - ElementId b = eid(5, 100); - ASSERT(elementid_eq(a, b) == true); -} - -TEST(eq_different_origin) { - ElementId a = eid(5, 100); - ElementId b = eid(6, 100); - ASSERT(elementid_eq(a, b) == false); -} - -TEST(eq_different_seq) { - ElementId a = eid(5, 100); - ElementId b = eid(5, 101); - ASSERT(elementid_eq(a, b) == false); -} - -TEST(eq_completely_different) { - ElementId a = eid(5, 100); - ElementId b = eid(99, 9999); - ASSERT(elementid_eq(a, b) == false); -} - -// --- ordering (cmp) --- - -// Equality returns 0. -TEST(cmp_equal_returns_zero) { - ElementId a = eid(5, 100); - ElementId b = eid(5, 100); - ASSERT_EQ(elementid_cmp(a, b), 0); -} - -// Same origin: seq decides. -TEST(cmp_same_origin_lower_seq_less) { - ElementId a = eid(5, 100); - ElementId b = eid(5, 200); - ASSERT(elementid_cmp(a, b) < 0); -} - -TEST(cmp_same_origin_higher_seq_greater) { - ElementId a = eid(5, 200); - ElementId b = eid(5, 100); - ASSERT(elementid_cmp(a, b) > 0); -} - -// Different origin: origin decides (via clientid_cmp), seq is irrelevant. -TEST(cmp_origin_dominates_seq) { - // a has smaller origin but huge seq; b has bigger origin but seq 0. - ElementId a = eid(1, UINT64_MAX); - ElementId b = eid(2, 0); - ASSERT(elementid_cmp(a, b) < 0); -} - -// Anti-symmetric: cmp(a, b) and cmp(b, a) have opposite signs (or both 0). -TEST(cmp_anti_symmetric_origin) { - ElementId a = eid(1, 100); - ElementId b = eid(2, 100); - int ab = elementid_cmp(a, b); - int ba = elementid_cmp(b, a); - ASSERT((ab < 0 && ba > 0) || (ab > 0 && ba < 0)); -} - -TEST(cmp_anti_symmetric_seq) { - ElementId a = eid(5, 1); - ElementId b = eid(5, 2); - int ab = elementid_cmp(a, b); - int ba = elementid_cmp(b, a); - ASSERT((ab < 0 && ba > 0) || (ab > 0 && ba < 0)); -} - -// Transitive: a < b and b < c implies a < c. -TEST(cmp_transitive) { - ElementId a = eid(5, 1); - ElementId b = eid(5, 2); - ElementId c = eid(5, 3); - ASSERT(elementid_cmp(a, b) < 0); - ASSERT(elementid_cmp(b, c) < 0); - ASSERT(elementid_cmp(a, c) < 0); -} - -// Trichotomy: for any two ids, exactly one of (a < b, b < a, equal) holds. -TEST(cmp_trichotomy_distinct) { - ElementId a = eid(1, 0); - ElementId b = eid(2, 0); - int ab = elementid_cmp(a, b); - int ba = elementid_cmp(b, a); - ASSERT((ab < 0 && ba > 0) || (ab > 0 && ba < 0)); -} - -TEST(cmp_trichotomy_equal) { - ElementId a = eid(5, 100); - ElementId b = eid(5, 100); - ASSERT_EQ(elementid_cmp(a, b), 0); - ASSERT_EQ(elementid_cmp(b, a), 0); -} - -int main(void) { - RUN(new_sets_origin_and_seq); - RUN(seq_zero_is_valid); - RUN(seq_max_is_valid); - - RUN(root_is_stable); - RUN(root_distinct_from_regular_ids); - - RUN(eq_same); - RUN(eq_different_origin); - RUN(eq_different_seq); - RUN(eq_completely_different); - - RUN(cmp_equal_returns_zero); - RUN(cmp_same_origin_lower_seq_less); - RUN(cmp_same_origin_higher_seq_greater); - RUN(cmp_origin_dominates_seq); - RUN(cmp_anti_symmetric_origin); - RUN(cmp_anti_symmetric_seq); - RUN(cmp_transitive); - RUN(cmp_trichotomy_distinct); - RUN(cmp_trichotomy_equal); - - TEST_SUMMARY(); -} diff --git a/test_map.c b/test_map.c index 129e537..fa78bfc 100644 --- a/test_map.c +++ b/test_map.c @@ -2,15 +2,13 @@ #include "clientid.h" #include "counter.h" #include "element.h" -#include "elementid.h" #include "map.h" #include "register.h" #include "scalar.h" #include "stamp.h" #include "string.h" #include "test_util.h" - -// Helpers — keep tests compact. +#include static ClientId cid(uint8_t first_byte) { uint8_t b[16] = {0}; @@ -18,13 +16,6 @@ static ClientId cid(uint8_t first_byte) { return clientid_from_bytes(b); } -static ElementId eid(uint8_t origin_byte, uint64_t seq) { - return elementid_new(cid(origin_byte), seq); -} - -// Default id for the Map under test when identity does not matter. -static ElementId default_id(void) { return eid(0xFF, 0); } - static Stamp stmp(uint64_t lamport, uint8_t client_first_byte) { return (Stamp){.lamport = lamport, .client_id = cid(client_first_byte)}; } @@ -38,26 +29,15 @@ static Stamp stmp(uint64_t lamport, uint8_t client_first_byte) { static Map *fresh(void) { Arena *arena = arena_create(); - return map_create(arena, default_id()); + return map_create(arena); } -// Assert helper: out is a SCALAR element equal to expected Scalar. #define ASSERT_SCALAR_EQ(out, expected) \ do { \ ASSERT_EQ(element_kind(out), ELEMENT_SCALAR); \ ASSERT(scalar_eq((out).as.scalar, (expected))); \ } while (0) -// --- identity --- - -TEST(map_create_stores_id) { - Arena *a = arena_create(); - ElementId id = eid(7, 42); - Map *m = map_create(a, id); - ASSERT(elementid_eq(map_id(m), id) == true); - arena_destroy(a); -} - // --- local set / get (scalar slots) --- TEST(empty_get_returns_false) { @@ -86,7 +66,7 @@ TEST(set_overwrites_with_newer_stamp) { TEST(set_lower_stamp_ignored) { Map *m = fresh(); map_set(m, SK("k"), EI(20), stmp(5, 1)); - map_set(m, SK("k"), EI(10), stmp(3, 1)); // older — ignored + map_set(m, SK("k"), EI(10), stmp(3, 1)); Element out; ASSERT(map_get(m, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(20)); @@ -95,7 +75,7 @@ TEST(set_lower_stamp_ignored) { TEST(set_equal_lamport_higher_client_wins) { Map *m = fresh(); map_set(m, SK("k"), EI(10), stmp(5, 1)); - map_set(m, SK("k"), EI(20), stmp(5, 2)); // same lamport, > client + map_set(m, SK("k"), EI(20), stmp(5, 2)); Element out; ASSERT(map_get(m, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(20)); @@ -119,7 +99,6 @@ TEST(set_same_stamp_idempotent) { ASSERT_SCALAR_EQ(out, scalar_int(42)); } -// A newer write can change the Scalar kind. TEST(set_can_change_value_kind) { Map *m = fresh(); map_set(m, SK("k"), EI(42), stmp(1, 1)); @@ -129,7 +108,6 @@ TEST(set_can_change_value_kind) { ASSERT_SCALAR_EQ(out, scalar_string((const uint8_t *)"hi", 2)); } -// Distinct keys are independent — writing one must not affect the other. TEST(distinct_keys_are_independent) { Map *m = fresh(); map_set(m, SK("a"), EI(1), stmp(1, 1)); @@ -141,8 +119,6 @@ TEST(distinct_keys_are_independent) { ASSERT_SCALAR_EQ(b, scalar_int(2)); } -// Headline reason for byte keys: keys with embedded NUL bytes must be -// distinguished past the NUL. TEST(keys_with_embedded_nul_are_distinct) { Map *m = fresh(); uint8_t k1[3] = {0x01, 0x00, 0x02}; @@ -166,17 +142,15 @@ TEST(delete_makes_get_return_false) { ASSERT(map_get(m, SK("k"), &out) == false); } -// A delete with a stamp older than the existing value must NOT clobber. TEST(delete_with_lower_stamp_ignored) { Map *m = fresh(); map_set(m, SK("k"), EI(42), stmp(5, 1)); - map_delete(m, SK("k"), stmp(3, 1)); // older — ignored + map_delete(m, SK("k"), stmp(3, 1)); Element out; ASSERT(map_get(m, SK("k"), &out) == true); ASSERT_SCALAR_EQ(out, scalar_int(42)); } -// After delete, a set with a higher stamp must resurrect the slot. TEST(set_after_delete_with_higher_stamp_resurrects) { Map *m = fresh(); map_set(m, SK("k"), EI(10), stmp(1, 1)); @@ -187,18 +161,15 @@ TEST(set_after_delete_with_higher_stamp_resurrects) { ASSERT_SCALAR_EQ(out, scalar_int(20)); } -// After delete, a set with a lower-or-equal stamp must NOT resurrect. TEST(set_after_delete_with_lower_stamp_ignored) { Map *m = fresh(); map_set(m, SK("k"), EI(10), stmp(1, 1)); map_delete(m, SK("k"), stmp(5, 1)); - map_set(m, SK("k"), EI(20), stmp(3, 1)); // older than delete + map_set(m, SK("k"), EI(20), stmp(3, 1)); Element out; ASSERT(map_get(m, SK("k"), &out) == false); } -// Concurrent set vs delete: stamp decides which wins. Here delete has the -// higher stamp, so the slot ends up tombstoned. TEST(set_vs_delete_higher_stamp_wins_delete) { Map *m = fresh(); map_set(m, SK("k"), EI(10), stmp(1, 1)); @@ -216,12 +187,10 @@ TEST(delete_idempotent_same_stamp) { ASSERT(map_get(m, SK("k"), &out) == false); } -// Deleting a key that was never set is a no-op but installs a tombstone with -// the given stamp — a later set with a lower stamp must still be rejected. TEST(delete_absent_key_still_installs_tombstone) { Map *m = fresh(); map_delete(m, SK("ghost"), stmp(10, 1)); - map_set(m, SK("ghost"), EI(1), stmp(5, 1)); // older than delete + map_set(m, SK("ghost"), EI(1), stmp(5, 1)); Element out; ASSERT(map_get(m, SK("ghost"), &out) == false); } @@ -262,8 +231,8 @@ TEST(size_recovers_on_resurrect) { TEST(set_counter_then_get_returns_element_counter) { Arena *ar = arena_create(); - Map *m = map_create(ar, default_id()); - Counter *c = counter_create(ar, eid(1, 1)); + Map *m = map_create(ar); + Counter *c = counter_create(ar); counter_inc(c, cid(1), 5); map_set(m, SK("votes"), element_counter(c), stmp(1, 1)); @@ -276,8 +245,8 @@ TEST(set_counter_then_get_returns_element_counter) { TEST(set_register_then_get_returns_element_register) { Arena *ar = arena_create(); - Map *m = map_create(ar, default_id()); - Register *r = register_create(ar, eid(1, 1), scalar_int(7), stmp(1, 1)); + Map *m = map_create(ar); + Register *r = register_create(ar, scalar_int(7), stmp(1, 1)); map_set(m, SK("title"), element_register(r), stmp(1, 1)); Element out; @@ -289,8 +258,8 @@ TEST(set_register_then_get_returns_element_register) { 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, eid(1, 1)); + Map *outer = map_create(ar); + Map *inner = map_create(ar); map_set(inner, SK("a"), EI(1), stmp(1, 1)); map_set(outer, SK("child"), element_map(inner), stmp(1, 1)); @@ -306,8 +275,8 @@ TEST(set_nested_map_then_get_returns_element_map) { // --- 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 = map_create(arena_create()); + Map *b = map_create(arena_create()); map_set(a, SK("x"), EI(1), stmp(1, 1)); map_set(b, SK("y"), EI(2), stmp(1, 2)); @@ -321,10 +290,10 @@ TEST(merge_disjoint_keys_unions) { } TEST(merge_same_key_newer_wins) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); + Map *a = map_create(arena_create()); + Map *b = map_create(arena_create()); map_set(a, SK("k"), EI(10), stmp(1, 1)); - map_set(b, SK("k"), EI(20), stmp(2, 2)); // newer + map_set(b, SK("k"), EI(20), stmp(2, 2)); map_merge(a, b); Element out; @@ -333,9 +302,9 @@ TEST(merge_same_key_newer_wins) { } TEST(merge_src_older_loses) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); - map_set(a, SK("k"), EI(20), stmp(5, 1)); // newer + Map *a = map_create(arena_create()); + Map *b = map_create(arena_create()); + map_set(a, SK("k"), EI(20), stmp(5, 1)); map_set(b, SK("k"), EI(10), stmp(2, 2)); map_merge(a, b); @@ -344,24 +313,22 @@ TEST(merge_src_older_loses) { ASSERT_SCALAR_EQ(out, scalar_int(20)); } -// Concurrent: dst has a value, src has a delete with a higher stamp. TEST(merge_delete_beats_older_set) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); + Map *a = map_create(arena_create()); + Map *b = map_create(arena_create()); map_set(a, SK("k"), EI(10), stmp(1, 1)); - map_delete(b, SK("k"), stmp(5, 1)); // newer + map_delete(b, SK("k"), stmp(5, 1)); map_merge(a, b); Element out; ASSERT(map_get(a, SK("k"), &out) == false); } -// Concurrent: dst has a delete, src has a value with a higher stamp. TEST(merge_set_beats_older_delete) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); + Map *a = map_create(arena_create()); + Map *b = map_create(arena_create()); map_delete(a, SK("k"), stmp(1, 1)); - map_set(b, SK("k"), EI(42), stmp(5, 1)); // newer + map_set(b, SK("k"), EI(42), stmp(5, 1)); map_merge(a, b); Element out; @@ -370,16 +337,14 @@ TEST(merge_set_beats_older_delete) { } TEST(merge_commutative) { - // path 1: a <- b - Map *a1 = map_create(arena_create(), default_id()); - Map *b1 = map_create(arena_create(), default_id()); + Map *a1 = map_create(arena_create()); + Map *b1 = map_create(arena_create()); map_set(a1, SK("k"), EI(10), stmp(5, 1)); map_set(b1, SK("k"), EI(20), stmp(5, 2)); map_merge(a1, b1); - // path 2: b <- a - Map *a2 = map_create(arena_create(), default_id()); - Map *b2 = map_create(arena_create(), default_id()); + Map *a2 = map_create(arena_create()); + Map *b2 = map_create(arena_create()); map_set(a2, SK("k"), EI(10), stmp(5, 1)); map_set(b2, SK("k"), EI(20), stmp(5, 2)); map_merge(b2, a2); @@ -394,8 +359,8 @@ TEST(merge_commutative) { } TEST(merge_idempotent) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); + Map *a = map_create(arena_create()); + Map *b = map_create(arena_create()); map_set(a, SK("k"), EI(10), stmp(1, 1)); map_set(b, SK("k"), EI(20), stmp(2, 1)); @@ -412,20 +377,18 @@ TEST(merge_idempotent) { } TEST(merge_associative) { - // (a <- b) <- c - 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 = map_create(arena_create()); + Map *b = map_create(arena_create()); + Map *c = map_create(arena_create()); 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); - // a <- (b <- 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 = map_create(arena_create()); + Map *b2 = map_create(arena_create()); + Map *c2 = map_create(arena_create()); 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)); @@ -442,23 +405,20 @@ TEST(merge_associative) { } TEST(merge_does_not_mutate_src) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); - map_set(a, SK("k"), EI(99), stmp(10, 1)); // newer + Map *a = map_create(arena_create()); + Map *b = map_create(arena_create()); + map_set(a, SK("k"), EI(99), stmp(10, 1)); map_set(b, SK("k"), EI(7), stmp(1, 1)); map_merge(a, b); Element out; ASSERT(map_get(b, SK("k"), &out) == true); - ASSERT_SCALAR_EQ(out, scalar_int(7)); // b unchanged + ASSERT_SCALAR_EQ(out, scalar_int(7)); } -// When merge accepts a winning string value from src, dst must own its own -// copy in dst's arena. Mutating the source bytes after merge must not affect -// dst's stored value. TEST(merge_copies_string_into_dst_arena) { - Map *a = map_create(arena_create(), default_id()); - Map *b = map_create(arena_create(), default_id()); + Map *a = map_create(arena_create()); + Map *b = map_create(arena_create()); uint8_t src_bytes[8]; memcpy(src_bytes, "hello", 5); @@ -466,7 +426,7 @@ TEST(merge_copies_string_into_dst_arena) { map_set(a, SK("k"), EI(0), stmp(1, 1)); map_set(b, SK("k"), ES(src_bytes, 5), stmp(5, 1)); - map_merge(a, b); // a takes b's string + map_merge(a, b); src_bytes[0] = 'X'; src_bytes[1] = 'X'; @@ -476,11 +436,9 @@ TEST(merge_copies_string_into_dst_arena) { ASSERT_SCALAR_EQ(out, scalar_string((const uint8_t *)"hello", 5)); } -// Tombstones survive merge: dst with a tombstone merged with src that has an -// older value must keep the tombstone (the higher stamp wins). 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 = map_create(arena_create()); + Map *b = map_create(arena_create()); map_delete(a, SK("k"), stmp(5, 1)); map_set(b, SK("k"), EI(10), stmp(2, 1)); @@ -489,53 +447,47 @@ TEST(merge_preserves_tombstone_against_older_set) { ASSERT(map_get(a, SK("k"), &out) == false); } -// --- nested merge: same id same kind recurses into element_merge --- +// --- recursive merge: same kind at same key recurses regardless of stamp --- +// +// Position is identity. Two replicas with a composite of the same kind at +// the same key are by definition the same logical object — recurse into +// element_merge. Slot stamp advances to max(dst, src). -// Two replicas hold the same Counter at "votes" (same id). Merge must combine -// their per-client tallies via counter_merge, NOT do LWW on the slot stamp. -// The dst slot has the OLDER stamp on purpose: if the implementation chose -// LWW, dst would inherit src's counter (=3) instead of the union (=8). -TEST(merge_same_id_counter_recurses) { +TEST(merge_same_kind_counter_recurses) { Arena *ad = arena_create(); Arena *as = arena_create(); - ElementId votes_id = eid(7, 1); - Map *dst = map_create(ad, default_id()); - Counter *dc = counter_create(ad, votes_id); + Map *dst = map_create(ad); + Counter *dc = counter_create(ad); counter_inc(dc, cid(1), 5); - map_set(dst, SK("votes"), element_counter(dc), - stmp(1, 1)); // older slot stamp + map_set(dst, SK("votes"), element_counter(dc), stmp(1, 1)); - Map *src = map_create(as, default_id()); - Counter *sc = counter_create(as, votes_id); + Map *src = map_create(as); + Counter *sc = counter_create(as); counter_inc(sc, cid(2), 3); - map_set(src, SK("votes"), element_counter(sc), - stmp(10, 1)); // newer slot stamp + map_set(src, SK("votes"), element_counter(sc), stmp(10, 1)); map_merge(dst, src); Element out; ASSERT(map_get(dst, SK("votes"), &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_COUNTER); - ASSERT_EQ(counter_read(out.as.counter), 8); // unioned, not replaced + ASSERT(out.as.counter == dc); // dst kept its own pointer + ASSERT_EQ(counter_read(out.as.counter), 8); arena_destroy(ad); arena_destroy(as); } -// Same shape with Register: same id → element_merge (register_merge by stamp). -// Pick stamps so register_merge picks src's value; that's distinct from "dst -// took src's whole Register" because dst's Register pointer must be preserved. -TEST(merge_same_id_register_recurses) { +TEST(merge_same_kind_register_recurses) { Arena *ad = arena_create(); Arena *as = arena_create(); - ElementId reg_id = eid(7, 1); - Map *dst = map_create(ad, default_id()); - Register *dr = register_create(ad, reg_id, scalar_int(10), stmp(1, 1)); + Map *dst = map_create(ad); + Register *dr = register_create(ad, scalar_int(10), stmp(1, 1)); map_set(dst, SK("title"), element_register(dr), stmp(1, 1)); - Map *src = map_create(as, default_id()); - Register *sr = register_create(as, reg_id, scalar_int(20), stmp(5, 1)); + Map *src = map_create(as); + Register *sr = register_create(as, scalar_int(20), stmp(5, 1)); map_set(src, SK("title"), element_register(sr), stmp(1, 1)); map_merge(dst, src); @@ -543,27 +495,23 @@ TEST(merge_same_id_register_recurses) { Element out; ASSERT(map_get(dst, SK("title"), &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_REGISTER); - // dst kept its OWN Register pointer; that Register absorbed src's value. ASSERT(out.as.reg == dr); ASSERT(scalar_eq(register_read(dr), scalar_int(20))); arena_destroy(ad); arena_destroy(as); } -// Same shape with nested Map: same id → element_merge recurses into map_merge -// on the inner maps. Inner slot from src must show up in dst's inner map. -TEST(merge_same_id_nested_map_recurses) { +TEST(merge_same_kind_nested_map_recurses) { Arena *ad = arena_create(); Arena *as = arena_create(); - ElementId inner_id = eid(7, 1); - Map *dst = map_create(ad, default_id()); - Map *di = map_create(ad, inner_id); + Map *dst = map_create(ad); + Map *di = map_create(ad); map_set(di, SK("a"), EI(1), stmp(1, 1)); map_set(dst, SK("child"), element_map(di), stmp(1, 1)); - Map *src = map_create(as, default_id()); - Map *si = map_create(as, inner_id); + Map *src = map_create(as); + Map *si = map_create(as); map_set(si, SK("b"), EI(2), stmp(1, 2)); map_set(src, SK("child"), element_map(si), stmp(1, 1)); @@ -572,7 +520,7 @@ TEST(merge_same_id_nested_map_recurses) { Element out; ASSERT(map_get(dst, SK("child"), &out) == true); ASSERT_EQ(element_kind(out), ELEMENT_MAP); - ASSERT(out.as.map == di); // dst kept its own inner Map pointer + ASSERT(out.as.map == di); Element a_out, b_out; ASSERT(map_get(di, SK("a"), &a_out) == true); ASSERT(map_get(di, SK("b"), &b_out) == true); @@ -582,54 +530,50 @@ TEST(merge_same_id_nested_map_recurses) { arena_destroy(as); } -// Recursive merge does not touch src's composite — dst absorbs, src untouched. -TEST(merge_same_id_counter_does_not_mutate_src) { +TEST(merge_same_kind_counter_does_not_mutate_src) { Arena *ad = arena_create(); Arena *as = arena_create(); - ElementId votes_id = eid(7, 1); - Map *dst = map_create(ad, default_id()); - Counter *dc = counter_create(ad, votes_id); + Map *dst = map_create(ad); + Counter *dc = counter_create(ad); counter_inc(dc, cid(1), 5); map_set(dst, SK("votes"), element_counter(dc), stmp(1, 1)); - Map *src = map_create(as, default_id()); - Counter *sc = counter_create(as, votes_id); + Map *src = map_create(as); + Counter *sc = counter_create(as); 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); // src counter unchanged + ASSERT_EQ(counter_read(sc), 3); arena_destroy(ad); arena_destroy(as); } -// After recursive same-id merge the slot stamp must advance to the max of -// dst's and src's slot stamps. Otherwise a later slot-level op on dst could -// flip the slot using a stamp that src has already moved past, and the -// replicas diverge. Probe externally: a subsequent set with a stamp above -// dst's old slot stamp but below src's must be ignored. -TEST(merge_same_id_counter_advances_slot_stamp) { +// Recursive merge must advance the slot stamp to max(dst, src). Otherwise +// future slot-level ops on this key can diverge between replicas. Probe: +// 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(); - ElementId votes_id = eid(7, 1); - Map *dst = map_create(ad, default_id()); - Counter *dc = counter_create(ad, votes_id); + Map *dst = map_create(ad); + Counter *dc = counter_create(ad); counter_inc(dc, cid(1), 5); map_set(dst, SK("votes"), element_counter(dc), stmp(1, 1)); - Map *src = map_create(as, default_id()); - Counter *sc = counter_create(as, votes_id); + Map *src = map_create(as); + Counter *sc = counter_create(as); counter_inc(sc, cid(2), 3); map_set(src, SK("votes"), element_counter(sc), stmp(10, 1)); map_merge(dst, src); - // Concurrent later write with stamp between (1,1) and (10,1). - // On src this would be ignored (src.slot.stamp = 10 > 5). dst must - // ignore it too; otherwise replicas diverge on the same key. + // A set at stamp(5,1) is below src's slot stamp(10,1) but above dst's + // old slot stamp(1,1). If dst's stamp wasn't advanced to 10, this set + // would replace the Counter — and replicas diverge. map_set(dst, SK("votes"), EI(99), stmp(5, 1)); Element out; @@ -640,9 +584,411 @@ TEST(merge_same_id_counter_advances_slot_stamp) { arena_destroy(as); } -int main(void) { - RUN(map_create_stores_id); +// --- type-flip via LWW --- +// +// 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). + +TEST(set_composite_displaces_scalar_at_lww) { + Arena *ar = arena_create(); + Map *m = map_create(ar); + + map_set(m, SK("score"), EI(42), stmp(1, 1)); // scalar first + Counter *c = counter_create(ar); + map_set(m, SK("score"), element_counter(c), stmp(5, 1)); // newer composite + + 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); +} + +TEST(set_scalar_displaces_composite_at_lww) { + Arena *ar = arena_create(); + Map *m = map_create(ar); + + Counter *c = counter_create(ar); + map_set(m, SK("score"), element_counter(c), stmp(1, 1)); + 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); +} + +TEST(set_different_kind_composite_displaces_at_lww) { + Arena *ar = arena_create(); + Map *m = map_create(ar); + + Counter *c = counter_create(ar); + map_set(m, SK("score"), element_counter(c), stmp(1, 1)); + Register *r = register_create(ar, scalar_int(42), stmp(5, 1)); + map_set(m, SK("score"), element_register(r), stmp(5, 1)); + + 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); +} + +// --- cross-arena composite LWW: clone winner into dst's arena --- +TEST(merge_composite_src_wins_into_empty_slot_clones) { + Arena *ad = arena_create(); + Arena *as = arena_create(); + Map *dst = map_create(ad); + Map *src = map_create(as); + Counter *sc = counter_create(as); + counter_inc(sc, cid(1), 5); + map_set(src, SK("votes"), element_counter(sc), stmp(5, 1)); + + map_merge(dst, src); + + Element out; + ASSERT(map_get(dst, SK("votes"), &out) == true); + 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 + Element out2; + ASSERT(map_get(dst, SK("votes"), &out2) == true); + ASSERT_EQ(counter_read(out2.as.counter), 5); + arena_destroy(ad); +} + +TEST(merge_kind_mismatch_clones_winner_into_dst) { + Arena *ad = arena_create(); + Arena *as = arena_create(); + + Map *dst = map_create(ad); + Counter *dc = counter_create(ad); + counter_inc(dc, cid(1), 5); + map_set(dst, SK("x"), element_counter(dc), stmp(1, 1)); + + Map *src = map_create(as); + Register *sr = register_create(as, scalar_int(42), stmp(10, 1)); + map_set(src, SK("x"), element_register(sr), stmp(10, 1)); + + map_merge(dst, src); + + Element out; + ASSERT(map_get(dst, SK("x"), &out) == true); + 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); +} + +// --- 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. + +TEST(map_counter_creates_and_installs_at_key) { + Arena *ar = arena_create(); + Map *m = map_create(ar); + + Counter *c = map_counter(m, SK("votes"), stmp(1, 1)); + ASSERT(c != NULL); + + Element out; + ASSERT(map_get(m, SK("votes"), &out) == true); + ASSERT_EQ(element_kind(out), ELEMENT_COUNTER); + ASSERT(out.as.counter == c); + arena_destroy(ar); +} + +TEST(map_counter_returns_same_pointer_on_repeat) { + Arena *ar = arena_create(); + Map *m = map_create(ar); + + 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); +} + +TEST(map_register_creates_and_installs_at_key) { + Arena *ar = arena_create(); + Map *m = map_create(ar); + + Register *r = map_register(m, SK("title"), scalar_int(42), stmp(1, 1)); + ASSERT(r != NULL); + ASSERT(scalar_eq(register_read(r), scalar_int(42))); + + Element out; + ASSERT(map_get(m, SK("title"), &out) == true); + ASSERT_EQ(element_kind(out), ELEMENT_REGISTER); + ASSERT(out.as.reg == r); + arena_destroy(ar); +} + +TEST(map_register_returns_same_pointer_on_repeat) { + Arena *ar = arena_create(); + Map *m = map_create(ar); + + Register *first = map_register(m, SK("title"), scalar_int(1), stmp(1, 1)); + // Second call's seed value is ignored — slot already exists. + Register *second = + 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); +} + +TEST(map_map_creates_and_installs_at_key) { + Arena *ar = arena_create(); + Map *outer = map_create(ar); + + Map *child = map_map(outer, SK("child"), stmp(1, 1)); + ASSERT(child != NULL); + + Element out; + ASSERT(map_get(outer, SK("child"), &out) == true); + ASSERT_EQ(element_kind(out), ELEMENT_MAP); + ASSERT(out.as.map == child); + arena_destroy(ar); +} + +TEST(map_map_returns_same_pointer_on_repeat) { + Arena *ar = arena_create(); + Map *outer = map_create(ar); + + 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); +} + +// Helper called over a different-kind slot with a winning stamp must flip +// the kind via LWW and return a fresh composite. +TEST(map_register_after_map_counter_flips_kind_via_lww) { + Arena *ar = arena_create(); + Map *m = map_create(ar); + + Counter *c = map_counter(m, SK("score"), stmp(1, 1)); + ASSERT(c != NULL); + + Register *r = map_register(m, SK("score"), scalar_int(42), stmp(5, 1)); + ASSERT(r != NULL); + + Element out; + ASSERT(map_get(m, SK("score"), &out) == true); + 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. + ASSERT_EQ(counter_read(c), 0); + arena_destroy(ar); +} + +// 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. +TEST(map_helper_losing_stamp_returns_detached_and_keeps_slot) { + Arena *ar = arena_create(); + Map *m = map_create(ar); + + Counter *c = map_counter(m, SK("score"), stmp(10, 1)); + ASSERT(c != NULL); + + 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))); + + // Slot kept its Counter — detached Register did not displace. + 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); +} + +// 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. +TEST(map_counter_cross_replica_merge_recurses) { + Arena *ad = arena_create(); + Arena *as = arena_create(); + Map *dst = map_create(ad); + Map *src = map_create(as); + + Counter *dc = map_counter(dst, SK("votes"), stmp(1, 1)); + Counter *sc = map_counter(src, SK("votes"), stmp(1, 2)); + counter_inc(dc, cid(1), 5); + counter_inc(sc, cid(2), 3); + + map_merge(dst, src); + + Element out; + 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_clone: deep recursive copy into a target arena --- + +TEST(clone_empty_map_is_empty) { + Arena *as = arena_create(); + Arena *ad = arena_create(); + Map *src = map_create(as); + Map *clone = map_clone(ad, src); + ASSERT(clone != NULL); + ASSERT(clone != src); + ASSERT_EQ(map_size(clone), 0); + arena_destroy(as); + arena_destroy(ad); +} + +TEST(clone_preserves_scalar_slots) { + Arena *as = arena_create(); + Arena *ad = arena_create(); + Map *src = map_create(as); + 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); + 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); +} + +// 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); + map_set(src, SK("k"), ES("hello", 5), stmp(1, 1)); + Map *clone = map_clone(ad, src); + arena_destroy(as); + Element out; + ASSERT(map_get(clone, SK("k"), &out) == true); + ASSERT_SCALAR_EQ(out, scalar_string((const uint8_t *)"hello", 5)); + arena_destroy(ad); +} + +// Composite slots are recursively cloned — the clone's nested composites +// are independent objects in dst's arena. +TEST(clone_recurses_into_composite_slots) { + Arena *as = arena_create(); + Arena *ad = arena_create(); + Map *src = map_create(as); + Counter *sc = counter_create(as); + counter_inc(sc, cid(1), 5); + map_set(src, SK("votes"), element_counter(sc), stmp(1, 1)); + + Map *clone = map_clone(ad, src); + + Element out; + ASSERT(map_get(clone, SK("votes"), &out) == true); + ASSERT_EQ(element_kind(out), ELEMENT_COUNTER); + ASSERT(out.as.counter != sc); // recursive clone, independent object + ASSERT_EQ(counter_read(out.as.counter), 5); + + arena_destroy(as); + Element out2; + ASSERT(map_get(clone, SK("votes"), &out2) == true); + ASSERT_EQ(counter_read(out2.as.counter), 5); + arena_destroy(ad); +} + +// 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); + map_set(src, SK("k"), EI(1), stmp(1, 1)); + map_delete(src, SK("k"), stmp(5, 1)); + + Map *clone = map_clone(ad, 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); +} + +// 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); + map_set(src, SK("k"), EI(1), stmp(1, 1)); + Map *clone = map_clone(ad, src); + map_set(src, SK("k"), EI(99), stmp(5, 1)); + map_set(src, SK("new"), EI(7), stmp(1, 1)); + + Element out; + 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); +} + +// 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. +// +// 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. +TEST(clone_tombstone_does_not_recurse_into_stale_value) { + Arena *as = arena_create(); + Arena *ad = arena_create(); + Map *src = map_create(as); + + // Big inner map under the to-be-tombstoned key. + Map *inner = map_create(as); + for (int i = 0; i < 50; 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_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); + + // 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); +} + +int main(void) { RUN(empty_get_returns_false); RUN(set_then_get); RUN(set_overwrites_with_newer_stamp); @@ -683,11 +1029,36 @@ int main(void) { RUN(merge_copies_string_into_dst_arena); RUN(merge_preserves_tombstone_against_older_set); - RUN(merge_same_id_counter_recurses); - RUN(merge_same_id_register_recurses); - RUN(merge_same_id_nested_map_recurses); - RUN(merge_same_id_counter_does_not_mutate_src); - RUN(merge_same_id_counter_advances_slot_stamp); + RUN(merge_same_kind_counter_recurses); + RUN(merge_same_kind_register_recurses); + RUN(merge_same_kind_nested_map_recurses); + RUN(merge_same_kind_counter_does_not_mutate_src); + RUN(merge_same_kind_counter_advances_slot_stamp); + + RUN(set_composite_displaces_scalar_at_lww); + RUN(set_scalar_displaces_composite_at_lww); + RUN(set_different_kind_composite_displaces_at_lww); + + RUN(merge_composite_src_wins_into_empty_slot_clones); + RUN(merge_kind_mismatch_clones_winner_into_dst); + + RUN(map_counter_creates_and_installs_at_key); + RUN(map_counter_returns_same_pointer_on_repeat); + RUN(map_register_creates_and_installs_at_key); + RUN(map_register_returns_same_pointer_on_repeat); + RUN(map_map_creates_and_installs_at_key); + 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_cross_replica_merge_recurses); + + RUN(clone_empty_map_is_empty); + RUN(clone_preserves_scalar_slots); + RUN(clone_survives_src_arena_destroy); + RUN(clone_recurses_into_composite_slots); + RUN(clone_preserves_tombstones); + RUN(clone_independent_of_src); + RUN(clone_tombstone_does_not_recurse_into_stale_value); TEST_SUMMARY(); } diff --git a/test_map_abort.c b/test_map_abort.c deleted file mode 100644 index 7cbad24..0000000 --- a/test_map_abort.c +++ /dev/null @@ -1,45 +0,0 @@ -// Death test: cross-arena composite LWW must host_abort. -// -// src holds a composite (Counter) at "votes" but dst has no entry. map_merge's -// LWW fallthrough must abort rather than store a cross-arena pointer. -// Deterministic id derivation (PR 5) keeps this path unreachable in normal -// use; the abort guards against silent dangling-pointer corruption. -// -// The Makefile target inverts the exit status: success means the binary died. - -#include "arena.h" -#include "clientid.h" -#include "counter.h" -#include "element.h" -#include "elementid.h" -#include "map.h" -#include "stamp.h" -#include -#include - -static ClientId cid(uint8_t first_byte) { - uint8_t b[16] = {0}; - b[0] = first_byte; - return clientid_from_bytes(b); -} - -static ElementId eid(uint8_t origin_byte, uint64_t seq) { - return elementid_new(cid(origin_byte), seq); -} - -int main(void) { - Arena *ad = arena_create(); - Arena *as = arena_create(); - Map *dst = map_create(ad, eid(0xFF, 0)); - Map *src = map_create(as, eid(0xFF, 0)); - Counter *sc = counter_create(as, eid(7, 1)); - counter_inc(sc, cid(1), 3); - map_set(src, (const void *)"votes", 5, element_counter(sc), - (Stamp){.lamport = 5, .client_id = cid(1)}); - - map_merge(dst, src); // expected: host_abort, process dies - - fprintf(stderr, - "test_map_abort: map_merge returned without aborting (bug)\n"); - return 0; // 0 exit will be inverted by Makefile -> failure -} diff --git a/test_register.c b/test_register.c index de38616..fa71ad5 100644 --- a/test_register.c +++ b/test_register.c @@ -1,6 +1,5 @@ #include "arena.h" #include "clientid.h" -#include "elementid.h" #include "register.h" #include "scalar.h" #include "stamp.h" @@ -14,12 +13,6 @@ static ClientId cid(uint8_t first_byte) { return clientid_from_bytes(b); } -static ElementId eid(uint8_t origin_byte, uint64_t seq) { - return elementid_new(cid(origin_byte), seq); -} - -static ElementId default_id(void) { return eid(0xFF, 0); } - // Build a Stamp from lamport + a ClientId's first byte. Tests stay readable. static Stamp stmp(uint64_t lamport, uint8_t client_first_byte) { return (Stamp){.lamport = lamport, .client_id = cid(client_first_byte)}; @@ -27,15 +20,7 @@ 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); -} - -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)); - ASSERT(elementid_eq(register_id(r), id) == true); - arena_destroy(a); + return register_create(arena, value, stamp); } // --- create / read --- @@ -240,8 +225,62 @@ TEST(merge_copies_string_into_dst_arena) { scalar_string((const uint8_t *)"hello", 5))); } +// --- register_clone: deep copy into a target arena --- + +TEST(clone_preserves_value) { + Arena *as = arena_create(); + Arena *ad = arena_create(); + Register *src = register_create(as, scalar_int(42), stmp(5, 1)); + Register *clone = register_clone(ad, 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, scalar_string((const uint8_t *)"hello", 5), stmp(1, 1)); + Register *clone = register_clone(ad, src); + arena_destroy(as); + ASSERT(scalar_eq(register_read(clone), + scalar_string((const uint8_t *)"hello", 5))); + arena_destroy(ad); +} + +// 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, scalar_int(1), stmp(1, 1)); + Register *clone = register_clone(ad, 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); +} + +// Clone preserves the stamp — 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, scalar_int(10), stmp(5, 1)); + Register *clone = register_clone(ad, 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); +} + int main(void) { - RUN(register_create_stores_id); RUN(create_seeds_value); RUN(create_with_string); RUN(create_with_null); @@ -265,5 +304,10 @@ int main(void) { RUN(merge_does_not_mutate_src); RUN(merge_copies_string_into_dst_arena); + RUN(clone_preserves_value); + RUN(clone_string_survives_src_arena_destroy); + RUN(clone_independent_of_src); + RUN(clone_preserves_stamp); + TEST_SUMMARY(); } diff --git a/test_scalar.c b/test_scalar.c index e881136..73804f1 100644 --- a/test_scalar.c +++ b/test_scalar.c @@ -1,6 +1,9 @@ +#include "arena.h" #include "scalar.h" +#include "string.h" #include "test_util.h" #include +#include // --- constructors set the right kind/value --- @@ -138,6 +141,72 @@ 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); +} + int main(void) { RUN(null_has_null_kind); RUN(bool_true_kind_and_value); @@ -166,5 +235,13 @@ int main(void) { RUN(int_neq_string); RUN(string_neq_null); + 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_empty_string_handled); + RUN(clone_string_with_embedded_nul); + TEST_SUMMARY(); } From 6a6591b86241d57e88434a31602c892e35452bea Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Wed, 3 Jun 2026 20:51:28 -0300 Subject: [PATCH 2/3] docs: refresh header comments for positional-identity model --- counter.h | 5 +++-- map.h | 38 +++++++++++++++++++++++--------------- register.h | 6 +++--- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/counter.h b/counter.h index a7968b5..ff4ea6b 100644 --- a/counter.h +++ b/counter.h @@ -2,8 +2,9 @@ #define _CRDT_COUNTER_H // PN-Counter: integer counter with concurrent increments and decrements. -// Carries an ElementId set at create, exposed via counter_id — that's how -// parent containers identify "same logical Counter across replicas". +// Identity is positional: a Counter is "the Counter for this slot" via the +// (key, kind) tuple of the containing Map. The Counter struct itself holds +// no identifier. // // Semantics: // - Per-client (inc, dec) tallies, one CounterEntry per ClientId that diff --git a/map.h b/map.h index ff34baa..d45e3cf 100644 --- a/map.h +++ b/map.h @@ -2,8 +2,9 @@ #define _CRDT_MAP_H // LWW Map with tombstones, keyed on raw bytes (binary-safe), Element-valued. -// Map itself carries an ElementId, set at create, exposed via map_id — -// that's how parent containers identify "same logical Map across replicas". +// Identity for a composite slot is positional: "the Counter / Register / +// nested Map at this key in this Map." The composites themselves hold no +// identifier; map_merge uses (key, kind) to decide whether to recurse. // // Semantics: // - Each slot carries a Stamp. set / delete take effect iff the new stamp @@ -15,22 +16,23 @@ // same delete decision. // // Merge (per src slot): -// - Both alive, same composite kind (REGISTER / COUNTER / MAP) and same -// ElementId → element_merge(dst, src) recurses in place. Slot stamps -// are ignored on this path; the composite owns its own merge order. -// - Otherwise → LWW on slot stamp. Scalar winners are dup'd into dst's -// arena. A composite winner is a cross-arena dangling-pointer hazard; -// map_merge host_aborts. Deterministic id derivation keeps this path -// unreachable in normal use. +// - Both alive AND same composite kind (REGISTER / COUNTER / MAP) → +// element_merge(dst, src) recurses in place. Slot stamp advances to +// max(dst, src) so future slot-level ops stay LWW-deterministic. +// - 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. // // Ownership: -// - SCALAR_STRING values are dup'd into the Map's arena on every accepted +// - 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. -// The pointed-to object must live in the same arena as the Map, or at -// least outlive it. map_set does not clone composites. +// 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). // @@ -61,14 +63,20 @@ void map_merge(Map *dst, const Map *src); // Count of live (non-tombstone) entries. size_t map_size(const Map *map); -// Helper for "get or create" pattern: returns the Counter at key if present; +// Get-or-create helpers. Behaviour per call: +// 1. Live slot with matching kind at `key` → return the existing pointer +// (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. Counter *map_counter(Map *map, const void *key, size_t key_len, Stamp stamp); -// Helper for "get or create" pattern: returns the Register at key if present; Register *map_register(Map *map, const void *key, size_t key_len, Scalar seed, Stamp stamp); -// Helper for "get or create" pattern: returns the Map at key if present; Map *map_map(Map *map, const void *key, size_t key_len, Stamp stamp); Map *map_clone(Arena *arena, const Map *map); diff --git a/register.h b/register.h index e8fd7eb..19c82b9 100644 --- a/register.h +++ b/register.h @@ -2,9 +2,9 @@ #define _CRDT_REGISTER_H // LWW (last-writer-wins) Register holding a Scalar value with a -// (lamport, client_id) stamp. Carries an ElementId set at create, -// exposed via register_id — that's how parent containers identify -// "same logical Register across replicas". +// (lamport, client_id) stamp. Identity is positional: a Register is "the +// Register for this slot" via the (key, kind) tuple of the containing Map. +// The Register struct itself holds no identifier. // // Semantics: // - Register always holds a value (seeded at create); there is no "unset" From 92ea0e9fdbbb5232623b2c0220d24b0a878e5d3d Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Wed, 3 Jun 2026 20:57:43 -0300 Subject: [PATCH 3/3] fix: avoid wasted clone on losing LWW merge; guard memcmp on empty strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - map_merge: skip element_clone + map_set when src would lose LWW against an existing dst entry. The clone is a deep recursive copy into dst's arena, and map_set would discard the write anyway — so the work was guaranteed-unreachable when src lost. - scalar_eq: short-circuit zero-length string comparison before memcmp. memcmp on NULL pointers is UB pre-C2x even with size 0; the original byte loop was safe here, my prior refactor was not. --- map.c | 8 ++++++++ scalar.c | 5 +++++ test_map.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/map.c b/map.c index e9d38f8..8bf1cd0 100644 --- a/map.c +++ b/map.c @@ -144,6 +144,14 @@ void map_merge(Map *dst, const Map *src) { continue; } + // 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. + if (dst_has && !stamp_gt(se->stamp, de->stamp)) { + continue; + } + Element cloned = element_clone(dst->arena, se->value); map_set(dst, k, klen, cloned, se->stamp); } diff --git a/scalar.c b/scalar.c index 8da2014..c19c5c6 100644 --- a/scalar.c +++ b/scalar.c @@ -48,6 +48,11 @@ bool scalar_eq(Scalar a, Scalar b) { if (a.as.s.len != b.as.s.len) { return false; } + // Guard zero-length: memcmp(NULL, NULL, 0) is UB pre-C2x even though + // libc impls typically tolerate it. Two empty strings are equal. + if (a.as.s.len == 0) { + return true; + } return memcmp(a.as.s.bytes, b.as.s.bytes, a.as.s.len) == 0; } } diff --git a/test_map.c b/test_map.c index fa78bfc..24711c2 100644 --- a/test_map.c +++ b/test_map.c @@ -660,6 +660,43 @@ TEST(merge_composite_src_wins_into_empty_slot_clones) { arena_destroy(ad); } +// 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. +TEST(merge_does_not_clone_when_src_loses_lww) { + Arena *ad = arena_create(); + Arena *as = arena_create(); + Map *dst = map_create(ad); + Map *src = map_create(as); + + // 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); + 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)); + + 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); +} + TEST(merge_kind_mismatch_clones_winner_into_dst) { Arena *ad = arena_create(); Arena *as = arena_create(); @@ -1040,6 +1077,7 @@ int main(void) { RUN(set_different_kind_composite_displaces_at_lww); RUN(merge_composite_src_wins_into_empty_slot_clones); + RUN(merge_does_not_clone_when_src_loses_lww); RUN(merge_kind_mismatch_clones_winner_into_dst); RUN(map_counter_creates_and_installs_at_key);