Skip to content

[Bugfix] Don't reuse engine core payload buffer while zmq is sending it - #50053

Merged
njhill merged 3 commits into
vllm-project:mainfrom
njhill:fix-engine-core-payload-buffer-reuse
Jul 28, 2026
Merged

[Bugfix] Don't reuse engine core payload buffer while zmq is sending it#50053
njhill merged 3 commits into
vllm-project:mainfrom
njhill:fix-engine-core-payload-buffer-reuse

Conversation

@njhill

@njhill njhill commented Jul 27, 2026

Copy link
Copy Markdown
Member

EngineCoreProc.process_output_sockets recycles the msgpack payload bytearray across messages via MsgpackEncoder.encode_into, gated on the tracker returned by send_multipart(copy=False, track=True).

That gate never engages. Socket.send_multipart() returns a tracker for the last frame only, and pyzmq copies any frame smaller than zmq.COPY_THRESHOLD (64KiB), returning the always-done _FINISHED_TRACKER for it. The last frame is a small tensor buffer, so the tracker is always done and pending is dead code, while the payload frame is genuinely zero-copy once it exceeds 64KiB. The next encode_into then overwrites it mid-flight, and the client decodes a newer payload alongside the older message's tensor frames:

RuntimeError: shape '[29, 2]' is invalid for input of size 60
msgspec.ValidationError: cannot unpack non-iterable int object
    - at `$[1][69][3][0]`

Send the payload frame separately so the tracker covers the buffer we actually reuse. pyzmq's copy threshold then works in our favour: a small payload is copied and reused immediately, a large one is tracked properly. Also cap reuse_buffers when reclaiming from pending, which previously could not grow because that path was unreachable.

Seen intermittently in CI as v1/sample/test_logprobs_e2e.py failures, where lm_eval's large prompt-logprobs batches push the payload frame either side of the 64KiB threshold.

Also drop the retention of objects whose buffers were extracted for zero-copy send, both here and in MPClient.pending_messages. For a zero-copy frame pyzmq registers the buffer in its own gc registry (zmq/backend/cython/_zmq.py, _gc.store(data, ...)) and releases it only once libzmq is finished, and frames below zmq.COPY_THRESHOLD are copied outright. tensor_data() hands zmq a memoryview whose base chain reaches the source tensor, so refcounting - not timing - keeps the memory from being freed and reused. The retained references never covered device tensors anyway, since tensor_data() copies those to a temporary host tensor that the retained object does not reference. Neither client path reuses a send buffer, so that bookkeeping had no effect beyond looking like protection it did not provide.

`EngineCoreProc.process_output_sockets` recycles the msgpack payload
bytearray across messages via `MsgpackEncoder.encode_into`, gated on the
tracker returned by `send_multipart(copy=False, track=True)`.

That gate never engages. `Socket.send_multipart()` returns a tracker for
the *last* frame only, and pyzmq copies any frame smaller than
`zmq.COPY_THRESHOLD` (64KiB), returning the always-done
`_FINISHED_TRACKER` for it. The last frame is a small tensor buffer, so
the tracker is always done and `pending` is dead code, while the payload
frame is genuinely zero-copy once it exceeds 64KiB. The next
`encode_into` then overwrites it mid-flight, and the client decodes a
newer payload alongside the older message's tensor frames:

    RuntimeError: shape '[29, 2]' is invalid for input of size 60
    msgspec.ValidationError: cannot unpack non-iterable int object
        - at `$[1][69][3][0]`

Send the payload frame separately so the tracker covers the buffer we
actually reuse. pyzmq's copy threshold then works in our favour: a small
payload is copied and reused immediately, a large one is tracked
properly. Also cap `reuse_buffers` when reclaiming from `pending`, which
previously could not grow because that path was unreachable.

Seen intermittently in CI as v1/sample/test_logprobs_e2e.py failures,
where lm_eval's large prompt-logprobs batches push the payload frame
either side of the 64KiB threshold.

Also drop the retention of objects whose buffers were extracted for
zero-copy send, both here and in `MPClient.pending_messages`. For a
zero-copy frame pyzmq registers the buffer in its own gc registry
(`zmq/backend/cython/_zmq.py`, `_gc.store(data, ...)`) and releases it
only once libzmq is finished, and frames below `zmq.COPY_THRESHOLD` are
copied outright. `tensor_data()` hands zmq a memoryview whose base chain
reaches the source tensor, so refcounting - not timing - keeps the memory
from being freed and reused. The retained references never covered device
tensors anyway, since `tensor_data()` copies those to a temporary host
tensor that the retained object does not reference. Neither client path
reuses a send buffer, so that bookkeeping had no effect beyond looking
like protection it did not provide.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Signed-off-by: Nick Hill <nickhill123@gmail.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@njhill

njhill commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

CI failure unrelated, should be fixed by #50060

@AndreasKaratzas AndreasKaratzas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@njhill
njhill enabled auto-merge (squash) July 28, 2026 13:40
@njhill
njhill merged commit 6453fc0 into vllm-project:main Jul 28, 2026
106 checks passed
@njhill
njhill deleted the fix-engine-core-payload-buffer-reuse branch July 28, 2026 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants