Base: Add a handle verifier to ScopedHandle.

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 <set>
+
+#include "base/debug/alias.h"
+#include "base/lazy_instance.h"
+#include "base/synchronization/lock.h"
+#include "base/win/windows_version.h"
+
+namespace {
+
+struct Info {
+  const void* owner;
+  const void* pc;
+  DWORD thread_id;

[alexeypa (please no reviews), 2012/05/30 20:19:30]

I think |pc| and |thread_id| can be hosted inside of GenericScopedHandle<T>
instances. See my other comment about LIST_ENTRY.

+};
+typedef std::map<HANDLE, Info> HandleMap;
+
+base::LazyInstance<HandleMap> g_handle_map = LAZY_INSTANCE_INITIALIZER;
+base::LazyInstance<std::set<const void*> > g_owner_set =

[alexeypa (please no reviews), 2012/05/30 20:19:30]

|g_owner_set| is a way too heavy for the job it is doing. You could add
LIST_ENTRY to each instance of GenericScopedHandle<T> and link all of them
together. This will give you constant time on insertion and removal; and it will
does the same sort of check. 

[rvargas (doing something else), 2012/05/30 21:39:01]

Background: this is not a long term feature (it should not even get to a beta
channel, and maybe not even dev). It is just so that we can track the linked
bug... As of App verifier, we need data from the field, not on our tests (a
debug build would be enough in that case).

+    LAZY_INSTANCE_INITIALIZER;
+base::LazyInstance<base::Lock> g_lock = LAZY_INSTANCE_INITIALIZER;
+
+}  // namespace
+
+namespace base {
+namespace win {
+
+// Static.
+void VerifierTraits::StartTracking(HANDLE handle, const void* owner,
+                                   const void* pc) {
+  if (OSInfo::GetInstance()->version() > VERSION_XP)
+    return;
+
+  // Grab the thread id before the lock.
+  DWORD thread_id = GetCurrentThreadId();
+
+  AutoLock(g_lock.Get());
+
+  if (handle == INVALID_HANDLE_VALUE) {
+    // Cannot track this handle.
+    g_owner_set.Get().insert(owner);

[rvargas (doing something else), 2012/05/30 22:12:33]

I still hope this is not the common case.

+    return;
+  }
+
+  HandleMap::iterator i = g_handle_map.Get().find(handle);

[alexeypa (please no reviews), 2012/05/30 20:19:30]

I think a simple linked list would be much simpler and more efficient. The
lookups can be accelerated by a bitmap or a bloom filter since all we need to
know if a value is in the set.

+  if (i != g_handle_map.Get().end()) {
+    Info other = i->second;
+    debug::Alias(&other);
+    CHECK(false);
+  }
+
+  Info handle_info = { owner, pc, thread_id };
+  g_handle_map.Get()[handle] = handle_info;

[alexeypa (please no reviews), 2012/05/30 20:19:30]

This will do a second lookup in the map. it is better to use map::insert and
check the return value to see if the element exists in the map already.

[rvargas (doing something else), 2012/05/30 22:12:33]

Done.

+}
+
+// Static.
+void VerifierTraits::StopTracking(HANDLE handle, const void* owner,
+                                  const void* pc) {
+  if (OSInfo::GetInstance()->version() > VERSION_XP)
+    return;
+
+  AutoLock(g_lock.Get());
+  HandleMap::iterator i = g_handle_map.Get().find(handle);
+  if (i == g_handle_map.Get().end()) {
+    std::set<const void*>::iterator j = g_owner_set.Get().find(owner);
+    if (j != g_owner_set.Get().end()) {
+      g_owner_set.Get().erase(j);
+      return;
+    }
+    CHECK(false);
+  }
+
+  Info other = i->second;
+  if (other.owner != owner) {
+    debug::Alias(&other);
+    CHECK(false);
+  }
+
+  g_handle_map.Get().erase(i);
+}
+
+}  // namespace win
+}  // namespace base