Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(188)

Unified Diff: components/metrics/leak_detector/leak_detector.cc

Issue 1665553002: metrics: Connect leak detector to allocator (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: primiano's feedback; Register observer sooner Created 4 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/metrics/leak_detector/leak_detector.cc
diff --git a/components/metrics/leak_detector/leak_detector.cc b/components/metrics/leak_detector/leak_detector.cc
index 0fbd868ccfa2df810e08ea523bd7f82b093f07c6..0240b58bd96028de8439ab3856f3a47923c5a204 100644
--- a/components/metrics/leak_detector/leak_detector.cc
+++ b/components/metrics/leak_detector/leak_detector.cc
@@ -4,45 +4,276 @@
#include "components/metrics/leak_detector/leak_detector.h"
+#include <stdint.h>
+
+#include "base/allocator/allocator_extension.h"
+#include "base/bind.h"
+#include "base/lazy_instance.h"
+#include "base/logging.h"
+#include "base/numerics/safe_conversions.h"
+#include "base/threading/thread_local.h"
+#include "components/metrics/leak_detector/custom_allocator.h"
+#include "components/metrics/leak_detector/leak_detector_impl.h"
#include "content/public/browser/browser_thread.h"
+#if defined(OS_CHROMEOS)
+#include <link.h> // for dl_iterate_phdr
+#else
+#error "Getting binary mapping info is not supported on this platform."
+#endif // defined(OS_CHROMEOS)
+
namespace metrics {
+using LeakReport = LeakDetector::LeakReport;
+using InternalLeakReport = leak_detector::LeakDetectorImpl::LeakReport;
+template <typename T>
+using InternalVector = leak_detector::LeakDetectorImpl::InternalVector<T>;
+
+namespace {
+
+#if defined(OS_CHROMEOS)
+// For storing the address range of the Chrome binary in memory.
+struct MappingInfo {
+ uintptr_t addr;
+ size_t size;
+};
+
+// Callback for dl_iterate_phdr() to find the Chrome binary mapping.
+int IterateLoadedObjects(struct dl_phdr_info* shared_object,
+ size_t /* size */,
+ void* data) {
+ for (int i = 0; i < shared_object->dlpi_phnum; i++) {
+ // Find the ELF segment header that contains the actual code of the Chrome
+ // binary.
+ const ElfW(Phdr)& segment_header = shared_object->dlpi_phdr[i];
+ if (segment_header.p_type == SHT_PROGBITS && segment_header.p_offset == 0 &&
+ data) {
+ MappingInfo* mapping = reinterpret_cast<MappingInfo*>(data);
+
+ // Make sure the fields in the ELF header and MappingInfo have the
+ // same size.
+ static_assert(sizeof(mapping->addr) == sizeof(shared_object->dlpi_addr),
+ "Integer size mismatch between MappingInfo::addr and "
+ "dl_phdr_info::dlpi_addr.");
+ static_assert(sizeof(mapping->size) == sizeof(segment_header.p_offset),
+ "Integer size mismatch between MappingInfo::size and "
+ "ElfW(Phdr)::p_memsz.");
+
+ mapping->addr = shared_object->dlpi_addr + segment_header.p_offset;
+ mapping->size = segment_header.p_memsz;
+ return 1;
+ }
+ }
+ return 0;
+}
+#endif // defined(OS_CHROMEOS)
+
+// Convert a pointer to a hash value. Returns only the upper eight bits.
+inline uint64_t PointerToHash(const void* ptr) {
+ // The input data is the pointer address, not the location in memory pointed
+ // to by the pointer.
+ // The multiplier is taken from Farmhash code:
+ // https://github.com/google/farmhash/blob/master/src/farmhash.cc
+ const uint64_t kMultiplier = 0x9ddfea08eb382d69ULL;
+ return reinterpret_cast<uint64_t>(ptr) * kMultiplier;
+}
+
+// Converts a vector of leak reports generated by LeakDetectorImpl
+// (InternalLeakReport) to a vector of leak reports suitable for sending to
+// LeakDetector's observers (LeakReport).
+void GetReportsForObservers(
+ const InternalVector<InternalLeakReport>& leak_reports,
+ std::vector<LeakReport>* reports_for_observers) {
+ reports_for_observers->clear();
+ reports_for_observers->resize(leak_reports.size());
+ for (const InternalLeakReport& report : leak_reports) {
+ reports_for_observers->push_back(LeakReport());
+ LeakReport* new_report = &reports_for_observers->back();
+
+ new_report->alloc_size_bytes = report.alloc_size_bytes();
+ if (!report.call_stack().empty()) {
+ new_report->call_stack.resize(report.call_stack().size());
+ memcpy(new_report->call_stack.data(), report.call_stack().data(),
+ report.call_stack().size() * sizeof(report.call_stack()[0]));
+ }
+ }
+}
+
+// The only instance of LeakDetector that should be used.
+base::LazyInstance<LeakDetector>::Leaky g_instance = LAZY_INSTANCE_INITIALIZER;
+
+// Thread-specific flag indicating that one of the alloc hooks have already been
+// entered. Used to handle recursive hook calls. Anything allocated when this
+// flag is set should also be freed when this flag is set.
+base::LazyInstance<base::ThreadLocalBoolean>::Leaky g_entered_hook_tls;
+
+} // namespace
+
LeakDetector::LeakReport::LeakReport() {}
LeakDetector::LeakReport::~LeakReport() {}
-LeakDetector::LeakDetector(float sampling_rate,
- size_t max_call_stack_unwind_depth,
- uint64_t analysis_interval_bytes,
- uint32_t size_suspicion_threshold,
- uint32_t call_stack_suspicion_threshold)
- : weak_factory_(this) {
- // TODO(sque): Connect this class to LeakDetectorImpl and base::allocator.
+// static
+LeakDetector* LeakDetector::GetInstance() {
+ return g_instance.Pointer();
}
-LeakDetector::~LeakDetector() {}
+void LeakDetector::Init(float sampling_rate,
+ size_t max_call_stack_unwind_depth,
+ uint64_t analysis_interval_bytes,
+ uint32_t size_suspicion_threshold,
+ uint32_t call_stack_suspicion_threshold) {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ DCHECK(sampling_rate > 0) << "Sampling rate cannot be zero or negative.";
+
+ sampling_factor_ = base::saturated_cast<uint64_t>(sampling_rate * UINT64_MAX);
+
+ analysis_interval_bytes_ = analysis_interval_bytes;
+ max_call_stack_unwind_depth_ = max_call_stack_unwind_depth;
+
+ MappingInfo mapping = {0};
+#if defined(OS_CHROMEOS)
+ // Locate the Chrome binary mapping info.
+ dl_iterate_phdr(IterateLoadedObjects, &mapping);
+#endif // defined(OS_CHROMEOS)
+
+ // CustomAllocator can use the default allocator, as long as the hook
+ // functions can handle recursive calls.
+ leak_detector::CustomAllocator::Initialize();
+
+ // The initialization should be done only once. Check for this by examining
+ // whether |impl_| has already been initialized.
+ CHECK(!impl_.get()) << "Cannot initialize LeakDetector more than once!";
+ impl_.reset(new leak_detector::LeakDetectorImpl(
+ mapping.addr, mapping.size, size_suspicion_threshold,
+ call_stack_suspicion_threshold));
+
+ // Register allocator hook functions.
+ base::allocator::SetHooks(&AllocHook, &FreeHook);
+}
void LeakDetector::AddObserver(Observer* observer) {
- DCHECK(thread_checker_.CalledOnValidThread());
+ base::AutoLock lock(observers_lock_);
observers_.AddObserver(observer);
}
void LeakDetector::RemoveObserver(Observer* observer) {
- DCHECK(thread_checker_.CalledOnValidThread());
+ base::AutoLock lock(observers_lock_);
observers_.RemoveObserver(observer);
}
+LeakDetector::LeakDetector()
+ : total_alloc_size_(0),
+ last_analysis_alloc_size_(0),
+ analysis_interval_bytes_(0),
+ max_call_stack_unwind_depth_(0),
+ sampling_factor_(0) {}
+
+LeakDetector::~LeakDetector() {}
+
+// static
+void LeakDetector::AllocHook(const void* ptr, size_t size) {
+ base::ThreadLocalBoolean& entered_hook = g_entered_hook_tls.Get();
+ if (entered_hook.Get())
+ return;
+
+ LeakDetector* detector = GetInstance();
+
+ // Add the allocation size to the total size across all threads. This should
+ // be done before sampling because it is more accurate than attempting to
+ // extrapolate it from the post-sampling total size.
+ {
+ base::AutoLock lock(detector->recording_lock_);
+ detector->total_alloc_size_ += size;
+ }
+
+ if (!detector->ShouldSample(ptr))
+ return;
+
+ entered_hook.Set(true);
+
+ // Get stack trace if necessary.
+ std::vector<void*> stack;
+ int depth = 0;
+ if (detector->impl_->ShouldGetStackTraceForSize(size)) {
+ stack.resize(detector->max_call_stack_unwind_depth_);
+ depth = base::allocator::GetCallStack(stack.data(), stack.size());
+ }
+
+ {
+ base::AutoLock lock(detector->recording_lock_);
+ detector->impl_->RecordAlloc(ptr, size, depth, stack.data());
+
+ // Check for leaks after |analysis_interval_bytes_| bytes have been
+ // allocated since the last time that was done.
+ const auto& total_alloc_size = detector->total_alloc_size_;
+ const auto& analysis_interval_bytes = detector->analysis_interval_bytes_;
+ if (total_alloc_size >
+ detector->last_analysis_alloc_size_ + analysis_interval_bytes) {
+ // Try to maintain regular intervals of size |analysis_interval_bytes_|.
+ detector->last_analysis_alloc_size_ =
+ total_alloc_size - total_alloc_size % analysis_interval_bytes;
+
+ InternalVector<InternalLeakReport> leak_reports;
+ detector->impl_->TestForLeaks(&leak_reports);
+
+ // Pass leak reports to observers.
+ std::vector<LeakReport> leak_reports_for_observers;
+ GetReportsForObservers(leak_reports, &leak_reports_for_observers);
+ detector->NotifyObservers(leak_reports_for_observers);
+ }
+ }
+
+ {
+ // The internal memory of |stack| should be freed before setting
+ // |entered_hook| to false at the end of this function. Free it here by
+ // moving the internal memory to a temporary variable that will go out of
+ // scope.
+ std::vector<void*> dummy_stack;
+ dummy_stack.swap(stack);
+ }
+
+ entered_hook.Set(false);
+}
+
+// static
+void LeakDetector::FreeHook(const void* ptr) {
+ LeakDetector* detector = GetInstance();
+ if (!detector->ShouldSample(ptr))
+ return;
+
+ base::ThreadLocalBoolean& entered_hook = g_entered_hook_tls.Get();
+ if (entered_hook.Get())
+ return;
+
+ entered_hook.Set(true);
+
+ {
+ base::AutoLock lock(detector->recording_lock_);
+ detector->impl_->RecordFree(ptr);
+ }
+
+ entered_hook.Set(false);
+}
+
+inline bool LeakDetector::ShouldSample(const void* ptr) const {
+ return PointerToHash(ptr) < sampling_factor_;
+}
+
void LeakDetector::NotifyObservers(const std::vector<LeakReport>& reports) {
+ if (reports.empty())
+ return;
+
if (!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) {
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE,
- base::Bind(&LeakDetector::NotifyObservers, weak_factory_.GetWeakPtr(),
+ base::Bind(&LeakDetector::NotifyObservers, base::Unretained(this),
reports));
return;
}
for (const LeakReport& report : reports) {
+ base::AutoLock lock(observers_lock_);
FOR_EACH_OBSERVER(Observer, observers_, OnLeakFound(report));
}
}
« no previous file with comments | « components/metrics/leak_detector/leak_detector.h ('k') | components/metrics/leak_detector/leak_detector_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698