[https://nvbugs/6244695][fix] Revert Pass IPC HMAC key through file descriptor - #14782
Conversation
📝 WalkthroughWalkthroughSpawn-proxy IPC authentication migrates from file-descriptor-based passing with cached helpers to direct hex-encoded environment variable export. The core accessor function, launcher script generation, and MPI server integration all simplify by removing FD plumbing and cached state, and the test module covering the old FD mechanism is removed. ChangesIPC HMAC Key Mechanism Refactor
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tensorrt_llm/executor/utils.py`:
- Line 26: The new constant TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY exposes the
IPC HMAC secret via process environment—restore the previous non-environment
mechanism: remove usage of TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY from any
subprocess env setup and instead pass the HMAC key over the narrowly-scoped
inherited-FD / dedicated IPC channel used by ZeroMqQueue (or via explicit file
descriptor/pipe passed to the child), update the code paths in ZeroMqQueue and
the launcher/spawn code that currently read/write this env var so they accept
the key from the inherited-FD/IPC handle, and delete the environment-constant
definition so the secret is never placed into os.environ or subprocess env.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 90405160-bfc2-4fc3-a9e1-5c178eafe732
📒 Files selected for processing (4)
tensorrt_llm/commands/serve.pytensorrt_llm/executor/utils.pytensorrt_llm/llmapi/trtllm-llmapi-launchtests/unittest/executor/test_launcher_envs.py
💤 Files with no reviewable changes (1)
- tests/unittest/executor/test_launcher_envs.py
| TLLM_SPAWN_PROXY_PROCESS_IPC_ADDR = "TLLM_SPAWN_PROXY_PROCESS_IPC_ADDR" | ||
| TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY_FD = ( | ||
| "TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY_FD") | ||
| TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY = "TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY" |
There was a problem hiding this comment.
Keep the IPC HMAC key off the environment.
ZeroMqQueue uses this key to authenticate pickle traffic before pickle.loads() in tensorrt_llm/executor/ipc.py:33-103,354-389. Switching the contract back to TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY means the secret now propagates through process environments in both launcher paths instead of staying on a narrowly scoped channel like the previous inherited-FD mechanism. That weakens the only barrier guarding unauthorized IPC deserialization.
Also applies to: 39-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tensorrt_llm/executor/utils.py` at line 26, The new constant
TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY exposes the IPC HMAC secret via process
environment—restore the previous non-environment mechanism: remove usage of
TLLM_SPAWN_PROXY_PROCESS_IPC_HMAC_KEY from any subprocess env setup and instead
pass the HMAC key over the narrowly-scoped inherited-FD / dedicated IPC channel
used by ZeroMqQueue (or via explicit file descriptor/pipe passed to the child),
update the code paths in ZeroMqQueue and the launcher/spawn code that currently
read/write this env var so they accept the key from the inherited-FD/IPC handle,
and delete the environment-constant definition so the secret is never placed
into os.environ or subprocess env.
99d27ff to
588868b
Compare
|
/bot run --post-merge --disable-fail-fast |
|
PR_Github #51187 [ run ] triggered by Bot. Commit: |
|
PR_Github #51187 [ run ] completed with state
|
…descriptor (NVIDIA#14378)" This reverts commit 50ca49f. Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>
588868b to
a003643
Compare
|
/bot run --disable-fail-fast --stage-list "*PerfSanity*" |
|
PR_Github #51230 [ run ] triggered by Bot. Commit: |
|
PR_Github #51230 [ run ] completed with state
|
…escriptor (NVIDIA#15654) Reverts NVIDIA#15654 (48fc753). PR NVIDIA#15654 passes the IPC HMAC key from the parent process to the LLM-API proxy child via a file descriptor. In the QA-pytest Slurm/MPI launch path used by trtllm-bench and trtllm-serve, the fd handshake between Rank0's forked subshell (spawning internal mpirun) and the mgmn_leader_node RemoteMpiCommSessionServer deadlocks silently: - trtllm-bench (nvbugs/6388787): after "start MpiSession with N workers" the bench process emits no further stdout for ~30 min, workers stay alive at 0% GPU / 4 MiB VRAM, no exception, no MPI abort. Perf harness SIGKILLs at _STALL_TIMEOUT=1800s. - trtllm-serve (nvbugs/6405747): serve mode's /health endpoint never binds, and pytest fails with "Server http://localhost:<port>/health did not become ready within 3600s" on both baseline and candidate wheels. This is the third time the "Pass IPC HMAC key via fd" idea has broken bench/serve (previously PR NVIDIA#14378 was reverted by PR NVIDIA#14782 for the same class of failure: BlockingIOError in _read_spawn_proxy_process_ ipc_hmac_key_fd). Reverting until the fd inheritance across fork+exec into the mpirun child is verified end-to-end for both bench and serve launch paths. Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>
…escriptor (NVIDIA#15654) Reverts NVIDIA#15654 (48fc753). PR NVIDIA#15654 passes the IPC HMAC key from the parent process to the LLM-API proxy child via a file descriptor. In the QA-pytest Slurm/MPI launch path used by trtllm-bench and trtllm-serve, the fd handshake between Rank0's forked subshell (spawning internal mpirun) and the mgmn_leader_node RemoteMpiCommSessionServer deadlocks silently: - trtllm-bench (nvbugs/6388787): after "start MpiSession with N workers" the bench process emits no further stdout for ~30 min, workers stay alive at 0% GPU / 4 MiB VRAM, no exception, no MPI abort. Perf harness SIGKILLs at _STALL_TIMEOUT=1800s. - trtllm-serve (nvbugs/6405747): serve mode's /health endpoint never binds, and pytest fails with "Server http://localhost:<port>/health did not become ready within 3600s" on both baseline and candidate wheels. This is the third time the "Pass IPC HMAC key via fd" idea has broken bench/serve (previously PR NVIDIA#14378 was reverted by PR NVIDIA#14782 for the same class of failure: BlockingIOError in _read_spawn_proxy_process_ ipc_hmac_key_fd). Reverting until the fd inheritance across fork+exec into the mpirun child is verified end-to-end for both bench and serve launch paths. Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>
…escriptor (NVIDIA#15654) Reverts NVIDIA#15654 (48fc753). PR NVIDIA#15654 passes the IPC HMAC key from the parent process to the LLM-API proxy child via a file descriptor. In the QA-pytest Slurm/MPI launch path used by trtllm-bench and trtllm-serve, the fd handshake between Rank0's forked subshell (spawning internal mpirun) and the mgmn_leader_node RemoteMpiCommSessionServer deadlocks silently: - trtllm-bench (nvbugs/6388787): after "start MpiSession with N workers" the bench process emits no further stdout for ~30 min, workers stay alive at 0% GPU / 4 MiB VRAM, no exception, no MPI abort. Perf harness SIGKILLs at _STALL_TIMEOUT=1800s. - trtllm-serve (nvbugs/6405747): serve mode's /health endpoint never binds, and pytest fails with "Server http://localhost:<port>/health did not become ready within 3600s" on both baseline and candidate wheels. This is the third time the "Pass IPC HMAC key via fd" idea has broken bench/serve (previously PR NVIDIA#14378 was reverted by PR NVIDIA#14782 for the same class of failure: BlockingIOError in _read_spawn_proxy_process_ ipc_hmac_key_fd). Reverting until the fd inheritance across fork+exec into the mpirun child is verified end-to-end for both bench and serve launch paths. Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>
…descriptor (#14378)"
This reverts commit 50ca49f.
Summary by CodeRabbit
Release Notes
Refactor
Tests
Description
Test Coverage
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-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin 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.