Provide a mechanism for SymbolSuppliers to interrupt processing (#93)

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@80 4c0a9323-5329-0410-9bdc-e9ce6186880e

13 files changed, 174 insertions, 96 deletions

diff --git a/src/google_airbag/processor/call_stack.h b/src/google_airbag/processor/call_stack.h
index 580d4927..d938944e 100644
--- a/src/google_airbag/processor/call_stack.h
+++ b/src/google_airbag/processor/call_stack.h
@@ -56,17 +56,18 @@ template<typename T> class linked_ptr;
 
 class CallStack {
  public:
+  CallStack() { Clear(); }
   ~CallStack();
 
+  // Resets the CallStack to its initial empty state
+  void Clear();
+  
   const vector<StackFrame*>* frames() const { return &frames_; }
 
  private:
   // Stackwalker is responsible for building the frames_ vector.
   friend class Stackwalker;
 
-  // Disallow instantiation other than by friends.
-  CallStack() : frames_() {}
-
   // Storage for pushed frames.
   vector<StackFrame*> frames_;
 };
diff --git a/src/google_airbag/processor/minidump_processor.h b/src/google_airbag/processor/minidump_processor.h
index 06c185ff..604eae45 100644
--- a/src/google_airbag/processor/minidump_processor.h
+++ b/src/google_airbag/processor/minidump_processor.h
@@ -42,15 +42,21 @@ class SymbolSupplier;
 
 class MinidumpProcessor {
  public:
+  // Return type for Process()
+  enum ProcessResult {
+    PROCESS_OK,  // the minidump was processed successfully
+    PROCESS_ERROR,  // there was an error processing the minidump
+    PROCESS_INTERRUPTED,  // processing was interrupted by the SymbolSupplier
+  };
+
   // Initializes this MinidumpProcessor.  supplier should be an
   // implementation of the SymbolSupplier abstract base class.
   explicit MinidumpProcessor(SymbolSupplier *supplier);
   ~MinidumpProcessor();
 
-  // Returns a new ProcessState object produced by processing the minidump
-  // file.  The caller takes ownership of the ProcessState.  Returns NULL on
-  // failure.
-  ProcessState* Process(const string &minidump_file);
+  // Processes the minidump file and fills process_state with the result.
+  ProcessResult Process(const string &minidump_file,
+                        ProcessState *process_state);
 
   // Returns a textual representation of the base CPU type that the minidump
   // in dump was produced on.  Returns an empty string if this information
diff --git a/src/google_airbag/processor/process_state.h b/src/google_airbag/processor/process_state.h
index 69005e2b..6fe12155 100644
--- a/src/google_airbag/processor/process_state.h
+++ b/src/google_airbag/processor/process_state.h
@@ -47,8 +47,12 @@ class CodeModules;
 
 class ProcessState {
  public:
+  ProcessState() : modules_(NULL) { Clear(); }
   ~ProcessState();
 
+  // Resets the ProcessState to its default values
+  void Clear();
+
   // Accessors.  See the data declarations below.
   u_int32_t time_date_stamp() const { return time_date_stamp_; }
   bool crashed() const { return crashed_; }
@@ -66,11 +70,6 @@ class ProcessState {
   // MinidumpProcessor is responsible for building ProcessState objects.
   friend class MinidumpProcessor;
 
-  // Disallow instantiation other than by friends.
-  ProcessState() : time_date_stamp_(0), crashed_(false), crash_reason_(),
-                   crash_address_(0), requesting_thread_(-1), threads_(),
-                   os_(), os_version_(), cpu_(), cpu_info_(), modules_(NULL) {}
-
   // The time-date stamp of the minidump (time_t format)
   u_int32_t time_date_stamp_;
 
diff --git a/src/google_airbag/processor/stackwalker.h b/src/google_airbag/processor/stackwalker.h
index b3f2333e..70da125e 100644
--- a/src/google_airbag/processor/stackwalker.h
+++ b/src/google_airbag/processor/stackwalker.h
@@ -61,10 +61,11 @@ class Stackwalker {
  public:
   virtual ~Stackwalker() {}
 
-  // Creates a new CallStack and populates it by calling GetContextFrame and
+  // Populates the given CallStack by calling GetContextFrame and
   // GetCallerFrame.  The frames are further processed to fill all available
-  // data.  The caller takes ownership of the CallStack returned by Walk.
-  CallStack* Walk();
+  // data.  Returns true if the stackwalk completed, or false if it was
+  // interrupted by SymbolSupplier::GetSymbolFile().
+  bool Walk(CallStack *stack);
 
   // Returns a new concrete subclass suitable for the CPU that a stack was
   // generated on, according to the CPU type indicated by the context
diff --git a/src/google_airbag/processor/symbol_supplier.h b/src/google_airbag/processor/symbol_supplier.h
index 47a2be08..d456174b 100644
--- a/src/google_airbag/processor/symbol_supplier.h
+++ b/src/google_airbag/processor/symbol_supplier.h
@@ -42,10 +42,24 @@ class CodeModule;
 
 class SymbolSupplier {
  public:
+  // Result type for GetSymbolFile
+  enum SymbolResult {
+    // no symbols were found, but continue processing
+    NOT_FOUND,
+
+    // symbols were found, and the path has been placed in symbol_file
+    FOUND,
+
+    // stops processing the minidump immediately
+    INTERRUPT,
+  };
+
   virtual ~SymbolSupplier() {}
 
-  // Returns the path to the symbol file for the given module.
-  virtual string GetSymbolFile(const CodeModule *module) = 0;
+  // Retrieves the symbol file for the given CodeModule, placing the
+  // path in symbol_file if successful.
+  virtual SymbolResult GetSymbolFile(const CodeModule *module,
+                                     string *symbol_file) = 0;
 };
 
 }  // namespace google_airbag
diff --git a/src/processor/call_stack.cc b/src/processor/call_stack.cc
index 8c1362f3..ee67aace 100644
--- a/src/processor/call_stack.cc
+++ b/src/processor/call_stack.cc
@@ -39,6 +39,10 @@
 namespace google_airbag {
 
 CallStack::~CallStack() {
+  Clear();
+}
+
+void CallStack::Clear() {
   for (vector<StackFrame *>::const_iterator iterator = frames_.begin();
        iterator != frames_.end();
        ++iterator) {
diff --git a/src/processor/minidump_processor.cc b/src/processor/minidump_processor.cc
index 99aa04cc..ab027e8a 100644
--- a/src/processor/minidump_processor.cc
+++ b/src/processor/minidump_processor.cc
@@ -45,13 +45,14 @@ MinidumpProcessor::MinidumpProcessor(SymbolSupplier *supplier)
 MinidumpProcessor::~MinidumpProcessor() {
 }
 
-ProcessState* MinidumpProcessor::Process(const string &minidump_file) {
+MinidumpProcessor::ProcessResult MinidumpProcessor::Process(
+    const string &minidump_file, ProcessState *process_state) {
   Minidump dump(minidump_file);
   if (!dump.Read()) {
-    return NULL;
+    return PROCESS_ERROR;
   }
 
-  scoped_ptr<ProcessState> process_state(new ProcessState());
+  process_state->Clear();
 
   const MDRawHeader *header = dump.header();
   assert(header);
@@ -91,7 +92,7 @@ ProcessState* MinidumpProcessor::Process(const string &minidump_file) {
 
   MinidumpThreadList *threads = dump.GetThreadList();
   if (!threads) {
-    return NULL;
+    return PROCESS_ERROR;
   }
 
   bool found_requesting_thread = false;
@@ -101,12 +102,12 @@ ProcessState* MinidumpProcessor::Process(const string &minidump_file) {
        ++thread_index) {
     MinidumpThread *thread = threads->GetThreadAtIndex(thread_index);
     if (!thread) {
-      return NULL;
+      return PROCESS_ERROR;
     }
 
     u_int32_t thread_id;
     if (!thread->GetThreadID(&thread_id)) {
-      return NULL;
+      return PROCESS_ERROR;
     }
 
     // If this thread is the thread that produced the minidump, don't process
@@ -122,7 +123,7 @@ ProcessState* MinidumpProcessor::Process(const string &minidump_file) {
     if (has_requesting_thread && thread_id == requesting_thread_id) {
       if (found_requesting_thread) {
         // There can't be more than one requesting thread.
-        return NULL;
+        return PROCESS_ERROR;
       }
 
       // Use processed_state->threads_.size() instead of thread_index.
@@ -148,7 +149,7 @@ ProcessState* MinidumpProcessor::Process(const string &minidump_file) {
 
     MinidumpMemoryRegion *thread_memory = thread->GetMemory();
     if (!thread_memory) {
-      return NULL;
+      return PROCESS_ERROR;
     }
 
     // Use process_state->modules_ instead of module_list, because the
@@ -165,23 +166,22 @@ ProcessState* MinidumpProcessor::Process(const string &minidump_file) {
                                        process_state->modules_,
                                        supplier_));
     if (!stackwalker.get()) {
-      return NULL;
+      return PROCESS_ERROR;
     }
 
-    scoped_ptr<CallStack> stack(stackwalker->Walk());
-    if (!stack.get()) {
-      return NULL;
+    scoped_ptr<CallStack> stack(new CallStack());
+    if (!stackwalker->Walk(stack.get())) {
+      return PROCESS_INTERRUPTED;
     }
-
     process_state->threads_.push_back(stack.release());
   }
 
   // If a requesting thread was indicated, it must be present.
   if (has_requesting_thread && !found_requesting_thread) {
-    return NULL;
+    return PROCESS_ERROR;
   }
 
-  return process_state.release();
+  return PROCESS_OK;
 }
 
 // Returns the MDRawSystemInfo from a minidump, or NULL if system info is
diff --git a/src/processor/minidump_processor_unittest.cc b/src/processor/minidump_processor_unittest.cc
index c8a0777f..766c2a30 100644
--- a/src/processor/minidump_processor_unittest.cc
+++ b/src/processor/minidump_processor_unittest.cc
@@ -62,18 +62,33 @@ using google_airbag::SymbolSupplier;
 
 class TestSymbolSupplier : public SymbolSupplier {
  public:
-  virtual string GetSymbolFile(const CodeModule *module);
+  TestSymbolSupplier() : interrupt_(false) {}
+
+  virtual SymbolResult GetSymbolFile(const CodeModule *module,
+                                     string *symbol_file);
+
+  // When set to true, causes the SymbolSupplier to return INTERRUPT
+  void set_interrupt(bool interrupt) { interrupt_ = interrupt; }
+
+ private:
+  bool interrupt_;
 };
 
-string TestSymbolSupplier::GetSymbolFile(const CodeModule *module) {
+SymbolSupplier::SymbolResult TestSymbolSupplier::GetSymbolFile(
+    const CodeModule *module, string *symbol_file) {
+  if (interrupt_) {
+    return INTERRUPT;
+  }
+
   if (module && module->code_file() == "C:\\test_app.exe") {
-    return string(getenv("srcdir") ? getenv("srcdir") : ".") +
-      "/src/processor/testdata/symbols/test_app.pdb/" +
-      module->debug_identifier() +
-      "/test_app.sym";
+      *symbol_file = string(getenv("srcdir") ? getenv("srcdir") : ".") +
+                     "/src/processor/testdata/symbols/test_app.pdb/" +
+                     module->debug_identifier() +
+                     "/test_app.sym";
+    return FOUND;
   }
 
-  return "";
+  return NOT_FOUND;
 }
 
 static bool RunTests() {
@@ -83,19 +98,20 @@ static bool RunTests() {
   string minidump_file = string(getenv("srcdir") ? getenv("srcdir") : ".") +
                          "/src/processor/testdata/minidump2.dmp";
 
-  scoped_ptr<ProcessState> state(processor.Process(minidump_file));
-  ASSERT_TRUE(state.get());
-  ASSERT_EQ(state->cpu(), "x86");
-  ASSERT_EQ(state->cpu_info(), "GenuineIntel family 6 model 13 stepping 8");
-  ASSERT_EQ(state->os(), "Windows NT");
-  ASSERT_EQ(state->os_version(), "5.1.2600 Service Pack 2");
-  ASSERT_TRUE(state->crashed());
-  ASSERT_EQ(state->crash_reason(), "EXCEPTION_ACCESS_VIOLATION");
-  ASSERT_EQ(state->crash_address(), 0x45);
-  ASSERT_EQ(state->threads()->size(), 1);
-  ASSERT_EQ(state->requesting_thread(), 0);
-
-  CallStack *stack = state->threads()->at(0);
+  ProcessState state;
+  ASSERT_EQ(processor.Process(minidump_file, &state),
+            MinidumpProcessor::PROCESS_OK);
+  ASSERT_EQ(state.cpu(), "x86");
+  ASSERT_EQ(state.cpu_info(), "GenuineIntel family 6 model 13 stepping 8");
+  ASSERT_EQ(state.os(), "Windows NT");
+  ASSERT_EQ(state.os_version(), "5.1.2600 Service Pack 2");
+  ASSERT_TRUE(state.crashed());
+  ASSERT_EQ(state.crash_reason(), "EXCEPTION_ACCESS_VIOLATION");
+  ASSERT_EQ(state.crash_address(), 0x45);
+  ASSERT_EQ(state.threads()->size(), 1);
+  ASSERT_EQ(state.requesting_thread(), 0);
+
+  CallStack *stack = state.threads()->at(0);
   ASSERT_TRUE(stack);
   ASSERT_EQ(stack->frames()->size(), 4);
 
@@ -131,17 +147,23 @@ static bool RunTests() {
   ASSERT_TRUE(stack->frames()->at(3)->source_file_name.empty());
   ASSERT_EQ(stack->frames()->at(3)->source_line, 0);
 
-  ASSERT_EQ(state->modules()->module_count(), 13);
-  ASSERT_TRUE(state->modules()->GetMainModule());
-  ASSERT_EQ(state->modules()->GetMainModule()->code_file(), "C:\\test_app.exe");
-  ASSERT_FALSE(state->modules()->GetModuleForAddress(0));
-  ASSERT_EQ(state->modules()->GetMainModule(),
-            state->modules()->GetModuleForAddress(0x400000));
-  ASSERT_EQ(state->modules()->GetModuleForAddress(0x7c801234)->debug_file(),
+  ASSERT_EQ(state.modules()->module_count(), 13);
+  ASSERT_TRUE(state.modules()->GetMainModule());
+  ASSERT_EQ(state.modules()->GetMainModule()->code_file(), "C:\\test_app.exe");
+  ASSERT_FALSE(state.modules()->GetModuleForAddress(0));
+  ASSERT_EQ(state.modules()->GetMainModule(),
+            state.modules()->GetModuleForAddress(0x400000));
+  ASSERT_EQ(state.modules()->GetModuleForAddress(0x7c801234)->debug_file(),
             "kernel32.pdb");
-  ASSERT_EQ(state->modules()->GetModuleForAddress(0x77d43210)->version(),
+  ASSERT_EQ(state.modules()->GetModuleForAddress(0x77d43210)->version(),
             "5.1.2600.2622");
 
+  // Test that the symbol supplier can interrupt processing
+  state.Clear();
+  supplier.set_interrupt(true);
+  ASSERT_EQ(processor.Process(minidump_file, &state),
+            MinidumpProcessor::PROCESS_INTERRUPTED);
+
   return true;
 }
 
diff --git a/src/processor/minidump_stackwalk.cc b/src/processor/minidump_stackwalk.cc
index 62bfd555..3c55933d 100644
--- a/src/processor/minidump_stackwalk.cc
+++ b/src/processor/minidump_stackwalk.cc
@@ -195,18 +195,18 @@ static bool PrintMinidumpProcess(const string &minidump_file,
   MinidumpProcessor minidump_processor(symbol_supplier.get());
 
   // Process the minidump.
-  scoped_ptr<ProcessState> process_state(
-      minidump_processor.Process(minidump_file));
-  if (!process_state.get()) {
+  ProcessState process_state;
+  if (minidump_processor.Process(minidump_file, &process_state) !=
+      MinidumpProcessor::PROCESS_OK) {
     fprintf(stderr, "MinidumpProcessor::Process failed\n");
     return false;
   }
 
   // Print OS and CPU information.
-  string cpu = process_state->cpu();
-  string cpu_info = process_state->cpu_info();
-  printf("Operating system: %s\n", process_state->os().c_str());
-  printf("                  %s\n", process_state->os_version().c_str());
+  string cpu = process_state.cpu();
+  string cpu_info = process_state.cpu_info();
+  printf("Operating system: %s\n", process_state.os().c_str());
+  printf("                  %s\n", process_state.os_version().c_str());
   printf("CPU: %s\n", cpu.c_str());
   if (!cpu_info.empty()) {
     // This field is optional.
@@ -215,36 +215,36 @@ static bool PrintMinidumpProcess(const string &minidump_file,
   printf("\n");
 
   // Print crash information.
-  if (process_state->crashed()) {
-    printf("Crash reason:  %s\n", process_state->crash_reason().c_str());
-    printf("Crash address: 0x%llx\n", process_state->crash_address());
+  if (process_state.crashed()) {
+    printf("Crash reason:  %s\n", process_state.crash_reason().c_str());
+    printf("Crash address: 0x%llx\n", process_state.crash_address());
   } else {
     printf("No crash\n");
   }
 
   // If the thread that requested the dump is known, print it first.
-  int requesting_thread = process_state->requesting_thread(); 
+  int requesting_thread = process_state.requesting_thread();
   if (requesting_thread != -1) {
     printf("\n");
     printf("Thread %d (%s)\n",
           requesting_thread,
-          process_state->crashed() ? "crashed" :
-                                     "requested dump, did not crash");
-    PrintStack(process_state->threads()->at(requesting_thread), cpu);
+          process_state.crashed() ? "crashed" :
+                                    "requested dump, did not crash");
+    PrintStack(process_state.threads()->at(requesting_thread), cpu);
   }
 
   // Print all of the threads in the dump.
-  int thread_count = process_state->threads()->size();
+  int thread_count = process_state.threads()->size();
   for (int thread_index = 0; thread_index < thread_count; ++thread_index) {
     if (thread_index != requesting_thread) {
       // Don't print the crash thread again, it was already printed.
       printf("\n");
       printf("Thread %d\n", thread_index);
-      PrintStack(process_state->threads()->at(thread_index), cpu);
+      PrintStack(process_state.threads()->at(thread_index), cpu);
     }
   }
 
-  PrintModules(process_state->modules());
+  PrintModules(process_state.modules());
 
   return true;
 }
diff --git a/src/processor/process_state.cc b/src/processor/process_state.cc
index 895ee5f7..d1e21cb8 100644
--- a/src/processor/process_state.cc
+++ b/src/processor/process_state.cc
@@ -40,13 +40,27 @@
 namespace google_airbag {
 
 ProcessState::~ProcessState() {
+  Clear();
+}
+
+void ProcessState::Clear() {
+  time_date_stamp_ = 0;
+  crashed_ = false;
+  crash_reason_.clear();
+  crash_address_ = 0;
+  requesting_thread_ = -1;
   for (vector<CallStack *>::const_iterator iterator = threads_.begin();
        iterator != threads_.end();
        ++iterator) {
     delete *iterator;
   }
-
+  threads_.clear();
+  os_.clear();
+  os_version_.clear();
+  cpu_.clear();
+  cpu_info_.clear();
   delete modules_;
+  modules_ = NULL;
 }
 
 }  // namespace google_airbag
diff --git a/src/processor/simple_symbol_supplier.cc b/src/processor/simple_symbol_supplier.cc
index 32b94342..e2bd9184 100644
--- a/src/processor/simple_symbol_supplier.cc
+++ b/src/processor/simple_symbol_supplier.cc
@@ -33,16 +33,19 @@
 //
 // Author: Mark Mentovai
 
+#include <cassert>
+
 #include "processor/simple_symbol_supplier.h"
 #include "google_airbag/processor/code_module.h"
 #include "processor/pathname_stripper.h"
 
 namespace google_airbag {
 
-string SimpleSymbolSupplier::GetSymbolFileAtPath(const CodeModule *module,
-                                                 const string &root_path) {
+SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetSymbolFileAtPath(
+    const CodeModule *module, const string &root_path, string *symbol_file) {
+  assert(symbol_file);
   if (!module)
-    return "";
+    return NOT_FOUND;
 
   // Start with the base path.
   string path = root_path;
@@ -51,14 +54,14 @@ string SimpleSymbolSupplier::GetSymbolFileAtPath(const CodeModule *module,
   path.append("/");
   string debug_file_name = PathnameStripper::File(module->debug_file());
   if (debug_file_name.empty())
-    return "";
+    return NOT_FOUND;
   path.append(debug_file_name);
 
   // Append the identifier as a directory name.
   path.append("/");
   string identifier = module->debug_identifier();
   if (identifier.empty())
-    return "";
+    return NOT_FOUND;
   path.append(identifier);
 
   // Transform the debug file name into one ending in .sym.  If the existing
@@ -76,7 +79,8 @@ string SimpleSymbolSupplier::GetSymbolFileAtPath(const CodeModule *module,
   }
   path.append(".sym");
 
-  return path;
+  *symbol_file = path;
+  return FOUND;
 }
 
 }  // namespace google_airbag
diff --git a/src/processor/simple_symbol_supplier.h b/src/processor/simple_symbol_supplier.h
index 30485d24..fdfcf0ca 100644
--- a/src/processor/simple_symbol_supplier.h
+++ b/src/processor/simple_symbol_supplier.h
@@ -92,13 +92,15 @@ class SimpleSymbolSupplier : public SymbolSupplier {
 
   // Returns the path to the symbol file for the given module.  See the
   // description above.
-  virtual string GetSymbolFile(const CodeModule *module) {
-    return GetSymbolFileAtPath(module, path_);
+  virtual SymbolResult GetSymbolFile(const CodeModule *module,
+                                     string *symbol_file) {
+    return GetSymbolFileAtPath(module, path_, symbol_file);
   }
 
  protected:
-  string GetSymbolFileAtPath(const CodeModule *module,
-                             const string &root_path);
+  SymbolResult GetSymbolFileAtPath(const CodeModule *module,
+                                   const string &root_path,
+                                   string *symbol_file);
 
  private:
   string path_;
diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc
index 4e4a6b9f..edbe428a 100644
--- a/src/processor/stackwalker.cc
+++ b/src/processor/stackwalker.cc
@@ -34,6 +34,8 @@
 // Author: Mark Mentovai
 
 
+#include <cassert>
+
 #include "google_airbag/processor/stackwalker.h"
 #include "google_airbag/processor/call_stack.h"
 #include "google_airbag/processor/code_module.h"
@@ -57,10 +59,10 @@ Stackwalker::Stackwalker(MemoryRegion *memory, const CodeModules *modules,
 }
 
 
-CallStack* Stackwalker::Walk() {
+bool Stackwalker::Walk(CallStack *stack) {
+  assert(stack);
   SourceLineResolver resolver;
-
-  scoped_ptr<CallStack> stack(new CallStack());
+  stack->Clear();
 
   // stack_frame_info parallels the CallStack.  The vector is passed to the
   // GetCallerFrame function.  It contains information that may be helpful
@@ -87,9 +89,18 @@ CallStack* Stackwalker::Walk() {
       if (module) {
         frame->module = module;
         if (!resolver.HasModule(frame->module->code_file()) && supplier_) {
-          string symbol_file = supplier_->GetSymbolFile(module);
-          if (!symbol_file.empty()) {
-            resolver.LoadModule(frame->module->code_file(), symbol_file);
+          string symbol_file;
+          SymbolSupplier::SymbolResult symbol_result =
+              supplier_->GetSymbolFile(module, &symbol_file);
+
+          switch (symbol_result) {
+            case SymbolSupplier::FOUND:
+              resolver.LoadModule(frame->module->code_file(), symbol_file);
+              break;
+            case SymbolSupplier::NOT_FOUND:
+              break;  // nothing to do
+            case SymbolSupplier::INTERRUPT:
+              return false;
           }
         }
         frame_info.reset(resolver.FillSourceLineInfo(frame.get()));
@@ -105,10 +116,10 @@ CallStack* Stackwalker::Walk() {
     frame_info.reset(NULL);
 
     // Get the next frame and take ownership.
-    frame.reset(GetCallerFrame(stack.get(), stack_frame_info));
+    frame.reset(GetCallerFrame(stack, stack_frame_info));
   }
 
-  return stack.release();
+  return true;
 }