Multi-Process Tracking Support

base/debug/activity_tracker.h

Index: base/debug/activity_tracker.h
diff --git a/base/debug/activity_tracker.h b/base/debug/activity_tracker.h
index 9ffcfaa844ba22ee9edee16dcade2481f5a4f011..1fb9662c8f42d96f97969815a13b92b3de09ad03 100644
--- a/base/debug/activity_tracker.h
+++ b/base/debug/activity_tracker.h
@@ -18,6 +18,7 @@
 #include <atomic>
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -27,8 +28,10 @@
 #include "base/gtest_prod_util.h"
 #include "base/location.h"
 #include "base/metrics/persistent_memory_allocator.h"
+#include "base/process/process_handle.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/task_runner.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread_checker.h"
 #include "base/threading/thread_local_storage.h"
@@ -41,7 +44,6 @@ class FilePath;
 class Lock;
 class PlatformThreadHandle;
 class Process;
-class StaticAtomicSequenceNumber;
 class WaitableEvent;
 
 namespace debug {
@@ -56,6 +58,37 @@ enum : int {
   kActivityCallStackSize = 10,
 };
 
+struct ProcessInfo {
+  ProcessInfo();
+  ~ProcessInfo();
+
+  // Initializes structure with the current process id and the current time.
+  // These can uniquely identify a process. A unique non-zero data_id will be
+  // set making it possible to tell using atomic reads if the data has changed.
+  void Release_Initialize();
+
+  // Explicitly sets the process ID.
+  void SetOwningProcessIdForTesting(ProcessId pid, int64_t stamp);
+
+  // Gets the associated process ID, in native form, and the creation timestamp
+  // from tracker memory without loading the entire structure for analysis. This
+  // will return false if no valid process ID is available.
+  static bool OwningProcessId(const void* memory,
+                              ProcessId* out_id,
+                              int64_t* out_stamp);
+
+  // SHA1(base::debug::ProcessInfo): Increment this if structure changes!
+  static constexpr uint32_t kPersistentTypeId = 0xD485C718 + 1;
+
+  // Expected size for 32/64-bit check by PersistentMemoryAllocator.
+  static constexpr size_t kExpectedInstanceSize = 24;
+
+  std::atomic<uint32_t> data_id;
+  uint32_t padding;
+  int64_t process_id;
+  int64_t create_stamp;
+};
+
 // The data associated with an activity is dependent upon the activity type.
 // This union defines all of the various fields. All fields must be explicitly
 // sized types to ensure no interoperability problems between 32-bit and
@@ -349,7 +382,9 @@ class BASE_EXPORT ActivityUserData {
   // contents have been overwritten by another thread. The return value is
   // always non-zero unless it's actually just a data "sink".
   uint32_t id() const {
-    return memory_ ? id_->load(std::memory_order_relaxed) : 0;
+    return header_
+               ? header_->process_info.data_id.load(std::memory_order_relaxed)
+               : 0;
   }
 
   // Writes a |value| (as part of a key/value pair) that will be included with
@@ -405,6 +440,16 @@ class BASE_EXPORT ActivityUserData {
   // Gets the base memory address used for storing data.
   const void* GetBaseAddress();
 
+  // Explicitly sets the process ID.
+  void SetOwningProcessIdForTesting(ProcessId pid, int64_t stamp);
+
+  // Gets the associated process ID, in native form, and the creation timestamp
+  // from tracker memory without loading the entire structure for analysis. This
+  // will return false if no valid process ID is available.
+  static bool OwningProcessId(const void* memory,
+                              ProcessId* out_id,
+                              int64_t* out_stamp);
+
  protected:
   virtual void Set(StringPiece name,
                    ValueType type,
@@ -422,8 +467,19 @@ class BASE_EXPORT ActivityUserData {
     uint64_t size;
   };
 
+  // A structure that defines the structure header in memory.
+  struct MemoryHeader {
+    MemoryHeader();
+    ~MemoryHeader();
+
+    ProcessInfo process_info;  // Information about the creating process.
+  };
+
   // Header to a key/value record held in persistent memory.
-  struct Header {
+  struct FieldHeader {
+    FieldHeader();
+    ~FieldHeader();
+
     std::atomic<uint8_t> type;         // Encoded ValueType
     uint8_t name_size;                 // Length of "name" key.
     std::atomic<uint16_t> value_size;  // Actual size of of the stored value.
@@ -464,12 +520,8 @@ class BASE_EXPORT ActivityUserData {
   mutable char* memory_;
   mutable size_t available_;
 
-  // A pointer to the unique ID for this instance.
-  std::atomic<uint32_t>* const id_;
-
-  // This ID is used to create unique indentifiers for user data so that it's
-  // possible to tell if the information has been overwritten.
-  static StaticAtomicSequenceNumber next_id_;
+  // A pointer to the memory header for this instance.
+  MemoryHeader* const header_;
 
   DISALLOW_COPY_AND_ASSIGN(ActivityUserData);
 };
@@ -612,6 +664,19 @@ class BASE_EXPORT ThreadActivityTracker {
   // implementation does not support concurrent snapshot operations.
   bool CreateSnapshot(Snapshot* output_snapshot) const;
 
+  // Gets the base memory address used for storing data.
+  const void* GetBaseAddress();
+
+  // Explicitly sets the process ID.
+  void SetOwningProcessIdForTesting(ProcessId pid, int64_t stamp);
+
+  // Gets the associated process ID, in native form, and the creation timestamp
+  // from tracker memory without loading the entire structure for analysis. This
+  // will return false if no valid process ID is available.
+  static bool OwningProcessId(const void* memory,
+                              ProcessId* out_id,
+                              int64_t* out_stamp);
+
   // Calculates the memory size required for a given stack depth, including
   // the internal header structure for the stack.
   static size_t SizeForStackDepth(int stack_depth);
@@ -643,13 +708,15 @@ class BASE_EXPORT GlobalActivityTracker {
   // will be safely ignored. These are public so that an external process
   // can recognize records of this type within an allocator.
   enum : uint32_t {
-    kTypeIdActivityTracker = 0x5D7381AF + 3,   // SHA1(ActivityTracker) v3
-    kTypeIdUserDataRecord = 0x615EDDD7 + 2,    // SHA1(UserDataRecord) v2
+    kTypeIdActivityTracker = 0x5D7381AF + 4,   // SHA1(ActivityTracker) v4
+    kTypeIdUserDataRecord = 0x615EDDD7 + 3,    // SHA1(UserDataRecord) v3
     kTypeIdGlobalLogMessage = 0x4CF434F9 + 1,  // SHA1(GlobalLogMessage) v1
-    kTypeIdGlobalDataRecord = kTypeIdUserDataRecord + 1000,
+    kTypeIdProcessDataRecord = kTypeIdUserDataRecord + 0x100,
+    kTypeIdGlobalDataRecord = kTypeIdUserDataRecord + 0x200,
 
     kTypeIdActivityTrackerFree = ~kTypeIdActivityTracker,
     kTypeIdUserDataRecordFree = ~kTypeIdUserDataRecord,
+    kTypeIdProcessDataRecordFree = ~kTypeIdProcessDataRecord,
   };
 
   // This structure contains information about a loaded module, as shown to
@@ -780,6 +847,23 @@ class BASE_EXPORT GlobalActivityTracker {
   // Releases the activity-tracker for the current thread (for testing only).
   void ReleaseTrackerForCurrentThreadForTesting();
 
+  // Sets a task-runner that can be used for background work.
+  void SetBackgroundTaskRunner(const scoped_refptr<TaskRunner>& runner);
+
+  // Manages process lifetimes.
+  void RecordProcessLaunch(ProcessId process_id);
+  void RecordProcessExit(ProcessId process_id, int exit_code);
+  static void RecordProcessLaunchIfEnabled(ProcessId process_id) {
+    GlobalActivityTracker* tracker = Get();
+    if (tracker)
+      tracker->RecordProcessLaunch(process_id);
+  }
+  static void RecordProcessExitIfEnabled(ProcessId process_id, int exit_code) {
+    GlobalActivityTracker* tracker = Get();
+    if (tracker)
+      tracker->RecordProcessExit(process_id, exit_code);
+  }
+
   // Records a log message. The current implementation does NOT recycle these
   // only store critical messages such as FATAL ones.
   void RecordLogMessage(StringPiece message);
@@ -793,7 +877,12 @@ class BASE_EXPORT GlobalActivityTracker {
   // active field trials to be fetched and recorded.
   void RecordFieldTrial(const std::string& trial_name, StringPiece group_name);
 
+  // Accesses the process data record for storing arbitrary key/value pairs.
+  // Updates to this are thread-safe.
+  ActivityUserData& process_data() { return process_data_; }
+
   // Accesses the global data record for storing arbitrary key/value pairs.
+  // Updates to this are thread-safe.
   ActivityUserData& global_data() { return global_data_; }
 
  private:
@@ -812,10 +901,10 @@ class BASE_EXPORT GlobalActivityTracker {
   // A wrapper around ActivityUserData that is thread-safe and thus can be used
   // in the global scope without the requirement of being called from only one
   // thread.
-  class GlobalUserData : public ActivityUserData {
+  class ThreadSafeUserData : public ActivityUserData {
    public:
-    GlobalUserData(void* memory, size_t size);
-    ~GlobalUserData() override;
+    ThreadSafeUserData(void* memory, size_t size);
+    ~ThreadSafeUserData() override;
 
    private:
     void Set(StringPiece name,
@@ -825,7 +914,7 @@ class BASE_EXPORT GlobalActivityTracker {
 
     Lock data_lock_;
 
-    DISALLOW_COPY_AND_ASSIGN(GlobalUserData);
+    DISALLOW_COPY_AND_ASSIGN(ThreadSafeUserData);
   };
 
   // State of a module as stored in persistent memory. This supports a single
@@ -837,7 +926,8 @@ class BASE_EXPORT GlobalActivityTracker {
     static constexpr uint32_t kPersistentTypeId = 0x05DB5F41 + 1;
 
     // Expected size for 32/64-bit check by PersistentMemoryAllocator.
-    static constexpr size_t kExpectedInstanceSize = 56;
+    static constexpr size_t kExpectedInstanceSize =
+        ProcessInfo::kExpectedInstanceSize + 56;
 
     // The atomic unfortunately makes this a "complex" class on some compilers
     // and thus requires an out-of-line constructor & destructor even though
@@ -845,6 +935,7 @@ class BASE_EXPORT GlobalActivityTracker {
     ModuleInfoRecord();
     ~ModuleInfoRecord();
 
+    ProcessInfo process_info;       // The process that created this record.
     uint64_t address;               // The base address of the module.
     uint64_t load_time;             // Time of last load/unload.
     uint64_t size;                  // The size of the module in bytes.
@@ -908,6 +999,9 @@ class BASE_EXPORT GlobalActivityTracker {
   // be tracked. |value| is a pointer to a ManagedActivityTracker.
   static void OnTLSDestroy(void* value);
 
+  // Does process-exit work. This can be run on any thread.
+  void CleanupAfterProcess(ProcessId process_id, int64_t exit_stamp);

[manzagop (departed), 2017/02/22 20:44:15]

I'm still not sure how we call CleanupAfterProcess if the child crashes. Eg, for
renderers, do we need to add a call in RenderProcessHost::ProcessDied or the
like?

[bcwhite, 2017/02/22 22:13:02]

Shouldn't.  The base Process code deals with the cleanup of child processes and
it updates the tracker.

+
   // The persistent-memory allocator from which the memory for all trackers
   // is taken.
   std::unique_ptr<PersistentMemoryAllocator> allocator_;
@@ -930,9 +1024,9 @@ class BASE_EXPORT GlobalActivityTracker {
   ActivityTrackerMemoryAllocator user_data_allocator_;
   base::Lock user_data_allocator_lock_;
 
-  // An object for holding global arbitrary key value pairs. Values must always
-  // be written from the main UI thread.
-  GlobalUserData global_data_;
+  // An object for holding arbitrary key value pairs with thread-safe access.
+  ThreadSafeUserData process_data_;
+  ThreadSafeUserData global_data_;
 
   // A map of global module information, keyed by module path.
   std::map<const std::string, ModuleInfoRecord*> modules_;
@@ -941,6 +1035,15 @@ class BASE_EXPORT GlobalActivityTracker {
   // The active global activity tracker.
   static subtle::AtomicWord g_tracker_;
 
+  // A lock that is used to procect access to the following fields.
+  base::Lock global_tracker_lock_;
+
+  // The collection of processes being tracked.

[manzagop (departed), 2017/02/22 20:44:15]

This is not persistent information. Should it be?

The reason would be to avoid the time window after we've posted
CleanupAfterProcess but before the task was processed. If we crash during that
time window, we'll think the other process is still running.

[bcwhite, 2017/02/22 22:13:02]

That race would continue to exist because the process could exit but the browser
might crash before updating the list.

In any case it is possible to find a process that has "exited" among the crumbs
but that'll be recognizable during analysis because that process will have a
"phase" value indicating where it was at the time.

Perhaps more problematic is a browser crash where the subprocesses continue to
operate.  The crumbs would hold state information from after the browser death,
perhaps including a clean exit (though no clean-up).

+  std::set<int64_t> known_processes_;
+
+  // A task-runner that can be used for doing background processing.
+  scoped_refptr<TaskRunner> background_task_runner_;
+
   DISALLOW_COPY_AND_ASSIGN(GlobalActivityTracker);
 };