Improve the ScopedHandle verifier.

1. Automate the selection of the proper channel to enable the verifier.
   Now the code is enabled at runtime.

2. Switch to a hash_map to track handles.

3. Intercept CloseHandle to detect the code that is closing handles owned
   by ScopedHandles. The initial implementation only covers chrome.exe/dll,
   but the plan is to extend that in the future to all modules loaded in the
   process.

BUG=362176
R=cpu@chromium.org
R=sky@chromium.org

See https://codereview.chromium.org/490043002/ for the actual review.

TBR=cpu@chromium.org
TBR=sky@chromium.org

Review URL: https://codereview.chromium.org/506013004

Cr-Commit-Position: refs/heads/master@{#291969}

2 files changed, 105 insertions, 13 deletions

diff --git a/base/win/scoped_handle.cc b/base/win/scoped_handle.cc
index 7b38369..fbc7d40 100644
--- a/base/win/scoped_handle.cc
+++ b/base/win/scoped_handle.cc
@@ -4,11 +4,12 @@
 
 #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 {
@@ -19,23 +20,86 @@ struct Info {
   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);
+};
+
+inline size_t HashHandle(const HANDLE& handle) {
+  char buffer[sizeof(handle)];
+  memcpy(buffer, &handle, sizeof(handle));
+  return base::Hash(buffer, sizeof(buffer));
+}
 
 }  // namespace
 
+namespace BASE_HASH_NAMESPACE {
+#if defined(COMPILER_MSVC)
+inline size_t hash_value(const HANDLE& handle) {
+  return HashHandle(handle);
+}
+#elif defined (COMPILER_GCC)
+template <>
+struct hash<HANDLE> {
+  size_t operator()(const HANDLE& handle) const {
+    return HashHandle(handle);
+  }
+};
+#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);
@@ -50,7 +114,10 @@ void VerifierTraits::StartTracking(HANDLE handle, const void* owner,
 // 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);
@@ -64,5 +131,23 @@ void VerifierTraits::StopTracking(HANDLE handle, const void* owner,
   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
diff --git a/base/win/scoped_handle.h b/base/win/scoped_handle.h
index 00063af..e1e0f5d 100644
--- a/base/win/scoped_handle.h
+++ b/base/win/scoped_handle.h
@@ -101,9 +101,7 @@ class GenericScopedHandle {
       Verifier::StopTracking(handle_, this, BASE_WIN_GET_CALLER,
                              tracked_objects::GetProgramCounter());
 
-      if (!Traits::CloseHandle(handle_))
-        CHECK(false);
-
+      Traits::CloseHandle(handle_);
       handle_ = Traits::NullHandle();
     }
   }
@@ -120,9 +118,7 @@ class HandleTraits {
   typedef HANDLE Handle;
 
   // Closes the handle.
-  static bool CloseHandle(HANDLE handle) {
-    return ::CloseHandle(handle) != FALSE;
-  }
+  static bool BASE_EXPORT CloseHandle(HANDLE handle);
 
   // Returns true if the handle value is valid.
   static bool IsHandleValid(HANDLE handle) {
@@ -168,6 +164,17 @@ class BASE_EXPORT VerifierTraits {
 
 typedef GenericScopedHandle<HandleTraits, VerifierTraits> ScopedHandle;
 
+// This function should be called by the embedder to enable the use of
+// VerifierTraits at runtime. It has no effect if DummyVerifierTraits is used
+// for ScopedHandle.
+void BASE_EXPORT EnableHandleVerifier();
+
+// This should be called whenever the OS is closing a handle, if extended
+// verification of improper handle closing is desired. If |handle| is being
+// tracked by the handle verifier and ScopedHandle is not the one closing it,
+// a CHECK is generated.
+void BASE_EXPORT OnHandleBeingClosed(HANDLE handle);
+
 }  // namespace win
 }  // namespace base