From 954679bb0f96c103376cff954c93f0d2bc4ef437 Mon Sep 17 00:00:00 2001 From: Igor Canadi Date: Wed, 26 Mar 2014 11:24:52 -0700 Subject: [PATCH] AssertHeld() should do things Summary: AssertHeld() was a no-op before. Now it does things. Also, this change caught a bad bug in SuperVersion::Init(). The method is calling db->mutex.AssertHeld(), but db variable is not initialized yet! I also fixed that issue. Test Plan: make check Reviewers: dhruba, haobo, ljin, sdong, yhchiang Reviewed By: haobo CC: leveldb Differential Revision: https://reviews.facebook.net/D17193 --- db/db_impl.cc | 2 +- port/port_posix.cc | 27 +++++++++++++++++++++++++-- port/port_posix.h | 7 ++++++- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index d1003bfdc..9cc957f85 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -3331,10 +3331,10 @@ void DBImpl::InstallSuperVersion(DeletionState& deletion_state) { DBImpl::SuperVersion* DBImpl::InstallSuperVersion( SuperVersion* new_superversion) { mutex_.AssertHeld(); + new_superversion->db = this; new_superversion->Init(mem_, imm_.current(), versions_->current()); SuperVersion* old_superversion = super_version_; super_version_ = new_superversion; - super_version_->db = this; ++super_version_number_; super_version_->version_number = super_version_number_; diff --git a/port/port_posix.cc b/port/port_posix.cc index f7025f461..911cebdf2 100644 --- a/port/port_posix.cc +++ b/port/port_posix.cc @@ -11,6 +11,7 @@ #include #include +#include #include #include "util/logging.h" @@ -45,9 +46,25 @@ Mutex::Mutex(bool adaptive) { Mutex::~Mutex() { PthreadCall("destroy mutex", pthread_mutex_destroy(&mu_)); } -void Mutex::Lock() { PthreadCall("lock", pthread_mutex_lock(&mu_)); } +void Mutex::Lock() { + PthreadCall("lock", pthread_mutex_lock(&mu_)); +#ifndef NDEBUG + locked_ = true; +#endif +} -void Mutex::Unlock() { PthreadCall("unlock", pthread_mutex_unlock(&mu_)); } +void Mutex::Unlock() { +#ifndef NDEBUG + locked_ = false; +#endif + PthreadCall("unlock", pthread_mutex_unlock(&mu_)); +} + +void Mutex::AssertHeld() { +#ifndef NDEBUG + assert(locked_); +#endif +} CondVar::CondVar(Mutex* mu) : mu_(mu) { @@ -57,7 +74,13 @@ CondVar::CondVar(Mutex* mu) CondVar::~CondVar() { PthreadCall("destroy cv", pthread_cond_destroy(&cv_)); } void CondVar::Wait() { +#ifndef NDEBUG + mu_->locked_ = false; +#endif PthreadCall("wait", pthread_cond_wait(&cv_, &mu_->mu_)); +#ifndef NDEBUG + mu_->locked_ = true; +#endif } void CondVar::Signal() { diff --git a/port/port_posix.h b/port/port_posix.h index aaea0b574..d393af6da 100644 --- a/port/port_posix.h +++ b/port/port_posix.h @@ -97,11 +97,16 @@ class Mutex { void Lock(); void Unlock(); - void AssertHeld() { } + // this will assert if the mutex is not locked + // it does NOT verify that mutex is held by a calling thread + void AssertHeld(); private: friend class CondVar; pthread_mutex_t mu_; +#ifndef NDEBUG + bool locked_; +#endif // No copying Mutex(const Mutex&);