Assert on deleting a held lock.

base/synchronization/lock.cc

 // Copyright (c) 2011 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.
 
 #if !defined(NDEBUG)
 
 #include "base/synchronization/lock.h"
 #include "base/logging.h"
 
 namespace base {
 
+const PlatformThreadId kNoThreadId = static_cast<PlatformThreadId>(0);
+
 Lock::Lock() : lock_() {
   owned_by_thread_ = false;
-  owning_thread_id_ = static_cast<PlatformThreadId>(0);
+  owning_thread_id_ = kNoThreadId;
+}
+
+Lock::~Lock() {
+  DCHECK(!owned_by_thread_);
+  DCHECK_EQ(kNoThreadId, owning_thread_id_);
+
+  // This may trip the assert on unlock in the owning thread.

[willchan no longer on Chromium, 2013/04/08 15:28:13]

I don't understand this one. We know that it's false already due to the new
DCHECK on line 24, right?

[Sigurður Ásgeirsson, 2013/04/08 15:32:35]

Depends on whether the DCHECK is set to log fatally, it seems in some builds
and/or on some bots we just spew a stack trace and move on. In a build with
those settings, you *may* get a second stack trace as e.g. AssertAcquired and/or
CheckHeldAndUnmark are invoked on the owning thread as part of unlocking. It
depends on whether the lock is e.g. a static instance, and if not, on what
happens to the allocation afterwards.
This is definitely heuristic, though - I'll remove it if you think it has no
value?

+  owned_by_thread_ = false;
 }
 
 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();
 }
 
 }  // namespace base
 
 #endif  // NDEBUG