From ceb340e2eb5b4c8edee8d0dbd71195389ebd86b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20P=C3=A9rez=20de=20Algaba?= <124347725+jperezdealgaba@users.noreply.github.com> Date: Mon, 17 Aug 2026 19:45:40 +0200 Subject: [PATCH] fix: prevent PyNvVideoCodec decoder slot limit bypass via ClassVar shadowing (#52126) Signed-off-by: jperezde --- tests/multimodal/test_video.py | 163 ++++++++++++++++++++++----------- vllm/multimodal/video.py | 70 ++++++++------ 2 files changed, 151 insertions(+), 82 deletions(-) diff --git a/tests/multimodal/test_video.py b/tests/multimodal/test_video.py index 8e27a5c58f..17d29f850e 100644 --- a/tests/multimodal/test_video.py +++ b/tests/multimodal/test_video.py @@ -24,12 +24,14 @@ from vllm.multimodal.video import ( Molmo2VideoBackend, PyNvVideoCodecDecoderSlot, PyNvVideoCodecVideoBackend, + PyNvVideoCodecVideoBackendMixin, Qwen2VLVideoBackend, Qwen3VLVideoBackend, VideoBackend, VideoLoader, VideoSourceMetadata, VideoTargetMetadata, + _pynv_decoder_pool, get_video_loader_backend_for_processor, ) from vllm.platforms import current_platform @@ -47,6 +49,27 @@ FAKE_OUTPUT_1 = np.random.rand(NUM_FRAMES, 1280, 720, 3) FAKE_OUTPUT_2 = np.random.rand(NUM_FRAMES, 1280, 720, 3) +@contextmanager +def _fresh_decoder_pool(): + """Reset module-level decoder pool for isolated test runs.""" + pool = _pynv_decoder_pool + old_slots = pool.slots + old_active = pool.active + old_cond = pool.cond + old_max = pool.max_slots + pool.slots = [] + pool.active = 0 + pool.cond = threading.Condition() + pool.max_slots = None + try: + yield pool + finally: + pool.slots = old_slots + pool.active = old_active + pool.cond = old_cond + pool.max_slots = old_max + + @VIDEO_LOADER_REGISTRY.register("test_video_loader_1") class TestVideoLoader1(VideoLoader): @classmethod @@ -206,16 +229,7 @@ def test_pynvvideocodec_corrupted_videos_raise_value_error(): corrupted_video = (ASSETS_DIR / "corrupted.mp4").read_bytes() malformed_video = corrupted_video[:128] - old_slots = PyNvVideoCodecVideoBackend._decoder_slots - old_active_slots = PyNvVideoCodecVideoBackend._active_decoder_slots - old_cond = PyNvVideoCodecVideoBackend._decoder_slot_cond - old_max_slots = PyNvVideoCodecVideoBackend._max_decoder_slots - try: - PyNvVideoCodecVideoBackend._decoder_slots = [] - PyNvVideoCodecVideoBackend._active_decoder_slots = 0 - PyNvVideoCodecVideoBackend._decoder_slot_cond = threading.Condition() - PyNvVideoCodecVideoBackend._max_decoder_slots = None - + with _fresh_decoder_pool(): loader = VIDEO_LOADER_REGISTRY.load(PYNVVIDEOCODEC_VIDEO_BACKEND) with pytest.raises( ValueError, @@ -247,11 +261,6 @@ def test_pynvvideocodec_corrupted_videos_raise_value_error(): hw_decoders=1, ) assert frames.shape[0] == 1 - finally: - PyNvVideoCodecVideoBackend._decoder_slots = old_slots - PyNvVideoCodecVideoBackend._active_decoder_slots = old_active_slots - PyNvVideoCodecVideoBackend._decoder_slot_cond = old_cond - PyNvVideoCodecVideoBackend._max_decoder_slots = old_max_slots @pytest.mark.parametrize("hw_decoders", [1, 3]) @@ -263,15 +272,7 @@ def test_pynvvideocodec_decoder_slots_are_bounded( pass create_count = 0 - old_slots = PyNvVideoCodecVideoBackend._decoder_slots - old_active_slots = PyNvVideoCodecVideoBackend._active_decoder_slots - old_cond = PyNvVideoCodecVideoBackend._decoder_slot_cond - old_max_slots = PyNvVideoCodecVideoBackend._max_decoder_slots - try: - PyNvVideoCodecVideoBackend._decoder_slots = [] - PyNvVideoCodecVideoBackend._active_decoder_slots = 0 - PyNvVideoCodecVideoBackend._decoder_slot_cond = threading.Condition() - PyNvVideoCodecVideoBackend._max_decoder_slots = None + with _fresh_decoder_pool(): PyNvVideoCodecVideoBackend._configure_decoder_slots(hw_decoders) def fake_create_slot(cls): @@ -309,17 +310,12 @@ def test_pynvvideocodec_decoder_slots_are_bounded( assert seen_slots[0] in retained_slots assert create_count == hw_decoders - finally: - PyNvVideoCodecVideoBackend._decoder_slots = old_slots - PyNvVideoCodecVideoBackend._active_decoder_slots = old_active_slots - PyNvVideoCodecVideoBackend._decoder_slot_cond = old_cond - PyNvVideoCodecVideoBackend._max_decoder_slots = old_max_slots def test_pynvvideocodec_decoder_slots_are_configured_once( monkeypatch: pytest.MonkeyPatch, ): - monkeypatch.setattr(PyNvVideoCodecVideoBackend, "_max_decoder_slots", None) + monkeypatch.setattr(_pynv_decoder_pool, "max_slots", None) PyNvVideoCodecVideoBackend._configure_decoder_slots(2) PyNvVideoCodecVideoBackend._configure_decoder_slots(2) @@ -358,15 +354,16 @@ def test_pynvvideocodec_failed_rebuild_invalidates_decoder_slot(): assert slot.source_path is None raise RuntimeError("construct failed") - old_slots = PyNvVideoCodecVideoBackend._decoder_slots - old_active_slots = PyNvVideoCodecVideoBackend._active_decoder_slots - old_cond = PyNvVideoCodecVideoBackend._decoder_slot_cond - old_max_slots = PyNvVideoCodecVideoBackend._max_decoder_slots + pool = _pynv_decoder_pool + old_slots = pool.slots + old_active = pool.active + old_cond = pool.cond + old_max = pool.max_slots try: - PyNvVideoCodecVideoBackend._decoder_slots = [slot] - PyNvVideoCodecVideoBackend._active_decoder_slots = 1 - PyNvVideoCodecVideoBackend._decoder_slot_cond = threading.Condition() - PyNvVideoCodecVideoBackend._max_decoder_slots = 1 + pool.slots = [slot] + pool.active = 1 + pool.cond = threading.Condition() + pool.max_slots = 1 with ( pytest.raises(RuntimeError, match="construct failed"), @@ -386,12 +383,12 @@ def test_pynvvideocodec_failed_rebuild_invalidates_decoder_slot(): assert old_decoder.poisoned assert slot.decoder is None assert slot.source_path is None - assert PyNvVideoCodecVideoBackend._decoder_slots == [slot] + assert pool.slots == [slot] finally: - PyNvVideoCodecVideoBackend._decoder_slots = old_slots - PyNvVideoCodecVideoBackend._active_decoder_slots = old_active_slots - PyNvVideoCodecVideoBackend._decoder_slot_cond = old_cond - PyNvVideoCodecVideoBackend._max_decoder_slots = old_max_slots + pool.slots = old_slots + pool.active = old_active + pool.cond = old_cond + pool.max_slots = old_max @pytest.mark.skipif(not current_platform.is_cuda(), reason="Requires CUDA") @@ -405,15 +402,15 @@ def test_pynvvideocodec_h200_recovers_after_unsupported_8k(): valid_video = create_long_gop_video(num_frames=2, width=64, height=64) unsupported_video = (ASSETS_DIR / "unsupported_8k_h264.mp4").read_bytes() - old_slots = PyNvVideoCodecVideoBackend._decoder_slots - old_active_slots = PyNvVideoCodecVideoBackend._active_decoder_slots - old_cond = PyNvVideoCodecVideoBackend._decoder_slot_cond - old_max_slots = PyNvVideoCodecVideoBackend._max_decoder_slots + old_slots = _pynv_decoder_pool.slots + old_active = _pynv_decoder_pool.active + old_cond = _pynv_decoder_pool.cond + old_max = _pynv_decoder_pool.max_slots try: - PyNvVideoCodecVideoBackend._decoder_slots = [] - PyNvVideoCodecVideoBackend._active_decoder_slots = 0 - PyNvVideoCodecVideoBackend._decoder_slot_cond = threading.Condition() - PyNvVideoCodecVideoBackend._max_decoder_slots = None + _pynv_decoder_pool.slots = [] + _pynv_decoder_pool.active = 0 + _pynv_decoder_pool.cond = threading.Condition() + _pynv_decoder_pool.max_slots = None loader = VIDEO_LOADER_REGISTRY.load(PYNVVIDEOCODEC_VIDEO_BACKEND) frames_before, _ = loader.load_bytes( @@ -443,12 +440,66 @@ def test_pynvvideocodec_h200_recovers_after_unsupported_8k(): assert frames_after.shape == frames_before.shape finally: - for slot in PyNvVideoCodecVideoBackend._decoder_slots: + for slot in _pynv_decoder_pool.slots: slot.invalidate() - PyNvVideoCodecVideoBackend._decoder_slots = old_slots - PyNvVideoCodecVideoBackend._active_decoder_slots = old_active_slots - PyNvVideoCodecVideoBackend._decoder_slot_cond = old_cond - PyNvVideoCodecVideoBackend._max_decoder_slots = old_max_slots + _pynv_decoder_pool.slots = old_slots + _pynv_decoder_pool.active = old_active + _pynv_decoder_pool.cond = old_cond + _pynv_decoder_pool.max_slots = old_max + + +def test_pynvvideocodec_cross_subclass_shares_single_pool(): + """Regression test for GHSA-j682-9xp5-rrf3. + + Multiple subclasses of PyNvVideoCodecVideoBackendMixin must share the + same process-wide decoder slot limit rather than getting independent + counters via ClassVar shadowing. + """ + + class FakeSlot: + pass + + create_count = 0 + + def fake_create_slot(cls): + nonlocal create_count + create_count += 1 + return FakeSlot() + + with _fresh_decoder_pool() as pool: + pool.max_slots = 2 + + orig_create = PyNvVideoCodecVideoBackendMixin._create_decoder_slot + PyNvVideoCodecVideoBackendMixin._create_decoder_slot = classmethod( + fake_create_slot + ) + try: + with ExitStack() as stack: + stack.enter_context(VideoBackend._borrow_decoder_slot()) + stack.enter_context(Qwen3VLVideoBackend._borrow_decoder_slot()) + assert pool.active == 2 + + blocked = threading.Event() + acquired = threading.Event() + + def try_borrow(): + blocked.set() + with Qwen2VLVideoBackend._borrow_decoder_slot(): + acquired.set() + + t = threading.Thread(target=try_borrow) + t.start() + blocked.wait(timeout=2.0) + assert not acquired.wait(timeout=0.3) + + assert acquired.wait(timeout=2.0) + t.join(timeout=2.0) + assert not t.is_alive() + + assert create_count == 2 + assert len(pool.slots) == 2 + finally: + PyNvVideoCodecVideoBackendMixin._create_decoder_slot = orig_create @pytest.mark.parametrize("hw_decoders", [0, -1, 1.5, True, "2"]) diff --git a/vllm/multimodal/video.py b/vllm/multimodal/video.py index 8d4d7090ed..c50ce77614 100644 --- a/vllm/multimodal/video.py +++ b/vllm/multimodal/video.py @@ -671,13 +671,38 @@ def _pynvvc_frames_to_nhwc(frames: torch.Tensor) -> torch.Tensor: return frames.contiguous() +class _PyNvDecoderPool: + """Process-wide singleton managing PyNvVideoCodec decoder slot state. + + Prevents subclass counter shadowing (GHSA-j682-9xp5-rrf3) by storing + all mutable pool state in a single module-level instance rather than + in ClassVar attributes that get shadowed by Python's augmented + assignment semantics on subclasses. + """ + + def __init__(self) -> None: + self.slots: list[PyNvVideoCodecDecoderSlot] = [] + self.active: int = 0 + self.cond: threading.Condition = threading.Condition() + self.max_slots: int | None = None + + def configure(self, hw_decoders: int) -> None: + with self.cond: + if self.max_slots is None: + self.max_slots = hw_decoders + elif self.max_slots != hw_decoders: + raise RuntimeError( + "PyNvVideoCodec decoder count is already configured as " + f"{self.max_slots}, got {hw_decoders}" + ) + + +_pynv_decoder_pool = _PyNvDecoderPool() + + class PyNvVideoCodecVideoBackendMixin: """PyNvVideoCodec utilities for GPU-backed frame decode.""" - _decoder_slots: ClassVar[list[PyNvVideoCodecDecoderSlot]] = [] - _active_decoder_slots: ClassVar[int] = 0 - _decoder_slot_cond: ClassVar[threading.Condition] = threading.Condition() - _max_decoder_slots: ClassVar[int | None] = None _DEVICE_INDEX: ClassVar[int] = 0 @classmethod @@ -704,14 +729,7 @@ class PyNvVideoCodecVideoBackendMixin: @classmethod def _configure_decoder_slots(cls, hw_decoders: object) -> None: hw_decoders = validate_pynvvideocodec_hw_decoders(hw_decoders) - with cls._decoder_slot_cond: - if cls._max_decoder_slots is None: - cls._max_decoder_slots = hw_decoders - elif cls._max_decoder_slots != hw_decoders: - raise RuntimeError( - "PyNvVideoCodec decoder count is already configured as " - f"{cls._max_decoder_slots}, got {hw_decoders}" - ) + _pynv_decoder_pool.configure(hw_decoders) @staticmethod @contextmanager @@ -729,28 +747,28 @@ class PyNvVideoCodecVideoBackendMixin: @classmethod @contextmanager def _borrow_decoder_slot(cls): + pool = _pynv_decoder_pool create_slot = False - with cls._decoder_slot_cond: - max_decoder_slots = cls._max_decoder_slots - if max_decoder_slots is None: + with pool.cond: + if pool.max_slots is None: raise RuntimeError("PyNvVideoCodec decoder slots are not configured") while True: - if cls._decoder_slots: - slot = cls._decoder_slots.pop() + if pool.slots: + slot = pool.slots.pop() break - if cls._active_decoder_slots < max_decoder_slots: - cls._active_decoder_slots += 1 + if pool.active < pool.max_slots: + pool.active += 1 create_slot = True break - cls._decoder_slot_cond.wait() + pool.cond.wait() if create_slot: try: slot = cls._create_decoder_slot() except Exception: - with cls._decoder_slot_cond: - cls._active_decoder_slots -= 1 - cls._decoder_slot_cond.notify() + with pool.cond: + pool.active -= 1 + pool.cond.notify() raise borrow_succeeded = False @@ -760,9 +778,9 @@ class PyNvVideoCodecVideoBackendMixin: finally: if not borrow_succeeded: slot.invalidate() - with cls._decoder_slot_cond: - cls._decoder_slots.append(slot) - cls._decoder_slot_cond.notify() + with pool.cond: + pool.slots.append(slot) + pool.cond.notify() @staticmethod def _metadata_value(metadata, *names: str, default=None):