[JhaShweta1]

The TearDown safety-net cancelled a nonblocking collective (MPI_Ibarrier) request on timeout, which the MPI standard forbids. OpenMPI raises MPI_ERR_REQUEST and, because MPI_COMM_WORLD uses MPI_ERRORS_ARE_FATAL, that MPI_Cancel call itself aborts the job with a misleading error before the intended MPI_Abort runs. Remove the MPI_Cancel/MPI_Request_free and call MPI_Abort directly (it tears the job down anyway).

Also raise the teardown-barrier timeout 3s -> 60s (overridable via RCCL_TEST_TEARDOWN_TIMEOUT_SEC) so benign rank skew in heavy multi-phase MPI tests (GrowMPITest, NetIbMPITest CtsDepthStress) no longer trips a false 'out of sync' abort.

Test-harness only; no librccl behavior change. Recovers Grow_MultiRankJump and NetIB_CtsDepthStress (ib-cast) from false failures.

Motivation

Fix an illegal MPI_Cancel on a nonblocking-barrier request that crashed MPI test teardown, causing passing tests to be reported as failures.

Technical Details

MPIEnvironment::TearDown() uses an MPI_Ibarrier + poll-with-timeout as a safety net. On timeout it called MPI_Cancel(&barrier_req) — but cancelling a nonblocking collective request is forbidden by the MPI standard. OpenMPI returns MPI_ERR_REQUEST, and since MPI_COMM_WORLD uses the default MPI_ERRORS_ARE_FATAL handler, that MPI_Cancel call itself aborts the job (before the intended MPI_Abort). Fix: drop the MPI_Cancel/MPI_Request_free and call MPI_Abort directly. Also raised the barrier timeout 3s→60s (env-overridable via RCCL_TEST_TEARDOWN_TIMEOUT_SEC) so benign rank skew in heavy multi-phase tests doesn't trigger a false abort. Test-harness only; no librccl change.

Issue Tracking

Test Plan

Test Result

Submission Checklist

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

[Copilot]

Pull request overview

Fixes RCCL MPI test-harness teardown behavior by removing an illegal MPI_Cancel of a nonblocking collective (MPI_Ibarrier) on timeout, and by increasing the teardown-barrier timeout to reduce false “out of sync” aborts in long/heavy multi-phase tests.

Changes:

• Remove MPI_Cancel/MPI_Request_free on MPI_Ibarrier timeout and call MPI_Abort directly.
• Increase default teardown-barrier timeout from 3s to 60s.
• Add RCCL_TEST_TEARDOWN_TIMEOUT_SEC environment override and improve timeout diagnostics.

Comments suppressed due to low confidence (1)

projects/rccl/test/common/MPIEnvironment.cpp:255

• Using std::atoi for RCCL_TEST_TEARDOWN_TIMEOUT_SEC is unsafe: it provides no error reporting and has undefined behavior on overflow. The test codebase already uses validated parsing via strtol (e.g., projects/rccl/test/NetSocketTests.cpp:574). Consider switching to std::strtol with end-pointer + ERANGE checks before updating timeout_sec.

        if(const char* env = std::getenv("RCCL_TEST_TEARDOWN_TIMEOUT_SEC"))
        {
            int v = std::atoi(env);
            if(v > 0) timeout_sec = v;
        }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[atulkulk]

Did we see failures in these tests without this change?

[JhaShweta1]

Yes: GrowMPITest, NetIbMPITest are passing now.