From d66a8a98b67e47429b44b12c2ab30c7276a517a1 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 7 May 2026 09:51:37 +0000 Subject: [PATCH] fix: use acquire/release ordering for inuse atomic ops in xzarray (P0-2) Changed all memory_order_relaxed accesses on xzarray_item_t.inuse to use: - memory_order_acquire for loads - memory_order_release for stores This ensures proper synchronization semantics as specified in the P0-2 spec, even in xzarray_thaw() where accesses are under an exclusive write lock. 15 sites changed: - xzarray_destroy: 1 load (line 424) - xzarray_ensure_items: 1 store, 1 load (lines 457, 464) - xzarray_store: 1 load, 1 store (lines 487, 489) - xzarray_get_vec: 3 loads (lines 525, 529, 543) - xzarray_put_vec: 1 load, 1 store (lines 591, 600) - xzarray_clear: 1 load, 1 store (lines 624, 631) - xzarray_iter_next: 1 load (line 654) - xzarray_thaw: 2 loads, 1 store (lines 732, 743, 745) All COW tests pass (test_freeze_thaw_slots_exist through test_double_freeze_fails). Same 4 pre-existing failures expected. --- src/lib/zarray.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/lib/zarray.c b/src/lib/zarray.c index abd050a3..88df02fd 100644 --- a/src/lib/zarray.c +++ b/src/lib/zarray.c @@ -421,7 +421,7 @@ static void xzarray_destroy(meta_obj_t* obj) } for (i = 0; i < (int)a->caps; i++) { - if (atomic_load_explicit(&a->data[i].inuse, memory_order_relaxed)) { + if (atomic_load_explicit(&a->data[i].inuse, memory_order_acquire)) { if (a->killer != NULL) { a->killer(a->data[i].data); } @@ -454,14 +454,14 @@ static int xzarray_ensure_items(xzarray_t* array, int mincap) array->data = p; for (i = array->caps; i < nc; i++) { - atomic_store_explicit(&array->data[i].inuse, false, memory_order_relaxed); + atomic_store_explicit(&array->data[i].inuse, false, memory_order_release); array->data[i].data = NULL; } /* this will make we can get next free * location quickly */ - if (atomic_load_explicit(&array->data[array->pos].inuse, memory_order_relaxed)) { + if (atomic_load_explicit(&array->data[array->pos].inuse, memory_order_acquire)) { array->pos = array->caps; } @@ -484,9 +484,9 @@ static int xzarray_store(meta_obj_t* obj, void* data) idx = a->pos; for (i = 0; i < a->caps; i++) { - if (!atomic_load_explicit(&a->data[idx].inuse, memory_order_relaxed)) { + if (!atomic_load_explicit(&a->data[idx].inuse, memory_order_acquire)) { a->data[idx].data = data; - atomic_store_explicit(&a->data[idx].inuse, true, memory_order_relaxed); + atomic_store_explicit(&a->data[idx].inuse, true, memory_order_release); a->count++; rc = idx; @@ -522,11 +522,11 @@ static int xzarray_get_vec(meta_obj_t* obj, int count, int* idxs, void** data) /* During COW, check active first, then shadow */ bool in_active = (idx < (int)a->caps) && - atomic_load_explicit(&a->data[idx].inuse, memory_order_relaxed); + atomic_load_explicit(&a->data[idx].inuse, memory_order_acquire); bool in_shadow = !in_active && priv->in_cow && priv->shadow != NULL && (idx < (int)priv->shadow_caps) && - atomic_load_explicit(&priv->shadow[idx].inuse, memory_order_relaxed); + atomic_load_explicit(&priv->shadow[idx].inuse, memory_order_acquire); if (!in_active && !in_shadow) { rc = -ENOENT; @@ -540,7 +540,7 @@ static int xzarray_get_vec(meta_obj_t* obj, int count, int* idxs, void** data) /* Active takes precedence over shadow */ if ((idx < (int)a->caps) && - atomic_load_explicit(&a->data[idx].inuse, memory_order_relaxed)) { + atomic_load_explicit(&a->data[idx].inuse, memory_order_acquire)) { data[i] = a->data[idx].data; } else { data[i] = priv->shadow[idx].data; @@ -588,7 +588,7 @@ static int xzarray_put_vec(meta_obj_t* obj, int count, int* idxs, void** data) for (i = 0; i < count; i++) { int idx = idxs[i]; - if (atomic_load_explicit(&a->data[idx].inuse, memory_order_relaxed)) { + if (atomic_load_explicit(&a->data[idx].inuse, memory_order_acquire)) { if (a->killer != NULL) { a->killer(a->data[idx].data); } @@ -597,7 +597,7 @@ static int xzarray_put_vec(meta_obj_t* obj, int count, int* idxs, void** data) } a->data[idx].data = data[i]; - atomic_store_explicit(&a->data[idx].inuse, true, memory_order_relaxed); + atomic_store_explicit(&a->data[idx].inuse, true, memory_order_release); } } @@ -621,14 +621,14 @@ static int xzarray_clear(meta_obj_t* obj, int idx) if (idx < (int)a->caps) { rc = 0; - if (atomic_load_explicit(&a->data[idx].inuse, memory_order_relaxed)) { + if (atomic_load_explicit(&a->data[idx].inuse, memory_order_acquire)) { if (a->killer != NULL) { a->killer(a->data[idx].data); } a->count--; } - atomic_store_explicit(&a->data[idx].inuse, false, memory_order_relaxed); + atomic_store_explicit(&a->data[idx].inuse, false, memory_order_release); a->data[idx].data = NULL; a->pos = idx; /* we can find the available idx next time */ } @@ -651,7 +651,7 @@ static int xzarray_iter_next(iterator_t* iter, void** data) int i; for (i = (int)(long)iter->data; i < (int)a->caps; i++) { - if (atomic_load_explicit(&a->data[i].inuse, memory_order_relaxed)) { + if (atomic_load_explicit(&a->data[i].inuse, memory_order_acquire)) { *data = a->data[i].data; iter->data = (void*)(long)(i + 1); @@ -729,7 +729,7 @@ static int xzarray_thaw(meta_obj_t* obj, retain_fn_t should_retain, void* arg) /* Merge: for each shadow entry not already written to active, copy it */ for (size_t i = 0; i < priv->shadow_caps; i++) { - if (!atomic_load_explicit(&priv->shadow[i].inuse, memory_order_relaxed)) + if (!atomic_load_explicit(&priv->shadow[i].inuse, memory_order_acquire)) continue; void* item = priv->shadow[i].data; @@ -740,9 +740,9 @@ static int xzarray_thaw(meta_obj_t* obj, retain_fn_t should_retain, void* arg) /* Only copy to active if active slot not already updated during COW */ if (i < a->caps && - !atomic_load_explicit(&a->data[i].inuse, memory_order_relaxed)) { + !atomic_load_explicit(&a->data[i].inuse, memory_order_acquire)) { a->data[i].data = item; - atomic_store_explicit(&a->data[i].inuse, true, memory_order_relaxed); + atomic_store_explicit(&a->data[i].inuse, true, memory_order_release); a->count++; } /* If active[i].inuse == true: mutation during COW wins — keep it */