[dlamd1dai]

Motivation

Follow-up to #11569 / AICOMRCCL-2339. ncclGinAnvilBindResourceWindowSignals walked g_pendingByComm[comm] with no lock. ginAnvilPendingAdd, ginAnvilPendingRemove, and ginAnvilPendingClear all take pluginMutex. operator[] also default-inserts an empty list when the comm has nothing pending. std::mutex is not recursive, so the walk cannot hold the lock across ginAnvilRegisterLsaSignals or ginAnvilPendingClear, both of which lock it again.

Technical Details

• find the comm under pluginMutex and copy the pending ginAnvilGinCtx pointers
• Walk that snapshot without the lock, then clear as before
• A comm with no pending entry is left out of the map

#11569 rewrites the bind path and still walks g_pendingByComm unlocked (the span scan, the slot loop, and the failure path). If that PR merges first, rebase this change onto it.

JIRA ID: AICOMRCCL-2339

Test Plan

• Single-node Anvil (NCCL_GIN_TYPE=7, 1 node / 2 ranks) still binds the pending contexts and clears the list
• No multi-node run planned for this change

Test Result

• Not run on a SUT in this session

Submission Checklist

• Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

Made with Cursor

[therock-pr-bot]

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	⚠️ Warning	Error: Source/code files changed without an accompanying unit test. Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*). Current: code file(s) changed: projects/rccl/src/gin/gin_plugin_anvil_sdma.cc; no test file found
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

[amd-argus]

ARGUS

Findings

1 finding(s) that could not be posted as inline comments

• projects/rccl/src/gin/gin_plugin_anvil_sdma.cc:90 (Low) ginAnvilPendingRemove leaves a permanent empty map node per destroyed context ginAnvilPendingRemove at gin_plugin_anvil_sdma.cc:90 reads g_pendingByComm[comm], which re-inserts a {comm, nullptr} node whenever the bind path has already erased the entry at line 836, and it never erases the node when it unlinks the last context. Every ginAnvilDestroyContext (line 956) therefore leaves one entry behind, keyed by an ncclComm* that is freed shortly after, so the map grows once per comm for the life of the process. This predates the PR and the same find() shape you used at line 802 would close it, so it seems better as a separate change than folded in here.

Bottom line: Looks approval ready.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

[dlamd1dai]

Superseded by #11569 (AICOMRCCL-2339).

#11569 was rebased onto current develop and now carries this fix directly. It restructures ncclGinAnvilBindResourceWindowSignals, which left three unlocked reads of g_pendingByComm rather than the single one patched here: the span geometry scan, the slot-binding loop, and the fail: rollback. All three now walk one std::vector snapshot taken under pluginMutex via find(), so the non-recursive mutex is still free for ginAnvilRegisterSignalSpan and ginAnvilPendingClear.

Closing in favour of #11569.