mirror of
https://github.com/XRPLF/rippled.git
synced 2026-06-08 19:26:45 +00:00
fix: Resolve TSAN-detected data races and suppress lock-order-inversion
- Make Journal::Sink::thresh_ atomic to fix data race between concurrent log threshold reads (hot path) and writes (RPC/startup) - Make SHAMap::ledgerSeq_ atomic to fix data race between concurrent node fetch reads and ledger acquisition writes - Suppress known lock-order-inversion between getMasterMutex() and RCLConsensus::mutex_ (requires larger lock-ordering redesign) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
16
.github/scripts/strategy-matrix/generate.py
vendored
16
.github/scripts/strategy-matrix/generate.py
vendored
@@ -64,7 +64,7 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
|
||||
if os["distro_version"] == "bookworm":
|
||||
if (
|
||||
f"{os['compiler_name']}-{os['compiler_version']}" == "gcc-13"
|
||||
and build_type == "Release"
|
||||
and build_type == "Debug"
|
||||
and architecture["platform"] == "linux/amd64"
|
||||
):
|
||||
cmake_args = f"-DUNIT_TEST_REFERENCE_FEE=500 {cmake_args}"
|
||||
@@ -236,10 +236,12 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
|
||||
# names get truncated.
|
||||
# Add Address and Thread (both coupled with UB) sanitizers for specific bookworm distros.
|
||||
# GCC-Asan rippled-embedded tests are failing because of https://github.com/google/sanitizers/issues/856
|
||||
if (
|
||||
os["distro_version"] == "bookworm"
|
||||
and f"{os['compiler_name']}-{os['compiler_version']}" == "clang-20"
|
||||
):
|
||||
if os[
|
||||
"distro_version"
|
||||
] == "bookworm" and f"{os['compiler_name']}-{os['compiler_version']}" in [
|
||||
"clang-20",
|
||||
"gcc-13",
|
||||
]:
|
||||
# Add ASAN + UBSAN configuration.
|
||||
configurations.append(
|
||||
{
|
||||
@@ -258,14 +260,14 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
|
||||
if activate_tsan:
|
||||
configurations.append(
|
||||
{
|
||||
"config_name": config_name + "-tsan",
|
||||
"config_name": config_name + "-tsan-ubsan",
|
||||
"cmake_args": cmake_args,
|
||||
"cmake_target": cmake_target,
|
||||
"build_only": build_only,
|
||||
"build_type": build_type,
|
||||
"os": os,
|
||||
"architecture": architecture,
|
||||
"sanitizers": "thread",
|
||||
"sanitizers": "thread,undefinedbehavior",
|
||||
}
|
||||
)
|
||||
else:
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
#include <xrpl/beast/utility/instrumentation.h>
|
||||
|
||||
#include <atomic>
|
||||
#include <sstream>
|
||||
|
||||
namespace beast {
|
||||
@@ -56,7 +57,7 @@ public:
|
||||
{
|
||||
protected:
|
||||
Sink() = delete;
|
||||
explicit Sink(Sink const& sink) = default;
|
||||
explicit Sink(Sink const& sink);
|
||||
Sink(Severity thresh, bool console);
|
||||
Sink&
|
||||
operator=(Sink const& lhs) = delete;
|
||||
@@ -104,7 +105,7 @@ public:
|
||||
writeAlways(Severity level, std::string const& text) = 0;
|
||||
|
||||
private:
|
||||
Severity thresh_;
|
||||
std::atomic<Severity> thresh_;
|
||||
bool m_console;
|
||||
};
|
||||
|
||||
|
||||
@@ -14,6 +14,7 @@
|
||||
#include <xrpl/shamap/SHAMapMissingNode.h>
|
||||
#include <xrpl/shamap/SHAMapTreeNode.h>
|
||||
|
||||
#include <atomic>
|
||||
#include <set>
|
||||
#include <stack>
|
||||
#include <vector>
|
||||
@@ -82,8 +83,11 @@ private:
|
||||
/** ID to distinguish this map for all others we're sharing nodes with. */
|
||||
std::uint32_t cowid_ = 1;
|
||||
|
||||
/** The sequence of the ledger that this map references, if any. */
|
||||
std::uint32_t ledgerSeq_ = 0;
|
||||
/** The sequence of the ledger that this map references, if any.
|
||||
* Atomic because it is written during ledger acquisition and read
|
||||
* concurrently by node fetch and sync operations.
|
||||
*/
|
||||
std::atomic<std::uint32_t> ledgerSeq_ = 0;
|
||||
|
||||
intr_ptr::SharedPtr<SHAMapTreeNode> root_;
|
||||
mutable SHAMapState state_;
|
||||
@@ -537,7 +541,7 @@ SHAMap::setFull()
|
||||
inline void
|
||||
SHAMap::setLedgerSeq(std::uint32_t lseq)
|
||||
{
|
||||
ledgerSeq_ = lseq;
|
||||
ledgerSeq_.store(lseq, std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
inline void
|
||||
|
||||
@@ -36,6 +36,12 @@ deadlock:pthread_create
|
||||
deadlock:pthread_rwlock_rdlock
|
||||
deadlock:boost::asio
|
||||
|
||||
# Known lock-order-inversion between getMasterMutex() and RCLConsensus::mutex_.
|
||||
# NetworkOPsImp::processHeartbeatTimer acquires master → consensus, while
|
||||
# RCLConsensus::startRound (via acceptLedger) acquires consensus → master.
|
||||
# Requires a larger lock-ordering redesign to fix properly.
|
||||
deadlock:RCLConsensus
|
||||
|
||||
# Signal/crash suppressions for GCC TSAN instrumentation issues
|
||||
signal:std::__cxx11::basic_stringbuf
|
||||
signal:basic_stringbuf
|
||||
|
||||
@@ -68,6 +68,11 @@ Journal::getNullSink()
|
||||
|
||||
//------------------------------------------------------------------------------
|
||||
|
||||
Journal::Sink::Sink(Sink const& sink)
|
||||
: thresh_(sink.thresh_.load(std::memory_order_relaxed)), m_console(sink.m_console)
|
||||
{
|
||||
}
|
||||
|
||||
Journal::Sink::Sink(Severity thresh, bool console) : thresh_(thresh), m_console(console)
|
||||
{
|
||||
}
|
||||
@@ -77,7 +82,7 @@ Journal::Sink::~Sink() = default;
|
||||
bool
|
||||
Journal::Sink::active(Severity level) const
|
||||
{
|
||||
return level >= thresh_;
|
||||
return level >= thresh_.load(std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
bool
|
||||
@@ -95,13 +100,13 @@ Journal::Sink::console(bool output)
|
||||
severities::Severity
|
||||
Journal::Sink::threshold() const
|
||||
{
|
||||
return thresh_;
|
||||
return thresh_.load(std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
void
|
||||
Journal::Sink::threshold(Severity thresh)
|
||||
{
|
||||
thresh_ = thresh;
|
||||
thresh_.store(thresh, std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
//------------------------------------------------------------------------------
|
||||
|
||||
@@ -47,7 +47,7 @@ SHAMap::SHAMap(SHAMap const& other, bool isMutable)
|
||||
: f_(other.f_)
|
||||
, journal_(other.f_.journal())
|
||||
, cowid_(other.cowid_ + 1)
|
||||
, ledgerSeq_(other.ledgerSeq_)
|
||||
, ledgerSeq_(other.ledgerSeq_.load(std::memory_order_relaxed))
|
||||
, root_(other.root_)
|
||||
, state_(isMutable ? SHAMapState::Modifying : SHAMapState::Immutable)
|
||||
, type_(other.type_)
|
||||
|
||||
Reference in New Issue
Block a user