Fix stack CC request lifetime races

[thweetkomputer]

Context

This fixes a lifetime race in stack-owned CC requests observed while debugging the EloqKV main-branch CI crash.

CcShard::ProcessRequests() keeps a raw request pointer in req_buf_, calls Execute(), and calls req->Free() when Execute() returns true. Some stack-owned requests signal their waiter from inside Execute() before that scheduler-side Free() happens. The waiting thread can then reuse or destroy the stack object while ProcessRequests() still has the pointer.

The concrete crash path is KickoutCcEntryCc: SetFinish() calls res_->SetFinished(), the background worker wakes up and resets the stack request for the next table/node group, and the CC worker then returns to ProcessRequests() and calls Free() on the same object.

Behavior before and after

Before:

• Stack KickoutCcEntryCc requests in LocalCcShards::PurgeDeletedData() and KickoutDataForTest() waited only for CcHandlerResult completion.
• UploadRangeSlicesCc and UploadBatchSlicesCc error paths could return true or call Free() after waking their stack-owner waiter.
• Those paths allowed the owner to reuse or destroy stack storage before the scheduler finished its final request access.

After:

• Stack KickoutCcEntryCc requests mark themselves in use before enqueueing and wait for InUse() == false after business completion, so the stack owner does not reset or destroy the object until scheduler-side Free() is complete.
• Stack range upload error and abort paths wake their waiters but return false / avoid Free(), matching their normal completion paths.
• The concurrency-control notes document the stack-owned versus pool-owned request lifetime rule.

Compatibility: pooled CC request behavior is unchanged.

Implementation

• Added WaitForStackCcRequestFree() in local_cc_shards.cpp using bthread_usleep() bounded backoff against the existing CcRequestBase::InUse() flag.
• Added Use() before enqueueing the two stack KickoutCcEntryCc instances and wait for scheduler-side free after the result callback has fired.
• Changed UploadRangeSlicesCc and UploadBatchSlicesCc stack-owned error paths to complete the waiter without returning true to ProcessRequests().
• Changed the UploadBatchSlicesCc cache-reject path in TemplateCcMap to return false after setting the error.

Design decisions and alternatives

This keeps the fix local to the unsafe stack-owned requests instead of changing the generic CC scheduler contract. Use() / InUse() is already the scheduler-side ownership signal, so the stack request can use it as a final lifetime fence without adding a new request subclass or another flag.

The wait happens outside Execute() and outside CC worker threads. It uses bthread sleep/backoff instead of blocking inside the coroutine execution model.

Test plan

• Unit/TCL tests
• Integration or manual validation
• Formatting/build checks
• Compatibility or performance validation, when relevant

Commands and results:

git -C data_substrate diff --check
# passed

cmake --build bld-verify --target txservice -j4
# passed: [100%] Built target txservice

clang-format-18 --version
# failed: clang-format-18 not installed in this workspace

Full EloqKV TCL/CI tests were not run locally.

Risk assessment

The main regression surface is stack KickoutCcEntryCc callers waiting slightly longer after logical completion. The wait should normally be short because it only covers the gap between SetFinished() and ProcessRequests() calling Free(), and it backs off with bthread_usleep().

Range upload changes intentionally affect stack-owned request paths; pooled request recycling remains unchanged.

Rollback plan

Revert this PR. That restores the previous request lifetime behavior.

Reviewer guide

• tx_service/src/cc/local_cc_shards.cpp: stack KickoutCcEntryCc lifetime fence.
• tx_service/include/cc/cc_request.h: stack range upload error/abort paths.
• tx_service/include/cc/template_cc_map.h: batch upload reject path.
• docs/03-concurrency-control.md: documented request lifetime invariant.

Follow-up work

Run the full EloqKV GitHub Actions matrix after the parent EloqKV submodule pointer is updated.

Summary by CodeRabbit

• Bug Fixes
  • Fixed CC request execution/abort flows to return consistent failure states when leadership/initialization checks fail.
  • Improved error handling for rejected uploads and scan-related operations when initialization fails.
  • Strengthened safe cleanup for stack-owned shard requests to avoid lifetime/race issues during scheduler processing.
• Documentation
  • Clarified concurrency-control ownership, cancellation/abort, and completion requirements to prevent premature reuse or destruction.
• Chores
  • Updated the eloqstore subproject to a newer revision.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cb33451d-4a8a-498c-bba6-cd2dfd6b92e1

📥 Commits

Reviewing files that changed from the base of the PR and between 2ef3762 and 4082f0e.

📒 Files selected for processing (2)

• store_handler/eloq_data_store_service/eloqstore
• tx_service/include/cc/cc_request.h

---

Walkthrough

The PR updates CC request error and abort control flow, adds synchronization for stack-owned requests dispatched to shards, documents ownership and cancellation requirements, and advances the eloqstore submodule reference.

Changes

CC request lifetime and completion

Layer / File(s)	Summary
Request error and abort semantics tx_service/include/cc/cc_request.h, tx_service/include/cc/template_cc_map.h	Invalid-term, initialization, scan, upload rejection, and abort paths now explicitly set errors and return false where required; batch abort no longer conditionally calls Free().
Stack-owned request lifetime coordination tx_service/src/cc/local_cc_shards.cpp, docs/03-concurrency-control.md	Stack-owned requests are marked in use before dispatch, awaited after completion notification, and documented with ownership and abort-finalization rules.
Eloqstore revision update store_handler/eloq_data_store_service/eloqstore	The eloqstore submodule pointer is updated to a newer commit.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• eloqdata/tx_service#528: Addresses related CC request completion lifetime and abort signaling behavior.
• eloqdata/tx_service#527: Updates the same eloqstore submodule reference.
• eloqdata/tx_service#510: Updates the same eloqstore submodule reference.

Suggested reviewers: liunyl, githubzilla

Poem

I’m a rabbit guarding requests in the queue,
Keeping stack-held pointers safe and true.
Errors now settle before paths return,
While aborting waits its proper turn.
Shards may hop, but fences stay bright—
Safe request lifetimes through the night.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: fixing stack CC request lifetime races.
Description check	✅ Passed	The description covers context, behavior changes, implementation, risks, rollback, reviewer guide, and tests, so it is mostly complete.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch agent-fix-cc-request-lifetimes

---

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.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tx_service/include/cc/cc_request.h`:
- Around line 7959-7962: The completion state in UploadRangeSlicesCc and
UploadBatchSlicesCc must be lock-free: replace mutex/condition-variable
signaling with atomic finish flags, make err_code_ atomic where required, and
update Wait() to poll with bthread_usleep exponential backoff. Remove all mutex
locking from ValidTermCheck and SetError, including UploadRangeSlicesCc at
tx_service/include/cc/cc_request.h:7959-7962 and UploadBatchSlicesCc at
tx_service/include/cc/cc_request.h:8220-8221 and 8243-8244; the latter site
requires SetError to be completely lock-free.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e1b01708-9c43-48a9-99bb-fb25b7e99d05

📥 Commits

Reviewing files that changed from the base of the PR and between f7e62c5 and 2ef3762.

📒 Files selected for processing (4)

• docs/03-concurrency-control.md
• tx_service/include/cc/cc_request.h
• tx_service/include/cc/template_cc_map.h
• tx_service/src/cc/local_cc_shards.cpp