Move windows specific lock-recursion-counter into windows impl file

Unix implementation of lock leaks the underlying lock_impl_ member
so that the condition variable implementation can directly acquire
and release the lock (without going through our abstract interface). This
causse the recursion counter to become incorrect on such platforms.

Windows uses an implementation of condition variables that uses our abstract
interface, and hence is the only implementation that can track the
recursion count (and besides... windows is the only platform that currently
allows recursive (multiple) acquisitions of a lock by a single thread.

I'll work on gracefully removing the depricated lock.cc
after I've landed this change.

r=cpu
Review URL: http://codereview.chromium.org/7660

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

base/lock.cc

@@ -2,74 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Provide place to put profiling methods for the
 // Lock class.
-// The Lock class is used everywhere, and hence any changes
-// to lock.h tend to require a complete rebuild.  To facilitate
-// profiler development, all the profiling methods are listed
-// here.  
 
-#include "base/lock.h"
-#include "base/logging.h"
-
-#ifndef NDEBUG
-Lock::Lock()
-    : lock_(),
-      recursion_count_shadow_(0),
-      recursion_used_(false),
-      acquisition_count_(0),
-      contention_count_(0) {
-}
-#else  // NDEBUG
-Lock::Lock()
-    : lock_() {
-}
-#endif  // NDEBUG
-
-Lock::~Lock() {
-}
-
-void Lock::Acquire() {
-#ifdef NDEBUG
-  lock_.Lock();
-#else  // NDEBUG
-  if (!lock_.Try()) {
-    // We have contention.
-    lock_.Lock();
-    contention_count_++;
-  }
-  // ONLY access data after locking.
-  recursion_count_shadow_++;
-  acquisition_count_++;
-  if (2 == recursion_count_shadow_ && !recursion_used_) {
-    recursion_used_ = true;
-    // TODO(jar): this is causing failures in ThreadTest.Restart and
-    // ChromeThreadTest.Get on Linux.
-    // DCHECK(false);  // Catch accidental redundant lock acquisition.
-  }
-#endif  // NDEBUG
-}
-
-void Lock::Release() {
-#ifndef NDEBUG
-  --recursion_count_shadow_;  // ONLY access while lock is still held.
-  DCHECK(0 <= recursion_count_shadow_);
-#endif  // NDEBUG
-  lock_.Unlock();
-}
-
-bool Lock::Try() {
-  if (lock_.Try()) {
-#ifndef NDEBUG
-    recursion_count_shadow_++;
-    acquisition_count_++;
-    if (2 == recursion_count_shadow_ && !recursion_used_) {
-      recursion_used_ = true;
-      DCHECK(false);  // Catch accidental redundant lock acquisition.
-    }
-#endif
-    return true;
-  } else {
-    return false;
-  }
-}
+// Depricated file.  See lock_impl_*.cc for platform specific versions.

base/lock.h

@@ -8,24 +8,16 @@
 #include "base/lock_impl.h"
 
 // A convenient wrapper for an OS specific critical section.
-//
-// 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).
-//
-// Complication: UnitTest for DeathTests catch DCHECK exceptions, so we need
-// to write code assuming DCHECK will throw.  This means we need to save any
-// assertable value in a local until we can safely throw.
 
 class Lock {
  public:
-  Lock();
-  ~Lock();
-  void Acquire();
-  void Release();
+  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();
+  bool Try() { return lock_.Try(); }
 
   // Return the underlying lock implementation.
   // TODO(awalker): refactor lock and condition variables so that this is
@@ -33,16 +25,7 @@ class Lock {
   LockImpl* lock_impl() { return &lock_; }
 
  private:
-  LockImpl lock_;  // User-supplied underlying lock implementation.
-
-#ifndef NDEBUG
-  // All private data is implicitly protected by lock_.
-  // Be VERY careful to only access members under that lock.
-  int32 recursion_count_shadow_;
-  bool recursion_used_;      // Allow debugging to continued after a DCHECK().
-  int32 acquisition_count_;  // Number of times lock was acquired.
-  int32 contention_count_;   // Number of times there was contention.
-#endif  // NDEBUG
+  LockImpl lock_;  // Platform specific underlying lock implementation.
 
   DISALLOW_COPY_AND_ASSIGN(Lock);
 };

base/lock_impl.h

@@ -48,6 +48,13 @@ 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.
+  int32 recursion_count_shadow_;
+  bool recursion_used_;      // Allow debugging to continued after a DCHECK().
+#endif  // NDEBUG
+
   DISALLOW_COPY_AND_ASSIGN(LockImpl);
 };
 

base/lock_impl_win.cc

@@ -3,8 +3,17 @@
 // found in the LICENSE file.
 
 #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;
+#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);
@@ -15,14 +24,35 @@ LockImpl::~LockImpl() {
 }
 
 bool LockImpl::Try() {
-  return ::TryEnterCriticalSection(&os_lock_) != FALSE;
+  if (::TryEnterCriticalSection(&os_lock_) != FALSE) {
+#ifndef NDEBUG
+    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;
 }
 
 void LockImpl::Lock() {
   ::EnterCriticalSection(&os_lock_);
+#ifndef NDEBUG
+  // ONLY access data after locking.
+  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_);
+#endif  // NDEBUG
   ::LeaveCriticalSection(&os_lock_);
 }
-