[Bugfix] Fix allocation & free logic of SingleWriterShmRingBuffer

[imkero]

Purpose

Fix the buggy free logic of SingleWriterShmRingBuffer, it may break multimodal shm cache because of over-allocation.

details:

1. correct the data_buffer_start computation in SingleWriterShmRingBuffer.free_buf
2. SingleWriterShmRingBuffer.metadata should be empty on init

Test Plan

Test case added in tests.distributed.test_shm_buffer

python -m unittest tests.distributed.test_shm_buffer.TestSingleWriterShmRingBuffer.test_allocation_cycles

Test Result

Test Result (main): FAILED

python -m unittest tests.distributed.test_shm_buffer.TestSingleWriterShmRingBuffer.test_allocation_cycles
F
======================================================================
FAIL: test_allocation_cycles (tests.distributed.test_shm_buffer.TestSingleWriterShmRingBuffer.test_allocation_cycles)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/workspaces/vllm/tests/distributed/test_shm_buffer.py", line 172, in test_allocation_cycles
    ring_allocate(2)
  File "/workspaces/vllm/tests/distributed/test_shm_buffer.py", line 160, in ring_allocate
    mark_allocated_with_assertion(monotonic_id, addr, allocate_size_with_md)
  File "/workspaces/vllm/tests/distributed/test_shm_buffer.py", line 135, in mark_allocated_with_assertion
    self.assertEqual(count_allocated(allocated_bitmap[addr : addr + size]), 0)
AssertionError: 10 != 0

Ran 1 test in 0.002s
FAILED (failures=1)

Test Result (this PR): OK

python -m unittest tests.distributed.test_shm_buffer.TestSingleWriterShmRingBuffer.test_allocation_cycles
.
----------------------------------------------------------------------
Ran 1 test in 0.006s
OK

---

Essential Elements of an Effective PR Description Checklist

• The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
• The test plan, such as providing test command.
• The test results, such as pasting the results comparison before and after, or e2e results
• (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
• (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

[ywang96]

/gemini-review

[gemini-code-assist]

Code Review

This pull request provides a much-needed fix for the allocation and freeing logic in SingleWriterShmRingBuffer. The previous implementation had several bugs related to pointer management and wrap-around behavior, which could lead to memory over-allocation and incorrect state. The new implementation is a complete rewrite of the core logic, introducing a clearer and more robust state management for the ring buffer. Key improvements include treating start == end as an empty buffer, reserving one byte to distinguish empty from full, and resetting pointers to zero when the buffer is empty to maximize contiguous allocation. The allocation logic is now split into two clear cases (wrapped vs. non-wrapped buffer), which is much easier to reason about. The fix to the free_buf logic, which now correctly advances the start pointer, is also crucial. I have thoroughly reviewed the new logic and it appears to be sound and correct, addressing the described bugs effectively. I did not find any high or critical severity issues with the proposed changes.

[gemini-code-assist]

Acknowledged. Initiating a comprehensive code review for this pull request.

[ywang96]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. More of your lovely PRs please.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[dongluw]

would be nice if you can help adding your test case to vllm/tests/distributed/test_shm_buffer.py

[imkero]

would be nice if you can help adding your test case to vllm/tests/distributed/test_shm_buffer.py

Yeah, I put the test case into vllm/tests/distributed/test_shm_buffer.py (test_allocation_cycles) with necessary assertions on allocation / free ops.

[dongluw]

looks good to me, thanks @imkero
maybe @ywang96 can have another look

[ywang96]

I updated an attribute error since np.bool has been deprecated, but otherwise LGTM!