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)
- runtime-tsan-options.txt: halt_on_error=true so CI fails fast on the
first unsuppressed report instead of only relying on the exit-code-66
behavior at process end.
- tsan.supp: drop entries that were redundant with the boost/asio/ and
boost/context/ path patterns (executor.hpp, boost::asio,
fiber::resume, spawned_fiber_thread*), and remove over-broad ones:
race:__tsan_memcpy (matches any race whose top frame is the memcpy
interceptor), race:pipe (substring-matches any function containing
'pipe'; narrowed to __pipe), called_from_lib:libclang_rt (GCC builds
link libtsan, and it would blanket-suppress interceptor reports),
signal:execute_native_thread_routine (matches every std::thread entry
point), and the duplicate std::__cxx11::basic_stringbuf. Document
that path-pattern suppressions match ANY frame in a report.
- workflow: add a lower-bound guard so BUILD_NPROC never drops below 1
after the coverage decrement on small runners.
- docs/build/sanitizers.md: document the halt_on_error=true default and
how to collect all reports locally.
- 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).
- resume() now skips the handle resume when the task is null or done
(duplicate external post() after completion), matching the old
Coro::resume() 'if (coro_)' guard instead of invoking UB in release
builds. The runCount_ bookkeeping still runs to balance post().
- Exceptions escaping a top-level coroutine body were captured by
unhandled_exception() and destroyed unobserved with the frame; they
are now logged at error level before the frame is released.
- Document that join() may return via the finished_ disjunct while the
final resume() is still completing its bookkeeping.
- Remove BoostToStdCoroutineSwitchPlan.md and BoostToStdCoroutineTaskList.md
working documents from the repo root; the rename script rewrites their
'rippled' references and the job fails on the resulting dirty tree.
- Reword two CoroTask.h comments to use 'xrpld'.
- Drop cspell words (cppcoro, gantt, Pratik, Mankawde) that existed only
for the removed documents.
Forward-merge PR 6429 (and develop) into the TSAN branch.
Conflict resolutions:
- JobQueue.h: keep the TSAN-safe postCoroTask (jobCounter_.wrap slot
reservation) on top of the legacy Coro removal.
- CoroTaskRunner.ipp: keep atomic finished_ (acquire loads) with the
renamed mutexRun_ from develop.
- reusable-build-test-config.yml: keep the single sanitizer-options step
and fold the sanitizer single-threaded test parallelism into the
coverage parallelism logic.
- linux.json: add "thread" to the amd64 sanitizer matrix (replaces the
removed generate.py logic).
- CoroTask_test.cpp: rename create_t/jtCLIENT to CreateT/JtClient in
testExpectEarlyExit.
Forward-merge the test-code migration (carrying the latest develop) into the
Boost.Coroutine cleanup.
Conflict resolutions: keep this branch's removal of JobQueue::Coro,
CoroCreateT, postCoro, Coro.ipp and the Boost.Coroutine2 includes, while
taking develop's include set, Doxygen comment style and formatting for the
retained CoroTaskRunner / postCoroTask declarations.
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.
