Skip to content

Commit 8f76761

Browse files
avsejavsej
committed
Prefer libc++ when running sanitizers
Ideally we would have matrix with both standard libraries, but it looks like Vagrind does not like implementation of the std::future in libstdc++, while libc++ passes without false positives
1 parent 4dbe09c commit 8f76761

File tree

6 files changed

+54
-45
lines changed

6 files changed

+54
-45
lines changed

‎.github/workflows/sanitizers.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ jobs:
3131
- name: Install dependencies
3232
run: |
3333
sudo apt-get update -y
34-
sudo apt-get install -y libssl-dev cmake curl wget gnupg2 gdb clang clang-tools valgrind
34+
sudo apt-get install -y libssl-dev cmake curl wget gnupg2 gdb clang clang-tools valgrind libc++-dev libc++abi-dev
3535
- uses: actions/checkout@v4
3636
with:
3737
submodules: recursive
@@ -69,7 +69,7 @@ jobs:
6969
- name: Install dependencies
7070
run: |
7171
sudo apt-get update -y
72-
sudo apt-get install -y libssl-dev cmake curl wget gnupg2 gdb clang clang-tools valgrind
72+
sudo apt-get install -y libssl-dev cmake curl wget gnupg2 gdb clang clang-tools valgrind libc++-dev libc++abi-dev
7373
- uses: actions/checkout@v4
7474
with:
7575
submodules: recursive
@@ -114,7 +114,7 @@ jobs:
114114
- name: Install dependencies
115115
run: |
116116
sudo apt-get update -y
117-
sudo apt-get install -y libssl-dev cmake curl wget gnupg2 gdb clang clang-tools valgrind
117+
sudo apt-get install -y libssl-dev cmake curl wget gnupg2 gdb clang clang-tools valgrind libc++-dev libc++abi-dev
118118
- uses: actions/checkout@v4
119119
with:
120120
submodules: recursive

‎bin/build-tests‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -50,38 +50,39 @@ echo "CB_CMAKE_BUILD_TYPE=${CB_CMAKE_BUILD_TYPE}"
5050
echo "CB_CACHE_OPTION=${CB_CACHE_OPTION}"
5151
echo "CB_TEST_SUITE=${CB_TEST_SUITE}"
5252
echo "CB_BORINGSSL=${CB_BORINGSSL}"
53+
echo "CB_SANITIZER=${CB_SANITIZER}"
54+
55+
CB_CMAKE_EXTRAS=()
56+
57+
if [ "x${CB_SANITIZER}" != "x" ]; then
58+
CB_CMAKE_EXTRAS+=(-DCMAKE_CXX_FLAGS="-stdlib=libc++" -DCMAKE_EXE_LINKER_FLAGS="-stdlib=libc++ -lc++abi")
59+
fi
5360

54-
CB_CMAKE_EXTRAS=
5561
case "${CB_SANITIZER}" in
5662
asan | address)
57-
CB_CMAKE_EXTRAS="-DENABLE_SANITIZER_ADDRESS=ON"
63+
CB_CMAKE_EXTRAS+=(-DENABLE_SANITIZER_ADDRESS=ON)
5864
;;
59-
6065
lsan | leak)
61-
CB_CMAKE_EXTRAS="-DENABLE_SANITIZER_LEAK=ON"
66+
CB_CMAKE_EXTRAS+=(-DENABLE_SANITIZER_LEAK=ON)
6267
;;
63-
6468
ubsan | undefined_behaviour)
65-
CB_CMAKE_EXTRAS="-DENABLE_SANITIZER_UNDEFINED_BEHAVIOUR=ON"
69+
CB_CMAKE_EXTRAS+=(-DENABLE_SANITIZER_UNDEFINED_BEHAVIOUR=ON)
6670
;;
67-
6871
tsan | thread)
69-
CB_CMAKE_EXTRAS="-DENABLE_SANITIZER_THREAD=ON"
72+
CB_CMAKE_EXTRAS+=(-DENABLE_SANITIZER_THREAD=ON)
7073
export TSAN_OPTIONS="second_deadlock_stack=1"
7174
;;
72-
7375
msan | memory)
74-
CB_CMAKE_EXTRAS="-DENABLE_SANITIZER_MEMORY=ON"
76+
CB_CMAKE_EXTRAS+=(-DENABLE_SANITIZER_MEMORY=ON)
7577
;;
76-
7778
valgrind)
78-
CB_CMAKE_EXTRAS="-DCOUCHBASE_CXX_CLIENT_ENABLE_VALGRIND=ON"
79+
CB_CMAKE_EXTRAS+=(-DCOUCHBASE_CXX_CLIENT_ENABLE_VALGRIND=ON)
7980
;;
8081
esac
8182

8283
CB_COLUMNAR=${CB_COLUMNAR:-""}
83-
if [ ! -z "$CB_COLUMNAR" ] ; then
84-
CB_CMAKE_EXTRAS="${CB_CMAKE_EXTRAS} -DCOUCHBASE_CXX_CLIENT_COLUMNAR=ON"
84+
if [ ! -z "${CB_COLUMNAR}" ]; then
85+
CB_CMAKE_EXTRAS+=(-DCOUCHBASE_CXX_CLIENT_COLUMNAR=ON)
8586
fi
8687
echo "CB_COLUMNAR=${CB_COLUMNAR}"
8788

@@ -101,6 +102,8 @@ if [ "x${CB_TEST_SUITE}" != "x" ] ; then
101102
CB_CMAKE_BUILD_EXTRAS="--target build_${CB_TEST_SUITE}_tests"
102103
fi
103104

105+
echo "CB_CMAKE_BUILD_EXTRAS=${CB_CMAKE_BUILD_EXTRAS}"
106+
104107
${CB_CMAKE} \
105108
-DCMAKE_BUILD_TYPE=${CB_CMAKE_BUILD_TYPE} \
106109
-DCMAKE_C_COMPILER="${CB_CC}" \
@@ -110,7 +113,7 @@ ${CB_CMAKE} \
110113
-DCOUCHBASE_CXX_CLIENT_BUILD_SHARED=ON \
111114
-DCOUCHBASE_CXX_CLIENT_STATIC_BORINGSSL=${CB_BORINGSSL} \
112115
-DCACHE_OPTION="${CB_CACHE_OPTION}" \
113-
${CB_CMAKE_EXTRAS} \
116+
"${CB_CMAKE_EXTRAS[@]}" \
114117
-B "${BUILD_DIR}" \
115118
-S "${PROJECT_ROOT}"
116119

‎core/io/http_session_manager.hxx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public:
114114
void update_config(topology::configuration config) override
115115
{
116116
{
117-
std::scoped_lock config_lock(config_mutex_, sessions_mutex_, next_index_mutex_);
117+
std::scoped_lock config_lock(sessions_mutex_, config_mutex_, next_index_mutex_);
118118
config_ = std::move(config);
119119
if (!config_.nodes.empty() && next_index_ >= config_.nodes.size()) {
120120
next_index_ = 0;

‎core/io/mcbp_session.cxx‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,16 +1751,20 @@ class mcbp_session_impl
17511751
}
17521752
}
17531753
state_ = diag::endpoint_state::connected;
1754-
const std::scoped_lock lock(pending_buffer_mutex_);
17551754
bootstrapped_ = true;
17561755
bootstrap_handler_->stop();
17571756
handler_ = std::make_shared<message_handler>(shared_from_this());
17581757
handler_->start();
1759-
if (!pending_buffer_.empty()) {
1760-
for (auto& buf : pending_buffer_) {
1758+
1759+
std::vector<std::vector<std::byte>> pending_buffer{};
1760+
{
1761+
const std::scoped_lock lock(pending_buffer_mutex_);
1762+
std::swap(pending_buffer, pending_buffer_);
1763+
}
1764+
if (!pending_buffer.empty()) {
1765+
for (auto& buf : pending_buffer) {
17611766
write(std::move(buf));
17621767
}
1763-
pending_buffer_.clear();
17641768
flush();
17651769
}
17661770
}

‎test/test_integration_connect.cxx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,10 @@ TEST_CASE("integration: destroy cluster without waiting for close completion", "
156156
"SDK. FIXME");
157157
}
158158

159-
asio::io_context io{};
159+
asio::io_context io{ ASIO_CONCURRENCY_HINT_SAFE };
160160

161161
couchbase::core::cluster cluster(io);
162-
auto io_thread = std::thread([&io]() {
162+
auto io_thread = std::thread([&io]() -> void {
163163
io.run();
164164
});
165165

‎test/utils/integration_shortcuts.cxx‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,15 @@ namespace test::utils
2727
void
2828
open_cluster(const couchbase::core::cluster& cluster, const couchbase::core::origin& origin)
2929
{
30-
auto barrier = std::make_shared<std::promise<std::error_code>>();
31-
auto f = barrier->get_future();
30+
std::promise<std::error_code> barrier;
31+
auto f = barrier.get_future();
3232
#ifdef COUCHBASE_CXX_CLIENT_COLUMNAR
33-
cluster.open_in_background(origin, [barrier](std::error_code ec) mutable {
34-
barrier->set_value(ec);
33+
cluster.open_in_background(origin, [barrier = std::move(barrier)](std::error_code ec) mutable {
34+
barrier.set_value(ec);
3535
});
3636
#else
37-
cluster.open(origin, [barrier](std::error_code ec) mutable {
38-
barrier->set_value(ec);
37+
cluster.open(origin, [barrier = std::move(barrier)](std::error_code ec) mutable -> void {
38+
barrier.set_value(ec);
3939
});
4040
#endif
4141
auto rc = f.get();
@@ -49,22 +49,23 @@ open_cluster(const couchbase::core::cluster& cluster, const couchbase::core::ori
4949
void
5050
close_cluster(const couchbase::core::cluster& cluster)
5151
{
52-
auto barrier = std::make_shared<std::promise<void>>();
53-
auto f = barrier->get_future();
54-
cluster.close([barrier]() {
55-
barrier->set_value();
52+
std::promise<void> barrier;
53+
auto f = barrier.get_future();
54+
cluster.close([barrier = std::move(barrier)]() mutable -> void {
55+
barrier.set_value();
5656
});
5757
f.get();
5858
}
5959

6060
void
6161
open_bucket(const couchbase::core::cluster& cluster, const std::string& bucket_name)
6262
{
63-
auto barrier = std::make_shared<std::promise<std::error_code>>();
64-
auto f = barrier->get_future();
65-
cluster.open_bucket(bucket_name, [barrier](std::error_code ec) mutable {
66-
barrier->set_value(ec);
67-
});
63+
std::promise<std::error_code> barrier;
64+
auto f = barrier.get_future();
65+
cluster.open_bucket(bucket_name,
66+
[barrier = std::move(barrier)](std::error_code ec) mutable -> void {
67+
barrier.set_value(ec);
68+
});
6869
auto rc = f.get();
6970
if (rc) {
7071
CB_LOG_CRITICAL("unable to open bucket: {}, name={}", rc.message(), bucket_name);
@@ -75,11 +76,12 @@ open_bucket(const couchbase::core::cluster& cluster, const std::string& bucket_n
7576
void
7677
close_bucket(const couchbase::core::cluster& cluster, const std::string& bucket_name)
7778
{
78-
auto barrier = std::make_shared<std::promise<std::error_code>>();
79-
auto f = barrier->get_future();
80-
cluster.close_bucket(bucket_name, [barrier](std::error_code ec) mutable {
81-
barrier->set_value(ec);
82-
});
79+
std::promise<std::error_code> barrier;
80+
auto f = barrier.get_future();
81+
cluster.close_bucket(bucket_name,
82+
[barrier = std::move(barrier)](std::error_code ec) mutable -> void {
83+
barrier.set_value(ec);
84+
});
8385
auto rc = f.get();
8486
if (rc) {
8587
CB_LOG_CRITICAL("unable to close bucket: {}, name={}", rc.message(), bucket_name);

0 commit comments

Comments
 (0)