🚀 Advanced: fix: Switch to boost::coroutine2 by pratikmankawde · Pull Request #6372 · XRPLF/rippled · GitHub

pratikmankawde · 2026-02-16T16:12:51Z

High Level Overview of Change

Switch boost::coroutine with boost::coroutine2, to fix ASAN issues

Context of Change

ASAN wasn't able to keep track of boost::coroutine context switches. It would lead to many false positives from ASAN. Switched to boost::coroutine2 and ucontext, allows ASAN to know about the context switches advertized by boost::fiber class. This leads to more cleaner ASAN analysis.

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
Test based testing.

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

codecov · 2026-02-16T16:38:43Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.8%. Comparing base (b585dc7) to head (866250f).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #6372     +/-   ##
=========================================
- Coverage     79.8%   79.8%   -0.0%     
=========================================
  Files          878     878             
  Lines        67887   67888      +1     
  Branches      7549    7555      +6     
=========================================
- Hits         54206   54197      -9     
- Misses       13681   13691     +10

Files with missing lines	Coverage Δ
include/xrpl/core/Coro.ipp	`100.0% <100.0%> (ø)`
include/xrpl/core/JobQueue.h	`100.0% <ø> (ø)`

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

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

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

…RPLF/rippled into pratik/Fix_asan_boost_coroutine_issues

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

…RPLF/rippled into pratik/Fix_asan_boost_coroutine_issues

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

godexsoft

Leaving some questions.

godexsoft · 2026-03-11T12:19:45Z

    )
 endif()
+
+# if (SANITIZERS_ENABLED AND is_clang)


Is this comment still needed here?

I was testing if we need to instrument boost with asan or not. It was flaky a bit. Removing now.

godexsoft · 2026-03-11T12:29:05Z

+#interceptor_via_fun:swapcontext
+#interceptor_via_fun:makecontext
+#interceptor_via_fun:boost::context::basic_fixedsize_stack*deallocate
+#interceptor_via_fun:boost::context::fiber::~fiber


Does this work in practice? i recall these kind of suppressions did not actually work for me because suppressions "only work on your code", not library code.. or so i read online.

These are commented anyway. I put them for GCC actually, since we can't pass ignorelist at compile time to gcc.

godexsoft · 2026-03-11T12:30:37Z

            self.options["boost"].visibility = "global"
        if self.settings.compiler in ["clang", "gcc"]:
            self.options["boost"].without_cobalt = True
+        self.options["boost"].without_context = False


Does this not duplicate lines 61-63?

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

godexsoft

👍

bthomee · 2026-03-20T17:22:48Z

/ai-review

xrplf-ai-reviewer

Review by Claude Opus 4.6 · Prompt: V12

xrplf-ai-reviewer · 2026-03-20T17:40:24Z

          [this, fn = std::forward<F>(f)](
-              boost::coroutines::asymmetric_coroutine<void>::push_type& do_yield) {
+              boost::coroutines2::asymmetric_coroutine<void>::push_type& do_yield) {
              yield_ = &do_yield;


Dangling raw pointer to coroutine push_type parameter

The yield_ member is assigned the address of do_yield, which is the push_type& parameter passed into the coroutine lambda body. While this pointer is valid during coroutine execution (the parameter lives in the coroutine frame), it becomes a dangling pointer once the coroutine completes or is destroyed. The migration from boost::coroutines to boost::coroutines2 preserves this pre-existing pattern, but boost::coroutines2 has stricter lifetime semantics: after fn(shared_from_this()) returns and the coroutine lambda exits, do_yield is destroyed. Any subsequent call through yield_ (e.g., via a stale Coro reference) would access freed memory. The finished_ debug flag partially guards against this but is only active in debug builds.

There is not path in the code which can create or preserve such a dangling ref. to Coro.
yield can only be called from inside a coroutine's stack, there are no semantics to call it from outside and hence reference any dangling yield_. So the current implementation is safe.

This reverts commit 9838162.

xrplf-ai-reviewer

Three correctness issues flagged inline: wrong coroutine2 type name in Coro.ipp, a no-op default_options mutation in conanfile.py, and missing context-impl=ucontext for the TSAN path in the sanitizers profile.

Review by Claude Opus 4.6 · Prompt: V15

Copilot

Pull request overview

This PR migrates the core JobQueue coroutine implementation from Boost.Coroutine to Boost.Coroutine2 (with Boost.Context/ucontext) to improve AddressSanitizer (ASAN) accuracy around context switches, and updates build/config tooling to match.

Changes:

Replace Boost.Coroutine usage in JobQueue with Boost.Coroutine2 and a protected fixed-size stack allocator.
Update Conan/CMake/CI sanitizer configuration to build/link Boost.Context appropriately and set ASAN runtime options.
Remove now-unneeded ASAN suppressions and update spelling dictionary.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
sanitizers/suppressions/asan.supp	Removes Boost-related suppressions that should no longer be needed with the coroutine2/ucontext approach.
include/xrpl/core/JobQueue.h	Switches coroutine member types and includes to Boost.Coroutine2/Boost.Context.
include/xrpl/core/Coro.ipp	Updates coroutine construction to use protected_fixedsize_stack and coroutine2 API.
cspell.config.yaml	Adds `fcontext` to spelling allowlist.
conanfile.py	Adjusts Boost options/components and adds sanitizer-driven configuration logic.
conan/profiles/sanitizers	Adds sanitizer defines and Boost build options for ucontext/ASAN annotations.
cmake/deps/Boost.cmake	Switches linked Boost component from coroutine to context; adds gcc14 warning workaround and ASAN-related defines.
cmake/XrplInterface.cmake	Updates deprecation-warning suppression macro for Coroutine2.
.github/workflows/reusable-build-test-config.yml	Extends sanitizer job timeout and refines ASAN_OPTIONS (plus gcc-specific tweak).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ASAN wasn't able to keep track of `boost::coroutine` context switches, and would lead to many false positives being detected. By switching to `boost::coroutine2` and `ucontext`, ASAN is able to know about the context switches advertised by the `boost::fiber` class, which in turn leads to more cleaner ASAN analysis.

switch to coroutine 2

78f4ef1

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

pratikmankawde added the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Feb 16, 2026

pratikmankawde added 5 commits February 16, 2026 18:42

set coro stack size of 2mb

45e53a8

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

don't use parallel test exe

b405d23

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

reset stack size

17479dd

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

reduce cell offers

376abfc

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

explicitely set parallel to 1

5d58331

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

pratikmankawde commented Feb 18, 2026

View reviewed changes

Comment thread sanitizers/suppressions/asan.supp

Apply suggestion from @pratikmankawde

8eda790

pratikmankawde commented Feb 18, 2026

View reviewed changes

Comment thread sanitizers/suppressions/asan.supp Outdated

pratikmankawde added 3 commits February 18, 2026 16:11

Apply suggestion from @pratikmankawde

4a8c6c8

added boost context files to ignorelist

580de04

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

Merge branch 'pratik/Fix_asan_boost_coroutine_issues' of github.com:X…

9bad601

…RPLF/rippled into pratik/Fix_asan_boost_coroutine_issues

pratikmankawde commented Feb 18, 2026

View reviewed changes

Comment thread sanitizers/suppressions/asan.supp Outdated

pratikmankawde added 6 commits February 18, 2026 17:35

Apply suggestion from @pratikmankawde

cf3bedb

only run asan

09b8602

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

halt on error=0

6b94f7a

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

Merge branch 'develop' into pratik/Fix_asan_boost_coroutine_issues

550ba26

don't intrument boost code

b926559

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

put nft test changes back

f04338a

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

pratikmankawde commented Feb 19, 2026

View reviewed changes

Comment thread .github/workflows/reusable-build-test-config.yml Outdated

pratikmankawde added 8 commits February 19, 2026 16:31

use_sigaltstack=0

e8ca732

Merge branch 'pratik/Fix_asan_boost_coroutine_issues' of github.com:X…

2397ad7

…RPLF/rippled into pratik/Fix_asan_boost_coroutine_issues

remove duplicates

0893228

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

detect_stack_use_after_return=0

7319b8f

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

alloc_dealloc_mismatch=0 for gcc

ec64579

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

increase ci timeout and use coroutines2::asymmetric_coroutine

76a4900

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

Merge branch 'develop' into pratik/Fix_asan_boost_coroutine_issues

2201264

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

multi run on tests

5956430

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

pratikmankawde added QE test required RippleX QE Team must look at this PR. and removed QE test desired RippleX QE Team should consider looking at this PR. labels Mar 10, 2026

godexsoft reviewed Mar 11, 2026

View reviewed changes

pratikmankawde added 3 commits March 11, 2026 14:26

code review comments

d9aa956

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

remove duplicate

432e3fd

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

Merge branch 'develop' into pratik/Fix_asan_boost_coroutine_issues

cf2db26

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

pratikmankawde requested a review from godexsoft March 11, 2026 14:30

pratikmankawde mentioned this pull request Mar 12, 2026

chore: Move sanitizer runtime options out to files #6371

Merged

Merge branch 'develop' into pratik/Fix_asan_boost_coroutine_issues

0d8919a

godexsoft approved these changes Mar 12, 2026

View reviewed changes

Merge branch 'develop' into pratik/Fix_asan_boost_coroutine_issues

f096d39

pratikmankawde added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Mar 16, 2026

Merge branch 'develop' into pratik/Fix_asan_boost_coroutine_issues

866250f

bthomee enabled auto-merge (squash) March 16, 2026 14:55

bthomee merged commit 9838162 into develop Mar 16, 2026
3 checks passed

bthomee deleted the pratik/Fix_asan_boost_coroutine_issues branch March 16, 2026 15:34

pratikmankawde mentioned this pull request Mar 20, 2026

fix: Guard Coro::resume() against completed coroutines #6608

Merged

2 tasks

xrplf-ai-reviewer Bot reviewed Mar 20, 2026

View reviewed changes

ximinez added a commit that referenced this pull request Apr 8, 2026

Revert "fix: Switch to boost::coroutine2 (#6372)"

421a109

This reverts commit 9838162.

ximinez added a commit that referenced this pull request Apr 9, 2026

Revert "fix: Switch to boost::coroutine2 (#6372)"

e003a57

This reverts commit 9838162.

ximinez added a commit that referenced this pull request Apr 15, 2026

Revert "fix: Switch to boost::coroutine2 (#6372)"

686d175

This reverts commit 9838162.

bthomee requested a review from Copilot April 20, 2026 14:29

Copilot started reviewing on behalf of bthomee April 20, 2026 14:29 View session

xrplf-ai-reviewer Bot reviewed Apr 20, 2026

View reviewed changes

Comment thread conan/profiles/sanitizers

Comment thread conanfile.py

Comment thread include/xrpl/core/Coro.ipp

Copilot AI reviewed Apr 20, 2026

View reviewed changes

Comment thread conan/profiles/sanitizers

Comment thread include/xrpl/core/Coro.ipp

Comment thread conanfile.py

pratikmankawde mentioned this pull request Apr 29, 2026

fix: Address code review comments regarding boost::coroutine2 #6977

Merged

mvadari added this to the 3.2.0 milestone May 20, 2026

Uh oh!

Conversation

pratikmankawde commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

Context of Change

Type of Change

Uh oh!

codecov Bot commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

godexsoft left a comment

Choose a reason for hiding this comment

Uh oh!

godexsoft Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

pratikmankawde Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

godexsoft Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

pratikmankawde Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

godexsoft Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

pratikmankawde Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

godexsoft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bthomee commented Mar 20, 2026

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

pratikmankawde May 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

pratikmankawde commented Feb 16, 2026 •

edited

Loading

codecov Bot commented Feb 16, 2026 •

edited

Loading