[alogfans]

Description

A TENT RDMA software timeout could publish a terminal status while the posted WR could still access the caller's buffer. Reusing that address after TIMEOUT could race with a delayed RDMA READ or WRITE.

Cancel further attempts of the expired task, retire its affected endpoints, and wait for completion/flush evidence or successful QP destruction before publishing the terminal task status. Keep early task cancellation so queued siblings and flush completions cannot resubmit the expired task. A late successful completion still leaves a timeout-canceled task as TIMEOUT once all its slices resolve.

Other tasks sharing the QP are not marked timed out or task-canceled; their flush completions retain the normal retry path. Validate that the triggering slice is still on the QP queue before retiring the endpoint, so a stale worker-set entry cannot tear down unrelated work.

On teardown, return quota and detach slices separately for each successfully destroyed QP under the same endpoint lock. A later QP's destruction failure keeps only that QP pending; repeated cleanup and delayed CQ handlers cannot return the successful QP's quota again.

Completion is sufficient evidence for the corresponding work even if the QP object remains alive. Failed destruction alone never permits terminal publication. If neither completions nor destruction succeed, the task stays PENDING and its buffers must remain valid. This uses the existing endpoint reclamation fallback and is not a hard completion deadline. The TENT API documentation now describes this contract.

Scope: TENT RDMA software timeouts and teardown accounting. Classic TE (#5034), Python/Store wait deadlines, GPU qualification, and a general physical-termination receipt API are outside this change. Related work checked: #4964 covers expiry before posting, #4383 covers terminal destructor failures, #4780 covers the Store wait layer, and #4816 proposes the broader termination receipt API.

Suggested Merge Priority

• P0 — Urgent
• P1 — High
• P2 — Normal: Focused correctness fix
• P3 — Low

Module

• Transfer Engine (mooncake-transfer-engine)
• Docs

Type of Change

• Bug fix
• Documentation update

How Has This Been Tested?

CPU build with USE_TENT=ON, USE_CUDA=OFF, based on 880bcf6763b79592e509cace4b5818b5202fa1f1 plus this PR.

Test commands:

cmake --build /tmp/mooncake-pr5011-revision-build --parallel 6 --target \
  tent_rdma_transport_test tent_endpoint_lifecycle_test \
  tent_endpoint_store_test rdma_cancel_test
ctest --test-dir /tmp/mooncake-pr5011-revision-build \
  -R '^(tent_rdma_transport_test|tent_endpoint_lifecycle_test|tent_endpoint_store_test|rdma_cancel_test)$' \
  --output-on-failure
MC_TE_FILTERS=mlx5_4 /tmp/mooncake-pr5011-revision-build/mooncake-transfer-engine/tent/tests/tent_rdma_transport_test \
  --gtest_filter=RdmaTransportIntegrationTest.WriteThenReadAcrossProcesses

• Normal unit suites: 146 RDMA + 24 endpoint lifecycle + 8 endpoint store + 2 cancellation tests passed. Three hardware-dependent cases skipped in the sandbox.
• Real-device cross-process READ/WRITE integration passed separately.
• Ten timeout-drain regressions cover pending-until-completion ordering, younger tasks retrying or completing, failed destruction with no CQE/flush/late success, partial destruction with a delayed CQE, stale set entries, and siblings on multiple QPs/endpoints.
• AddressSanitizer results: 22 focused timeout/shared-QP/lifetime tests passed with leak detection enabled; lifecycle and cancellation suites also passed. The broader run found an EndpointStore base-pointer delete type mismatch and a 720-byte leak in RdmaSliceOrphanTest.AnOpportunisticReapDoesNotAgeAnOrphan. Both were reproduced using the pre-PR endpoint/worker implementations and original tests from 880bcf67; they remain outside this change. The full ASan run is therefore not reported as clean.
• Scoped pre-commit passed all applicable hooks for the six changed files, including staged-line clang-format.
• Documentation HTML build passed; the generated TENT API page was checked.

Manual real-RDMA A/B fault probe (local harness):

Both binaries used the same probe object and dependencies. The baseline substituted the original endpoint/worker implementations from 880bcf67. On each of mlx5_4 and mlx5_1, run ten 1 MiB READ and ten WRITE cases: 40 cases per version.

Post while the peer QP remains RESET and confirm one outstanding WR, no CQ completion, unchanged destination memory, and PENDING status. After the software deadline, pause immediately before the actual local QP-to-ERR call. The consumer reuses the registered address only after TIMEOUT, then enables the peer. Revised runs first handle a real flush CQE and observe memory/CQ for 600 ms after peer recovery.

Observation	Baseline	Revised
TIMEOUT visible while hardware QP is still RTS	40/40	0/40
Reused memory overwritten by the old operation	40/40	0/40
Real flush CQE handled before TIMEOUT	0/40	40/40
Subsequent transfer on a fresh connection succeeded	40/40	40/40

These are controlled same-host RNIC probes with a scheduling pause, not a physical link outage or qualification of already-delivered cross-host packets.

Test results:

• Unit tests pass (normal build; ASan qualification above)
• Integration tests pass (real-device READ/WRITE case)
• Manual testing done (A/B fault probe)

Checklist

• I have performed a self-review of my own code — human line-by-line review pending
• I have formatted my code using ./scripts/code_format.sh
• I have run pre-commit on the files changed in this PR and all applicable hooks pass
• I have updated the documentation
• I have added tests to prove my changes are effective
• For changes >500 LOC: I have filed an RFC issue (not applicable)

AI Assistance Disclosure

• No AI tools were used
• AI tools were used

OpenAI Codex assisted with implementation, review analysis, regression tests, real-RDMA A/B testing, and this description. Human review of every changed line remains pending; the submitter is responsible for understanding and defending the final change.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

[staryxchen]

Three comments on the software-timeout abort path.

[staryxchen]

Destroying this QP drops every queued WR, and this loop publishes TIMEOUT and sets cancel_requested on each of those tasks, including younger slices of other tasks. cancel_requested is per task, so their slices on other QPs are dropped as well. acknowledge() used to fail only the prefix through the expired slice; later WRs flushed and were retried via submitFromTick.

Is that the intended outcome? Same question when the expired slice is no longer in the queue (a neighbour already popped it for retry): this still calls beginDestroyNoLock() and ibv_destroy_qp before the num_slices == 0 path.

[alogfans]

Canceling unrelated tasks was not intended. Reworked this in cb8eebbb: timeout cancellation is now limited to the expired task. The endpoint is retired to request flushes, and other tasks sharing its QPs retain their normal completion/retry path. We no longer synchronously destroy the timed-out QP and mark every queued task TIMEOUT.

retireForTimeout() checks that the triggering slice is still in that QP's queue under the endpoint write lock before starting retirement. A stale worker-set entry therefore cannot retire the connection or cancel younger tasks.

Added YoungerTaskOnSharedQpCanRetry, YoungerTaskOnSharedQpCanComplete, and StaleSetEntryDoesNotRetireYoungerTask. They cover retry on a fresh connection, successful completion of a younger task, and a detached expired slice leaving the healthy endpoint unchanged.

[staryxchen]

If this QP is destroyed and a later one fails, these slices stay queued and PENDING (resetInflightSlices runs only when every data QP is destroyed) while wr_depth is already 0. The next timeout sweep calls abortTimedOutQp, which pops that queue and cancelQuota()s the same entries again. RdmaCQ::cancelQuota is a raw fetch_sub on the shared device CQ, so cqe_now_ can go negative and reserveQuota will admit posts past cqe_limit_. A poller that already copied a CQE does the same thing through acknowledge().

Release the quota and detach those slices once, in this critical section. FailedQpAbortKeepsBuffersPendingUntilRetry only covers a single QP whose destroy fails.

[alogfans]

Confirmed and fixed in cb8eebbb. deconstructUnlocked() now returns quota and detaches/resolves the queue for each successfully destroyed QP in the same endpoint-lock critical section, using resetInflightSlices(i). A failed QP retains its queue and quota independently of the successful ones.

Added PartialDestroyReleasesEachQpOnlyOnce: QP0 destruction succeeds and QP1 returns EIO; QP0's queue is empty and depth is zero while CQ quota remains 1. A repeated timeout sweep and a delayed CQE for QP0 leave those values unchanged. QP1's completion then returns the remaining quota to zero. This covers the partial-destruction case that the original single-QP test missed.

[staryxchen]

These flags are latched before destroy is known to have worked. On failure the sweep continues, but beginDestroyNoLock() has already moved every QP to ERR. The flush CQE then takes the cancel_requested path in handleCompletion and updateSliceStatus(CANCELED); with first_error already TIMEOUT, a one-slice task publishes TIMEOUT while the QP object is still alive. That path also skips disableEndpoint(), so the rail is not marked failed.

FailedQpAbortKeepsBuffersPendingUntilRetry never enters handleCompletion, so it misses this. Set these flags only after destroy succeeds. A failed attempt should stay pending: no publish, and no submitFromTick.

[alogfans]

The failed-destroy-plus-CQE case was missing, and the original description's requirement to destroy the QP before TIMEOUT was not enforced by that path. In cb8eebbb, I changed the implementation and documented contract to require completion/flush evidence for the posted work, or successful destruction of its owning QP, before publishing the task's terminal status.

I kept cancellation intent early so queued siblings and flush handlers cannot retry the expired task. Setting those flags alone does not resolve a posted slice. Failed destruction with no CQE leaves it PENDING; an actual flush or successful completion can resolve the work even while the QP object remains alive. The task stays PENDING until all its slices resolve, and late success preserves the timeout outcome. If neither completion nor destruction succeeds, it remains PENDING.

The timeout-specific error-CQE path now avoids retrying the canceled task or failing unrelated tasks in the QP prefix. Rail failure is recorded when retireForTimeout() starts endpoint retirement. In the previous revision, the normal retry branch also called disableEndpoint() before checking cancel_requested; the new path makes the retirement-side rail update explicit.

Added tests for failed destruction without a CQE, failed destruction followed by flush or late success, and task siblings across QPs/endpoints. The real-RDMA READ/WRITE probe also verified 40/40 revised runs handled a flush before TIMEOUT, with no overwrite after buffer reuse; the baseline showed an overwrite in 40/40 runs. The PR description and API documentation now state the completion-or-destruction condition and the absence of a hard completion deadline.

[staryxchen]

LGTM.