pytorch / pytorch/pytorch

[Distributed][NCCL] Request for Backport: SIGSEGV in HeartbeatMonitor due to thread-unsafe getenv/setenv race (#167523)

Open
#175,724 5 comments 0 reactions 0 assignees View on GitHub
bot-triaged oncall: distributed oncall: distributed infra ptd-bot-triaged
Dominant language
Python
Stars
103k
Forks
29.5k
PR merge metrics
PR metrics pending

Description

### 🐛 Describe the bug

### Summary
A SIGSEGV occurs in the pt_nccl_heartbt thread because it performs getenv while the main thread modifies os.environ. While this has been fixed in the main branch via PR [#167523](https://github.com/pytorch/pytorch/pull/167523), this remains a critical stability issue for multiple stable releases, including v2.9, v2.10, v2.11, and potentially earlier versions where environment lookups still occur in background threads.

### The Evidence
GDB captured the exact memory corruption caused by an environ reallocation during a setenv call on the main thread.

- Captured Argument: "RCH_NCCL_DEBUG_INFO_PIPE_FILE"

- Expected Argument: "TORCH_NCCL_DEBUG_INFO_PIPE_FILE"

The 2-byte offset (T-O-RCH) confirms the background thread accessed an invalidated pointer while the main thread's setenv was triggering a realloc.

Click to expand the full GDB Backtrace

```text
Thread 6 (Thread 0x7fff55dfe640 (LWP 2658121) "pt_nccl_heartbt"):
#0 __GI_getenv (name=0x7fbf40000c72 "RCH_NCCL_DEBUG_INFO_PIPE_FILE") at ./stdlib/getenv.c:84
ep_start =
len = 29
ep = 0xffbfb8
name_start = 20308
#1 0x00007fff790fa0a8 in c10::utils::get_env[abi:cxx11](char const*) ()
#2 0x00007fff7c8d6ee3 in c10d::getCvarString(...)
#3 0x00007fff7c8d7b68 in c10d::DumpPipe::DumpPipe(int) ()
#4 0x00007fff7c8a106a in c10d::ProcessGroupNCCL::HeartbeatMonitor::runLoop() ()
```

### **Vulnerable Code Location (v2.9 / v2.10 / v2.11)**
In the affected stable branches, the `getenv` call is still executed within the background thread's lifecycle:

1. **Background thread start:** `ProcessGroupNCCL::Watchdog::run()` calls `heartbeatMonitor_->start()`.
2. **Execution:** `HeartbeatMonitor::runLoop()` initializes `DumpPipe`.
3. **Crash point:** Inside the `DumpPipe` constructor, it directly calls `getCvarString`:
```cpp
// This is the vulnerable code in stable branches:
DumpPipe(int rank) {
std::string fileStem =
getCvarString({"TORCH_NCCL_DEBUG_INFO_PIPE_FILE"}, "");
...
}
```

### My Observations & Workaround

- Trigger: Modifying os.environ shortly after init_process_group triggers the memory reallocation of the global environment table, leading to the crash.

- Current Workaround: We ensured all os.environ configurations are set BEFORE calling torch.distributed.init_process_group, which avoids the race condition.

### Request
Please backport PR [#167523](https://github.com/pytorch/pytorch/pull/167523) to v2.9, v2.10, and v2.11. This fix correctly moves environment lookups to the main thread during ProcessGroupNCCL construction, ensuring background threads only access cached strings.

### Versions

PyTorch Version: release/2.9 (ROCm)
Hardware: AMD Instinct MI325

cc @awgu @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @pragupta @msaroufim @dcci @aditvenk @weifengpy @xmfan @jeffdaily

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.