[rishabhsinha17]

Summary

Every engine launch creates /tmp/kvcached-tp-<ipc>-<hash>/w<rank>.sock, at TP=1 too, and nothing removed it: the listener thread served the socket until the process died, and kvctl delete only knew about /dev/shm. One directory accumulates per launch (listing in #476).

Root cause

start_worker_listener_thread() binds the socket in a daemon thread with no stop path, shutdown_kvcached() never touched it, and kvctl delete (delete_kv_cache_segment()) unlinks only the shm segment.

Changes

• kvcached/tp_ipc_util.py: track each listener. stop_worker_listener_threads() wakes the accept loop, closes and unlinks the socket, and removes the directory (and the pp<N> subdirectory) once no other worker's socket is left in it. Also registered with atexit for clean exits.
• kvcached/integration/vllm/patches.py: GPUWorkerPatch also wraps Worker.shutdown(), which vLLM runs on every exit path that reaches EngineCore.shutdown(): in-process via UniProcExecutor.shutdown() at TP=1, and in each WorkerProc from worker_main()'s finally block. Releases without Worker.shutdown are left alone.
• shutdown_kvcached() (vLLM and SGLang) stops the listeners too.
• kvcached/utils.py: the directory naming rule moves next to DEFAULT_IPC_NAME as get_tp_socket_dir(ipc_name) so kvctl can derive it without loading the compiled extension. kvctl delete <ipc> now removes the directory as well and says so.

Validation

Current summary at 4cf1090, rebased onto 79d7c70 (conflicts: shutdown_kvcached's globals line vs #469's capacity-record clear, and the cpu manifest; both sides kept).

• tests/test_tp_socket_cleanup.py, 26 tests in the CPU manifest: stop unlinks the socket and removes the dir and pp subdir, keeps the dir while another rank's socket is in it, restart replaces the listener, the 09-17 shutdown-race pair (stop cancels a parked read; stop drains an executing handler before returning), the deferred-teardown retry from shipiyouniao's 26a4d68 (integrations keep state when a drain times out and retry later), the Worker.shutdown patch paths, delete_tp_socket_dir, kvctl delete output.
• Full CPU manifest: 335 passed; the 10 errors and 1 failure are the pre-existing macOS-only fixture issues, byte-identical to a clean run of main 79d7c70 here. ruff 0.11.7, isort 6.0.1 clean; mypy: the same 13 pre-existing notes as main, none in touched files.
• Teardown on real hardware is shipiyouniao's T4 work from the 09-17/09-18 rounds: timeout, drain and retry fault injections through real worker IPC against this branch, which is also where 26a4d68 came from.
• Connection-tracking overhead (09-21 ask): CPU-side microbenchmark of the worker IPC socket path, Apple M1 Pro, VMM calls stubbed so transport plus tracking dominate, one fresh connection per message exactly like _send_and_receive_message. Workloads: kv_tensors_created as the small frequent request, map_to_kv_tensors with 16 offsets, and the transactional unmap as a prepare+commit pair. 9 windows per arm, arms interleaved, per-op median of window medians: small 39.1us vs main 38.3us (+2.1%), map 43.0 vs 41.2 (+4.6%), unmap pair 87.7 vs 83.0 (+5.7%). A third arm with track_conn/untrack_conn patched to no-ops recovers most of the delta (the shipped head is +1.1%/+3.1%/+4.1% over it), so the added cost is the per-connection tracking itself, about 1 to 2us per accepted connection. These percentages are ceilings from a stub that removes the VMM and CUDA work real requests spend their time in; the registry lock is start/stop lifecycle only and never on the accepted-connection path.
• Not re-run on GPU since the rebase.

Found while testing kvcached for vllm-project/aibrix#2419. The shm segment half of the same shutdown gap is #477.

Fixes #476

[shipiyouniao]

The 14 socket-cleanup tests pass on 50090d0, but I reproduced a shutdown race with an accepted connection on Linux:

1. Start the real listener and send a valid message-length header, withholding the body.
2. Wait until the listener enters recv_msg(), then call stop_worker_listener_threads().
3. stop returns after 1.000 s while the listener is still alive.
4. Send the remaining map request body. The listener executes map_to_kv_tensors() and replies with status=success after stop has returned.

I used stubbed VMM functions for this test; this demonstrates the ordering problem, not a reproduced CUDA crash. Both integration shutdown paths call _shutdown_kvcached_impl() after stop_worker_listener_threads(), so the late handler can race allocator teardown.

Please cover both an accepted connection stalled in recv_msg() and a handler already executing a backend operation. Shutdown needs to cancel pending reads/prevent late dispatch and drain active operations before releasing their resources; a timed join followed by unconditional teardown is not sufficient.

[rishabhsinha17]

Thanks for the repro. I turned it into a regression test first; on 50090d0 it fails with "map_to_kv_tensors dispatched after stop returned", and a companion test that parks a handler inside a stubbed VMM call fails with "stop returned while a handler was still executing".

stop() no longer relies on the timed join. The listener tracks accepted connections, and stop() sets the stop flag under the same lock, then shuts each tracked connection down, which wakes a parked recv_msg() with a connection error. Its loop drops a request that finishes arriving after stop has begun and closes later-accepted connections without reading them. Teardown (unlink, rmdir, close) happens only once the thread has exited, so after a successful stop no handler is left in a VMM call. If the thread has not finished one within 5 s, stop tears nothing down, keeps the listener registered for a retry by a later call or the atexit hook, and stop_worker_listener_threads() returns False. Cleanup errors no longer escape the shutdown paths.

New tests pin the stalled read from your recipe, a handler that is already inside a backend operation when stop begins, and the retained listener retry after an incomplete stop. They fail on the previous head and pass on 300bb50 across ten consecutive local runs. I rebased the branch onto current main, resolving against #413 and #373 in tp_ipc_util.py; at the CI pins the local cpu manifest and mypy results are identical to main's, with lint clean.

[rishabhsinha17]

CI caught a mypy error in the stalled-read test's stub on 300bb50, an Event.set() inside an or expression. Amended and repushed, so the head is now 7d32e5e. Nothing else changed, and the torch-free mypy run at the CI pin is clean locally on 3.9, 3.11 and 3.13.

[rishabhsinha17]

Went through 26a4d68 and the rebased head. The caller-side gap was real in 7d32e5e. Both integrations ignored the False from stop_worker_listener_threads(), so a timed-out drain still ran _shutdown_kvcached_impl() against a live handler and reset _kvcached_initialized, which also made a retry impossible. Deferring teardown with the state intact closes it. Your registry rework also fixes two holes our version left, a cleanup exception dropped the entry with nothing left for the atexit retry to find, and a concurrent stop could see the cleared dict and report success while the first call was still draining. The current-thread return in stop() corrects another edge we got wrong, and the restart guard is what you asked for on the replacement path.

I compared both replayed commits line by line against the originals. Commit 2 has one difference, the OSError guard now wrapping the typed response, which is the right resolution. Commit 1 is identical, including a removal of import uuid that was correct before #418 and stale after it. At 4dd7e71 and a2169fe the transactional unmap broadcast would raise NameError. Your commit restores the import, and the squash merge keeps the intermediates out of main.

Locally at 4ddbf76 on macOS py3.11, test_tp_socket_cleanup.py passes 26/26 and the cpu manifest gives 275 passed with the same 1 failure and 10 errors as c48be96 on this machine, all environmental (socket path length under the macOS tmp root, /dev/shm). ruff 0.11.7, isort 6.0.1 and codespell are clean, and CI-mode mypy 1.11.1 is clean on 3.9, 3.11 and 3.13 with output identical to main's plus the new file.

One non-blocking note. test_tp_ipc_worker_failures.py reloads tp_ipc_util per test and leaves six fake listeners parked in a stubbed accept, so the atexit hook now spends 6x5s draining them at interpreter exit, about 30s per CI job that pytest's own clock never shows. Happy to push a small teardown for that here or in a follow-up, whichever you prefer.

Ready from our side. Thanks for finishing the caller side and for the careful rebase.

Withdrawing the unconditional approval pending a rebase and a focused IPC overhead check. The previous T4 teardown fault tests remain valid for their tested head; no new runtime regression is being claimed.

[shipiyouniao]

I revisited my approval and want to narrow it: the cleanup is useful, and the previous T4 timeout/drain/retry tests support the teardown fix. They don't measure the new connection-tracking overhead, though. I've withdrawn the unconditional approval while we close that gap and rebase the current conflicts.

The global listener registry lock is mainly on lifecycle operations. The additional per-listener connection lock is taken for accepted IPC connections, so the relevant check is a matched IPC/serving comparison, not just shutdown tests. Could you include that comparison on the rebased head, covering the small frequent requests as well as map/unmap? I'm not claiming a measured slowdown or a new CUDA race here.

Please also refresh the PR's validation section: it still describes the earlier test count and pending GPU work, although later comments contain the real teardown fault results. One concise current summary would make the scope much easier to review.

[rishabhsinha17]

measured the tracking overhead, about 1 to 2us per accepted connection, a 2 to 6 percent ceiling on a stub that strips the VMM work. the client mirrors _send_and_receive_message, one fresh connection per message, kv_tensors_created, 16-offset map, and the prepare/commit unmap pair, 9 interleaved windows per arm on an M1 Pro, per-op medians vs main. an arm with track_conn/untrack_conn patched out recovers most of the delta, and the registry lock stays on start/stop only. rebased and pushed, full numbers in the refreshed validation section, happy to rerun with a different request mix if you want.

[shipiyouniao]

LGTM at 4cf1090. The missing Linux serving comparison is now covered, alongside the earlier eight real-T4 VMM shutdown/retry cases on this head.

I ran A-B-B-A with Qwen2.5-0.5B-Instruct, FP16, vLLM 0.28 V1, TP=1 with a separate worker process, eager/synchronous scheduling and identical warmup/request sequences. Both arms use the same current-main/public-compatibility baseline and native extension; the difference is this PR.

Metric	Baseline	This PR
Output throughput, total tokens / total measured time	224.48 tok/s	224.09 tok/s
Mean TTFT	3325.01 ms	3350.84 ms
Mean TPOT	43.97 ms	43.92 ms
Complete requests	256/256	256/256

Each round exercised 64 map and 56 transactional-unmap IPC calls. All 65,536 measured output tokens arrived. The -0.17% aggregate throughput difference is within the observed baseline variation; this does not establish a universal overhead bound or multi-GPU qualification.

Both arms still emit identical FTensor cleanup errors at interpreter teardown. Those are recorded separately, not counted as clean-shutdown success. The candidate does stop and remove its worker listener as intended.

@RixinLiu, the listener cleanup change is ready for your final look from my side.

[RixinLiu]

The potential overhead comes from tracking every accepted IPC connection: the listener acquires a lock to add the connection to a set, then acquires it again to remove the connection after handling the request.

I tested this on an A100 40GB with Qwen2.5-7B BF16, vLLM 0.24, and background preallocation enabled using 32 concurrent requests, each with 4,096 input and 128 output tokens, totaling 2,560 measured requests. Every wave exercised 27 map and 23 unmap IPC calls, with physical KV usage growing from 2.41 to 8.31 GiB and returning to 2.41 GiB. Throughput was 372.40 tok/s baseline versus 372.47 tok/s with this PR (+0.019%). No meaningful throughput regression was observed in this workload.