[legionxiong]

What problem does this PR solve?

Issue Number: resolves #3410

Problem Summary:

An RDMA completion-queue callback may remain queued after its endpoint is
reset. If the main socket is later revived with a new CQ, the stale callback
can successfully acquire the revived socket and continue operating on the
endpoint's new generation.

This may cause the stale callback to access invalid or mismatched RDMA
resources in RdmaEndpoint::PollCq, potentially resulting in a crash.

What is changed and the side effects?

Changed:

• Verify that the CQ socket passed to RdmaEndpoint::PollCq still matches the
  endpoint's current _cq_sid.
• Return immediately when the callback belongs to an older CQ generation.
• Add a regression test covering a stale callback running after the endpoint
  has switched to a new CQ.

Side effects:

• Performance effects: One SocketId comparison is added to each PollCq
  invocation. The overhead is negligible.

• Breaking backward compatibility: No.

[Copilot]

Pull request overview

This PR hardens RDMA completion-queue (CQ) polling against “stale” queued callbacks that can survive an endpoint Reset/Revive cycle and inadvertently operate on a new CQ/resource generation. It does so by validating that the CQ socket invoking RdmaEndpoint::PollCq is still the endpoint’s current CQ socket, and adds a regression test to prevent reintroducing the crash class described in #3410.

Changes:

• Add a guard in RdmaEndpoint::PollCq to immediately return when the callback’s CQ socket ID does not match the endpoint’s current _cq_sid.
• Add a unit test that exercises PollCq with a “stale” CQ socket to ensure it does not touch the new/current generation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
test/brpc_rdma_unittest.cpp	Adds a regression test for stale CQ callbacks not polling a new generation.
src/brpc/rdma/rdma_endpoint.cpp	Rejects PollCq execution when invoked by a non-current CQ socket (m->id() != ep->_cq_sid).

---

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

[Huixxi]

This fix is on the right track — it addresses the core race condition described in issue #3410, but it's a targeted patch rather than the more complete solution the issue actually suggests. Here's the breakdown:

Is the fix correct?

• The core problem in the issue: a PollCq callback belonging to an "old-generation" CQ Socket may resume execution after RdmaEndpoint::Reset() → health check → Revive() → a fresh handshake (which allocates a new _resource and a new _cq_sid). At that point, the ep (RdmaEndpoint*) it holds is already "new generation," while m (the CQ Socket that triggered the callback) is still "old generation."
• The new check added:

if (m->id() != ep->_cq_sid) {
    return;
}

This is exactly one of the key validations the issue suggested — "verify that m->id() still equals ep->_cq_sid." Since _cq_sid is regenerated via Socket::Create every time AllocateResources() runs (brpc's SocketId carries a version number, so even if the fd slot is reused, the id differs), the old-generation CQ Socket's id will necessarily not match the new-generation _cq_sid. This correctly rejects the vast majority of stale callbacks and is a reasonable, low-cost fix (one integer comparison).

• The added test stale_cq_callback_does_not_poll_new_generation directly constructs the scenario "old CQ socket + mismatched new _cq_sid" and verifies PollCq returns early, covering the main scenario described in the issue.

What isn't fully addressed (the issue itself hints at a more complete solution):

1. A TOCTOU window still exists: the check happens once at the start of PollCq, but the function later continues to access ep->_resource (e.g., ep->_resource->recv_cq). If Reset() happens right after this check passes but before ep->_resource is dereferenced (setting _resource to NULL), a crash could still occur. Ideally, per the issue's suggestion, the callback should be bound to immutable context such as {main_socket_id, cq_socket_id, resource, generation}, and re-validated right before actually touching _resource — not just once at function entry.
2. No explicit "generation" field was introduced: the issue text explicitly notes that PR drain CQ after moving QP into RESET state #3261 considered adding a generation counter in Reset() and validating it in PollCq, but the merged implementation chose CQ draining only. This fix similarly just adds a _cq_sid comparison rather than an explicit generation. In practice, _cq_sid's regeneration mechanism likely serves as an effective equivalent to a generation counter (since SocketId carries a version), but semantically it's less explicit than a dedicated generation field, and it wouldn't cover edge cases like "the same CQ Socket being reused by later logic" (though the current code doesn't exhibit that pattern).
3. Only the shared entry point in PollCq (used by both polling and non-polling modes) gets this check; HandleCompletion itself still has no secondary validation. If any future call path were to invoke HandleCompletion directly, bypassing PollCq, the same issue could resurface (currently no such path exists, so this is a latent maintenance risk rather than an active bug).

Conclusion:
This PR's change is correct in direction and does fix the specific crash scenario described in issue #3410 (a stale callback incorrectly accessing new-generation resources after socket revival). The change is small, the performance overhead is negligible, and the test specifically validates the fix point. However, it is a "minimally invasive" fix rather than the "complete solution" envisioned in the issue (e.g., finer-grained generation validation, or re-checking right before dereferencing ep->_resource). If similar TOCTOU scenarios surface later, further hardening may be needed (e.g., re-checking ep->_cq_sid immediately before actually dereferencing ep->_resource, or introducing an atomic generation field). I'd suggest flagging this in the review comments, but the fix can reasonably be merged as an incremental improvement in the meantime.