kvcache-ai / kvcache-ai/Mooncake

[P2P-Store] Same-region concurrent Add/Remove races in RegisteredMemory

Open
#3,143 2 comments 0 reactions 1 assignee Claimed by @Icedcoco View on GitHub
Dominant language
C++
Stars
6.6k
Forks
1.2k
Avg merge
3d 5h
Merged PRs (30d)
312

Description

## Context

Follow-up from #2375 review discussion with @Icedcoco. Two same-region concurrency windows exist in `RegisteredMemory` (`mooncake-p2p-store/src/p2pstore/registered_memory.go`) that predate #2375:

## Race 1: concurrent Add() during registration

`Add()` inserts the `bufferList` entry (refCount=1) and releases the lock **before** the underlying `registerLocalMemory` chunks complete. A concurrent `Add()` for the same region hits the entry, increments refCount and returns success — but the memory may not be registered yet, and if the original registration fails, the rollback (added in #2375) removes the entry while the second caller still believes it holds a reference.

## Race 2: Add() overlapping a final Remove()

The last `Remove()` deletes the entry and releases the lock **before** the underlying `unregisterLocalMemory` calls finish. A new `Add()` for the same region can start registering while unregistration of the same addresses is still in flight.

## Open design question

Are same-region concurrent Add/Remove operations intended to be supported?

- If **yes**: entries need a per-entry state (e.g. registering/ready/removing) with waiters, or per-region synchronization held across the underlying register/unregister — a change to the synchronization semantics for all callers.
- If **no**: a doc comment on `RegisteredMemory` stating the contract (external serialization required for same-region calls) would make the current behavior well-defined.

#2375 narrows the damage (no permanent stale entry on failed Add, no premature unregister on refCount>0) but intentionally does not change the visibility windows.

Happy to take this after #2375 lands, once maintainers weigh in on the intended contract.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.