[stefankoncarevic]

Overview

AsyncLLM.shutdown() stops the engine core before it cancels the background output handler, and the launcher runs shutdown() in an executor thread, so the event loop keeps turning in between. A read already in flight then finds the core gone and raises EngineDeadError, which the handler logs as AsyncLLM output_handler failed. with a full traceback. That is a clean shutdown reported as a crash. This teaches the handler to tell the two apart.

Behaviour does not change. The handler still propagates the error to the output processor and still returns, and a core that dies outside shutdown() is still reported as a failure. Only the log line changes.

Validation

In the (H100) [Nightly] Elastic EP Scaling step of build 96848 the step passes, 5 passed in 1448 s, and still prints two of these:

(APIServer pid=17873) ERROR [async_llm.py:943] AsyncLLM output_handler failed.
(APIServer pid=17873) ERROR [async_llm.py:943] Traceback (most recent call last):
(APIServer pid=17873) ERROR [async_llm.py:943]   File ".../v1/engine/async_llm.py", line 880
(APIServer pid=17873) ERROR [async_llm.py:943]     outputs = await engine_core.get_output_async()
(APIServer pid=17873) ERROR [async_llm.py:943]   File ".../v1/engine/core_client.py", line 1272
(APIServer pid=17873) ERROR [async_llm.py:943]     raise self._format_exception(outputs) from None
(APIServer pid=17873) ERROR [async_llm.py:943] vllm.v1.engine.exceptions.EngineDeadError
INFO [launcher.py:281] [shutdown] API server: engine client stopped

That branch does not carry this change. The line right after the error is what identifies the window: the API server is shutting its engine client down, so the core was asked to stop. I see the same tracebacks in the same group on MI300X, on unmodified main, so this is neither platform specific nor something I introduced.

With the change I no longer see AsyncLLM output_handler failed. or any traceback in that window, on MI300X or on H100. The handler logs one info line instead.

tests/v1/engine/test_async_llm_shutdown_race.py covers both sides: a stop that shutdown() asked for is logged as info, and a death outside shutdown() is still logged as an error. The second test is the one that matters, since it keeps the new branch from swallowing a core that really died.

tests/v1/shutdown/ passes, 19 passed and 4 skipped on L4 and the same on MI300X. tests/multimodal/test_cache.py passes, and so do the four other test files that build an AsyncLLM without running __init__, 84 tests, with no ignored exception from __del__.

Details

Why a one element list. The output handler must not capture self, or the AsyncLLM keeps a circular reference through the task and is never garbage collected, which makes del engine hang. The file already solves this the same way for the logger, with self._logger_ref = [self.logger_manager], so I follow that pattern. I hit the trap first with a plain attribute read inside the closure, and test_async_llm_delete timed out on all four parametrizations.

Where the flag is set. First thing in shutdown(), before the engine core is stopped, because the handler reads it on the event loop while shutdown() runs in an executor thread. Setting it later leaves a window where the core is gone and the flag is still false, which is the case being fixed.

Why the read is guarded. __del__ calls shutdown(), so shutdown() can run on an engine whose __init__ never finished, which is why every other attribute it touches goes through getattr. I do the same, otherwise a half built engine raises AttributeError out of __del__ as noise on top of whatever really went wrong at startup.

Test stub. tests/multimodal/test_cache.py calls AsyncLLM._run_output_handler unbound against a SimpleNamespace that supplies the attributes the method reads. It now supplies _shutting_down_ref too.

Pull Request Checklist

• I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

• AI assistance was used during the creation of this PR.

• Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

• Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

• Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

• Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

[claude]

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.

[github-actions]

✅ @stefankoncarevic, CI is now available for this PR.

• /ci run starts upstream CI; /amd-ci run starts AMD CI only.
• Run commands test your branch with every commit currently on its upstream target branch. If it is behind, the bot merges the target branch into it first (pull before pushing again). Append --allow-stale to a run command to test an outdated branch as is, at your own risk.
• /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
• /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
• /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

[stefankoncarevic]

/ci run

[github-actions]

✅ Triggered Buildkite CI #96882 for commit 111fd60431b5.

[njhill]

Thanks @stefankoncarevic. I have a proposal for a simpler variation: njhill@bb1051e

Please take a look and feel free to cherry-pick here if it looks good to you.