fix: assertion failure in JobQueue::stop

[a1q123456]

High Level Overview of Change

We're getting the assertion failure in Antithesis in JobQueue::stop at line 316 and it's because m_processCount will be 0 when there's a coroutine suspended (either by the first yield() before it reaches the user function or by an yield() in the user function) and nSuspend_ will be 1. The assertion failure indicates that coroutines in rippled do not have threads to resume and exit cleanly when the job queue is getting closed.

This PR makes JobQueue resume all suspended coroutines so that coroutines can check if it should exit after waking up.

Context of Change

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (non-breaking change that only restructures code)
• Performance (increase or change in throughput and/or latency)
• Tests (you added tests for code that already exists, or your new feature included in this PR)
• Documentation update
• Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
• Release

API Impact

• Public API: New feature (new methods and/or new fields)
• Public API: Breaking change (in general, breaking changes should only impact the next api_version)
• libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
• Peer protocol change (must be backward compatible or bump the peer protocol version)

[codecov]

Codecov Report

❌ Patch coverage is 92.18750% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.9%. Comparing base (fe9c8d5) to head (02793cd).
⚠️ Report is 195 commits behind head on develop.

Files with missing lines	Patch %	Lines
include/xrpl/core/Coro.ipp	95.0%	2 Missing ⚠️
src/libxrpl/core/detail/JobQueue.cpp	87.5%	2 Missing ⚠️
include/xrpl/core/JobQueue.h	87.5%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5774     +/-   ##
=========================================
- Coverage     79.9%   79.9%   -0.0%     
=========================================
  Files          839     840      +1     
  Lines        65528   65582     +54     
  Branches      7273    7267      -6     
=========================================
+ Hits         52330   52373     +43     
- Misses       13198   13209     +11     

Files with missing lines	Coverage Δ
include/xrpl/core/JobQueue.h	90.9% <87.5%> (-9.1%)	⬇️
include/xrpl/core/Coro.ipp	94.8% <95.0%> (-5.2%)	⬇️
src/libxrpl/core/detail/JobQueue.cpp	90.3% <87.5%> (-0.5%)	⬇️

... and 438 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[a1q123456]

This case is very hard to cover in unit tests but this is valid, because we have to call jobQueue->stop() and then call coro->yield() right after m_suspendedCoros gets copied and before we call jobCounter_.join in JobQueue::stop

[a1q123456]

Seems that we're getting an error in Antithesis, converting this PR to draft.

[pratikmankawde]

Typo: JobQueue.

[pratikmankawde]

This will change the contract of the class. The class description suggests that when a JobQueue stops, it wait for all the jobs to finish. Here you seem to be adding an abort functionality. This will change the expected behaviour of the class. I would recommend finding an alternate solution for the problem.

[a1q123456]

I don't think there's an alternate solution and this approach is quite conventional, the only difference is that we could provide a std::stop_token to all async functions and then the stop token does the same as shouldStop, but adding shouldStop requires least amount of changes.

[pratikmankawde]

This might trigger an out of order execution of the job assigned to this coroutine. I think that could be buggy, if not borderline dangerous.

Example: We created 5 coroutines, each with a job. They all got scheduled in a job queue. We freed coroutine no. 3. It gets resumed/executed, before others.

[a1q123456]

This is the issue with the boost coroutine where we can't throw an exception directly outside a coroutine when it's suspended. To notify coroutines that the underlying thread pool is getting destructed or the coroutine itself is getting destructed, we have to resume the coroutine and let it exit itself, and then, inside the coroutine, it has to check if it should stop before doing any other job.

[a1q123456]

I've updated the JobQueue class so that we can leverage the internal clean up process of boost coroutine.

[pratikmankawde]

I think we need to fix this issue, where JobQueue gets stopped before coroutines finish. It shouldn't. It should keep waiting for coroutines.

[a1q123456]

JobQueue won't fully stop before all coroutines stop.

[pratikmankawde]

The class name is JobQueue, so I am going with the assumption of jobs being processed to completion in FIFO order. If you add a new job to the queue at this point, it will be the last job to get executed(since you deactivated queuing). We would know that when this job finishes, the queue is empty. This job can also do any clean-up at this point. It can signal that the queue is now empty and hence JobQueue can stop.

[a1q123456]

It is impossible to add a new job when the job queue finishes except for jobs that resumes coroutines, in which case, the order doesn't matter that much because the coroutines will just exit without doing any other things.

[pratikmankawde]

I didn't understand the argument(from PR description) that

The assertion failure indicates that coroutines in rippled do not have threads to resume and exit cleanly when the job queue is getting closed.

How come there are no threads? Workers object is alive in JobQueue class. What am I missing here?

[a1q123456]

I didn't understand the argument(from PR description) that

The assertion failure indicates that coroutines in rippled do not have threads to resume and exit cleanly when the job queue is getting closed.

How come there are no threads? Workers object is alive in JobQueue class. What am I missing here?

It happens when the thread pool in the job queue has been emptied (during destruction) and then a coroutine just suddenly realises that it needs to resume.

[github-actions]

This PR has conflicts, please resolve them in order for the PR to be reviewed.

[godexsoft]

We recently merged a refactor to develop that enables clang-tidy's readability-identifier-naming. Your branch now has heavy conflicts that are largely mechanical. Below is a workflow that aligns your branch's naming with develop before merging, which should minimize the merge conflicts.

One-time setup

If you don't already have clang-tidy working in your env, on macOS:

brew install llvm@21
# Follow brew's hint to put $(brew --prefix llvm@21)/bin on PATH so run-clang-tidy is found.

Workflow on your branch (before merging develop)

1. Grab the new .clang-tidy from develop without pulling anything else. Sync your fork on GitHub first, then:

git remote -v   # should show 'upstream' among others; if not:
# git remote set-url upstream git@github.com:XRPLF/rippled.git
git fetch upstream
git checkout upstream/develop -- .clang-tidy

2. Reconfigure conan/cmake so compile_commands.json is fresh.

3. Apply renames for the files modified in your PR:

git diff --name-only $(git merge-base HEAD upstream/develop) HEAD \
  | grep -E '\.(cpp|h|hpp|ipp)$' \
  | xargs run-clang-tidy -p build -fix -allow-no-checks
# or -p .build, or whatever your build dir is called

4. Build + test, then commit as a single dedicated commit:

cmake --build build -j8
git commit -am "refactor: Align identifier naming with develop"

5. Now merge develop:

git merge upstream/develop

Extra

Run clang-tidy once more after the merge to catch any stragglers introduced from develop's side:

run-clang-tidy -p build -fix -allow-no-checks src tests
# or -p .build, or whatever your build dir is called