Support running memory watch under vista, plus other tweaks

This version of memory_watcher can run under Vista, even though the
recursive calls that it handles are appearing often enough that there
is a performance penalty.  With this landed, it may be possible for other
folks to run the tool, and I can work on improving its performance.

This CL also resolves the problem with hanging processes.  Although
memory reporting can only be done once, and it leaves a pile of memory
"hanging around," the browser can be cleanly exited.

Tweaks include outputing the aggregate stacks such that the largest
stacks appear at the start of the output file.

This version avoids ongoing aggregation of stats in favor of only
doing the aggregation at dump-time.  This probably enhances performance
at run-time, although it is hidden (on Vista) by the recursive calling.
This also simplifies the tracking code a fair amount.

There is some evidence that a small number of duplicate calls are being
made to "track" the same memory region, without an intervening free (i.e.,
call to "untrack").  The code to better diagnose this is currently in
place, but ifdef'ed, as it is only useful under a debugger.  Exercise
of this code (turning a stack-frame list into a human readable stack
trace string) currently causes some corruption shortly after it triggers,
so I can't leave it on full time.


r=mbelshe
Review URL: http://codereview.chromium.org/366031

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@31299 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 8 insertions, 1 deletions

diff --git a/tools/memory_watcher/memory_hook.cc b/tools/memory_watcher/memory_hook.cc
index 9c8bc03..2340d84 100644
--- a/tools/memory_watcher/memory_hook.cc
+++ b/tools/memory_watcher/memory_hook.cc
@@ -217,6 +217,7 @@ static LPVOID WINAPI Perftools_HeapReAlloc(HANDLE hHeap, DWORD dwFlags,
   // block via Perftools_HeapAlloc.
 
   LPVOID rv = Perftools_HeapAlloc(hHeap, dwFlags, dwBytes);
+  DCHECK_EQ((HEAP_REALLOC_IN_PLACE_ONLY & dwFlags), 0);
 
   // If there was an old buffer, now copy the data to the new buffer.
   if (lpMem != 0) {
@@ -237,8 +238,10 @@ static LPVOID WINAPI Perftools_VirtualAllocEx(HANDLE process, LPVOID address,
   if (address != NULL) {
     MEMORY_BASIC_INFORMATION info;
     CHECK(VirtualQuery(address, &info, sizeof(info)));
-    if (info.State & MEM_COMMIT)
+    if (info.State & MEM_COMMIT) {
       already_committed = true;
+      CHECK(size >= info.RegionSize);
+    }
   }
   bool reserving = (address == NULL) || (type & MEM_RESERVE);
   bool committing = !already_committed && (type & MEM_COMMIT);
@@ -355,6 +358,7 @@ static HGLOBAL WINAPI Perftools_GlobalFree(HGLOBAL hMem) {
 
 static HGLOBAL WINAPI Perftools_GlobalReAlloc(HGLOBAL hMem, SIZE_T dwBytes,
                                               UINT uFlags) {
+  // TODO(jar): [The following looks like a copy/paste typo from LocalRealloc.]
   // GlobalDiscard is a macro which calls LocalReAlloc with size 0.
   if (dwBytes == 0) {
     return patch_GlobalReAlloc()(hMem, dwBytes, uFlags);
@@ -515,6 +519,9 @@ bool MemoryHook::RegisterWatcher(MemoryObserver* watcher) {
 bool MemoryHook::UnregisterWatcher(MemoryObserver* watcher) {
   DCHECK(hooked_);
   DCHECK(global_hook_->watcher_ == watcher);
+  // TODO(jar): changing watcher_ here is very racy.  Other threads may (without
+  // a lock) testing, and then calling through this value.  We probably can't
+  // remove this until we are single threaded.
   global_hook_->watcher_ = NULL;
 
   // For now, since there are no more watchers, unhook memory.