Add test for multithreaded ActiveVerifier behavior.

base/win/scoped_handle_test_dll.cc

+// Copyright 2016 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 <windows.h>
+
+#include <vector>
+
+#include "base/win/scoped_handle.h"
+
+// http://blogs.msdn.com/oldnewthing/archive/2004/10/25/247180.aspx
+extern "C" IMAGE_DOS_HEADER __ImageBase;
+
+namespace base {
+namespace win {
+namespace testing {
+
+extern "C" bool __declspec(dllexport) RunTest();
+
+namespace {
+
+// Note, this must use all native functions to avoid instantiating the
+// ActiveVerifier. e.g. can't use base::Thread or even base::PlatformThread.
+DWORD __stdcall ThreadFunc(void* params) {
+  HANDLE handle = ::CreateMutex(nullptr, false, nullptr);
+  HANDLE event = reinterpret_cast<HANDLE>(params);
+  ::WaitForSingleObject(event, INFINITE);
+  ScopedHandle handle_holder(handle);

[scottmg, 2016/03/11 22:13:09]

Does ActiveVerifier CHECK or DCHECK? Do we need to make it fatal-er always?

[Will Harris, 2016/03/11 22:23:44]

it does a CHECK, so it is certainly quite fatal.

the test failing looks like this:
https://paste.googleplex.com/4738316201099264?raw

[scottmg, 2016/03/11 22:29:00]

OK, I was more worried about this test becoming pointless if someone decided
that should only be DCHECK later.

[Will Harris, 2016/03/11 23:07:47]

Acknowledged.

+  return 0;
+}
+
+bool InternalRunThreadTest() {
+  std::vector<HANDLE> threads_;
+  // From manual testing, the bug fixed by crrev.com/678736a starts reliably
+  // causing handle verifier asserts to trigger at around 100 threads, so make
+  // it 200 to be sure to detect any future regressions.
+  const size_t kNumThreads = 200;
+
+  // bManualReset is set to true to allow signalling multiple threads.
+  HANDLE start_event = ::CreateEvent(nullptr, true, false, nullptr);
+  if (!start_event)
+    return false;
+
+  for (size_t i = 0; i < kNumThreads; i++) {
+    HANDLE thread_handle =
+        ::CreateThread(nullptr, 0, ThreadFunc,
+                       reinterpret_cast<void*>(start_event), 0, nullptr);
+    if (!thread_handle)
+      break;
+    threads_.push_back(thread_handle);
+  }
+
+  if (threads_.size() != kNumThreads) {

[scottmg, 2016/03/11 22:13:09]

I wouldn't bother with this, just return false instead of break since you're
trying to get something to assert anyway. (And if CreateThread fails, the world
is over anyway.).

Does LOG break the test? Otherwise, consider logging on failures for
CreateEvent, CreateThread, and WFSO.

[Will Harris, 2016/03/11 22:23:44]

bah why u not like my cleanup code?

[scottmg, 2016/03/11 22:29:00]

I don't really care either way. You can keep it if you like, it just seems
pointless to me.

[Will Harris, 2016/03/11 23:07:47]

I left it. I hate leaks.

[scottmg, 2016/03/11 23:14:19]

You wouldn't appreciate the irony of a handle-checker test leaking handles?
Pfft.

+    for (const auto& thread : threads_)
+      ::CloseHandle(thread);
+    ::CloseHandle(start_event);
+    return false;
+  }
+
+  ::SetEvent(start_event);
+  ::CloseHandle(start_event);
+  for (const auto& thread : threads_) {
+    ::WaitForSingleObject(thread, INFINITE);
+    ::CloseHandle(thread);
+  }
+
+  return true;
+}
+
+bool InternalRunLocationTest() {
+  // Create a new handle and then set LastError again.
+  HANDLE handle = ::CreateMutex(nullptr, false, nullptr);
+  if (!handle)
+    return false;
+  ScopedHandle handle_holder(handle);
+
+  HMODULE verifier_module = GetHandleVerifierModuleForTesting();
+  if (!verifier_module)
+    return false;
+
+  // Get my module
+  HMODULE my_module = reinterpret_cast<HMODULE>(&__ImageBase);
+  if (!my_module)
+    return false;
+
+  HMODULE main_module = ::GetModuleHandle(NULL);
+
+#if defined(COMPONENT_BUILD)
+  // In a component build ActiveVerifier will always be created inside base.dll
+  // as the code always lives there.
+  if (verifier_module == my_module || verifier_module == main_module)
+    return false;
+#else
+  // In a non-component build, ActiveVerifier should always be created in the
+  // version of base linked with the main executable.
+  if (verifier_module == my_module || verifier_module != main_module)
+    return false;
+#endif
+  return true;
+}
+
+}  // namespace
+
+bool RunTest() {
+  return InternalRunThreadTest() && InternalRunLocationTest();
+}
+
+}  // testing
+}  // win
+}  // base