Skip to content

Support ThreadSanitizer #57

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chenBright wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support ThreadSanitizer #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
232 changes: 3 additions & 229 deletions .github/workflows/ci-linux.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,215 +19,14 @@ concurrency:

# https://github.com/actions/runner-images
jobs:
compile-with-make:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v2
- uses: ./.github/actions/install-all-dependencies

- name: gcc with default options
uses: ./.github/actions/compile-with-make
with:
options: --headers=/usr/include --libs=/usr/lib /usr/lib64 --cc=gcc --cxx=g++ --werror

- name: gcc with all options
uses: ./.github/actions/compile-with-make
with:
options: --headers=/usr/include --libs=/usr/lib /usr/lib64 --cc=gcc --cxx=g++ --werror \
--with-thrift --with-glog --with-rdma --with-debug-bthread-sche-safety \
--with-debug-lock --with-bthread-tracer --with-asan

- name: clang with default options
uses: ./.github/actions/compile-with-make
with:
options: --headers=/usr/include --libs=/usr/lib /usr/lib64 --cc=clang --cxx=clang++ --werror

- name: clang with all options
uses: ./.github/actions/compile-with-make
with:
options: --headers=/usr/include --libs=/usr/lib /usr/lib64 --cc=clang --cxx=clang++ --werror \
--with-thrift --with-glog --with-rdma --with-debug-bthread-sche-safety \
--with-debug-lock --with-bthread-tracer --with-asan

compile-with-cmake:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v2
- uses: ./.github/actions/install-all-dependencies

- name: gcc with default options
run: |
export CC=gcc && export CXX=g++
mkdir gcc_build && cd gcc_build && cmake -DCMAKE_POLICY_VERSION_MINIMUM=3.5 ..
make -j ${{env.proc_num}} && make clean

- name: gcc with all options
run: |
export CC=gcc && export CXX=g++
mkdir gcc_build_all && cd gcc_build_all
cmake -DWITH_MESALINK=OFF -DWITH_GLOG=ON -DWITH_THRIFT=ON -DWITH_RDMA=ON \
-DWITH_DEBUG_BTHREAD_SCHE_SAFETY=ON -DWITH_DEBUG_LOCK=ON -DWITH_BTHREAD_TRACER=ON \
-DWITH_ASAN=ON -DCMAKE_POLICY_VERSION_MINIMUM=3.5 ..
make -j ${{env.proc_num}} && make clean

- name: clang with default options
run: |
export CC=clang && export CXX=clang++
mkdir clang_build && cd clang_build && cmake -DCMAKE_POLICY_VERSION_MINIMUM=3.5 ..
make -j ${{env.proc_num}} && make clean

- name: clang with all options
run: |
export CC=clang && export CXX=clang++
mkdir clang_build_all && cd clang_build_all
cmake -DWITH_MESALINK=OFF -DWITH_GLOG=ON -DWITH_THRIFT=ON -DWITH_RDMA=ON \
-DWITH_DEBUG_BTHREAD_SCHE_SAFETY=ON -DWITH_DEBUG_LOCK=ON -DWITH_BTHREAD_TRACER=ON \
-DWITH_ASAN=ON -DCMAKE_POLICY_VERSION_MINIMUM=3.5 ..
make -j ${{env.proc_num}} && make clean

gcc-compile-with-make-protobuf:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v2
- uses: ./.github/actions/install-essential-dependencies

- name: protobuf 3.5.1
uses: ./.github/actions/compile-with-make-protobuf
with:
protobuf-version: 3.5.1
protobuf-cpp-version: 3.5.1
protobuf-install-dir: /protobuf-3.5.1
config-brpc-options: --cc=gcc --cxx=g++ --werror

- name: protobuf 3.12.4
uses: ./.github/actions/compile-with-make-protobuf
with:
protobuf-version: 3.12.4
protobuf-cpp-version: 3.12.4
protobuf-install-dir: /protobuf-3.12.4
config-brpc-options: --cc=gcc --cxx=g++ --werror

- name: protobuf 21.12
uses: ./.github/actions/compile-with-make-protobuf
with:
protobuf-version: 21.12
protobuf-cpp-version: 3.21.12
protobuf-install-dir: /protobuf-3.21.12
config-brpc-options: --cc=gcc --cxx=g++ --werror

gcc-unittest-with-bazel:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v2
# Install redis-server/mysql-server so the integration tests that fork a
# real server (e.g. brpc_redis_unittest) actually run under bazel instead
# of skipping. Same shared action the make-based unittest jobs use.
- uses: ./.github/actions/install-essential-dependencies
- run: bazel test //test/...

gcc-compile-with-bazel-all-options:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v2
- run: |
bazel build --define with_mesalink=false \
--define with_glog=true \
--define with_thrift=true \
--define BRPC_WITH_BORINGSSL=true \
--define with_debug_bthread_sche_safety=true \
--define with_debug_lock=true \
--define with_asan=true \
--define with_bthread_tracer=true \
--define BRPC_WITH_NO_PTHREAD_MUTEX_HOOK=true \
--define with_babylon_counter=true \
-- //:brpc

clang-compile-with-make-protobuf:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v2
- uses: ./.github/actions/install-essential-dependencies

- name: protobuf 3.5.1
uses: ./.github/actions/compile-with-make-protobuf
with:
protobuf-version: 3.5.1
protobuf-cpp-version: 3.5.1
protobuf-install-dir: /protobuf-3.5.1
config-brpc-options: --cc=clang --cxx=clang++ --werror

- name: protobuf 3.12.4
uses: ./.github/actions/compile-with-make-protobuf
with:
protobuf-version: 3.12.4
protobuf-cpp-version: 3.12.4
protobuf-install-dir: /protobuf-3.12.4
config-brpc-options: --cc=clang --cxx=clang++ --werror

- name: protobuf 21.12
uses: ./.github/actions/compile-with-make-protobuf
with:
protobuf-version: 21.12
protobuf-cpp-version: 3.21.12
protobuf-install-dir: /protobuf-3.21.12
config-brpc-options: --cc=clang --cxx=clang++ --werror

clang-unittest-with-bazel:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v2
# Install redis-server/mysql-server so the forked-server integration tests
# actually run under bazel (see gcc-unittest-with-bazel).
- uses: ./.github/actions/install-essential-dependencies
- run: |
bazel test --test_output=streamed \
--action_env=CC=clang \
//test/...

clang-compile-with-bazel-all-options:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v2
- run: |
bazel build --action_env=CC=clang \
--define with_mesalink=false \
--define with_glog=true \
--define with_thrift=true \
--define BRPC_WITH_BORINGSSL=true \
--define with_debug_bthread_sche_safety=true \
--define with_debug_lock=true \
--define with_asan=true \
--define with_bthread_tracer=true \
--define BRPC_WITH_NO_PTHREAD_MUTEX_HOOK=true \
--define with_babylon_counter=true \
-- //:brpc

clang-unittest:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v2
- uses: ./.github/actions/install-essential-dependencies
- uses: ./.github/actions/init-ut-make-config
with:
options: --with-bthread-tracer
- name: compile tests
run: |
cat config.mk
cd test
make -j ${{env.proc_num}}
- name: run tests
run: |
cd test
sh ./run_tests.sh

clang-unittest-asan:
clang-unittest-tsan:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v2
- uses: ./.github/actions/install-essential-dependencies
- uses: ./.github/actions/init-ut-make-config
with:
options: --with-bthread-tracer --with-asan
options: --with-bthread-tracer --with-tsan
- name: compile tests
run: |
cat config.mk
Expand All @@ -238,32 +37,7 @@ jobs:
cd test
# The redis integration tests (sanity/keys_with_spaces/incr_and_decr/by_components/auth)
# fork a real redis-server and connect after a fixed 50ms wait; under ASan redis starts
# too slowly, so they flake here (connection refused). Skip just those under ASan; the
# too slowly, so they flake here (connection refused). Skip just those under TSan; the
# redis codec/server tests still run, and the full suite runs in clang-unittest.
GTEST_FILTER='-RedisTest.sanity:RedisTest.keys_with_spaces:RedisTest.incr_and_decr:RedisTest.by_components:RedisTest.auth' sh ./run_tests.sh

clang-unittest-bazel-with-babylon-and-new-pb:
runs-on: ubuntu-22.04
env:
TEST_PROTOBUF_VERSION: "34.1"
# protobuf >= 34.x uses new ProtoInfo fields (option_deps,
# extension_declarations) introduced in Bazel 8.x. The repo's
# .bazelversion (7.2.1) is too old. bazelisk honors USE_BAZEL_VERSION.
USE_BAZEL_VERSION: "8.3.1"
steps:
- uses: actions/checkout@v2
# Install redis-server/mysql-server so the forked-server integration tests
# actually run under bazel (see gcc-unittest-with-bazel).
- uses: ./.github/actions/install-essential-dependencies
- name: Override protobuf version for testing
run: |
sed -i -E "s/(bazel_dep\(name = ['\"]protobuf['\"], version = ['\"])[^'\"]+/\1${TEST_PROTOBUF_VERSION}/" MODULE.bazel
echo "After override:"
grep -E "bazel_dep\(name = ['\"]protobuf['\"]" MODULE.bazel
grep -qE "bazel_dep\(name = ['\"]protobuf['\"], version = ['\"]${TEST_PROTOBUF_VERSION}['\"]" MODULE.bazel \
|| { echo "ERROR: failed to override protobuf version in MODULE.bazel to ${TEST_PROTOBUF_VERSION}"; exit 1; }
- run: |
bazel test --action_env=CC=clang --config=rdma \
--define with_bthread_tracer=true \
--define with_babylon_counter=true \
//test/... --test_arg=--gtest_filter=-RdmaRpcTest.*
6 changes: 3 additions & 3 deletions BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ LINKOPTS = [
],
"//conditions:default": [],
}) + select({
"//bazel/config:brpc_with_asan": ["-fsanitize=address"],
"//conditions:default": [],
})
"//bazel/config:brpc_with_asan": ["-fsanitize=address"],
"//conditions:default": [],
})

genrule(
name = "config_h",
Expand Down
12 changes: 12 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ option(WITH_RDMA "With RDMA" OFF)
option(WITH_DEBUG_BTHREAD_SCHE_SAFETY "With debugging bthread sche safety" OFF)
option(WITH_DEBUG_LOCK "With debugging lock" OFF)
option(WITH_ASAN "With AddressSanitizer" OFF)
option(WITH_TSAN "With ThreadSanitizer" OFF)
option(BUILD_UNIT_TESTS "Whether to build unit tests" OFF)
option(BUILD_FUZZ_TESTS "Whether to build fuzz tests" OFF)
option(BUILD_BRPC_TOOLS "Whether to build brpc tools" ON)
Expand Down Expand Up @@ -137,10 +138,21 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
endif()

set(CMAKE_CPP_FLAGS "${CMAKE_CPP_FLAGS} ${DEFINE_CLOCK_GETTIME} -DBRPC_WITH_GLOG=${WITH_GLOG_VAL} -DBRPC_WITH_RDMA=${WITH_RDMA_VAL} -DBRPC_DEBUG_BTHREAD_SCHE_SAFETY=${WITH_DEBUG_BTHREAD_SCHE_SAFETY_VAL} -DBRPC_DEBUG_LOCK=${WITH_DEBUG_LOCK_VAL}")
if (WITH_ASAN AND WITH_TSAN)
message(FATAL_ERROR "WITH_ASAN and WITH_TSAN are mutually exclusive")
endif()
if (WITH_ASAN)
set(CMAKE_CPP_FLAGS "${CMAKE_CPP_FLAGS} -fsanitize=address")
set(CMAKE_C_FLAGS "${CMAKE_CPP_FLAGS} -fsanitize=address")
endif()
if (WITH_TSAN)
# THREAD_SANITIZER switches butil/atomicops.h to the TSan-aware atomic
# implementation and lets other modules skip TSan-incompatible code paths.
# Benign-race annotations (butil/debug/thread_annotations.h) call the TSan
# runtime's AnnotateBenignRaceSized directly, so no extra defines are needed.
set(CMAKE_CPP_FLAGS "${CMAKE_CPP_FLAGS} -fsanitize=thread")
set(CMAKE_C_FLAGS "${CMAKE_CPP_FLAGS} -fsanitize=thread")
endif()
if(WITH_MESALINK)
set(CMAKE_CPP_FLAGS "${CMAKE_CPP_FLAGS} -DUSE_MESALINK")
endif()
Expand Down
17 changes: 16 additions & 1 deletion config_brpc.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,14 @@ else
LDD=ldd
fi

TEMP=`getopt -o v: --long headers:,libs:,cc:,cxx:,with-glog,with-thrift,with-rdma,with-mesalink,with-bthread-tracer,with-debug-bthread-sche-safety,with-debug-lock,with-asan,nodebugsymbols,werror -n 'config_brpc' -- "$@"`
TEMP=`getopt -o v: --long headers:,libs:,cc:,cxx:,with-glog,with-thrift,with-rdma,with-mesalink,with-bthread-tracer,with-debug-bthread-sche-safety,with-debug-lock,with-asan,with-tsan,nodebugsymbols,werror -n 'config_brpc' -- "$@"`
WITH_GLOG=0
WITH_THRIFT=0
WITH_RDMA=0
WITH_MESALINK=0
WITH_BTHREAD_TRACER=0
WITH_ASAN=0
WITH_TSAN=0
BRPC_DEBUG_BTHREAD_SCHE_SAFETY=0
DEBUGSYMBOLS=-g
WERROR=
Expand Down Expand Up @@ -92,6 +93,7 @@ while true; do
--with-debug-bthread-sche-safety ) BRPC_DEBUG_BTHREAD_SCHE_SAFETY=1; shift 1 ;;
--with-debug-lock ) BRPC_DEBUG_LOCK=1; shift 1 ;;
--with-asan) WITH_ASAN=1; shift 1 ;;
--with-tsan) WITH_TSAN=1; shift 1 ;;
--nodebugsymbols ) DEBUGSYMBOLS=; shift 1 ;;
--werror ) WERROR=-Werror; shift 1 ;;
-- ) shift; break ;;
Expand Down Expand Up @@ -385,11 +387,21 @@ fi

CPPFLAGS=

if [ $WITH_ASAN != 0 ] && [ $WITH_TSAN != 0 ]; then
>&2 $ECHO "--with-asan and --with-tsan can not be enabled at the same time"
exit 1
fi

if [ $WITH_ASAN != 0 ]; then
CPPFLAGS="${CPPFLAGS} -fsanitize=address"
DYNAMIC_LINKINGS="$DYNAMIC_LINKINGS -fsanitize=address"
fi

if [ $WITH_TSAN != 0 ]; then
CPPFLAGS="${CPPFLAGS} -fsanitize=thread"
DYNAMIC_LINKINGS="$DYNAMIC_LINKINGS -fsanitize=thread"
fi

LEVELDB_HDR=$(find_dir_of_header_or_die leveldb/db.h)

if [ $WITH_BTHREAD_TRACER != 0 ]; then
Expand Down Expand Up @@ -575,6 +587,8 @@ if [ -z "$TCMALLOC_LIB" ]; then
append_to_output " \$(error \"Fail to find gperftools\")"
elif [ $WITH_ASAN != 0 ]; then
append_to_output " \$(error \"gperftools is not compatible with ASAN\")"
elif [ $WITH_TSAN != 0 ]; then
append_to_output " \$(error \"gperftools is not compatible with TSAN\")"
else
append_to_output " CPPFLAGS+=-DBRPC_ENABLE_CPU_PROFILER"
append_to_output_libs "$TCMALLOC_LIB" " "
Expand Down Expand Up @@ -655,4 +669,5 @@ if [ $WITH_RDMA -ne 0 ]; then print_info "With RDMA: yes"; fi
if [ $WITH_MESALINK -ne 0 ]; then print_info "With MesaLink: yes"; fi
if [ $WITH_BTHREAD_TRACER -ne 0 ]; then print_info "With bthread tracer: yes"; fi
if [ $WITH_ASAN -ne 0 ]; then print_info "With ASAN: yes"; fi
if [ $WITH_TSAN -ne 0 ]; then print_info "With TSAN: yes"; fi
printf "\n${GREEN}brpc is now configured. You can build it with 'make'.${NC}\n"
7 changes: 6 additions & 1 deletion src/brpc/acceptor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@

#include <inttypes.h>
#include <gflags/gflags.h>
#include "butil/fd_guard.h" // fd_guard
#include "butil/fd_guard.h" // fd_guard
#include "butil/fd_utility.h" // make_close_on_exec
#include "butil/time.h" // gettimeofday_us
#include "butil/debug/thread_annotations.h" // BUTIL_TSAN_ANNOTATE_BENIGN_RACE_SIZED
#include "brpc/acceptor.h"
#include "brpc/transport_factory.h"

Expand Down Expand Up @@ -202,6 +203,10 @@ void Acceptor::Join() {
size_t Acceptor::ConnectionCount() const {
// Notice that _socket_map may be modified concurrently. This actually
// assumes that size() is safe to call concurrently.
// The data race here is benign: we're only reading the size for monitoring
// purposes, and occasional stale values are acceptable.
BUTIL_TSAN_ANNOTATE_BENIGN_RACE_SIZED(&_socket_map, sizeof(_socket_map),
"ConnectionCount reads _socket_map.size() without lock");
return _socket_map.size();
}

Expand Down
3 changes: 1 addition & 2 deletions src/brpc/builtin/bthreads_service.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ void BthreadsService::default_method(::google::protobuf::RpcController* cntl_bas
bthread_t tid = strtoull(constraint.c_str(), &endptr, 10);
if (*endptr == '\0' || *endptr == '/' || *endptr == '?') {
::bthread::print_task(os, tid, enable_trace);
}
else if (constraint != "all" && constraint != "all?st=1") {
} else if (constraint != "all" && constraint != "all?st=1") {
cntl->SetFailed(ENOMETHOD, "path=%s is not a bthread id or all, or all?st=1\n",
constraint.c_str());
} else {
Expand Down
Loading
Loading