Skip connUpdateState for async free clients for RDMA

[harrylin98]

Summary

In forkless processPendingCommandAndInputBuffer, we add the extra check:

if (c->flag.close_asap) return C_ERR;

In handleReadJobs(), when processPendingCommandAndInputBuffer returns C_ERR, the client is dying but may not yet be freed. The subsequent connUpdateState is still called, which for RDMA synchronously invokes the read handler and could cause use-after-free. Fix by skipping the rest of the loop iteration on C_ERR.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 88c9b6cf-21a6-4012-ab8a-0ec1c19d361c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[valkey-review-bot]

The test-rdma check fails on this head, while both runs for the base commit passed. This PR targets the RDMA path, so please inspect and rerun that job before merging: https://github.com/valkey-io/valkey/actions/runs/32284603270/job/96171081364

[valkey-review-bot]

The code change follows the existing C_ERR contract and avoids touching a dying client. The PR's test-rdma check is currently failing even though the base commit's two RDMA runs passed; I posted the failing job separately because it has no diff line to anchor to.

[harrylin98]

The test-rdma check fails on this head, while both runs for the base commit passed. This PR targets the RDMA path, so please inspect and rerun that job before merging: https://github.com/valkey-io/valkey/actions/runs/32284603270/job/96171081364

This seems a flaky test, not related with my fix.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.42%. Comparing base (50ec936) to head (8c70cdf).

Files with missing lines	Patch %	Lines
src/io_threads.c	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           forkless    #4474      +/-   ##
============================================
- Coverage     79.82%   79.42%   -0.41%     
============================================
  Files           175      175              
  Lines         94843    93223    -1620     
============================================
- Hits          75712    74044    -1668     
- Misses        19131    19179      +48     

Files with missing lines	Coverage Δ
src/io_threads.c	75.00% <0.00%> (-0.38%)	⬇️

... and 25 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[quanyeyang]

The control-flow change looks correct. C_ERR means the client must no longer be touched: it either has already been freed during command processing or has been scheduled for asynchronous free via close_asap. Skipping the remainder of the iteration prevents RDMA's connUpdateState() from synchronously re-entering handlers on such a client, while the C_OK path remains unchanged.

The failing test-rdma job appears unrelated. It fails during the phase-1 RDMA CM smoke test, before the IO-thread benchmark, with RDMA_CM_EVENT_REJECTED and server-side accept failures. This hunk is only reachable after an IO-thread read completion. Please rerun that job before merging.

No blocking findings from me.