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, 66 insertions, 3 deletions

diff --git a/tools/memory_watcher/call_stack.h b/tools/memory_watcher/call_stack.h
index 5af7ed6..2c026bc 100644
--- a/tools/memory_watcher/call_stack.h
+++ b/tools/memory_watcher/call_stack.h
@@ -51,8 +51,14 @@ class CallStack {
   // every frame in each is identical to the corresponding frame in the other.
   bool IsEqual(const CallStack &target);
 
+  typedef std::basic_string<char, std::char_traits<char>,
+                            PrivateHookAllocator<char> > PrivateAllocatorString;
+
   // Convert the callstack to a string stored in output.
-  void CallStack::ToString(std::string* output);
+  void CallStack::ToString(PrivateAllocatorString* output);
+
+  //
+  bool Valid() const { return valid_; }
 
  private:
   // The maximum number of frames to trace.
@@ -68,14 +74,67 @@ class CallStack {
   // Functions for manipulating the frame list.
   void ClearFrames();
 
+  // Dynamically load the DbgHelp library and supporting routines that we
+  // will use.
+  static bool LoadDbgHelp();
+
+  static void LockDbgHelp() {
+    dbghelp_lock_.Acquire();
+    active_thread_id_ = GetCurrentThreadId();
+  }
+
+  static void UnlockDbgHelp() {
+    active_thread_id_ = GetCurrentThreadId();
+    dbghelp_lock_.Release();
+  }
+
+  class AutoDbgHelpLock {
+  public:
+    AutoDbgHelpLock() {
+      CallStack::LockDbgHelp();
+    }
+    ~AutoDbgHelpLock() {
+      CallStack::UnlockDbgHelp();
+    }
+  };
+
+  // Check to see if this thread is already processing a stack.
+  bool LockedRecursionDetected() const;
+
+  // According to http://msdn2.microsoft.com/en-us/library/ms680650(VS.85).aspx
+  // "All DbgHelp functions, such as this one, are single threaded.  Therefore,
+  // calls from more than one thread to this function will likely result in
+  // unexpected behavior or memory corruption.  To avoid this, you must
+  // synchromize all concurrent calls from one thread to this function."
+  //
+  // dbghelp_lock_ is used to serialize access across all calls to the DbgHelp
+  // library.  This may be overly conservative (serializing them all together),
+  // but does guarantee correctness.
+  static Lock dbghelp_lock_;
+
+  // Record the fact that dbghelp has been loaded.
+  // Changes to this variable are protected by dbghelp_lock_.
+  // It will only changes once... from false to true.
+  static bool dbghelp_loaded_;
+
+  // To prevent infinite recursion due to unexpected side effects in libraries,
+  // we track the thread_id of the thread currently holding the dbghelp_lock_.
+  // We avoid re-aquiring said lock and return an !valid_ instance when we
+  // detect recursion.
+  static DWORD active_thread_id_;
+
   int frame_count_;  // Current size (in frames)
   DWORD_PTR frames_[kMaxTraceFrames];
   int32 hash_;
   int32 id_;
 
+  // Indicate is this is a valid stack.
+  // This is false if recursion precluded a real stack generation.
+  bool valid_;
+
   // Cache ProgramCounter -> Symbol lookups.
   // This cache is not thread safe.
-  typedef std::map<int32, std::string, std::less<int32>,
+  typedef std::map<int32, PrivateAllocatorString, std::less<int32>,
                    PrivateHookAllocator<int32> > SymbolCache;
   static SymbolCache* symbol_cache_;
 
@@ -88,14 +147,18 @@ class CallStack {
 // free instances.
 class AllocationStack : public CallStack {
  public:
-  AllocationStack() : next_(NULL), CallStack() {}
+  explicit AllocationStack(int32 size)
+      : next_(NULL), size_(size), CallStack() {}
 
   // We maintain a freelist of the AllocationStacks.
   void* operator new(size_t s);
   void operator delete(void*p);
 
+  int32 size() const { return size_; }
+
  private:
   AllocationStack* next_;     // Pointer used when on the freelist.
+  int32 size_;                // Size of block allocated.
   static AllocationStack* freelist_;
   static Lock freelist_lock_;