Skip to content

Commit

Permalink
Mutex: Remove destructor in release build
Browse files Browse the repository at this point in the history 
The Mutex destructor is needed only to clean up debug logging
and invariant checking synch events. These are not supposed
to be used in production, but the non-empty destructor has
costs for production builds.

Instead of removing synch events in destructor,
drop all of them if we accumulated too many.
For tests is should not matter (we maybe only consume
a bit more memory). Production builds should be either unaffected
(if don't use debug logging), or use periodic reset of all synch events.

PiperOrigin-RevId: 578123259
Change-Id: I0ec59183a5f63ea0a6b7fc50f0a77974e7f677be
  • Loading branch information
@dvyukov @copybara-github
dvyukov authored and copybara-github committed Oct 31, 2023
1 parent 6c8338c commit f3760b4
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 75 deletions.
150 changes: 76 additions & 74 deletions absl/synchronization/mutex.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,31 +203,23 @@ int MutexDelay(int32_t c, int mode) {
// Ensure that "(*pv & bits) == bits" by doing an atomic update of "*pv" to
// "*pv | bits" if necessary. Wait until (*pv & wait_until_clear)==0
// before making any change.
// Returns true if bits were previously unset and set by the call.
// This is used to set flags in mutex and condition variable words.
static void AtomicSetBits(std::atomic<intptr_t>* pv, intptr_t bits,
static bool AtomicSetBits(std::atomic<intptr_t>* pv, intptr_t bits,
intptr_t wait_until_clear) {
intptr_t v;
do {
v = pv->load(std::memory_order_relaxed);
} while ((v & bits) != bits &&
((v & wait_until_clear) != 0 ||
!pv->compare_exchange_weak(v, v | bits, std::memory_order_release,
std::memory_order_relaxed)));
}

// Ensure that "(*pv & bits) == 0" by doing an atomic update of "*pv" to
// "*pv & ~bits" if necessary. Wait until (*pv & wait_until_clear)==0
// before making any change.
// This is used to unset flags in mutex and condition variable words.
static void AtomicClearBits(std::atomic<intptr_t>* pv, intptr_t bits,
intptr_t wait_until_clear) {
intptr_t v;
do {
v = pv->load(std::memory_order_relaxed);
} while ((v & bits) != 0 &&
((v & wait_until_clear) != 0 ||
!pv->compare_exchange_weak(v, v & ~bits, std::memory_order_release,
std::memory_order_relaxed)));
for (;;) {
intptr_t v = pv->load(std::memory_order_relaxed);
if ((v & bits) == bits) {
return false;
}
if ((v & wait_until_clear) != 0) {
continue;
}
if (pv->compare_exchange_weak(v, v | bits, std::memory_order_release,
std::memory_order_relaxed)) {
return true;
}
}
}

//------------------------------------------------------------------
Expand Down Expand Up @@ -338,12 +330,49 @@ static SynchEvent* EnsureSynchEvent(std::atomic<intptr_t>* addr,
const char* name, intptr_t bits,
intptr_t lockbit) {
uint32_t h = reinterpret_cast<uintptr_t>(addr) % kNSynchEvent;
SynchEvent* e;
// first look for existing SynchEvent struct..
synch_event_mu.Lock();
for (e = synch_event[h];
e != nullptr && e->masked_addr != base_internal::HidePtr(addr);
e = e->next) {
// When a Mutex/CondVar is destroyed, we don't remove the associated
// SynchEvent to keep destructors empty in release builds for performance
// reasons. If the current call is the first to set bits (kMuEvent/kCVEvent),
// we don't look up the existing even because (if it exists, it must be for
// the previous Mutex/CondVar that existed at the same address).
// The leaking events must not be a problem for tests, which should create
// bounded amount of events. And debug logging is not supposed to be enabled
// in production. However, if it's accidentally enabled, or briefly enabled
// for some debugging, we don't want to crash the program. Instead we drop
// all events, if we accumulated too many of them. Size of a single event
// is ~48 bytes, so 100K events is ~5 MB.
// Additionally we could delete the old event for the same address,
// but it would require a better hashmap (if we accumulate too many events,
// linked lists will grow and traversing them will be very slow).
constexpr size_t kMaxSynchEventCount = 100 << 10;
// Total number of live synch events.
static size_t synch_event_count ABSL_GUARDED_BY(synch_event_mu);
if (++synch_event_count > kMaxSynchEventCount) {
synch_event_count = 0;
ABSL_RAW_LOG(ERROR,
"Accumulated %zu Mutex debug objects. If you see this"
" in production, it may mean that the production code"
" accidentally calls "
"Mutex/CondVar::EnableDebugLog/EnableInvariantDebugging.",
kMaxSynchEventCount);
for (auto*& head : synch_event) {
for (auto* e = head; e != nullptr;) {
SynchEvent* next = e->next;
if (--(e->refcount) == 0) {
base_internal::LowLevelAlloc::Free(e);
}
e = next;
}
head = nullptr;
}
}
SynchEvent* e = nullptr;
if (!AtomicSetBits(addr, bits, lockbit)) {
for (e = synch_event[h];
e != nullptr && e->masked_addr != base_internal::HidePtr(addr);
e = e->next) {
}
}
if (e == nullptr) { // no SynchEvent struct found; make one.
if (name == nullptr) {
Expand All @@ -359,7 +388,6 @@ static SynchEvent* EnsureSynchEvent(std::atomic<intptr_t>* addr,
e->log = false;
strcpy(e->name, name); // NOLINT(runtime/printf)
e->next = synch_event[h];
AtomicSetBits(addr, bits, lockbit);
synch_event[h] = e;
} else {
e->refcount++; // for return value
Expand All @@ -368,48 +396,18 @@ static SynchEvent* EnsureSynchEvent(std::atomic<intptr_t>* addr,
return e;
}

// Deallocate the SynchEvent *e, whose refcount has fallen to zero.
static void DeleteSynchEvent(SynchEvent* e) {
base_internal::LowLevelAlloc::Free(e);
}

// Decrement the reference count of *e, or do nothing if e==null.
static void UnrefSynchEvent(SynchEvent* e) {
if (e != nullptr) {
synch_event_mu.Lock();
bool del = (--(e->refcount) == 0);
synch_event_mu.Unlock();
if (del) {
DeleteSynchEvent(e);
base_internal::LowLevelAlloc::Free(e);
}
}
}

// Forget the mapping from the object (Mutex or CondVar) at address addr
// to SynchEvent object, and clear "bits" in its word (waiting until lockbit
// is clear before doing so).
static void ForgetSynchEvent(std::atomic<intptr_t>* addr, intptr_t bits,
intptr_t lockbit) {
uint32_t h = reinterpret_cast<uintptr_t>(addr) % kNSynchEvent;
SynchEvent** pe;
SynchEvent* e;
synch_event_mu.Lock();
for (pe = &synch_event[h];
(e = *pe) != nullptr && e->masked_addr != base_internal::HidePtr(addr);
pe = &e->next) {
}
bool del = false;
if (e != nullptr) {
*pe = e->next;
del = (--(e->refcount) == 0);
}
AtomicClearBits(addr, bits, lockbit);
synch_event_mu.Unlock();
if (del) {
DeleteSynchEvent(e);
}
}

// Return a refcounted reference to the SynchEvent of the object at address
// "addr", if any. The pointer returned is valid until the UnrefSynchEvent() is
// called.
Expand Down Expand Up @@ -733,25 +731,35 @@ static unsigned TsanFlags(Mutex::MuHow how) {
}
#endif

static bool DebugOnlyIsExiting() {
return false;
}
#if defined(__APPLE__) || defined(ABSL_BUILD_DLL)
// When building a dll symbol export lists may reference the destructor
// and want it to be an exported symbol rather than an inline function.
// Some apple builds also do dynamic library build but don't say it explicitly.
Mutex::~Mutex() { Dtor(); }
#endif

Mutex::~Mutex() {
intptr_t v = mu_.load(std::memory_order_relaxed);
if ((v & kMuEvent) != 0 && !DebugOnlyIsExiting()) {
ForgetSynchEvent(&this->mu_, kMuEvent, kMuSpin);
}
#if !defined(NDEBUG) || defined(ABSL_HAVE_THREAD_SANITIZER)
void Mutex::Dtor() {
if (kDebugMode) {
this->ForgetDeadlockInfo();
}
ABSL_TSAN_MUTEX_DESTROY(this, __tsan_mutex_not_static);
}
#endif

void Mutex::EnableDebugLog(const char* name) {
SynchEvent* e = EnsureSynchEvent(&this->mu_, name, kMuEvent, kMuSpin);
e->log = true;
UnrefSynchEvent(e);
// This prevents "error: undefined symbol: absl::Mutex::~Mutex()"
// in a release build (NDEBUG defined) when a test does "#undef NDEBUG"
// to use assert macro. In such case, the test does not get the dtor
// definition because it's supposed to be outline when NDEBUG is not defined,
// and this source file does not define one either because NDEBUG is defined.
// Since it's not possible to take address of a destructor, we move the
// actual destructor code into the separate Dtor function and force the
// compiler to emit this function even if it's inline by taking its address.
ABSL_ATTRIBUTE_UNUSED volatile auto dtor = &Mutex::Dtor;
}

void EnableMutexInvariantDebugging(bool enabled) {
Expand Down Expand Up @@ -2488,12 +2496,6 @@ void CondVar::EnableDebugLog(const char* name) {
UnrefSynchEvent(e);
}

CondVar::~CondVar() {
if ((cv_.load(std::memory_order_relaxed) & kCvEvent) != 0) {
ForgetSynchEvent(&this->cv_, kCvEvent, kCvSpin);
}
}

// Remove thread s from the list of waiters on this condition variable.
void CondVar::Remove(PerThreadSynch* s) {
SchedulingGuard::ScopedDisable disable_rescheduling;
Expand Down
18 changes: 17 additions & 1 deletion absl/synchronization/mutex.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
#include <iterator>
#include <string>

#include "absl/base/attributes.h"
#include "absl/base/const_init.h"
#include "absl/base/internal/identity.h"
#include "absl/base/internal/low_level_alloc.h"
Expand Down Expand Up @@ -536,6 +537,7 @@ class ABSL_LOCKABLE Mutex {
void Block(base_internal::PerThreadSynch* s);
// Wake a thread; return successor.
base_internal::PerThreadSynch* Wakeup(base_internal::PerThreadSynch* w);
void Dtor();

friend class CondVar; // for access to Trans()/Fer().
void Trans(MuHow how); // used for CondVar->Mutex transfer
Expand Down Expand Up @@ -909,7 +911,6 @@ class CondVar {
// A `CondVar` allocated on the heap or on the stack can use the this
// constructor.
CondVar();
~CondVar();

// CondVar::Wait()
//
Expand Down Expand Up @@ -1061,6 +1062,21 @@ inline Mutex::Mutex() : mu_(0) {

inline constexpr Mutex::Mutex(absl::ConstInitType) : mu_(0) {}

#if !defined(__APPLE__) && !defined(ABSL_BUILD_DLL)
ABSL_ATTRIBUTE_ALWAYS_INLINE
inline Mutex::~Mutex() { Dtor(); }
#endif

#if defined(NDEBUG) && !defined(ABSL_HAVE_THREAD_SANITIZER)
// Use default (empty) destructor in release build for performance reasons.
// We need to mark both Dtor and ~Mutex as always inline for inconsistent
// builds that use both NDEBUG and !NDEBUG with dynamic libraries. In these
// cases we want the empty functions to dissolve entirely rather than being
// exported from dynamic libraries and potentially override the non-empty ones.
ABSL_ATTRIBUTE_ALWAYS_INLINE
inline void Mutex::Dtor() {}
#endif

inline CondVar::CondVar() : cv_(0) {}

// static
Expand Down
27 changes: 27 additions & 0 deletions absl/synchronization/mutex_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1709,6 +1709,33 @@ TEST(Mutex, Logging) {
logged_cv.SignalAll();
}

TEST(Mutex, LoggingAddressReuse) {
// Repeatedly re-create a Mutex with debug logging at the same address.
alignas(absl::Mutex) char storage[sizeof(absl::Mutex)];
auto invariant =
+[](void *alive) { EXPECT_TRUE(*static_cast<bool *>(alive)); };
constexpr size_t kIters = 10;
bool alive[kIters] = {};
for (size_t i = 0; i < kIters; ++i) {
absl::Mutex *mu = new (storage) absl::Mutex;
alive[i] = true;
mu->EnableDebugLog("Mutex");
mu->EnableInvariantDebugging(invariant, &alive[i]);
mu->Lock();
mu->Unlock();
mu->~Mutex();
alive[i] = false;
}
}

TEST(Mutex, LoggingBankrupcy) {
// Test the case with too many live Mutexes with debug logging.
std::vector<absl::Mutex> mus(1 << 20);
for (auto &mu : mus) {
mu.EnableDebugLog("Mutex");
}
}

// --------------------------------------------------------

// Generate the vector of thread counts for tests parameterized on thread count.
Expand Down

0 comments on commit f3760b4

Please sign in to comment.