Skip to content

fix(mp/mq): isolate malformed server requests - #4934

Merged
maobaolong merged 5 commits into
LMCache:devfrom
0z5a:fix/mp-mq-malformed-request-isolation
Nov 4, 2026
Merged

maobaolong merged 5 commits into
LMCache:devfrom
0z5a:fix/mp-mq-malformed-request-isolation

Conversation

@0z5a

@0z5a 0z5a commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Incremental review: Files changed against dev.

What this PR does / why we need it:

A malformed DEALER peer could terminate the shared MessageQueueServer polling thread before handler-level exception isolation ran:

  • a request with fewer than three frames hit an assertion;
  • a request with an undecodable MessagePack request type raised before handler dispatch.

Both cases left otherwise healthy clients timing out. This change validates the frame count and isolates request-type decoding failures, logs the malformed request, and continues polling.

The regression test sends both malformed header variants, waits deterministically until the server processes each one, and then verifies that a healthy NOOP request still succeeds and the worker thread remains alive.

Special notes for your reviewers:

Test-first reproduction against the previous implementation:

  • focused test failed because the healthy request timed out;
  • pytest reported an unhandled server-thread AssertionError for the truncated multipart request.

Validation:

  • macOS: uv run pytest -q tests/v1/multiprocess/test_mq.py tests/v1/multiprocess/test_futures.py -> 39 passed, 14 skipped, 134 warnings in 30.27s
  • Linux CPU-only (CUDA_VISIBLE_DEVICES=""): same suite -> 39 passed, 14 skipped, 134 warnings in 47.78s
  • uvx ruff check lmcache/v1/multiprocess/mq.py tests/v1/multiprocess/test_mq.py -> passed
  • uvx ruff format --check lmcache/v1/multiprocess/mq.py tests/v1/multiprocess/test_mq.py -> passed
  • git diff --check -> passed

No GPU benchmark is applicable; this is a MessageQueue control-plane correctness fix.

If applicable:

  • this PR contains user facing changes - docs added
  • this PR contains unit tests

Final-head reproducible integration log

Validated from an isolated worktree at exact commit f6e7a04ab6b03366f4fd0cff9d16b90bfb432c9c on macOS arm64 / Python 3.12.13 with the locally built LMCache native extensions:

python -m pytest -q tests/v1/multiprocess/test_mq.py tests/v1/multiprocess/test_futures.py
39 passed, 14 skipped, 134 warnings in 32.61s

The verbose raw pytest log contains 89 lines and has SHA-256 0aeb5d02e23c7c17d000b92e253407c856a0985e48b3916030630baeb60f979a. The command above is the reviewer-runnable reproduction entry point and exercises the same checked-in tests and candidate code. This is a CPU control-plane/integration change, so a GPU notebook would add an indirect wrapper without increasing coverage; GPU/benchmark PRs carry Run-All notebooks separately.

0z5a added a commit to 0z5a/LMCache that referenced this pull request Sep 24, 2026
@0z5a
0z5a force-pushed the fix/mp-mq-malformed-request-isolation branch from 8618ee8 to 15481ff Compare September 24, 2026 06:40

@maobaolong maobaolong left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@0z5a This is a good catch! LGTM.

dev (LMCache#5614) moved per-request handling out of _main_loop into
_handle_request(). Keep dev's loop and move the malformed-frame-count
guard into _handle_request(), which now logs and returns instead of
asserting.

submit_request() on dev now returns MessagingFuture | MessagingStream,
so the regression test narrows the response like the other dev tests.

Signed-off-by: 0z5a <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
@0z5a
0z5a requested a review from VincyZhang as a code owner October 17, 2026 16:23
@maobaolong maobaolong added the skip expensive ci skip expensive ci, which includes k3 integration test and k3 multiprocess test now label Nov 4, 2026
@maobaolong
maobaolong enabled auto-merge (squash) November 4, 2026 03:33
@github-actions github-actions Bot added the full Run comprehensive tests on this PR label Nov 4, 2026
@maobaolong
maobaolong merged commit 47a3017 into LMCache:dev Nov 4, 2026
43 of 45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full Run comprehensive tests on this PR skip expensive ci skip expensive ci, which includes k3 integration test and k3 multiprocess test now

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants