Add TSAN fiber-switching support, sanitizer/valgrind bazel configs, and CI#33
Merged
Conversation
…nd CI
Following the same pattern as the existing AddressSanitizer integration,
the coroutine library now cooperates with ThreadSanitizer via its fiber
API (__tsan_create_fiber / __tsan_switch_to_fiber / ...) on every
coroutine context switch, eliminating spurious data-race reports across
cooperative yields between the scheduler and its coroutines.
* detect_sanitizers.h: add CO_THREAD_SANITIZER and
CO_DISABLE_THREAD_SANITIZER, mirroring the ASAN macros.
* coroutine.h: forward-declare the TSan fiber API and add a
per-coroutine tsan_fiber_ plus a per-scheduler tsan_fiber_.
* coroutine.cc: switch fibers on every Resume/Yield/Exit and at the
end of InvokeFunction; create and name a fiber in the Coroutine
ctor, destroy it in the dtor, and capture the scheduler thread's
fiber lazily in Run().
Also adds .bazelrc configurations for asan, tsan, and valgrind so the
test suite can be exercised under each, e.g.
bazel test --config=asan //co:coroutines_test //co:test_cpp20
bazel test --config=tsan //co:coroutines_test //co:test_cpp20
bazel test --config=valgrind //co:coroutines_test //co:test_cpp20
And adds a GitHub Actions CI workflow (.github/workflows/ci.yml)
modeled on the one used by dallison/subspace, extended to exercise
every config above on every push and pull request.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Re-opens the work from the (now-reverted) PR #32 with clean commit messages
(no
Co-authored-by: Cursor <[email protected]>trailers, since BCRrejects those). Same content, single squashable commit on top of
081378c.API (
__tsan_create_fiber/__tsan_switch_to_fiber/ ...), in the samespirit as the existing AddressSanitizer integration. Every cooperative
context switch now tells TSan which fiber it is leaving and which it is
entering, so yields between the scheduler and its coroutines no longer look
like data races.
--config=profiles in.bazelrcfor running thetest suite under sanitizers / Memcheck:
--config=asan—-fsanitize=addresswith frame pointers and debug info.--config=tsan—-fsanitize=threadwith frame pointers and debug info.--config=valgrind—-O1 -g, runs each test via--run_underwithvalgrind --error-exitcode=1 --leak-check=full --track-origins=yes .....github/workflows/ci.yml) modeled onthe one used by
dallison/subspace,extended to exercise every config above on every push and pull request.
Implementation notes
TSan integration
detect_sanitizers.hgainsCO_THREAD_SANITIZER(auto-detected from__has_feature(thread_sanitizer)for clang and__SANITIZE_THREAD__forgcc) and a matching
CO_DISABLE_THREAD_SANITIZERattribute.coroutine.hforward-declares the five TSan fiber entry points we use andadds a
tsan_fiber_member to bothCoroutineandCoroutineScheduler.coroutine.cc:CO_TSAN_SWITCH_TO_FIBER(fiber)helper that compiles to__tsan_switch_to_fiber(fiber, 0)under TSan and a no-op otherwise.SWAPCONTEXTmacro variants(setjmp / ucontext / custom-context) and into every
SETCONTEXT(resume_),SETCONTEXT(exit_), and the tail ofInvokeFunction.it in the destructor.
Run(),since the scheduler may have been constructed on a different thread.
Coroutine::ResumewithCO_DISABLE_THREAD_SANITIZER(itcontains hand-written stack-switching assembly that TSan should not try
to instrument) alongside the existing
CO_DISABLE_ADDRESS_SANITIZER.CI workflow
.github/workflows/ci.ymldefines three jobs, each usingbazel-contrib/[email protected]with bazelisk / disk / repository caching:Build & test(matrix)ubuntu-latest,ubuntu-24.04-arm,macos-latest(with--config=apple_silicon)bazel build //...thenbazel test //...sanitizers(matrix:asan,tsan)ubuntu-latestbazel test --config=$config //...valgrindubuntu-latestvalgrindvia apt, thenbazel test --config=valgrind //...Failed runs upload
bazel-testlogs/as artifacts for post-mortem.Test plan
bazel build //co:co //co:cotest //co:costress //co:coroutines_test— passes locally.bazel test --config=asan //co:coroutines_test //co:test_cpp20— both pass locally.bazel test --config=tsan //co:coroutines_test //co:test_cpp20— both pass locally.bazel build --config=valgrind //co:coroutines_test //co:test_cpp20— builds clean locally (running under Valgrind requires Linux).Build & test (macos-latest / ubuntu-24.04-arm / ubuntu-latest),asan,tsan,valgrind); CI on this PR will reproduce the run.Notes for review
Either way the resulting commit message will be clean (no Cursor trailer).
relying on the dangling 3.3.1 / 3.3.2 tags from the previous attempt.