kvcache-ai / kvcache-ai/Mooncake
Classic tebench crashes when the target segment is unavailable
- Dominant language
- C++
- Stars
- 6.6k
- Forks
- 1.2k
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 312
Description
## Bug Report
### Summary
The classic `tebench` initiator dereferences a null segment descriptor when the target segment cannot be opened. A missing target or an incorrect target RPC port causes SIGSEGV instead of a normal error exit.
### Environment
- Commit: `f84c27aa4032a1926676c6b4765fc9dd1be5dcd3`
- Linux x86_64, GCC 12.2, CMake 3.25.1
- Release (`-O3`), `USE_CUDA=OFF`, `USE_TCP=ON`, `USE_TENT=OFF`
- Classic backend, P2P metadata; `MC_FORCE_TCP=1` to avoid RDMA auto-discovery
### Reproduction
Use an unpatched `tebench` binary with shared libraries resolvable. Ensure no listener exists on `127.0.0.1:1`, then run without starting a target:
```bash
MC_FORCE_TCP=1 ./tebench --backend=classic --xport_type=tcp --metadata_type=p2p --target_seg_name=127.0.0.1:1 --seg_type=DRAM --total_buffer_size=4096 --start_block_size=4096 --max_block_size=4096 --start_batch_size=1 --max_batch_size=1 --start_num_threads=1 --max_num_threads=1 --duration=1 --logtostderr=true
```
The original failure was observed with a target port different from the dynamically advertised P2P RPC port. The unused loopback endpoint above exercises the same failed segment lookup path.
### Actual Behavior
Metadata connection refused, then SIGSEGV (shell exit status 139), before benchmark work starts.
### Expected Behavior
Log the unavailable target segment name and exit normally with a nonzero status, without starting worker threads or accessing a missing descriptor.
### Root Cause
`TEBenchRunner::startInitiator()` in `mooncake-transfer-engine/benchmark/te_backend.cpp` does not check the handle or descriptor before sorting buffers:
```cpp
SegmentID handle = engine_->openSegment(name);
auto info = engine_->getMetadata()->getSegmentDescByID(handle);
std::sort(info->buffers.begin(), info->buffers.end(), ...);
```
On lookup failure, `openSegment()` returns the invalid handle (`-1` represented as the unsigned segment type); `getSegmentDescByID()` returns `nullptr`. Accessing `info->buffers` dereferences it.
### Proposed Fix and Validation
Validate the handle and descriptor before accessing buffers. Reject descriptors with no registered buffers as well, since subsequent indexing assumes a nonempty buffer list.
A local guard patch makes the unavailable-target case exit with status 1 and an explicit target error. A valid-target, same-host Classic TCP DRAM `mix` run with consistency checking still succeeds. The regression test should check a normal error exit and a target-specific diagnostic, distinguishing SIGSEGV from expected failure.
### Related Work
- #2343 fixed a similar unchecked descriptor in `sendNotifyByID()`, not in `tebench`.
- #3562 records connection-refused/segmentation-fault failures in legacy `tcp_transport_test` with a fixed/dynamic P2P port mismatch. This is a related observation, not a claim that its crash site is identical or that this fix resolves it.
- #516 concerns exception unwinding with PyTorch/libunwind, not this null descriptor access.
### Before Submitting
- [x] Searched related issues/PRs and consulted benchmark documentation.
AI assistance was used to investigate the code path and draft this report.
Contributor guide
Research direction
Start in mooncake-transfer-engine/benchmark/te_backend.cpp at TEBenchRunner::startInitiator(), then run the provided tebench command against 127.0.0.1:1 to reproduce the failure. Add regression coverage for an unavailable target and verify that the command exits nonzero with a target-specific diagnostic instead of SIGSEGV, without starting worker threads.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cmake, cpp, linux
- Domain
- cli, networking, testing
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100