[BUG] SingleMutableCsr::put_edge has no locking — data race on concurrent writes to same vertex

[zhanglei1949]

Describe the bug

SingleMutableCsr::put_edge performs writes to the neighbor list without any locking mechanism. If two threads call put_edge on the same source vertex concurrently, this results in a data race on the MutableNbr fields (neighbor, data, timestamp).

Affected location

include/neug/storages/csr/mutable_csr.h:343-361

std::pair<int32_t, const void*> put_edge(vid_t src, vid_t dst,
                                         const EDATA_T& data, timestamp_t ts,
                                         Allocator& alloc) override {
    if (src >= vertex_capacity()) {
        THROW_INVALID_ARGUMENT_EXCEPTION(/* ... */);
    }
    auto* nbrs = reinterpret_cast<nbr_t*>(nbr_list_->GetData());
    nbrs[src].neighbor = dst;           // ← non-atomic write
    nbrs[src].data = data;               // ← non-atomic write
    CHECK_EQ(nbrs[src].timestamp,       // ← implicit atomic load (operator T())
             std::numeric_limits<timestamp_t>::max());
    nbrs[src].timestamp.store(ts);      // ← atomic store
    edge_num_.fetch_add(1, std::memory_order_relaxed);
    // ...
}

The race

Since SingleMutableCsr has no per-vertex lock (unlike MutableCsr which has locks_[src]):

1. Concurrent put_edge on same src: Two threads write neighbor and data simultaneously, resulting in a torn MutableNbr (e.g., neighbor from Thread A and data from Thread B).

2. Concurrent put_edge + read: A reader via get_generic_view could see a MutableNbr with a partially updated neighbor field (e.g., upper 16 bits from old value, lower 16 bits from new value on platforms where vid_t writes are not atomic).

3. CHECK_EQ is not a guard: CHECK_EQ(nbrs[src].timestamp, max) is meant to catch double-insertion, but:

   • In Release builds, CHECK_EQ aborts the process (FATAL), which is a crash, not a graceful handling.
   • In a concurrent scenario, both threads could pass the check before either stores the new timestamp, leading to a race.

4. Unused alloc parameter: The Allocator& alloc parameter is not used, suggesting the method signature was inherited from MutableCsr without adaptation.

Expected behavior

SingleMutableCsr::put_edge should be safe for concurrent calls on different src values (no contention), but must handle concurrent calls on the same src gracefully — either by:

1. Adding a per-vertex lock (like MutableCsr), or
2. Documenting that put_edge for the same src must not be called concurrently (single-writer guarantee), and enforcing it at a higher level.

Suggested fix

Option A — Add per-vertex lock (consistent with MutableCsr):

// Add member:
std::unique_ptr<SpinLock[]> locks_;

// In put_edge:
std::lock_guard<SpinLock> guard(locks_[src]);
// ... rest of the logic ...

Note: This adds memory overhead for the locks array. Since SingleMutableCsr stores exactly one edge per vertex, the lock could alternatively be embedded in the MutableNbr struct or use a striped lock approach.

Option B — Document single-writer constraint:

/// @pre The caller must ensure no concurrent put_edge calls for the same src.
/// This is typically enforced by the transaction layer.
std::pair<int32_t, const void*> put_edge(vid_t src, vid_t dst, ...) {
    // ...
}

Impact

• Severity: Medium — data race on neighbor and data fields. For vid_t (typically uint32_t), writes are likely atomic on x86, but this is not guaranteed by the C++ standard. The real risk is the interleaving of neighbor and data writes, which can produce a semantically incorrect edge (wrong neighbor paired with wrong data).
• Likelihood: Low-Medium — requires concurrent put_edge on the same vertex, which may happen if transaction routing doesn't shard by vertex.
• Note: The CHECK_EQ will cause a FATAL crash if an already-inserted edge is re-inserted, which is more severe than the race itself.