Support ThreadSanitizer#57
Open
chenBright wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds first-class ThreadSanitizer (TSan) support across brpc by introducing a unified BUTIL_USE_TSAN switch, teaching TSan about bthread fiber switches, and eliminating/annotating a large set of TSan-reported data races in both production code and unit tests.
Changes:
- Add build/CI knobs for TSan (
--with-tsan,WITH_TSAN) and adjust sanitizer-incompatible test cases. - Replace racy test globals/counters with atomics and add joins/synchronization to avoid thread leaks and races under TSan.
- Add TSan-specific annotations and ordering fixes in core runtime components (bthread scheduling, atomics, logging, sockets, bvar).
Reviewed changes
Copilot reviewed 97 out of 98 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci-linux.yml |
Adds a TSan CI job (needs flag fix). |
BUILD.bazel |
Minor linkopts select cleanup. |
CMakeLists.txt |
Adds WITH_TSAN and sanitizer flags. |
config_brpc.sh |
Adds --with-tsan and mutual exclusion with ASan. |
src/brpc/acceptor.cpp |
Annotate benign monitoring race. |
src/brpc/channel.cpp |
Serialize protocol field init vs concurrent cancel. |
src/brpc/controller.cpp |
Use acq/rel when publishing call-id. |
src/brpc/controller.h |
Atomic load for correlation id value. |
src/brpc/details/method_status.cpp |
Read concurrency counter atomically for bvar. |
src/brpc/event_dispatcher.h |
Make stop flag atomic. |
src/brpc/event_dispatcher_epoll.cpp |
Use atomic stop flag with ordering. |
src/brpc/event_dispatcher_kqueue.cpp |
Use atomic stop flag with ordering. |
src/brpc/input_messenger.cpp |
Annotate pthread_once publication of messenger. |
src/brpc/parallel_channel.cpp |
TSan-friendly ordering for completion fence. |
src/brpc/policy/http2_rpc_protocol.cpp |
Atomically describe last sent stream id. |
src/brpc/policy/http2_rpc_protocol.h |
Fix concurrent stream-id allocation + locking. |
src/brpc/policy/locality_aware_load_balancer.cpp |
Out-of-line no-tsan accessor for benign race. |
src/brpc/policy/locality_aware_load_balancer.h |
Declare no-tsan volatile weight accessor. |
src/brpc/policy/rtmp_protocol.cpp |
Annotate benign concurrent write state. |
src/brpc/rpc_dump.cpp |
Annotate benign reloadable-flag reads. |
src/brpc/rtmp.cpp |
Annotate one-time socket publication; lock timer fields. |
src/brpc/server.cpp |
Make server status atomic + annotate monitoring races. |
src/brpc/server.h |
Store server status as atomic. |
src/brpc/socket.cpp |
Atomics + benign-race annotations for hot paths. |
src/brpc/socket.h |
Atomics for write shutdown/overcrowded/preferred index. |
src/brpc/socket_inl.h |
TSan-friendly EOF decrement ordering. |
src/brpc/socket_map.cpp |
Avoid reloadable-flag races; no-tsan watcher. |
src/brpc/stream.cpp |
Annotate benign races on once-published members. |
src/brpc/stream_impl.h |
Document benign races covered by ctor annotation. |
src/bthread/butex.cpp |
TSan-friendly acquire load instead of fence. |
src/bthread/countdown_event.cpp |
Annotate benign concurrent debug flag writes. |
src/bthread/execution_queue.cpp |
Make queue node linking atomic + TSan HB edges. |
src/bthread/execution_queue_inl.h |
Atomic next + ordering fixes in _more_tasks. |
src/bthread/fd.cpp |
Make epoll thread fields atomic. |
src/bthread/id.cpp |
Annotate by-design races in Id/butex fast paths. |
src/bthread/key.cpp |
Annotate benign lock-free pool probe. |
src/bthread/remote_task_queue.h |
Annotate benign lock-free empty() fast path. |
src/bthread/stack.h |
Add TSan fiber handle to stacks (TSan builds). |
src/bthread/stack_inl.h |
Create/switch/destroy TSan fibers on context switches. |
src/bthread/task_group.cpp |
Add TSan annotations + fix REMOTE priority race. |
src/bthread/task_meta.h |
Annotate by-design races (interrupted/stat). |
src/bthread/timer_thread.cpp |
Annotate benign lock-free bucket/stat sampling. |
src/bthread/work_stealing_queue.h |
Make buffer atomic and avoid unsupported fences in TSan. |
src/butil/atomicops.h |
Select TSan-safe impl via BUTIL_USE_TSAN. |
src/butil/atomicops_internals_gcc_tsan.h |
New: __atomic-based impl for GCC+TSan. |
src/butil/compiler_specific.h |
Auto-detect TSan + add no-sanitize-thread attribute. |
src/butil/containers/mpsc_queue.h |
Mark lock-free enqueue/dequeue as no-tsan. |
src/butil/debug/thread_annotations.h |
New: TSan fiber + benign-race + HB annotations. |
src/butil/details/extended_endpoint.hpp |
Fix refcount ordering to avoid reuse races. |
src/butil/iobuf_inl.h |
Fold acquire fence into RMW for TSan builds. |
src/butil/logging.cc |
No-tsan for MPSC list + annotate benign init races. |
src/butil/logging.h |
Use explicit atomic load for log-every-second state. |
src/butil/object_pool_inl.h |
Annotate benign lock-free free-chunk peek. |
src/butil/resource_pool_inl.h |
Annotate benign lock-free fast paths. |
src/butil/shared_object.h |
Fold acquire fence into RMW under TSan. |
src/butil/threading/platform_thread_posix.cc |
Annotate stack-reuse false positive in params. |
src/bvar/collector.cpp |
Annotate benign stat sampling races. |
src/bvar/collector.h |
Annotate benign speed-limit races. |
src/bvar/default_variables.cpp |
Lock/copy cached values; annotate benign mtime race. |
src/bvar/variable.cpp |
Lock map while counting; stop/join dumper thread at exit. |
src/bvar/window.h |
Annotate benign vptr sampling race during shutdown. |
test/CMakeLists.txt |
Don’t link gperftools under ASan/TSan. |
test/bthread_butex_unittest.cpp |
Join all waiters to avoid leaks/races. |
test/bthread_cond_unittest.cpp |
Replace volatile flags with atomics; gate TSan-incompatible test. |
test/bthread_dispatcher_unittest.cpp |
Make stop/stats atomic and join before reading. |
test/bthread_execution_queue_unittest.cpp |
Atomics for flags + join leaked pthread. |
test/bthread_fd_unittest.cpp |
Replace volatile stop with atomic. |
test/bthread_futex_unittest.cpp |
Track and join waiter threads. |
test/bthread_id_unittest.cpp |
Make shared quit flag atomic. |
test/bthread_mutex_unittest.cpp |
Atomic butex reads; polling instead of fixed sleep; atomics for flags. |
test/bthread_once_unittest.cpp |
Atomic started flags with proper loads/stores. |
test/bthread_ping_pong_unittest.cpp |
Atomic counters for detached writers. |
test/bthread_rwlock_unittest.cpp |
Atomics + acquire/release for readiness. |
test/bthread_sched_yield_unittest.cpp |
Replace volatile stop with atomic. |
test/bthread_timer_thread_unittest.cpp |
Protect run_times with mutex; atomic task ids/sleep. |
test/bthread_unittest.cpp |
Gate fork/scale-sensitive tests under TSan; attr update. |
test/brpc_channel_unittest.cpp |
Make flag atomic; gate multi-thread channel tests under TSan. |
test/brpc_controller_unittest.cpp |
Atomic cancel flag + bounded wait. |
test/brpc_event_dispatcher_unittest.cpp |
Atomics + global accumulator to avoid recycled-object reads. |
test/brpc_hpack_unittest.cpp |
Gate fork-based test under TSan. |
test/brpc_http_rpc_protocol_unittest.cpp |
Atomics for cross-thread state; TSan-safe lifetime sync; gate tests under TSan. |
test/brpc_input_messenger_unittest.cpp |
Gate messenger test suite under TSan. |
test/brpc_load_balancer_unittest.cpp |
Atomics for stop/ready flags. |
test/brpc_protobuf_json_unittest.cpp |
Adds (currently inverted) TSan compilation guard. |
test/brpc_redis_cluster_unittest.cpp |
Atomics for cross-thread flags. |
test/brpc_rtmp_unittest.cpp |
Atomics for callback-written counters. |
test/brpc_socket_unittest.cpp |
Lock shared state; atomics for flags; gate perf tests under TSan. |
test/brpc_ssl_unittest.cpp |
Disables SSL sanity test (currently unconditional). |
test/brpc_streaming_rpc_unittest.cpp |
Atomics for handler state and writable callback. |
test/bvar_multi_dimension_unittest.cpp |
Atomic stop flag. |
test/bvar_reducer_unittest.cpp |
Atomic stop flag. |
test/bvar_sampler_unittest.cpp |
Atomics for counters + loads/stores. |
test/logging_unittest.cc |
Atomic stop flag in async logging loop. |
test/popen_unittest.cpp |
Gate clone test under TSan; atomics; heap arg to avoid stack reuse FP. |
test/safe_sprintf_unittest.cc |
Disable death tests under TSan. |
test/thread_collision_warner_unittest.cc |
Atomic failure flag. |
test/thread_key_unittest.cpp |
Replace shared bools with atomics. |
test/watchdog_unittest.cc |
Atomic counter; stop watchdog before vptr teardown. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c8a7f18 to
bdf43bf
Compare
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.
What problem does this PR solve?
Issue Number: resolve
Problem Summary:
What is changed and the side effects?
Changed:
Side effects:
Performance effects:
Breaking backward compatibility:
Check List: