[LeizhenZhang]

Dev Engineer Review

EngineDeadError.__reduce__ reconstructs the exception from root_cause and restores self.__dict__. This prevents the formatted message in args from being passed back into __init__, while preserving the root-cause attribute. No unrelated source changes are reported. Test execution status is not available.

QA Engineer Review

Modified tests/unittest/executor/test_proxy_fast_death.py. The new tests check pickle round trips with and without a cause, a second pickle hop, and shallow and deep copies. They verify message stability, args, and root-cause type and text. The test file is listed in tests/integration/test_lists/test-db/l0_a10.yml; no manual-QA listing was found. Coverage verdict: sufficient for the reported message-stability behavior.

Per-File QA Perspective

• tensorrt_llm/executor/utils.py: Verify that pickling and copying preserve the rendered message and root_cause. The new reduction method also applies to subclasses through self.__class__.
• tests/unittest/executor/test_proxy_fast_death.py: Tests message stability across pickle and copy operations, including repeated pickling and a None cause. Listed in the CI test list at tests/integration/test_lists/test-db/l0_a10.yml.

Description

Fixes #17910.

Thanks to @StephenLReed for the detailed report. @Lunemere offered to take this issue on 2026-09-25. I have not seen a PR from either of you yet, so here is a small fix. I am glad to close this in favor of yours if you already have one in progress.

EngineDeadError formats its message in __init__ from root_cause and passes that text to RuntimeError.__init__. The default exception reduction rebuilds an exception as cls(*self.args). For EngineDeadError that means the formatted message comes back in as root_cause, so every pickle or copy prefixes it once more.

Reproduction on current main (CPU only):

import copy, pickle
from tensorrt_llm.executor import EngineDeadError

err = EngineDeadError(RuntimeError("PyExecutor has already been shutdown"))
print(str(err))
print(str(pickle.loads(pickle.dumps(err))))
print(str(copy.deepcopy(err)))

Output before this change:

Engine has died: RuntimeError: PyExecutor has already been shutdown
Engine has died: str: Engine has died: RuntimeError: PyExecutor has already been shutdown
Engine has died: str: Engine has died: RuntimeError: PyExecutor has already been shutdown

A second pickle adds a third prefix, and so on. EngineDeadError() with no cause turns into Engine has died: str: Engine has died. root_cause itself survives because __dict__ is restored after __init__, so only the message is affected. The error is the one users see when a worker dies. On main it is raised in the proxy and handled in the same process, so the growing prefix shows up when code passes it to another process or copies it, for example user code or a deployment that forwards errors over multiprocessing. Sending it from a forked child to its parent over a multiprocessing.Pipe shows the doubled prefix in the parent.

Root cause: tensorrt_llm/executor/utils.py, EngineDeadError.__init__ stores the formatted text in self.args, and the class has no __reduce__, so BaseException.__reduce__ returns (cls, self.args, self.__dict__).

Fix: add EngineDeadError.__reduce__ that returns (self.__class__, (self.root_cause,), self.__dict__). Reconstruction now goes through __init__ with the original cause and produces the same message once. Keeping self.__dict__ in the tuple matches the default reduction, so root_cause and any other instance attributes (for example notes added with add_note) still round trip. self.args and str(err) are unchanged for code that creates the error directly.

Test Coverage

tests/unittest/executor/test_proxy_fast_death.py:

• test_engine_dead_error_pickle_round_trip_keeps_message[cause0] and [None]: one and two pickle hops keep str(), args, and the root_cause type and text.
• test_engine_dead_error_copy_keeps_message: copy.copy and copy.deepcopy keep the message and root_cause.

The three new cases fail on current main and pass with this change. The other tests in the file give the same results before and after the change.

PR Checklist

Please review the following before submitting your PR:

• PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

• PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

• Test cases are provided for new code paths (see test instructions)

• If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

• Any new dependencies have been scanned for license and vulnerabilities

• CODEOWNERS updated if ownership changes

• Documentation updated as needed

• Update tava architecture diagram if there is a significant design change in PR.

• The reviewers assigned automatically/manually are appropriate for the PR.

• Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: NVIDIA/TensorRT-LLM
• Branch: main
• File: .coderabbit.yaml
• Commit: 687359e
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 516163b6-c0b8-4566-85c3-60b2abe6f651

Commits

Reviewing files that changed from the base of the PR and between 687359e and fb8b735.

Files selected for processing: 2

• tensorrt_llm/executor/utils.py
• tests/unittest/executor/test_proxy_fast_death.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

EngineDeadError now defines __reduce__ to reconstruct the exception from root_cause and preserve its instance dictionary. Tests cover pickle round trips, shallow copies, and deep copies.

Changes

EngineDeadError serialization

Layer / File(s)	Summary
Exception reconstruction and coverage tensorrt_llm/executor/utils.py, tests/unittest/executor/test_proxy_fast_death.py	__reduce__ supplies the class, root_cause, and instance dictionary for reconstruction. Tests check message and cause preservation across pickle round trips and copies.

Priority: Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: Minimal · up to fb8b7

The change preserves EngineDeadError messages across serialization and copying, with regression coverage in CI.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the fix and the affected behavior: keeping EngineDeadError messages stable across pickle and copy operations.
Description check		The description explains the root cause, implementation, expected behavior, reproduction, test coverage, and checklist status. It satisfies the required template sections.
Linked Issues check		Issue #17910 requires stable EngineDeadError messages after reconstruction across process boundaries. tensorrt_llm/executor/utils.py now returns (self.__class__, (self.root_cause,), self.__dict__)…
Out of Scope Changes check		The pull request changes only EngineDeadError.__reduce__ and adds focused pickle and copy tests for Issue #17910. The implementation preserves self.__dict__, including root_cause, and the tests …

Full details: Linked Issues check

Explanation

Issue #17910 requires stable EngineDeadError messages after reconstruction across process boundaries. tensorrt_llm/executor/utils.py now returns (self.__class__, (self.root_cause,), self.__dict__) from __reduce__, so reconstruction uses the original cause instead of the formatted self.args message. The added tests in tests/unittest/executor/test_proxy_fast_death.py verify message and argument stability after one and two pickle hops, plus shallow and deep copy. The tests also verify the root-cause type and text.

Full details: Out of Scope Changes check

Explanation

The pull request changes only EngineDeadError.__reduce__ and adds focused pickle and copy tests for Issue #17910. The implementation preserves self.__dict__, including root_cause, and the tests directly cover the requested message behavior. No unrelated production behavior or unrelated tests appear in the whole-pull-request diff.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[JunyiXu-nv]

LGTM.

[svc-trtllm-gh-bot]

Generated by an AI-assisted triage bot; please correct any mistaken assumptions.

Your fix keeps the error message readable across pickle and copy, and the new tests directly cover that behavior. The demonstrated improvement is limited to diagnostic stability: the exception and its root cause already survive, and the engine-death handling still follows the same terminal failure path.

Based on the assessment above, we're giving this PR lower review priority for now and adding waiting for feedback and stale.

A concrete downstream use where stable exception messages restore recovery or another functional behavior beyond clearer reporting would warrant reconsideration.

You're welcome to reply, update the PR description, or request maintainer review; a code change is not necessarily required. If there is no further activity for 14 days, this PR will be automatically closed.

[LeizhenZhang]

Thanks for the assessment, it is accurate. I checked main: inside TensorRT-LLM, EngineDeadError is created in the proxy and consumed in the same process (result queues and the serve layer, where describe_stream_termination passes its message to the client), so no in-repo path pickles it. The doubled message shows up only when the error crosses a process boundary outside the library, for example user code or an orchestration layer that forwards exceptions over multiprocessing or similar. I do not have a recovery behaviour to point to beyond the readable message.

The change is small (a 6-line __reduce__ in tensorrt_llm/executor/utils.py plus 3 CPU-only tests that fail on main), and @JunyiXu-nv has approved it. @JunyiXu-nv, if you think it is worth taking, could you start CI with /bot run? If the team would rather not carry it, I am fine with closing it.