Chromium Code Reviews| Index: base/debug/activity_tracker.cc |
| diff --git a/base/debug/activity_tracker.cc b/base/debug/activity_tracker.cc |
| index 444dc7067eb5a47a82cf7f353317ed6ecbd4b29a..14d8eee5483f23df1fed3c7e44e651d538434332 100644 |
| --- a/base/debug/activity_tracker.cc |
| +++ b/base/debug/activity_tracker.cc |
| @@ -320,7 +320,10 @@ ActivityUserData::FieldHeader::~FieldHeader() {} |
| ActivityUserData::ActivityUserData(void* memory, size_t size) |
| : memory_(reinterpret_cast<char*>(memory)), |
| available_(RoundDownToAlignment(size, kMemoryAlignment)), |
| - header_(reinterpret_cast<MemoryHeader*>(memory)) { |
| + header_(reinterpret_cast<MemoryHeader*>(memory)), |
| + orig_data_id(0), |
| + orig_process_id(0), |
| + orig_create_stamp(0) { |
| // It's possible that no user data is being stored. |
| if (!memory_) |
| return; |
| @@ -332,6 +335,12 @@ ActivityUserData::ActivityUserData(void* memory, size_t size) |
| memory_ += sizeof(MemoryHeader); |
| available_ -= sizeof(MemoryHeader); |
| + // Make a copy of identifying information for later comparison. |
| + *const_cast<uint32_t*>(&orig_data_id) = |
| + header_->owner.data_id.load(std::memory_order_acquire); |
| + *const_cast<int64_t*>(&orig_process_id) = header_->owner.process_id; |
| + *const_cast<int64_t*>(&orig_create_stamp) = header_->owner.create_stamp; |
| + |
| // If there is already data present, load that. This allows the same class |
| // to be used for analysis through snapshots. |
| ImportExistingData(); |
| @@ -347,18 +356,18 @@ bool ActivityUserData::CreateSnapshot(Snapshot* output_snapshot) const { |
| // class that is adding records. |
| ImportExistingData(); |
| + // Add all the values to the snapshot. |
| for (const auto& entry : values_) { |
| TypedValue value; |
| + const size_t size = entry.second.size_ptr->load(std::memory_order_acquire); |
| value.type_ = entry.second.type; |
| - DCHECK_GE(entry.second.extent, |
| - entry.second.size_ptr->load(std::memory_order_relaxed)); |
| + DCHECK_GE(entry.second.extent, size); |
| switch (entry.second.type) { |
| case RAW_VALUE: |
| case STRING_VALUE: |
| value.long_value_ = |
| - std::string(reinterpret_cast<char*>(entry.second.memory), |
| - entry.second.size_ptr->load(std::memory_order_relaxed)); |
| + std::string(reinterpret_cast<char*>(entry.second.memory), size); |
| break; |
| case RAW_VALUE_REFERENCE: |
| case STRING_VALUE_REFERENCE: { |
| @@ -384,6 +393,16 @@ bool ActivityUserData::CreateSnapshot(Snapshot* output_snapshot) const { |
| DCHECK(inserted.second); // True if inserted, false if existed. |
| } |
| + // Another import attempt will validate that the underlying memory has not |
| + // been reused for another purpose. Entries added since the first import |
| + // will be ignored here but will be returned if another snapshot is created. |
| + ImportExistingData(); |
| + if (!memory_) { |
| + output_snapshot->clear(); |
| + return false; |
| + } |
| + |
| + // Successful snapshot. |
| return true; |
| } |
| @@ -416,6 +435,11 @@ void ActivityUserData::Set(StringPiece name, |
| size = std::min(std::numeric_limits<uint16_t>::max() - (kMemoryAlignment - 1), |
| size); |
| + // Find any new data that may have been added by an active instance of this |
|
manzagop (departed)
2017/03/21 21:10:05
Can you say more about what/where this other insta
bcwhite
2017/03/29 22:00:51
You're right. I was once thinking they could oper
|
| + // class that is adding records. This comes before the memory_ test below |
| + // because this call can clear the memory_ pointer if it finds a problem. |
| + ImportExistingData(); |
| + |
| // It's possible that no user data is being stored. |
| if (!memory_) |
| return; |
| @@ -517,6 +541,10 @@ void ActivityUserData::SetReference(StringPiece name, |
| } |
| void ActivityUserData::ImportExistingData() const { |
| + // It's possible that no user data is being stored. |
| + if (!memory_) |
| + return; |
| + |
| while (available_ > sizeof(FieldHeader)) { |
| FieldHeader* header = reinterpret_cast<FieldHeader*>(memory_); |
| ValueType type = |
| @@ -548,6 +576,14 @@ void ActivityUserData::ImportExistingData() const { |
| memory_ += header->record_size; |
| available_ -= header->record_size; |
| } |
| + |
| + // Check if memory has been completely reused. |
| + if (header_->owner.data_id.load(std::memory_order_acquire) != orig_data_id || |
| + header_->owner.process_id != orig_process_id || |
| + header_->owner.create_stamp != orig_create_stamp) { |
| + memory_ = nullptr; |
|
manzagop (departed)
2017/03/21 21:10:05
This behavior should be mentioned in the .h.
bcwhite
2017/03/29 22:00:51
Done.
|
| + values_.clear(); |
| + } |
| } |
| // This information is kept for every thread that is tracked. It is filled |
| @@ -878,6 +914,7 @@ bool ThreadActivityTracker::CreateSnapshot(Snapshot* output_snapshot) const { |
| // structure are valid (at least at the current moment in time). |
| const uint32_t starting_id = |
| header_->owner.data_id.load(std::memory_order_acquire); |
| + const int64_t starting_create_stamp = header_->owner.create_stamp; |
| const int64_t starting_process_id = header_->owner.process_id; |
| const int64_t starting_thread_id = header_->thread_ref.as_id; |
| @@ -911,6 +948,7 @@ bool ThreadActivityTracker::CreateSnapshot(Snapshot* output_snapshot) const { |
| // Get the general thread information. |
| output_snapshot->thread_name = |
| std::string(header_->thread_name, sizeof(header_->thread_name) - 1); |
| + output_snapshot->create_stamp = header_->owner.create_stamp; |
| output_snapshot->thread_id = header_->thread_ref.as_id; |
| output_snapshot->process_id = header_->owner.process_id; |
| @@ -923,6 +961,7 @@ bool ThreadActivityTracker::CreateSnapshot(Snapshot* output_snapshot) const { |
| // If the data ID has changed then the tracker has exited and the memory |
| // reused by a new one. Try again. |
| if (header_->owner.data_id.load(std::memory_order_seq_cst) != starting_id || |
| + output_snapshot->create_stamp != starting_create_stamp || |
| output_snapshot->process_id != starting_process_id || |
| output_snapshot->thread_id != starting_thread_id) { |
| continue; |