diff options
| -rw-r--r-- | base/condition_variable.h | 3 | ||||
| -rw-r--r-- | base/condition_variable_posix.cc | 14 | ||||
| -rw-r--r-- | base/lock.cc | 36 | ||||
| -rw-r--r-- | base/lock.h | 68 | ||||
| -rw-r--r-- | base/lock_impl.h | 19 | ||||
| -rw-r--r-- | base/lock_impl_win.cc | 41 |
6 files changed, 107 insertions, 74 deletions
diff --git a/base/condition_variable.h b/base/condition_variable.h index 430f57c..da87bfc 100644 --- a/base/condition_variable.h +++ b/base/condition_variable.h @@ -174,6 +174,9 @@ class ConditionVariable { pthread_cond_t condition_; pthread_mutex_t* user_mutex_; +#if !defined(NDEBUG) + Lock* user_lock_; // Needed to adjust shadow lock state on wait. +#endif #endif diff --git a/base/condition_variable_posix.cc b/base/condition_variable_posix.cc index 8c5a5c2..9d08f18 100644 --- a/base/condition_variable_posix.cc +++ b/base/condition_variable_posix.cc @@ -16,7 +16,7 @@ using base::Time; using base::TimeDelta; ConditionVariable::ConditionVariable(Lock* user_lock) - : user_mutex_(user_lock->lock_impl()->os_lock()) { + : user_mutex_(user_lock->lock_.os_lock()), user_lock_(user_lock) { int rv = pthread_cond_init(&condition_, NULL); DCHECK(rv == 0); } @@ -27,8 +27,14 @@ ConditionVariable::~ConditionVariable() { } void ConditionVariable::Wait() { +#if !defined(NDEBUG) + user_lock_->CheckHeldAndUnmark(); +#endif int rv = pthread_cond_wait(&condition_, user_mutex_); DCHECK(rv == 0); +#if !defined(NDEBUG) + user_lock_->CheckUnheldAndMark(); +#endif } void ConditionVariable::TimedWait(const TimeDelta& max_time) { @@ -46,8 +52,14 @@ void ConditionVariable::TimedWait(const TimeDelta& max_time) { abstime.tv_nsec %= Time::kNanosecondsPerSecond; DCHECK(abstime.tv_sec >= now.tv_sec); // Overflow paranoia +#if !defined(NDEBUG) + user_lock_->CheckHeldAndUnmark(); +#endif int rv = pthread_cond_timedwait(&condition_, user_mutex_, &abstime); DCHECK(rv == 0 || rv == ETIMEDOUT); +#if !defined(NDEBUG) + user_lock_->CheckUnheldAndMark(); +#endif } void ConditionVariable::Broadcast() { diff --git a/base/lock.cc b/base/lock.cc index 9ff963b..c73a458 100644 --- a/base/lock.cc +++ b/base/lock.cc @@ -1,7 +1,37 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// Lock class. +// This file is used for debugging assertion support. The Lock class +// is functionally a wrapper around the LockImpl class, so the only +// real intelligence in the class is in the debugging logic. -// Depricated file. See lock_impl_*.cc for platform specific versions. +#if !defined(NDEBUG) + +#include "base/lock.h" +#include "base/logging.h" + +Lock::Lock() : lock_() { + owned_by_thread_ = false; + owning_thread_id_ = static_cast<PlatformThreadId>(0); +} + +void Lock::AssertAcquired() const { + DCHECK(owned_by_thread_); + DCHECK_EQ(owning_thread_id_, PlatformThread::CurrentId()); +} + +void Lock::CheckHeldAndUnmark() { + DCHECK(owned_by_thread_); + DCHECK_EQ(owning_thread_id_, PlatformThread::CurrentId()); + owned_by_thread_ = false; + owning_thread_id_ = static_cast<PlatformThreadId>(0); +} + +void Lock::CheckUnheldAndMark() { + DCHECK(!owned_by_thread_); + owned_by_thread_ = true; + owning_thread_id_ = PlatformThread::CurrentId(); +} + +#endif // NDEBUG diff --git a/base/lock.h b/base/lock.h index 31ad9a0..64b8f74 100644 --- a/base/lock.h +++ b/base/lock.h @@ -7,30 +7,78 @@ #include "base/lock_impl.h" -// A convenient wrapper for an OS specific critical section. +// A convenient wrapper for an OS specific critical section. The only real +// intelligence in this class is in debug mode for the support for the +// AssertAcquired() method. class Lock { public: +#if defined(NDEBUG) // Optimized wrapper implementation Lock() : lock_() {} ~Lock() {} void Acquire() { lock_.Lock(); } void Release() { lock_.Unlock(); } + // If the lock is not held, take it and return true. If the lock is already // held by another thread, immediately return false. bool Try() { return lock_.Try(); } - // In debug builds this method checks that the lock has been acquired by the - // calling thread. If the lock has not been acquired, then the method - // will DCHECK(). In non-debug builds, the LockImpl's implementation of - // AssertAcquired() is an empty inline method. - void AssertAcquired() const { return lock_.AssertAcquired(); } + // Null implementation if not debug. + void AssertAcquired() const {} +#else + Lock(); + ~Lock() {} + + // NOTE: Although windows critical sections support recursive locks, we do not + // allow this, and we will commonly fire a DCHECK() if a thread attempts to + // acquire the lock a second time (while already holding it). + void Acquire() { + lock_.Lock(); + CheckUnheldAndMark(); + } + void Release() { + CheckHeldAndUnmark(); + lock_.Unlock(); + } - // Return the underlying lock implementation. - // TODO(awalker): refactor lock and condition variables so that this is - // unnecessary. - LockImpl* lock_impl() { return &lock_; } + bool Try() { + bool rv = lock_.Try(); + if (rv) { + CheckUnheldAndMark(); + } + return rv; + } + + void AssertAcquired() const; +#endif // NDEBUG + +#if defined(OS_POSIX) + // The posix implementation of ConditionVariable needs to be able + // to see our lock and tweak our debugging counters, as it releases + // and acquires locks inside of pthread_cond_{timed,}wait. + // Windows doesn't need to do this as it calls the Lock::* methods. + friend class ConditionVariable; +#endif private: +#if !defined(NDEBUG) + // Members and routines taking care of locks assertions. + // Note that this checks for recursive locks and allows them + // if the variable is set. This is allowed by the underlying implementation + // on windows but not on Posix, so we're doing unneeded checks on Posix. + // It's worth it to share the code. + void CheckHeldAndUnmark(); + void CheckUnheldAndMark(); + + // All private data is implicitly protected by lock_. + // Be VERY careful to only access members under that lock. + + // Determines validity of owning_thread_id_. Needed as we don't have + // a null owning_thread_id_ value. + bool owned_by_thread_; + PlatformThreadId owning_thread_id_; +#endif // NDEBUG + LockImpl lock_; // Platform specific underlying lock implementation. DISALLOW_COPY_AND_ASSIGN(Lock); diff --git a/base/lock_impl.h b/base/lock_impl.h index 2d4a921..5e323a6 100644 --- a/base/lock_impl.h +++ b/base/lock_impl.h @@ -41,17 +41,6 @@ class LockImpl { // a successful call to Try, or a call to Lock. void Unlock(); - // Debug-only method that will DCHECK() if the lock is not acquired by the - // current thread. In non-debug builds, no check is performed. - // Because linux and mac condition variables modify the underlyning lock - // through the os_lock() method, runtime assertions can not be done on those - // builds. -#if defined(NDEBUG) || !defined(OS_WIN) - void AssertAcquired() const {} -#else - void AssertAcquired() const; -#endif - // Return the native underlying lock. Not supported for Windows builds. // TODO(awalker): refactor lock and condition variables so that this is // unnecessary. @@ -62,14 +51,6 @@ class LockImpl { private: OSLockType os_lock_; -#if !defined(NDEBUG) && defined(OS_WIN) - // All private data is implicitly protected by lock_. - // Be VERY careful to only access members under that lock. - PlatformThreadId owning_thread_id_; - int32 recursion_count_shadow_; - bool recursion_used_; // Allow debugging to continued after a DCHECK(). -#endif // NDEBUG - DISALLOW_COPY_AND_ASSIGN(LockImpl); }; diff --git a/base/lock_impl_win.cc b/base/lock_impl_win.cc index 0d1ac93..14b76f8 100644 --- a/base/lock_impl_win.cc +++ b/base/lock_impl_win.cc @@ -5,16 +5,7 @@ #include "base/lock_impl.h" #include "base/logging.h" -// NOTE: Although windows critical sections support recursive locks, we do not -// allow this, and we will commonly fire a DCHECK() if a thread attempts to -// acquire the lock a second time (while already holding it). - LockImpl::LockImpl() { -#ifndef NDEBUG - recursion_count_shadow_ = 0; - recursion_used_ = false; - owning_thread_id_ = 0; -#endif // NDEBUG // The second parameter is the spin count, for short-held locks it avoid the // contending thread from going to sleep which helps performance greatly. ::InitializeCriticalSectionAndSpinCount(&os_lock_, 2000); @@ -26,16 +17,6 @@ LockImpl::~LockImpl() { bool LockImpl::Try() { if (::TryEnterCriticalSection(&os_lock_) != FALSE) { -#ifndef NDEBUG - // ONLY access data after locking. - owning_thread_id_ = PlatformThread::CurrentId(); - DCHECK_NE(owning_thread_id_, 0u); - recursion_count_shadow_++; - if (2 == recursion_count_shadow_ && !recursion_used_) { - recursion_used_ = true; - DCHECK(false); // Catch accidental redundant lock acquisition. - } -#endif return true; } return false; @@ -43,31 +24,9 @@ bool LockImpl::Try() { void LockImpl::Lock() { ::EnterCriticalSection(&os_lock_); -#ifndef NDEBUG - // ONLY access data after locking. - owning_thread_id_ = PlatformThread::CurrentId(); - DCHECK_NE(owning_thread_id_, 0u); - recursion_count_shadow_++; - if (2 == recursion_count_shadow_ && !recursion_used_) { - recursion_used_ = true; - DCHECK(false); // Catch accidental redundant lock acquisition. - } -#endif // NDEBUG } void LockImpl::Unlock() { -#ifndef NDEBUG - --recursion_count_shadow_; // ONLY access while lock is still held. - DCHECK(0 <= recursion_count_shadow_); - owning_thread_id_ = 0; -#endif // NDEBUG ::LeaveCriticalSection(&os_lock_); } -// In non-debug builds, this method is declared as an empty inline method. -#ifndef NDEBUG -void LockImpl::AssertAcquired() const { - DCHECK(recursion_count_shadow_ > 0); - DCHECK_EQ(owning_thread_id_, PlatformThread::CurrentId()); -} -#endif |