fix: Add Doxygen docs and remove temporary CI filter

- Add Doxygen documentation to new methods, members, constants, and
  macros introduced by the TSAN/ASAN PR (Database::startReadThreads,
  BasicApp::DeferStart, ResourceManager::start, XRPL_SANITIZER_ACTIVE,
  coroStackSize, yieldStackSize)
- Add @note thread-safety tags where atomics were introduced
- Update Database constructor docs to reflect deferred thread startup
- Remove temporary CI filter that restricted builds to sanitizer-only
  variants (must run full matrix before merge)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Pratik Mankawde
2026-03-23 10:46:16 +00:00
parent 390622f7c9
commit 02c9830184
8 changed files with 70 additions and 28 deletions

View File

@@ -324,17 +324,6 @@ def generate_strategy_matrix(all: bool, config: Config) -> list:
}
)
# TEMPORARY: Only build previously-failing sanitizer variants to save CI.
# Remove this filter once these configs pass.
sanitizer_only = [
c
for c in configurations
if c["sanitizers"]
and ("gcc-13" in c["config_name"] or "clang-20" in c["config_name"])
]
if sanitizer_only:
return sanitizer_only
return configurations

View File

@@ -10,9 +10,14 @@
#define XRPL_NO_SANITIZE_ADDRESS
#endif
// Detect whether a memory sanitizer (TSAN or ASAN) is active at compile time.
// GCC defines __SANITIZE_THREAD__ / __SANITIZE_ADDRESS__ directly.
// Clang uses __has_feature(thread_sanitizer) / __has_feature(address_sanitizer).
/** Detect whether a memory sanitizer (TSAN or ASAN) is active at compile time.
*
* Evaluates to 1 when the translation unit is compiled with ThreadSanitizer
* or AddressSanitizer instrumentation, 0 otherwise.
*
* GCC defines __SANITIZE_THREAD__ / __SANITIZE_ADDRESS__ directly.
* Clang uses __has_feature(thread_sanitizer) / __has_feature(address_sanitizer).
*/
#if defined(__SANITIZE_THREAD__) || defined(__SANITIZE_ADDRESS__)
#define XRPL_SANITIZER_ACTIVE 1
#elif defined(__has_feature)

View File

@@ -19,10 +19,15 @@
namespace beast {
namespace test {
// Sanitizers significantly increase stack frame sizes
// (TSAN ~3-5x, ASAN ~2-3x), requiring larger coroutine stacks.
// Note: This duplicates the detection logic from xrpl/basics/sanitizers.h
// because xrpl.beast cannot depend on xrpl.basics (levelization constraint).
/** Stack size for yield_to coroutines.
*
* Sanitizers significantly increase stack frame sizes
* (TSAN ~3-5x, ASAN ~2-3x), requiring larger coroutine stacks.
*
* @note This duplicates the detection logic from xrpl/basics/sanitizers.h
* because xrpl.beast cannot depend on xrpl.basics (levelization
* constraint).
*/
#if defined(__SANITIZE_THREAD__) || defined(__SANITIZE_ADDRESS__)
inline constexpr std::size_t yieldStackSize = 4 * 1024 * 1024;
#elif defined(__has_feature)

View File

@@ -105,6 +105,8 @@ public:
writeAlways(Severity level, std::string const& text) = 0;
private:
/// Minimum severity level. Atomic for safe concurrent reads/writes
/// (e.g. RPC threshold changes vs. hot-path log checks).
std::atomic<Severity> thresh_;
bool m_console;
};

View File

@@ -7,8 +7,11 @@
namespace xrpl {
// Sanitizers significantly increase stack frame sizes
// (TSAN ~3-5x, ASAN ~2-3x), requiring larger coroutine stacks.
/** Stack size for JobQueue coroutines.
*
* Sanitizers significantly increase stack frame sizes
* (TSAN ~3-5x, ASAN ~2-3x), requiring larger coroutine stacks.
*/
inline constexpr std::size_t coroStackSize = XRPL_SANITIZER_ACTIVE ? megabytes(4) : megabytes(2);
template <class F>

View File

@@ -34,16 +34,24 @@ public:
/** Construct the node store.
Construction configures the database but does not start read threads.
Call startReadThreads() after construction to begin asynchronous reads.
@param scheduler The scheduler to use for performing asynchronous tasks.
@param readThreads The number of asynchronous read threads to create.
@param config The configuration settings
@param readThreads The desired number of asynchronous read threads.
@param config The configuration settings.
@param journal Destination for logging output.
*/
Database(Scheduler& scheduler, int readThreads, Section const& config, beast::Journal j);
/** Start the asynchronous read threads.
Must be called after construction to start read threads.
It is safe to destroy the Database without calling this.
Must be called after construction to start read threads. It is safe
to destroy the Database without calling this; in that case no
asynchronous reads will be serviced.
@note Not thread-safe. Must be called exactly once, before any
concurrent access to the database.
*/
void
startReadThreads();
@@ -259,6 +267,8 @@ private:
std::atomic<bool> readStopping_ = false;
std::atomic<int> readThreads_ = 0;
std::atomic<int> runningThreads_ = 0;
/// The number of read threads to create when startReadThreads() is called.
int const desiredReadThreads_;
virtual std::shared_ptr<NodeObject>

View File

@@ -23,7 +23,12 @@ public:
virtual ~Manager() = 0;
/** Start the manager's background thread.
Must be called after construction.
Must be called after construction to begin periodic charge decay
and inactive-consumer sweeps.
@note Not thread-safe. Must be called exactly once, before any
concurrent access to the manager.
*/
virtual void
start() = 0;

View File

@@ -6,13 +6,22 @@
#include <thread>
#include <vector>
// This is so that the io_context can outlive all the children
/** Manages an io_context and its worker threads.
*
* Ensures the io_context outlives all derived classes by joining worker
* threads in the destructor. Supports immediate or deferred thread startup.
*
* @note Thread-safe after construction completes. The deferred-start
* constructor and startIOThreads() must be called from a single thread.
*/
class BasicApp
{
private:
std::optional<boost::asio::executor_work_guard<boost::asio::io_context::executor_type>> work_;
std::vector<std::thread> threads_;
boost::asio::io_context io_context_;
/// Number of IO worker threads to create.
std::size_t numberOfThreads_;
public:
@@ -32,13 +41,27 @@ public:
}
protected:
// Construct without starting threads. Derived classes must call
// startIOThreads() once construction is complete.
/** Tag type for deferred thread startup.
*
* Pass to the protected constructor to construct without starting IO
* threads. The derived class must call startIOThreads() once its own
* construction is complete.
*/
struct DeferStart
{
};
/** Construct without starting IO threads.
*
* @param numberOfThreads Desired number of IO worker threads.
* @param Tag to select deferred startup.
*/
BasicApp(std::size_t numberOfThreads, DeferStart);
/** Start the IO worker threads.
*
* @note Must be called exactly once after the deferred-start constructor.
*/
void
startIOThreads();
};