fix(v1): avoid false shutdown failures on clean exit

tests/v1/engine/test_shutdown_cleanup.py

@@ -0,0 +1,124 @@
+# SPDX-License-Identifier: Apache-2.0
+# SPDX-FileCopyrightText: Copyright contributors to the vLLM project
+
+from types import SimpleNamespace
+from unittest.mock import MagicMock
+
+import pytest
+
+import vllm.v1.engine.core_client as core_client_mod
+import vllm.v1.engine.llm_engine as llm_engine_mod
+from vllm.v1.engine.core_client import MPClient
+from vllm.v1.engine.llm_engine import LLMEngine
+
+pytestmark = pytest.mark.skip_global_cleanup
+
+
+class DummyResources:
+    def __init__(self, *, engine_dead: bool, engine_manager):
+        self.engine_dead = engine_dead
+        self.engine_manager = engine_manager
+        self.cleanup = MagicMock()
+
+    def __call__(self):
+        self.cleanup()
+
+
+def test_mp_client_shutdown_marks_engine_dead_before_manager_shutdown():
+    client = object.__new__(MPClient)
+    client._finalizer = MagicMock()
+    client._finalizer.detach.return_value = object()
+
+    engine_manager = MagicMock()
+    client.resources = DummyResources(
+        engine_dead=False,
+        engine_manager=engine_manager,
+    )
+
+    client.shutdown(timeout=3.0)
+
+    assert client.resources.engine_dead is True
+    engine_manager.shutdown.assert_called_once_with(timeout=3.0)
+    client.resources.cleanup.assert_called_once_with()
+
+
+def test_mp_client_monitor_cleans_up_after_clean_engine_exit(
+    monkeypatch: pytest.MonkeyPatch,
+):
+    client = object.__new__(MPClient)
+    client._finalizer = SimpleNamespace(alive=True)
+    client.resources = SimpleNamespace(
+        engine_dead=False,
+        engine_manager=SimpleNamespace(
+            failed_proc_name=None,
+            monitor_engine_liveness=lambda: None,
+        ),
+    )
+    client.shutdown = MagicMock()
+
+    thread_target = None
+
+    class ImmediateThread:
+        def __init__(self, *, target, daemon, name):
+            nonlocal thread_target
+            thread_target = target
+
+        def start(self):
+            thread_target()
+
+    monkeypatch.setattr(core_client_mod, "Thread", ImmediateThread)
+    logger_error = MagicMock()
+    monkeypatch.setattr(core_client_mod.logger, "error", logger_error)
+
+    client.start_engine_core_monitor()
+
+    assert client.resources.engine_dead is False
+    client.shutdown.assert_called_once_with()
+    logger_error.assert_not_called()
+
+
+def test_llm_engine_shutdown_cleans_up_owned_resources(
+    monkeypatch: pytest.MonkeyPatch,
+):
+    llm_engine = object.__new__(LLMEngine)
+    renderer = MagicMock()
+    engine_core = MagicMock()
+    dp_group = object()
+    llm_engine.renderer = renderer
+    llm_engine.engine_core = engine_core
+    llm_engine.dp_group = dp_group
+    llm_engine.external_launcher_dp = False
+
+    shutdown_prometheus = MagicMock()
+    destroy_dp_group = MagicMock()
+    monkeypatch.setattr(llm_engine_mod, "shutdown_prometheus", shutdown_prometheus)
+    monkeypatch.setattr(
+        llm_engine_mod,
+        "stateless_destroy_torch_distributed_process_group",
+        destroy_dp_group,
+    )
+
+    llm_engine.shutdown(timeout=1.5)
+
+    shutdown_prometheus.assert_called_once_with()
+    renderer.shutdown.assert_called_once_with()
+    engine_core.shutdown.assert_called_once_with(timeout=1.5)
+    destroy_dp_group.assert_called_once_with(dp_group)
+    assert llm_engine.renderer is None
+    assert llm_engine.engine_core is None
+    assert llm_engine.dp_group is None
+
+
+def test_llm_engine_shutdown_tolerates_renderer_without_shutdown(
+    monkeypatch: pytest.MonkeyPatch,
+):
+    llm_engine = object.__new__(LLMEngine)
+    llm_engine.renderer = object()
+    llm_engine.engine_core = None
+    llm_engine.dp_group = None
+    llm_engine.external_launcher_dp = False
+    monkeypatch.setattr(llm_engine_mod, "shutdown_prometheus", MagicMock())
+
+    llm_engine.shutdown()
+
+    assert llm_engine.renderer is None

vllm/v1/engine/core_client.py

@@ -653,6 +653,9 @@ def shutdown(self, timeout: float | None = None) -> None:
         if self._finalizer.detach() is not None:
             timeout_str = "default" if timeout is None else f"{timeout}s"
             logger.info("[shutdown] MPClient: start timeout=%s", timeout_str)
+            # Mark shutdown as intentional before tearing down child processes
+            # so the monitor thread can distinguish it from a real crash.
+            self.resources.engine_dead = True
             if self.resources.engine_manager is not None:
                 logger.info_once("[shutdown] MPClient: stopping engine manager")
                 self.resources.engine_manager.shutdown(timeout=timeout)
@@ -698,9 +701,16 @@ def monitor_engine_cores():
             _self = self_ref()
             if not _self or not _self._finalizer.alive or _self.resources.engine_dead:
                 return
+            failed_proc_name = getattr(engine_manager, "failed_proc_name", None)
+            if failed_proc_name is None:
+                # The manager exited cleanly, but the client still owns sockets,
+                # tasks, and other background resources that must be released.
+                _self.shutdown()
+                return
             _self.resources.engine_dead = True
-            logger.warning_once(
-                "[shutdown] MPClient: engine core exited unexpectedly; starting cleanup"
+            logger.error(
+                "Engine core proc %s died unexpectedly, shutting down client.",
+                failed_proc_name,
             )
             _self.shutdown()
             # Note: For MPClient, we don't have a failure callback mechanism

vllm/v1/engine/llm_engine.py

@@ -35,6 +35,7 @@
 from vllm.v1.engine.parallel_sampling import ParentRequest
 from vllm.v1.executor import Executor
 from vllm.v1.metrics.loggers import StatLoggerFactory, StatLoggerManager
+from vllm.v1.metrics.prometheus import shutdown_prometheus
 from vllm.v1.metrics.reader import Metric, get_metrics_snapshot
 from vllm.v1.metrics.stats import IterationStats
 from vllm.v1.utils import record_function_or_nullcontext
@@ -442,7 +443,22 @@ def _cleanup_instance_caches(model) -> None:
             if isinstance(module, TorchCompileWithNoGuardsWrapper):
                 module.cleanup()
 
-    def __del__(self):
+    def shutdown(self, timeout: float | None = None) -> None:
+        shutdown_prometheus()
+
+        if renderer := getattr(self, "renderer", None):
+            if shutdown := getattr(renderer, "shutdown", None):
+                shutdown()
+            self.renderer = None
+
+        if engine_core := getattr(self, "engine_core", None):
+            engine_core.shutdown(timeout=timeout)
+            self.engine_core = None
+
         dp_group = getattr(self, "dp_group", None)
         if dp_group is not None and not self.external_launcher_dp:
             stateless_destroy_torch_distributed_process_group(dp_group)
+            self.dp_group = None
+
+    def __del__(self):
+        self.shutdown()