clang-format placed the suppression comments two lines above the lambda
expressions (before the postCoroTask call), so NOLINTNEXTLINE suppressed
the wrong line and clang-tidy still reported
cppcoreguidelines-avoid-capturing-lambda-coroutines at the lambda. Move
the comments inside the argument list, immediately before each lambda,
matching the pattern already used in threadSpecificStorage.
The JobQueue_test and Coroutine_test coroutine lambdas capture pointers
to test-scope locals, but each test drives the coroutine to completion
(or, for the stopped-queue case, guarantees it never starts) before the
locals go out of scope. Add NOLINTNEXTLINE for
cppcoreguidelines-avoid-capturing-lambda-coroutines with comments
explaining the lifetime guarantee, and drop the unused <memory> include
flagged by misc-include-cleaner.
The Path_test and TestHelpers coroutine lambdas capture locals by
reference, but the caller blocks on Gate::waitFor() until the coroutine
signals completion, so the captures cannot dangle. Add NOLINTNEXTLINE
for cppcoreguidelines-avoid-capturing-lambda-coroutines with a comment
explaining the lifetime guarantee.
- Suppress cppcoreguidelines-avoid-capturing-lambda-coroutines with
NOLINT where lifetime is guaranteed: GRPCServer (thisShared keeps
CallData alive), ServerHandler RPC/WS clients (captures by value,
handler outlives JobQueue jobs), and PathMPT_test (test blocks on
Gate::waitFor until the coroutine completes).
- Change ServerHandler::processRequest to take Output const& to fix
cppcoreguidelines-rvalue-reference-param-not-moved.
- Replace std::lock_guard with std::scoped_lock in RipplePathFind.
- Remove redundant includes and default member initializer in
Context.h, GRPCServer.h, RipplePathFind.cpp.
- NOLINT the compiler-mandated coroutine protocol names (promise_type,
await_ready, ...) that conflict with readability-identifier-naming and
readability-convert-member-functions-to-static
- Add [[nodiscard]] to handle(), done(), await_ready()
- Replace std::lock_guard with std::scoped_lock const
- Pass CoroTaskRunner name by value and std::move it; default-init
runCount_ in-class
- Drop unused <coroutine> include from JobQueue.h; include
instrumentation.h directly in JobQueueAwaiter.h
- CoroTask_test: kN constant naming, const locals, file-level NOLINT for
cppcoreguidelines-avoid-capturing-lambda-coroutines (lifetimes are
gated and joined)
- PostCoroTest2 called runner->resume() directly from the test thread,
violating the documented precondition on CoroTaskRunner::resume() (a
post() must precede every resume) and driving runCount_ negative.
Rewrite the loop as post()+join(); the non-atomic yieldCount now also
verifies the happens-before edge join() provides via mutexRun_.
- PostCoroTest3 wrote false into an already-false flag, so it could
not detect the coroutine running after stop(). Write true and assert
the flag stays false.
- Coroutine_test: early-return when a Gate waitFor() times out instead
of falling through to c->join()/a[i]->join(), which would deref a
null shared_ptr (correctOrder, threadSpecificStorage first loop) or
block indefinitely (second loop).
- Replace the magic 30s in doRipplePathFind with
RPC::Tuning::kPathfindCompletionTimeout.
- In PathRequestManager::updateAll, invoke updateComplete() via a
ScopeExit guard on the one-shot (hasCompletion) path so the blocked
RPC handler is released immediately even if doUpdate throws, instead
of waiting out the full timeout.
The no-ledger branch of doRipplePathFind parks its JobQueue worker on a
condition_variable for up to 30s, waiting for a completion that is fired
from a JtUpdatePf job -- which itself needs a free worker to run. With
unbounded JtClientRpc concurrency, enough simultaneous ripple_path_find
calls could park every worker, stalling the entire JobQueue until the
30s timeouts expired. The old Boost.Coroutine implementation suspended
and released the worker, so it did not have this failure mode.
Reuse the RPC::LegacyPathFind guard (already applied on the
ledger-specified branch) to admit at most kMaxPathfindsInProgress
non-admin blocking requests, returning rpcTOO_BUSY beyond that. The
guard stays in scope across the wait.
- testExceptionPropagation now co_awaits an inner CoroTask<void> that
throws and asserts the rethrown message, covering the
CoroTask<void>::await_resume rethrow path; the old version could not
distinguish a throw from a normal return.
- testJobQueueAwaiter now actually awaits the JobQueueAwaiter struct
(single use per coroutine, per the GCC-12 note) and asserts the full
ordered step sequence instead of only the terminal value.
- testValueException asserts the caught exception's message.
- All waitFor() timeouts early-return on failure instead of falling
through to join()/state reads (null-deref and TSAN-race hazards on
timeout, plus the nSuspend_ assert in ~Env).
- Remove dead shared_ptr locals in testCorrectOrder/testMultipleYields.
- Add missing includes (<array>, <stdexcept>, <string>, <vector>,
LocalValue.h, CoroTask.h).
Forward-merge the entry-point migration (carrying the latest develop) into
the test-code migration.
Conflict resolutions in Coroutine_test.cpp and JobQueue_test.cpp: keep the
postCoroTask / CoroTaskRunner form from this branch, with develop's renames
(JtClient, Gate::waitFor, kN) and develop's initialization of the
unprotected flag.
Forward-merge the updated coroutine primitives (which themselves carry the
latest develop) into the entry-point migration.
Conflict resolutions:
- .cspell.config.yaml: keep develop's dotfile name, merge word lists.
- Context.h: drop the coro member; infoSub gets a default initializer.
- Application.cpp, ServerHandler.cpp, TestHelpers.cpp, Path_test.cpp,
PathMPT_test.cpp: adopt develop's designated-initializer JsonContext and
renamed identifiers (JtClient, kApiVersionIfUnspecified, kMaxSrcCur, Gate).
- GRPCServer: process(coro) becomes processRequest(), posted with
postCoroTask.
- RipplePathFind.cpp: follow develop's move to handlers/orderbook/ and
PathRequestManager; the completion state shared with the path-finding
continuation is now heap-allocated so it outlives an early return.
- AMMTest.cpp: develop moved find_paths_request into TestHelpers.cpp, so the
local copy is dropped and the migration applied there instead.
Resolves the develop rename of cspell.config.yaml and the JobQueue.h
conflicts, and aligns the new coroutine primitives with develop naming
(CreateT, mutex_/mutexRun_, JtClient, forceMultiThread,
beast::unit_test::Suite).
Also addresses two review findings:
- postCoroTask now holds a jobCounter_ reservation for the whole
function. JobQueue::stop() joins jobCounter_ before asserting
nSuspend_ == 0, so the reservation closes the window between the
++nSuspend_ and the balancing post()/expectEarlyExit(), and doubles
as the shutdown check.
- YieldPostAwaiter::await_suspend returns a coroutine_handle<>
(symmetric transfer) instead of resuming inline. This keeps a
yield loop against a stopping JobQueue from growing the stack
without bound and avoids touching a frame that resume() may have
already destroyed.
Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
