Enable storing last-dispatched exception per-thread.

base/debug/activity_tracker.cc

Index: base/debug/activity_tracker.cc
diff --git a/base/debug/activity_tracker.cc b/base/debug/activity_tracker.cc
index 59a66dfd097571051d89a6e3b8cc9ff583cc7ed7..c3f38017876e42724782ea4ccea4bc4253904d83 100644
--- a/base/debug/activity_tracker.cc
+++ b/base/debug/activity_tracker.cc
@@ -13,6 +13,7 @@
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/files/memory_mapped_file.h"
+#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
@@ -52,6 +53,9 @@ const char kProcessPhaseDataKey[] = "process-phase";
 // in the same memory space.
 StaticAtomicSequenceNumber g_next_id;
 
+// An reusable user-data sink that just discards all data saved to it.
+LazyInstance<ActivityUserData>::Leaky g_data_sink;
+
 union ThreadRef {
   int64_t as_id;
 #if defined(OS_WIN)
@@ -316,6 +320,8 @@ ActivityUserData::MemoryHeader::~MemoryHeader() {}
 ActivityUserData::FieldHeader::FieldHeader() {}
 ActivityUserData::FieldHeader::~FieldHeader() {}
 
+ActivityUserData::ActivityUserData() : ActivityUserData(nullptr, 0) {}
+
 ActivityUserData::ActivityUserData(void* memory, size_t size)
     : memory_(reinterpret_cast<char*>(memory)),
       available_(RoundDownToAlignment(size, kMemoryAlignment)),
@@ -557,7 +563,8 @@ struct ThreadActivityTracker::Header {
 
   // Expected size for 32/64-bit check.
   static constexpr size_t kExpectedInstanceSize =
-      OwningProcess::kExpectedInstanceSize + 72;
+      OwningProcess::kExpectedInstanceSize + Activity::kExpectedInstanceSize +
+      72;
 
   // This information uniquely identifies a process.
   OwningProcess owner;
@@ -593,7 +600,11 @@ struct ThreadActivityTracker::Header {
   // the contents didn't change while being copied. This can handle concurrent
   // snapshot operations only if each snapshot writes a different bit (which
   // is not the current implementation so no parallel snapshots allowed).
-  std::atomic<uint32_t> stack_unchanged;
+  std::atomic<uint32_t> data_unchanged;
+
+  // The last "exception" activity. This can't be stored on the stack because
+  // that could get popped as things unwind.
+  Activity last_exception;
 
   // The name of the thread (up to a maximum length). Dynamic-length names
   // are not practical since the memory has to come from the same persistent
@@ -671,7 +682,7 @@ ThreadActivityTracker::ThreadActivityTracker(void* base, size_t size)
     DCHECK_EQ(0, header_->start_ticks);
     DCHECK_EQ(0U, header_->stack_slots);
     DCHECK_EQ(0U, header_->current_depth.load(std::memory_order_relaxed));
-    DCHECK_EQ(0U, header_->stack_unchanged.load(std::memory_order_relaxed));
+    DCHECK_EQ(0U, header_->data_unchanged.load(std::memory_order_relaxed));
     DCHECK_EQ(0, stack_[0].time_internal);
     DCHECK_EQ(0U, stack_[0].origin_address);
     DCHECK_EQ(0U, stack_[0].call_stack[0]);
@@ -785,40 +796,28 @@ void ThreadActivityTracker::PopActivity(ActivityId id) {
 
   // The stack has shrunk meaning that some other thread trying to copy the
   // contents for reporting purposes could get bad data. That thread would
-  // have written a non-zero value into |stack_unchanged|; clearing it here
+  // have written a non-zero value into |data_unchanged|; clearing it here
   // will let that thread detect that something did change. This needs to
   // happen after the atomic |depth| operation above so a "release" store
   // is required.
-  header_->stack_unchanged.store(0, std::memory_order_release);
+  header_->data_unchanged.store(0, std::memory_order_release);
 }
 
 std::unique_ptr<ActivityUserData> ThreadActivityTracker::GetUserData(
     ActivityId id,
     ActivityTrackerMemoryAllocator* allocator) {
-  // User-data is only stored for activities actually held in the stack.
-  if (id < stack_slots_) {
-    // Don't allow user data for lock acquisition as recursion may occur.
-    if (stack_[id].activity_type == Activity::ACT_LOCK_ACQUIRE) {
-      NOTREACHED();
-      return MakeUnique<ActivityUserData>(nullptr, 0);
-    }
-
-    // Get (or reuse) a block of memory and create a real UserData object
-    // on it.
-    PersistentMemoryAllocator::Reference ref = allocator->GetObjectReference();
-    void* memory =
-        allocator->GetAsArray<char>(ref, PersistentMemoryAllocator::kSizeAny);
-    if (memory) {
-      std::unique_ptr<ActivityUserData> user_data =
-          MakeUnique<ActivityUserData>(memory, kUserDataSize);
-      stack_[id].user_data_ref = ref;
-      stack_[id].user_data_id = user_data->id();
-      return user_data;
-    }
+  // Don't allow user data for lock acquisition as recursion may occur.
+  if (stack_[id].activity_type == Activity::ACT_LOCK_ACQUIRE) {
+    NOTREACHED();
+    return MakeUnique<ActivityUserData>();
   }
 
-  // Return a dummy object that will still accept (but ignore) Set() calls.
-  return MakeUnique<ActivityUserData>(nullptr, 0);
+  // User-data is only stored for activities actually held in the stack.
+  if (id >= stack_slots_)
+    return MakeUnique<ActivityUserData>();
+
+  // Create and return a real UserData object.
+  return CreateUserDataForActivity(&stack_[id], allocator);
 }
 
 bool ThreadActivityTracker::HasUserData(ActivityId id) {
@@ -834,6 +833,43 @@ void ThreadActivityTracker::ReleaseUserData(
     allocator->ReleaseObjectReference(stack_[id].user_data_ref);
     stack_[id].user_data_ref = 0;
   }
+  // Release user-data for an exception.
+  if (header_->last_exception.user_data_ref) {
+    exception_data.reset();
+    allocator->ReleaseObjectReference(header_->last_exception.user_data_ref);
+    header_->last_exception.user_data_ref = 0;
+  }
+}
+
+ActivityUserData& ThreadActivityTracker::ExceptionActivity(
+    const void* program_counter,
+    const void* origin,

[Sigurður Ásgeirsson, 2017/02/24 18:37:41]

is this the exception address? If so, maybe name it that.
If not, we'd want to record the exception address somewhere, somehow.
Also we want to explicitly record the exception code.

[bcwhite, 2017/02/24 19:10:58]

Yes, it's for holding the address where the exception occurred.

I've got 4+64 bits of information I can save in the activity.  Anything beyond
that has to be "user data" in key/value pairs.

+    Activity::Type type,
+    const ActivityData& data,
+    ActivityTrackerMemoryAllocator* allocator) {

[Sigurður Ásgeirsson, 2017/02/24 18:37:41]

Note that here you will be executing in the context of an exception, which may -
as a case in point - occur in your own code, leading to reentrancy. EXTREME
caution is warranted here, as if you goof, most likely there will be no record
of this happening anywhere, as you've blown our chance to get a crash report
from the field.

[bcwhite, 2017/02/24 19:10:58]

There are two points that need hardening:

1) Creation of the thread-tracker object needs a lock but getting an existing
one does not.  I'll just get on the assumption that the tracker already exists. 
This will leave us blind during thread start-up, however.

2) The user-data object.  That also requires a lock to create.  The alternative
is to create one in advance, per-thread, and then return it when needed.  That
seems bad.  Perhaps I could keep one on reserve globally, extract it atomically
if needed, and refill it later on during normal program operation.  Two
exceptions in a row could result in no user-data for the second, but that's
probably the lesser concern.

[manzagop (departed), 2017/02/27 16:05:15]

IIRC the locks are to enable memory reuse. Another option is to just "leak"
(give up on reuse) the allocator memory used in those cases. 

That said, it would be a problem if threads keep spinning up and hitting
recoverable exceptions.

[bcwhite, 2017/03/14 12:53:25]

Correct on all counts.  Better to avoid the leaks if possible.

+  // A thread-checker creates a lock to check the thread-id which means
+  // re-entry into this code if lock acquisitions are being tracked.
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // If there was a previous exception, release its user-data.
+  if (header_->last_exception.user_data_ref) {
+    allocator->ReleaseObjectReference(header_->last_exception.user_data_ref);
+    header_->last_exception.user_data_ref = 0;
+  }
+
+  // Fill the reusable exception activity.
+  Activity::FillFrom(&header_->last_exception, program_counter, origin, type,
+                     data);
+
+  // Create a new user-data object for holding additional information.
+  exception_data =
+      CreateUserDataForActivity(&header_->last_exception, allocator);
+
+  // The data has changed meaning that some other thread trying to copy the
+  // contents for reporting purposes could get bad data.
+  header_->data_unchanged.store(0, std::memory_order_relaxed);
+
+  return *exception_data;
 }
 
 bool ThreadActivityTracker::IsValid() const {
@@ -878,12 +914,12 @@ bool ThreadActivityTracker::CreateSnapshot(Snapshot* output_snapshot) const {
     const int64_t starting_process_id = header_->owner.process_id;
     const int64_t starting_thread_id = header_->thread_ref.as_id;
 
-    // Write a non-zero value to |stack_unchanged| so it's possible to detect
+    // Write a non-zero value to |data_unchanged| so it's possible to detect
     // at the end that nothing has changed since copying the data began. A
     // "cst" operation is required to ensure it occurs before everything else.
     // Using "cst" memory ordering is relatively expensive but this is only
     // done during analysis so doesn't directly affect the worker threads.
-    header_->stack_unchanged.store(1, std::memory_order_seq_cst);
+    header_->data_unchanged.store(1, std::memory_order_seq_cst);
 
     // Fetching the current depth also "acquires" the contents of the stack.
     depth = header_->current_depth.load(std::memory_order_acquire);
@@ -895,16 +931,20 @@ bool ThreadActivityTracker::CreateSnapshot(Snapshot* output_snapshot) const {
              count * sizeof(Activity));
     }
 
+    // Capture the last exception.
+    memcpy(&output_snapshot->last_exception, &header_->last_exception,
+           sizeof(Activity));
+
+    // TODO(bcwhite): Snapshot other things here.
+
     // Retry if something changed during the copy. A "cst" operation ensures
     // it must happen after all the above operations.
-    if (!header_->stack_unchanged.load(std::memory_order_seq_cst))
+    if (!header_->data_unchanged.load(std::memory_order_seq_cst))
       continue;
 
     // Stack copied. Record it's full depth.
     output_snapshot->activity_stack_depth = depth;
 
-    // TODO(bcwhite): Snapshot other things here.
-
     // Get the general thread information.
     output_snapshot->thread_name =
         std::string(header_->thread_name, sizeof(header_->thread_name) - 1);
@@ -940,6 +980,11 @@ bool ThreadActivityTracker::CreateSnapshot(Snapshot* output_snapshot) const {
            TimeDelta::FromInternalValue(activity.time_internal - start_ticks))
               .ToInternalValue();
     }
+    output_snapshot->last_exception.time_internal =
+        (start_time +
+         TimeDelta::FromInternalValue(
+             output_snapshot->last_exception.time_internal - start_ticks))
+            .ToInternalValue();
 
     // Success!
     return true;
@@ -971,6 +1016,26 @@ size_t ThreadActivityTracker::SizeForStackDepth(int stack_depth) {
   return static_cast<size_t>(stack_depth) * sizeof(Activity) + sizeof(Header);
 }
 
+std::unique_ptr<ActivityUserData>
+ThreadActivityTracker::CreateUserDataForActivity(
+    Activity* activity,
+    ActivityTrackerMemoryAllocator* allocator) {
+  DCHECK_EQ(0U, activity->user_data_ref);
+
+  PersistentMemoryAllocator::Reference ref = allocator->GetObjectReference();
+  void* memory = allocator->GetAsArray<char>(ref, kUserDataSize);
+  if (memory) {
+    std::unique_ptr<ActivityUserData> user_data =
+        MakeUnique<ActivityUserData>(memory, kUserDataSize);
+    activity->user_data_ref = ref;
+    activity->user_data_id = user_data->id();
+    return user_data;
+  }
+
+  // Return a dummy object that will still accept (but ignore) Set() calls.
+  return MakeUnique<ActivityUserData>();
+}
+
 // The instantiation of the GlobalActivityTracker object.
 // The object held here will obviously not be destructed at process exit
 // but that's best since PersistentMemoryAllocator objects (that underlie
@@ -1124,7 +1189,7 @@ ActivityUserData& GlobalActivityTracker::ScopedThreadActivity::user_data() {
       user_data_ =
           tracker_->GetUserData(activity_id_, &global->user_data_allocator_);
     } else {
-      user_data_ = MakeUnique<ActivityUserData>(nullptr, 0);
+      user_data_ = MakeUnique<ActivityUserData>();
     }
   }
   return *user_data_;
@@ -1556,6 +1621,19 @@ void GlobalActivityTracker::ReturnTrackerMemory(
   thread_tracker_allocator_.ReleaseObjectReference(mem_reference);
 }
 
+ActivityUserData& GlobalActivityTracker::RecordExceptionImpl(
+    const void* pc,
+    const void* origin) {
+  ThreadActivityTracker* tracker = GetOrCreateTrackerForCurrentThread();
+  if (!tracker)
+    return g_data_sink.Get();
+
+  AutoLock lock(user_data_allocator_lock_);
+  return tracker->ExceptionActivity(pc, origin, Activity::ACT_EXCEPTION,
+                                    ActivityData::ForGeneric(0, 0),
+                                    &user_data_allocator_);
+}
+
 // static
 void GlobalActivityTracker::OnTLSDestroy(void* value) {
   delete reinterpret_cast<ManagedActivityTracker*>(value);