Merge branch 'release/v1.1.11'

Author: i-vovk

.github/workflows/run.sh

@@ -21,14 +21,16 @@ build() {
 }
 
 build_testing() {
-    if [[ $# -lt 1 ]]; then
-        echo "Build folder is absent"
+    if [[ $# -lt 2 ]]; then
+        echo "Build folder and/or build type are absent"
         return 1
     fi
 
     declare -r folder="$1"
+    declare -r build_type="$2"
 
-    build "$folder" "testing" "-DSTREAMCLIENT_BUILD_TESTING=ON" \
+    build "$folder" "testing" "-DCMAKE_BUILD_TYPE=$build_type" \
+                              "-DSTREAMCLIENT_BUILD_TESTING=ON" \
                               "-DSTREAMCLIENT_BUILD_DOCS=OFF" \
                               "-DSTREAMCLIENT_BUILD_EXAMPLES=OFF" \
                               "-DCMAKE_EXPORT_COMPILE_COMMANDS=ON"\
@@ -46,10 +48,12 @@ collect_coverage() {
 }
 
 if [[ "$1" == "test" ]] ; then
-    build_testing "build"
+    build_testing "build-tsan" "Tsan"
+    build_testing "build-asan" "Asan"
+    build_testing "build-ubsan" "Ubsan"
 
 elif [[ "$1" == "coverage" ]] ; then
-    build_testing "build"
+    build_testing "build" "Debug"
     collect_coverage "build"
 
 elif [[ "$1" == "collect-coverage" ]] ; then

.gitignore

@@ -41,5 +41,6 @@
 .idea/
 .gradle/
 build/
+build-*/
 coverage-build/
 coverage.*

CHANGELOG.md

@@ -1,3 +1,14 @@
+## 1.1.11 (2021-08-20)
+
+### Bug Fixes
+
+* connector::base_connector: fix UB in dealing with boolean flag
+
+### Misc
+
+* Add thread/address/UB sanitizers build to CI
+
+
 ## 1.1.10 (2021-08-17)
 
 ### Bug Fixes

CMakeLists.txt

@@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.0 FATAL_ERROR)
 
 set(STREAMCLIENT_VERSION_MAJOR "1")
 set(STREAMCLIENT_VERSION_MINOR "1")
-set(STREAMCLIENT_VERSION_RELEASE "10")
+set(STREAMCLIENT_VERSION_RELEASE "11")
 set(STREAMCLIENT_SUMMARY "C++ library")
 set(STREAMCLIENT_REPOSITORY_URL "https://github.com/TinkoffCreditSystems/stream-client")
 set(STREAMCLIENT_LICENSE "${CMAKE_CURRENT_SOURCE_DIR}/LICENSE")
@@ -32,6 +32,30 @@ if(NOT CMAKE_BUILD_TYPE)
     set(CMAKE_BUILD_TYPE "RelWithDebInfo" CACHE STRING "Choose the type of build, options are: Debug Release RelWithDebInfo MinSizeRel." FORCE)
 endif()
 
+# ThreadSanitizer
+set(CMAKE_CXX_FLAGS_TSAN "-fsanitize=thread -g -O1"
+    CACHE STRING "Flags used by the C++ compiler during ThreadSanitizer builds."
+    FORCE
+)
+# AddressSanitize
+set(CMAKE_CXX_FLAGS_ASAN "-fsanitize=address \
+                          -fno-optimize-sibling-calls \
+                          -fsanitize-address-use-after-scope \
+                          -fno-omit-frame-pointer -g -O1"
+    CACHE STRING "Flags used by the C++ compiler during AddressSanitizer builds."
+    FORCE
+)
+# LeakSanitizer
+set(CMAKE_CXX_FLAGS_LSAN "-fsanitize=leak -fno-omit-frame-pointer -g -O1"
+    CACHE STRING "Flags used by the C++ compiler during LeakSanitizer builds."
+    FORCE
+)
+# UndefinedBehaviour
+set(CMAKE_CXX_FLAGS_UBSAN "-fsanitize=undefined"
+    CACHE STRING "Flags used by the C++ compiler during UndefinedBehaviourSanitizer builds."
+    FORCE
+)
+
 option(STREAMCLIENT_BUILD_TESTING "Build included unit-tests" OFF)
 option(STREAMCLIENT_BUILD_DOCS "Build sphinx generated docs" OFF)
 option(STREAMCLIENT_BUILD_EXAMPLES "Build stream-client examples" OFF)
@@ -124,11 +148,13 @@ install(EXPORT ${PROJECT_NAME}-targets
 if(STREAMCLIENT_BUILD_TESTING)
     include(CTest)
 
-    set(CMAKE_C_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-arcs -ftest-coverage")
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-arcs -ftest-coverage")
-    set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-arcs -ftest-coverage")
-    if(NOT APPLE)
-        set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_CXX_FLAGS} -lgcov")
+    if(CMAKE_BUILD_TYPE STREQUAL Debug)
+        set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fprofile-arcs -ftest-coverage")
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-arcs -ftest-coverage")
+        set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fprofile-arcs -ftest-coverage")
+        if(NOT APPLE)
+            set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -lgcov")
+        endif()
     endif()
 
     add_subdirectory(tests)

include/stream-client/connector/connector.hpp

@@ -3,6 +3,7 @@
 #include "stream-client/resolver/resolver.hpp"
 #include "stream-client/stream/http_socket.hpp"
 
+#include <atomic>
 #include <condition_variable>
 #include <mutex>
 
@@ -277,17 +278,17 @@ class base_connector
     endpoint_container_type endpoints_; ///< Resolved endpoints.
     mutable std::mutex endpoints_mutex_; ///< @p endpoints_ mutex.
 
-    bool resolving_thread_running_ = true; ///< Flag to stop @p resolving_thread_.
+    std::atomic_bool resolving_thread_running_{true}; ///< Flag to stop @p resolving_thread_.
     std::thread resolving_thread_; ///< Thread to run resolve_routine() in.
 
     boost::system::error_code resolve_error_; ///< Error value propagated from @p resolving_thread_.
     mutable std::mutex resolve_error_mutex_; ///< @p resolve_error_ mutex.
 
     /* used to implement waits on update/done events without polling */
-    bool resolve_needed_ = true; ///< Flag to trigger @p resolving_thread_ to reresolve.
+    bool resolve_needed_{true}; ///< Flag to trigger @p resolving_thread_ to reresolve.
     std::timed_mutex resolve_needed_mutex_; ///< @p resolve_needed_ mutex.
     std::condition_variable_any resolve_needed_cv_; ///< @p resolve_needed_ condition variable.
-    bool resolve_done_ = false; ///< Flag to notify that @p resolving_thread_ has done @p endpoints_ update.
+    bool resolve_done_{false}; ///< Flag to notify that @p resolving_thread_ has done @p endpoints_ update.
     std::timed_mutex resolve_done_mutex_; ///< @p resolve_done_ mutex.
     std::condition_variable_any resolve_done_cv_; ///< @p resolve_done_ condition variable.
 };

include/stream-client/connector/impl/connector.ipp

@@ -31,14 +31,14 @@ base_connector<Stream>::base_connector(const std::string& host, const std::strin
 {
     resolve_done_ = false;
     resolve_needed_ = true;
-    resolving_thread_running_ = true;
+    resolving_thread_running_.store(true, std::memory_order_release);
     resolving_thread_ = std::thread([this]() { this->resolve_routine(); });
 }
 
 template <typename Stream>
 base_connector<Stream>::~base_connector()
 {
-    resolving_thread_running_ = false;
+    resolving_thread_running_.store(false, std::memory_order_release);
     if (resolving_thread_.joinable()) {
         resolving_thread_.join();
     }
@@ -89,7 +89,7 @@ void base_connector<Stream>::resolve_routine()
 {
     static const auto lock_timeout = std::chrono::milliseconds(100);
 
-    while (resolving_thread_running_) {
+    while (resolving_thread_running_.load(std::memory_order_acquire)) {
         std::unique_lock<std::timed_mutex> resolve_needed_lk(resolve_needed_mutex_, std::defer_lock);
         if (!resolve_needed_lk.try_lock_for(lock_timeout) ||
             !resolve_needed_cv_.wait_for(resolve_needed_lk, lock_timeout, [this] { return resolve_needed_; })) {

tests/timeout.cpp

@@ -12,8 +12,7 @@ TYPED_TEST(ServerEnv, ConnectZeroTimeout)
                       boost::asio::error::timed_out);
 
     const auto elapsed = std::chrono::steady_clock::now() - start_t;
-    EXPECT_TIME_GE(elapsed, std::chrono::milliseconds(0));
-    EXPECT_TIME_LE(elapsed, std::chrono::milliseconds(20));
+    EXPECT_TIME_LE(elapsed, std::chrono::milliseconds(50)); // actual time differs with sanitized builds
 }
 
 TEST_F(TCPServerEnv, ConnectTimeout)

tests/utils/echo_server.hpp

@@ -17,6 +17,11 @@ class echo_session
     {
     }
 
+    echo_session(const echo_session<Socket>& other) = delete;
+    echo_session<Socket>& operator=(const echo_session<Socket>& other) = delete;
+    echo_session(echo_session<Socket>&& other) = default;
+    echo_session<Socket>& operator=(echo_session<Socket>&& other) = default;
+
     virtual ~echo_session() = default;
 
 protected:
@@ -345,12 +350,12 @@ class tcp_base_server: public echo_server<Session>
         auto future = promise.get_future();
 
         session_threads_.emplace_back(
-            [=](std::promise<Session>&& promise) {
-                std::unique_lock<std::mutex> lk(acceptor_mutex_);
+            [this](std::promise<Session>&& promise) {
+                std::unique_lock<std::mutex> lk(this->acceptor_mutex_);
 
                 auto sock = std::make_shared<boost::asio::ip::tcp::socket>(*this->io_service_);
                 this->acceptor_.accept(*sock);
-                promise.set_value_at_thread_exit(Session(this->io_service_, std::move(sock)));
+                promise.set_value(Session(this->io_service_, std::move(sock)));
             },
             std::move(promise));
 
@@ -422,13 +427,13 @@ class ssl_server: public echo_server<ssl_session>
         auto future = promise.get_future();
 
         session_threads_.emplace_back(
-            [=](std::promise<ssl_session>&& promise) {
-                std::unique_lock<std::mutex> lk(acceptor_mutex_);
+            [this](std::promise<ssl_session>&& promise) {
+                std::unique_lock<std::mutex> lk(this->acceptor_mutex_);
 
                 auto session = ssl_session(this->io_service_, context_);
                 this->acceptor_.accept(session.get_socket());
                 session.get_ssl_socket().handshake(boost::asio::ssl::stream_base::server);
-                promise.set_value_at_thread_exit(std::move(session));
+                promise.set_value(std::move(session));
             },
             std::move(promise));
 
@@ -472,8 +477,8 @@ class udp_server: public echo_server<udp_session>
         auto future = promise.get_future();
 
         session_threads_.emplace_back(
-            [=](std::promise<udp_session>&& promise) {
-                promise.set_value_at_thread_exit(udp_session(this->io_service_, this->socket_));
+            [this](std::promise<udp_session>&& promise) {
+                promise.set_value(udp_session(this->io_service_, this->socket_));
             },
             std::move(promise));