Skip to content

Commit

Permalink
Ball log set category tsan race drqs 171004031 (#4560)
Browse files Browse the repository at this point in the history 
Fix TSAN failure with ball categories and refactor parts of ball_category.h
  • Loading branch information
@jfevold-bbg
jfevold-bbg authored and GitHub Enterprise committed Apr 26, 2024
1 parent 5c14056 commit 7837870
Show file tree
Hide file tree
Showing 6 changed files with 371 additions and 181 deletions.
30 changes: 21 additions & 9 deletions groups/bal/ball/ball_category.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ BSLS_IDENT_RCSID(ball_category_cpp,"$Id$ $CSID$")
#include <bslmf_assert.h>
#include <bslmf_issame.h>

#include <bslmt_mutexassert.h>

#include <bsls_assert.h>

#include <bsl_algorithm.h>
Expand Down Expand Up @@ -44,9 +46,10 @@ Category::Category(const char *categoryName,
triggerLevel,
triggerAllLevel))
, d_categoryName(categoryName, basicAllocator)
, d_categoryHolder(0)
, d_categoryHolder_p(0)
, d_relevantRuleMask()
, d_ruleThreshold(0)
, d_mutex()
{
BSLS_ASSERT(categoryName);
bsls::AtomicOperations::initUint(&d_relevantRuleMask, 0);
Expand All @@ -58,31 +61,39 @@ Category::linkCategoryHolder(CategoryHolder *categoryHolder)
{
BSLS_ASSERT(categoryHolder);

bslmt::LockGuard<bslmt::Mutex> guard(&d_mutex);
if (!categoryHolder->category()) {
categoryHolder->setThreshold(bsl::max(d_threshold, d_ruleThreshold));
categoryHolder->setThreshold(bsl::max(d_threshold.loadRelaxed(),
d_ruleThreshold));
categoryHolder->setCategory(this);
categoryHolder->setNext(d_categoryHolder);
d_categoryHolder = categoryHolder;
categoryHolder->setNext(d_categoryHolder_p);
d_categoryHolder_p = categoryHolder;
}
}

void Category::resetCategoryHolders()
{
CategoryHolder *holder = d_categoryHolder;
bslmt::LockGuard<bslmt::Mutex> guard(&d_mutex);

CategoryHolder *holder = d_categoryHolder_p;

while (holder) {
CategoryHolder *nextHolder = holder->next();
holder->reset();
holder = nextHolder;
}
d_categoryHolder = 0;
d_categoryHolder_p = 0;
}

// CLASS METHODS
void Category::updateThresholdForHolders()
{
if (d_categoryHolder) {
CategoryHolder *holder = d_categoryHolder;
const int threshold = bsl::max(d_threshold, d_ruleThreshold);
BSLMT_MUTEXASSERT_IS_LOCKED(&d_mutex);

if (d_categoryHolder_p) {
CategoryHolder *holder = d_categoryHolder_p;
const int threshold = bsl::max(d_threshold.loadRelaxed(),
d_ruleThreshold);
if (threshold != holder->threshold()) {
do {
holder->setThreshold(threshold);
Expand All @@ -102,6 +113,7 @@ int Category::setLevels(int recordLevel,
passLevel,
triggerLevel,
triggerAllLevel)) {
bslmt::LockGuard<bslmt::Mutex> guard(&d_mutex);

d_thresholdLevels.setLevels(recordLevel,
passLevel,
Expand Down
69 changes: 53 additions & 16 deletions groups/bal/ball/ball_category.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,11 @@ BSLS_IDENT("$Id: $")

#include <bslmf_nestedtraitdeclaration.h>

#include <bslmt_lockguard.h>
#include <bslmt_mutex.h>

#include <bsls_assert.h>
#include <bsls_atomic.h>
#include <bsls_atomicoperations.h>
#include <bsls_keyword.h>
#include <bsls_review.h>
Expand Down Expand Up @@ -134,23 +138,29 @@ class Category {
// logging system.

// DATA
ThresholdAggregate d_thresholdLevels; // record, pass, trigger, and
// trigger-all levels
ThresholdAggregate d_thresholdLevels; // record, pass, trigger, and
// trigger-all levels

bsls::AtomicInt d_threshold; // numerical maximum of the four
// levels

int d_threshold; // numerical maximum of the four
// levels
const bsl::string d_categoryName; // category name

bsl::string d_categoryName; // category name
CategoryHolder *d_categoryHolder_p; // linked list of holders of this
// category

CategoryHolder *d_categoryHolder; // linked list of holders of this
// category
mutable bsls::AtomicOperations::AtomicTypes::Uint
d_relevantRuleMask; // the mask indicating which rules
// are relevant (i.e., have been
// attached to this category)
d_relevantRuleMask; // the mask indicating which
// rules are relevant (i.e., have
// been attached to this
// category)

mutable int d_ruleThreshold; // numerical maximum of all four
// levels for all relevant rules
mutable int d_ruleThreshold; // numerical maximum of all four
// levels for all relevant rules

mutable bslmt::Mutex d_mutex; // mutex providing mutually
// exclusive access to all
// non-atomic members

// FRIENDS
friend class CategoryManagerImpUtil;
Expand All @@ -175,7 +185,8 @@ class Category {
void updateThresholdForHolders();
// Update the threshold of all category holders that hold the address
// of this object to the maximum of 'd_threshold' and
// 'd_ruleThreshold'.
// 'd_ruleThreshold'. The behavior is undefined unless 'd_mutex' is
// locked.

public:
// CLASS METHODS
Expand Down Expand Up @@ -221,6 +232,8 @@ class Category {
// otherwise (with no effect on the threshold levels of this category).

// ACCESSORS
// BDE_VERIFY pragma: push
// BDE_VERIFY pragma: -FABC01: Functions not in alphanumeric order
const char *categoryName() const;
// Return the name of this category.

Expand All @@ -245,7 +258,7 @@ class Category {
int triggerAllLevel() const;
// Return the trigger-all level of this category.

const ThresholdAggregate& thresholdLevels() const;
ThresholdAggregate thresholdLevels() const;
// Return the aggregate threshold levels of this category.

int threshold() const;
Expand All @@ -271,6 +284,7 @@ class Category {
// owns this category) applies at this category. Note that a rule
// applies to this category if the rule's pattern matches the name
// returned by 'categoryName'.
// BDE_VERIFY pragma: pop
};

// ====================
Expand All @@ -296,6 +310,7 @@ class CategoryHolder {
// NOT IMPLEMENTED
CategoryHolder& operator=(const CategoryHolder&) BSLS_KEYWORD_DELETED;

// PRIVATE TYPES
typedef bsls::AtomicOperations AtomicOps;
typedef bsls::AtomicOperations::AtomicTypes::Int AtomicInt;
typedef bsls::AtomicOperations::AtomicTypes::Pointer AtomicPointer;
Expand All @@ -317,16 +332,21 @@ class CategoryHolder {
// the range '[0 .. 255]'.

// PUBLIC DATA
// BDE_VERIFY pragma: push
// BDE_VERIFY pragma: -MN01 // Class data members must be private
AtomicInt d_threshold; // threshold level
AtomicPointer d_category_p; // held category (not owned)
AtomicPointer d_next_p; // next category holder in linked list
// BDE_VERIFY pragma: pop

// CREATORS

// No constructors or destructors are declared in order to allow for static
// initialization of instances of this class.

// MANIPULATORS
// BDE_VERIFY pragma: push
// BDE_VERIFY pragma: -FABC01: Functions not in alphanumeric order
void reset();
// Reset this object to its default value. The default value is:
//..
Expand Down Expand Up @@ -354,6 +374,7 @@ class CategoryHolder {

CategoryHolder *next() const;
// Return the address of the modifiable holder held by this holder.
// BDE_VERIFY pragma: pop
};

// ============================
Expand All @@ -368,7 +389,9 @@ class CategoryManagerImpUtil {
// implementation detail of the 'ball' logging system.

public:
// PRIVATE MANIPULATORS
// CLASS METHODS
// BDE_VERIFY pragma: push
// BDE_VERIFY pragma: -FABC01: Functions not in alphanumeric order
static void linkCategoryHolder(Category *category,
CategoryHolder *categoryHolder);
// Load the specified 'category' and its corresponding 'maxLevel()'
Expand Down Expand Up @@ -402,6 +425,7 @@ class CategoryManagerImpUtil {
RuleSet::MaskType mask);
// Set the rule-mask for the specified 'category' to the specified
// 'mask'.
// BDE_VERIFY pragma: pop
};

// ============================================================================
Expand All @@ -412,6 +436,9 @@ class CategoryManagerImpUtil {
// class Category
// --------------

// BDE_VERIFY pragma: push
// BDE_VERIFY pragma: -FABC01: Functions not in alphanumeric order

// CLASS METHODS
inline
bool Category::areValidThresholdLevels(int recordLevel,
Expand Down Expand Up @@ -447,30 +474,35 @@ int Category::maxLevel() const
inline
int Category::recordLevel() const
{
bslmt::LockGuard<bslmt::Mutex> guard(&d_mutex);
return d_thresholdLevels.recordLevel();
}

inline
int Category::passLevel() const
{
bslmt::LockGuard<bslmt::Mutex> guard(&d_mutex);
return d_thresholdLevels.passLevel();
}

inline
int Category::triggerLevel() const
{
bslmt::LockGuard<bslmt::Mutex> guard(&d_mutex);
return d_thresholdLevels.triggerLevel();
}

inline
int Category::triggerAllLevel() const
{
bslmt::LockGuard<bslmt::Mutex> guard(&d_mutex);
return d_thresholdLevels.triggerAllLevel();
}

inline
const ThresholdAggregate& Category::thresholdLevels() const
ThresholdAggregate Category::thresholdLevels() const
{
bslmt::LockGuard<bslmt::Mutex> guard(&d_mutex);
return d_thresholdLevels;
}

Expand All @@ -483,6 +515,7 @@ int Category::threshold() const
inline
int Category::ruleThreshold() const
{
bslmt::LockGuard<bslmt::Mutex> guard(&d_mutex);
return d_ruleThreshold;
}

Expand Down Expand Up @@ -564,13 +597,15 @@ void CategoryManagerImpUtil::updateThresholdForHolders(Category *category)
{
BSLS_ASSERT(category);

bslmt::LockGuard<bslmt::Mutex> guard(&category->d_mutex);
category->updateThresholdForHolders();
}

inline
void CategoryManagerImpUtil::setRuleThreshold(Category *category,
int ruleThreshold)
{
bslmt::LockGuard<bslmt::Mutex> guard(&category->d_mutex);
category->d_ruleThreshold = ruleThreshold;
}

Expand Down Expand Up @@ -616,6 +651,8 @@ void CategoryManagerImpUtil::setRelevantRuleMask(Category *category,
mask);
}

// BDE_VERIFY pragma: pop

} // close package namespace
} // close enterprise namespace

Expand Down
Loading

0 comments on commit 7837870

Please sign in to comment.