Revert 49648 - Initial implementation of new AssertAcquired() functionality for Posix.

Webkit compile failing.

BUG=44091
TEST=Try bot run on Windows, Linux, Mac.  Will land during low traffic time and revert on any problems or perf degradation.

Review URL: http://codereview.chromium.org/2196001

TBR=rdsmith@google.com
Review URL: http://codereview.chromium.org/2805001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@49649 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 74 insertions, 107 deletions

diff --git a/base/condition_variable.h b/base/condition_variable.h
index da87bfc..430f57c 100644
--- a/base/condition_variable.h
+++ b/base/condition_variable.h
@@ -174,9 +174,6 @@ 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 9d08f18..8c5a5c2 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_.os_lock()), user_lock_(user_lock) {
+    : user_mutex_(user_lock->lock_impl()->os_lock()) {
   int rv = pthread_cond_init(&condition_, NULL);
   DCHECK(rv == 0);
 }
@@ -27,14 +27,8 @@ 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) {
@@ -52,14 +46,8 @@ 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 c73a458..9ff963b 100644
--- a/base/lock.cc
+++ b/base/lock.cc
@@ -1,37 +1,7 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2006-2008 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.
 
-// 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.
+// Lock class.
 
-#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
+// Depricated file.  See lock_impl_*.cc for platform specific versions.
diff --git a/base/lock.h b/base/lock.h
index 64b8f74..31ad9a0 100644
--- a/base/lock.h
+++ b/base/lock.h
@@ -7,78 +7,30 @@
 
 #include "base/lock_impl.h"
 
-// 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.
+// A convenient wrapper for an OS specific critical section.
 
 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(); }
 
-  // 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();
-  }
+  // 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(); }
 
-  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
+  // Return the underlying lock implementation.
+  // TODO(awalker): refactor lock and condition variables so that this is
+  // unnecessary.
+  LockImpl* lock_impl() { return &lock_; }
 
  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 5e323a6..2d4a921 100644
--- a/base/lock_impl.h
+++ b/base/lock_impl.h
@@ -41,6 +41,17 @@ 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.
@@ -51,6 +62,14 @@ 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 14b76f8..0d1ac93 100644
--- a/base/lock_impl_win.cc
+++ b/base/lock_impl_win.cc
@@ -5,7 +5,16 @@
 #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);
@@ -17,6 +26,16 @@ 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;
@@ -24,9 +43,31 @@ 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