Fix RecursionError in MediaWithBytes unpickling#31191
Fix RecursionError in MediaWithBytes unpickling#31191DarkLight1337 merged 3 commits intovllm-project:mainfrom
Conversation
Guard __getattr__ against recursion when media attribute is not yet set during pickle deserialization. Adds regression test. Fixes: vllm-project#30818 Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com>
There was a problem hiding this comment.
Code Review
This pull request addresses a RecursionError that occurs when unpickling MediaWithBytes objects. The root cause is the __getattr__ method recursively calling itself when the media attribute is not yet set during deserialization. The proposed fix introduces a guard to check for the presence of media in the instance's __dict__, which is a correct and idiomatic way to prevent such recursion. A comprehensive regression test is also added, which validates the fix by performing a pickle roundtrip and asserting attribute delegation works correctly before and after. The changes are well-implemented and effectively resolve the issue.
|
Hi @nrghosh, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
- and fix ordering Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com>
75f5925 to
34a4067
Compare
Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com>
Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com>
Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com> Signed-off-by: dsuhinin <suhinin.dmitriy@gmail.com>
Signed-off-by: Nikhil Ghosh <nikhil@anyscale.com>
Guard
__getattr__against recursion when media attribute is not yet set during pickle deserialization. Adds regression test.Fixes: #30818
Purpose
Fix RecursionError when unpickling MediaWithBytes dataclass, which breaks vLLM usage with Ray. The
__getattr__method recursively calls itself whenself.mediais not yet set during pickle deserialization.Test Plan
pytest tests/multimodal/test_image.py::test_media_with_bytes_pickle_roundtrip -vTest Result
Test verifies pickle roundtrip succeeds and attribute delegation works before and after unpickling. Without the fix, unpickling causes RecursionError: maximum recursion depth exceeded.
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.