Improve the ScopedHandle verifier.

base/win/scoped_handle.cc

 // Copyright (c) 2012 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.
 
 #include "base/win/scoped_handle.h"
 
-#include <map>
-
+#include "base/containers/hash_tables.h"
 #include "base/debug/alias.h"
+#include "base/hash.h"
 #include "base/lazy_instance.h"
-#include "base/synchronization/lock.h"
+#include "base/logging.h"
+#include "base/synchronization/lock_impl.h"
 #include "base/win/windows_version.h"
 
 namespace {
 
 struct Info {
   const void* owner;
   const void* pc1;
   const void* pc2;
   DWORD thread_id;
 };
-typedef std::map<HANDLE, Info> HandleMap;
+typedef base::hash_map<HANDLE, Info> HandleMap;
 
+typedef base::internal::LockImpl NativeLock;
 base::LazyInstance<HandleMap>::Leaky g_handle_map = LAZY_INSTANCE_INITIALIZER;
-base::LazyInstance<base::Lock>::Leaky g_lock = LAZY_INSTANCE_INITIALIZER;
+base::LazyInstance<NativeLock>::Leaky g_lock = LAZY_INSTANCE_INITIALIZER;
+bool g_closing = false;
+bool g_verifier_enabled = false;
+
+bool CloseHandleWrapper(HANDLE handle) {
+  if (!::CloseHandle(handle))
+    CHECK(false);
+  return true;
+}
+
+// Simple automatic locking using a native critical section so it supports
+// recursive locking.
+class AutoNativeLock {
+ public:
+  explicit AutoNativeLock(NativeLock& lock) : lock_(lock) {
+    lock_.Lock();
+  }
+
+  ~AutoNativeLock() {
+    lock_.Unlock();
+  }
+
+ private:
+  NativeLock& lock_;
+  DISALLOW_COPY_AND_ASSIGN(AutoNativeLock);
+};
 
 }  // namespace
 
+namespace BASE_HASH_NAMESPACE {
+#if defined(COMPILER_MSVC)
+inline size_t hash_value(const HANDLE& handle) {
+  ULONG_PTR int_handle = reinterpret_cast<ULONG_PTR>(handle);
+  return base::Hash(reinterpret_cast<char*>(&int_handle), sizeof(int_handle));
+}
+#elif defined (COMPILER_GCC)
+template <>

[cpu_(ooo_6.6-7.5), 2014/08/22 01:52:04]

this branch is for nacl?

[rvargas (doing something else), 2014/08/22 02:10:32]

Not really. It is for completeness if people are trying to use another compiler
(we only ack those two on base_hash)

[cpu_(ooo_6.6-7.5), 2014/08/22 22:10:20]

Acknowledged.

+struct hash<HANDLE> {
+  size_t operator()(const HANDLE& key) const {
+    ULONG_PTR int_handle = reinterpret_cast<ULONG_PTR>(handle);
+    return base::Hash(reinterpret_cast<char*>(&int_handle), sizeof(int_handle));

[cpu_(ooo_6.6-7.5), 2014/08/22 01:52:05]

shouldn't we be doing bit_cast<char[N]> or somesuch?

I might be having dejavu here.

[rvargas (doing something else), 2014/08/22 02:10:31]

Maybe? what is N? sizeof? I wanted to be explicit/cautious and that's why I have
two casts instead of <const char*>(&handle) and let the compiler merge them at
will.


I'm not really sure if GCC's aliasing warning will trigger here given that it is
sending the pointer to another function. I can remove the whole GCC part,
although it may be better to have an alias warning (if at all) than a cryptic
error about a missing operator.

[cpu_(ooo_6.6-7.5), 2014/08/22 22:10:20]

yes sizeof.
I think we should use bit_cast and have a single path, is that possible, like
using struct hash?

Another option is that there seems to be some handy macros to has 32 and 64 bit
values which we could use when we cast to ULONG_PTR.

[rvargas (doing something else), 2014/08/23 02:22:07]

Done.

+  }
+};
+#endif
+}  // BASE_HASH_NAMESPACE
+
 namespace base {
 namespace win {
 
 // Static.
+bool HandleTraits::CloseHandle(HANDLE handle) {
+  if (!g_verifier_enabled)
+    return CloseHandleWrapper(handle);
+
+  AutoNativeLock lock(g_lock.Get());
+  g_closing = true;
+  CloseHandleWrapper(handle);
+  g_closing = false;
+
+  return true;
+}
+
+// Static.
 void VerifierTraits::StartTracking(HANDLE handle, const void* owner,
                                    const void* pc1, const void* pc2) {
+  if (!g_verifier_enabled)
+    return;
+
   // Grab the thread id before the lock.
   DWORD thread_id = GetCurrentThreadId();
 
-  AutoLock lock(g_lock.Get());
+  AutoNativeLock lock(g_lock.Get());
 
   Info handle_info = { owner, pc1, pc2, thread_id };
   std::pair<HANDLE, Info> item(handle, handle_info);
   std::pair<HandleMap::iterator, bool> result = g_handle_map.Get().insert(item);
   if (!result.second) {
     Info other = result.first->second;
     debug::Alias(&other);
     CHECK(false);
   }
 }
 
 // Static.
 void VerifierTraits::StopTracking(HANDLE handle, const void* owner,
                                   const void* pc1, const void* pc2) {
-  AutoLock lock(g_lock.Get());
+  if (!g_verifier_enabled)
+    return;
+
+  AutoNativeLock lock(g_lock.Get());
   HandleMap::iterator i = g_handle_map.Get().find(handle);
   if (i == g_handle_map.Get().end())
     CHECK(false);
 
   Info other = i->second;
   if (other.owner != owner) {
     debug::Alias(&other);
     CHECK(false);
   }
 
   g_handle_map.Get().erase(i);
 }
 
+void EnableHandleVerifier() {
+  g_verifier_enabled = true;
+}
+
+void OnHandleBeingClosed(HANDLE handle) {
+  AutoNativeLock lock(g_lock.Get());
+  if (g_closing)
+    return;
+
+  HandleMap::iterator i = g_handle_map.Get().find(handle);
+  if (i == g_handle_map.Get().end())
+    return;
+
+  Info other = i->second;
+  debug::Alias(&other);
+  CHECK(false);
+}
+
 }  // namespace win
 }  // namespace base