Skip to content

[BUG] put_edge lock leak on exception path — raw lock/unlock instead of RAII #721

Description

@zhanglei1949

Describe the bug

MutableCsr::put_edge uses raw lock()/unlock() instead of RAII guard. If any operation between lock and unlock throws an exception (e.g., Allocator::allocate() throws std::bad_alloc), the SpinLock is never released, causing a permanent deadlock for that vertex.

Affected location

include/neug/storages/csr/mutable_csr.h:134-162

std::pair<int32_t, const void*> put_edge(vid_t src, vid_t dst,
                                         const EDATA_T& data, timestamp_t ts,
                                         Allocator& alloc) override {
    // ... bounds check ...
    locks_[src].lock();                          // ← raw lock
    int sz = sizes[src].load(std::memory_order_relaxed);
    int cap = caps[src];
    if (sz == cap) {
        cap += (cap >> 1);
        cap = std::max(cap, 8);
        nbr_t* new_buffer =
            static_cast<nbr_t*>(alloc.allocate(cap * sizeof(nbr_t)));  // ← can throw!
        // ...
    }
    // ... other operations that could throw ...
    sizes[src].store(sz + 1, std::memory_order_release);
    locks_[src].unlock();                        // ← raw unlock
    return {sz, data_ptr};
}

Inconsistency

The same file's DetachVertex method (line 206) correctly uses RAII:

std::lock_guard<SpinLock> guard(locks_[vid]);

This confirms the raw lock/unlock in put_edge is an oversight, not a design choice.

Expected behavior

The lock should be automatically released if an exception occurs between lock() and unlock(), ensuring the vertex remains accessible for subsequent operations.

Suggested fix

Replace raw lock/unlock with std::lock_guard:

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** buffers = reinterpret_cast<nbr_t**>(adj_list_buffer_->GetData());
    auto* sizes = reinterpret_cast<std::atomic<int>*>(degree_list_->GetData());
    auto* caps = reinterpret_cast<int*>(cap_list_->GetData());

    std::lock_guard<SpinLock> guard(locks_[src]);   // ← RAII
    int sz = sizes[src].load(std::memory_order_relaxed);
    int cap = caps[src];
    // ... rest of the logic ...
    sizes[src].store(sz + 1, std::memory_order_release);
    return {sz, data_ptr};
}

Impact

  • Severity: High — permanent deadlock on the affected vertex, causing all subsequent operations on that vertex to hang indefinitely.
  • Likelihood: Low-Medium — requires alloc.allocate() or another operation to throw, which happens under memory pressure.
  • Reproducibility: Hard to reproduce deterministically, but can occur under memory-constrained environments or corrupted allocator state.

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingstoreStorage layer

Type

No type

Projects

Status
To do

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions