Skip to content

[ROCm] Work around DeepEP teardown SIGSEGV in MoE test harness - #51174

Merged
AndreasKaratzas merged 3 commits into
vllm-project:mainfrom
Rohan138:rocm-deepep-teardown-osexit
Aug 5, 2026
Merged

[ROCm] Work around DeepEP teardown SIGSEGV in MoE test harness#51174
AndreasKaratzas merged 3 commits into
vllm-project:mainfrom
Rohan138:rocm-deepep-teardown-osexit

Conversation

@Rohan138

@Rohan138 Rohan138 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Purpose

On ROCm, tests/kernels/moe/test_deepep_moe.py::test_deep_ep_moe fails with each
spawned rank dying via SIGSEGV at process exit (no traceback). The MoE compute
is correct; the crash is teardown-only — a ROCr use-after-free in HIP's atexit
handler, fixed upstream in ROCm/rocm-systems#6942 but not
yet in our base image's ROCm build.

Guard the DeepEP spawn worker so on ROCm it os._exit(exit_code) after
destroy_process_group(), bypassing the broken atexit path while preserving the
pass/fail signal (non-zero exit if the worker raised). No effect off ROCm;
test-harness only (no kernel/model change). Revert once the base image ships a
ROCm build with the ROCr fix.

Test plan

MI300X (gfx942), ROCm 7.2.3, torch 2.12, stock runtime (isolates the workaround):

pytest -q tests/kernels/moe/test_deepep_moe.py::test_deep_ep_moe
  • Before: 28 failed (every rank SIGSEGV at exit)
  • After: 28 passed, 0 failed

Notes

@mergify mergify Bot added the rocm Related to AMD ROCm label Aug 5, 2026
@github-project-automation github-project-automation Bot moved this to Todo in AMD Aug 5, 2026
@Rohan138
Rohan138 force-pushed the rocm-deepep-teardown-osexit branch from c0b4090 to 6ba8d48 Compare August 5, 2026 15:47
On ROCm 7.2.x, `test_deep_ep_moe` fails 28/28 with each rank dying via
`SIGSEGV` at process exit (no Python traceback). The MoE compute is correct;
the crash is teardown-only, inside HIP's atexit handler. Root cause is a ROCr
use-after-free: `Runtime::Unload()` clears the shared signal pool before
`DestroyAgents()`, and `GpuAgent::ReleaseResources()` never releases the GWS
(cooperative-dispatch) queue that DeepEP's `hipLaunchCooperativeKernel` leaves
live, so `~AqlQueue` stores into freed signal memory. The real fix is in ROCr
(submitted separately); this only unblocks CI.

Guard the spawn worker's teardown so that on ROCm it `os._exit(exit_code)` after
`destroy_process_group()`, bypassing the broken atexit path while preserving
failure reporting (non-zero exit when the worker raised). No effect off ROCm.
Verified locally: `test_deep_ep_moe` 28 passed with the ROCr fix; this guard
lets the same run pass on a stock base image.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Rohan138 <rohanpotdar138@gmail.com>
@Rohan138
Rohan138 force-pushed the rocm-deepep-teardown-osexit branch from 6ba8d48 to e07de74 Compare August 5, 2026 16:32
@Rohan138
Rohan138 marked this pull request as ready for review August 5, 2026 17:17

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Rohan138 Rohan138 added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 5, 2026
@Rohan138

Rohan138 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

/ci run

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

@Rohan138, CI is now available for this PR.

  • /ci run starts a CI build.
  • /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.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #82528 for commit 2ded847cde8f.

@AndreasKaratzas AndreasKaratzas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AndreasKaratzas
AndreasKaratzas merged commit c2d8009 into vllm-project:main Aug 5, 2026
36 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in AMD Aug 5, 2026
@Rohan138
Rohan138 deleted the rocm-deepep-teardown-osexit branch August 5, 2026 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed rocm Related to AMD ROCm

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants