ci: Enable tsan on linux64 build (#4563)

Make fMixing atomic as it has concurrent access Add tsan suppression for zmq namespace Suppress deadlock false positive in ConnectTip Switch ubsan target to linux32 Add new test job for linux64_cxx17 target without any sanitizers Increase rpc time out for block reward reallocation test Fix heap use after free in CConnman::GetExtraOutboundCount() Different builds for linux32 and linux64 tsan and ubsan Increase timeout for llmq_signing functional test
dashpay · Jan 10, 2022 · f73fce7 · f73fce7
1 parent 25f5be7
commit f73fce7
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 9 deletions.
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
@@ -234,6 +234,15 @@ linux32-build:
   variables:
     BUILD_TARGET: linux32
 
+linux32_ubsan-build:
+  extends:
+    - .build-template
+    - .skip-in-fast-mode-template
+  needs:
+    - i686-pc-linux-gnu
+  variables:
+    BUILD_TARGET: linux32_ubsan
+
 linux64-build:
   extends: .build-template
   needs:
@@ -248,6 +257,15 @@ linux64_cxx20-build:
   variables:
     BUILD_TARGET: linux64_cxx20
 
+linux64_tsan-build:
+  extends:
+    - .build-template
+    - .skip-in-fast-mode-template
+  needs:
+    - x86_64-unknown-linux-gnu-debug
+  variables:
+    BUILD_TARGET: linux64_tsan
+
 linux64_nowallet-build:
   extends:
     - .build-template
@@ -286,9 +304,27 @@ linux32-test:
   variables:
     BUILD_TARGET: linux32
 
+linux32_ubsan-test:
+  extends:
+    - .test-template
+    - .skip-in-fast-mode-template
+  needs:
+    - linux32_ubsan-build
+  variables:
+    BUILD_TARGET: linux32_ubsan
+
 linux64-test:
   extends: .test-template
   needs:
     - linux64-build
   variables:
     BUILD_TARGET: linux64
+
+linux64_tsan-test:
+  extends:
+    - .test-template
+    - .skip-in-fast-mode-template
+  needs:
+    - linux64_tsan-build
+  variables:
+    BUILD_TARGET: linux64_tsan
diff --git a/ci/dash/matrix.sh b/ci/dash/matrix.sh
@@ -35,6 +35,9 @@ export MAKEJOBS
 export RUN_UNITTESTS=true
 export RUN_INTEGRATIONTESTS=true
 
+# Configure sanitizers options
+export TSAN_OPTIONS="suppressions=${SRC_DIR}/test/sanitizer_suppressions/tsan"
+
 if [ "$BUILD_TARGET" = "arm-linux" ]; then
   export HOST=arm-linux-gnueabihf
   export CHECK_DOC=1
@@ -53,16 +56,27 @@ elif [ "$BUILD_TARGET" = "linux32" ]; then
   export BITCOIN_CONFIG="--enable-zmq --disable-bip70 --enable-reduce-exports --enable-crash-hooks"
   export USE_SHELL="/bin/dash"
   export PYZMQ=true
+elif [ "$BUILD_TARGET" = "linux32_ubsan" ]; then
+  export HOST=i686-pc-linux-gnu
+  export BITCOIN_CONFIG="--enable-zmq --disable-bip70 --enable-reduce-exports --enable-crash-hooks --with-sanitizers=undefined"
+  export USE_SHELL="/bin/dash"
+  export PYZMQ=true
 elif [ "$BUILD_TARGET" = "linux64" ]; then
   export HOST=x86_64-unknown-linux-gnu
   export DEP_OPTS="NO_UPNP=1 DEBUG=1"
-  export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks --with-sanitizers=undefined"
+  export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks"
+  export CPPFLAGS="-DDEBUG_LOCKORDER -DENABLE_DASH_DEBUG -DARENA_DEBUG"
+  export PYZMQ=true
+elif [ "$BUILD_TARGET" = "linux64_tsan" ]; then
+  export HOST=x86_64-unknown-linux-gnu
+  export DEP_OPTS="NO_UPNP=1 DEBUG=1"
+  export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks --with-sanitizers=thread"
   export CPPFLAGS="-DDEBUG_LOCKORDER -DENABLE_DASH_DEBUG -DARENA_DEBUG"
   export PYZMQ=true
 elif [ "$BUILD_TARGET" = "linux64_cxx20" ]; then
   export HOST=x86_64-unknown-linux-gnu
   export DEP_OPTS="NO_UPNP=1 DEBUG=1"
-  export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks --enable-c++20 --enable-suppress-external-warnings --enable-werror --with-sanitizers=undefined"
+  export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks --enable-c++20 --enable-suppress-external-warnings --enable-werror"
   export CPPFLAGS="-DDEBUG_LOCKORDER -DENABLE_DASH_DEBUG -DARENA_DEBUG"
   export PYZMQ=true
   export RUN_INTEGRATIONTESTS=false

diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp
@@ -235,10 +235,8 @@ void CCoinJoinClientSession::ProcessMessage(CNode* pfrom, const std::string& str
 }
 
 bool CCoinJoinClientManager::StartMixing() {
-    if (IsMixing()) {
-        return false;
-    }
-    return fMixing = true;
+    bool expected{false};
+    return fMixing.compare_exchange_strong(expected, true);
 }
 
 void CCoinJoinClientManager::StopMixing() {

diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h
@@ -9,6 +9,7 @@
 #include <coinjoin/coinjoin.h>
 
 #include <utility>
+#include <atomic>
 
 class CDeterministicMN;
 using CDeterministicMNCPtr = std::shared_ptr<const CDeterministicMN>;
@@ -178,7 +179,7 @@ class CCoinJoinClientManager
     // TODO: or map<denom, CCoinJoinClientSession> ??
     std::deque<CCoinJoinClientSession> deqSessions GUARDED_BY(cs_deqsessions);
 
-    bool fMixing{false};
+    std::atomic<bool> fMixing{false};
 
     int nCachedLastSuccessBlock{0};
     int nMinBlocksToWait{1}; // how many blocks to wait for after one successful mixing tx in non-multisession mode

diff --git a/src/net.cpp b/src/net.cpp
@@ -3218,13 +3218,14 @@ void CConnman::Stop()
         }
 
     // clean up some globals (to help leak detection)
-    for (CNode *pnode : vNodes) {
+    std::vector<CNode*> nodes;
+    WITH_LOCK(cs_vNodes, nodes.swap(vNodes));
+    for (CNode *pnode : nodes) {
         DeleteNode(pnode);
     }
     for (CNode *pnode : vNodesDisconnected) {
         DeleteNode(pnode);
     }
-    vNodes.clear();
     mapSocketToNode.clear();
     {
         LOCK(cs_vNodes);

diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan
@@ -1,9 +1,15 @@
 # ThreadSanitizer suppressions
 # ============================
 
+# Data races from zmq namespace
+race:zmq::*
+
 # WalletBatch (unidentified deadlock)
 deadlock:WalletBatch
 
+# deadlock false positive (see: https://github.com/dashpay/dash/pull/4563)
+deadlock:CChainState::ConnectTip
+
 # Intentional deadlock in tests
 deadlock:sync_tests::potential_deadlock_detected