[go: up one dir, main page]

[heap profiler] Fix a crash on Android.

Make use of acquire memory order to obtain the hash set to ensure its
internal fields have properly initialized values.

BUG=968801
TBR=alph@chromium.org

(cherry picked from commit 4e1dda6dfc46ed4908d4a324acdc3411dfd599b8)

Change-Id: Id93aed0a76ec559a5fe92b4443a94367fafaf2ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1643894
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Alexei Filippov <alph@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#666039}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1656968
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#269}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/base/sampling_heap_profiler/poisson_allocation_sampler.cc b/base/sampling_heap_profiler/poisson_allocation_sampler.cc
index 16bb6fe..515cf6c 100644
--- a/base/sampling_heap_profiler/poisson_allocation_sampler.cc
+++ b/base/sampling_heap_profiler/poisson_allocation_sampler.cc
@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <atomic>
 #include <cmath>
+#include <memory>
 #include <utility>
 
 #include "base/allocator/allocator_shim.h"
@@ -319,9 +320,8 @@
   CHECK_EQ(nullptr, instance_);
   instance_ = this;
   Init();
-  auto sampled_addresses = std::make_unique<LockFreeAddressHashSet>(64);
-  g_sampled_addresses_set = sampled_addresses.get();
-  sampled_addresses_stack_.push_back(std::move(sampled_addresses));
+  auto* sampled_addresses = new LockFreeAddressHashSet(64);
+  g_sampled_addresses_set.store(sampled_addresses, std::memory_order_release);
 }
 
 // static
@@ -509,16 +509,15 @@
       std::make_unique<LockFreeAddressHashSet>(current_set.buckets_count() * 2);
   new_set->Copy(current_set);
   // Atomically switch all the new readers to the new set.
-  g_sampled_addresses_set = new_set.get();
-  // We still have to keep all the old maps alive to resolve the theoretical
-  // race with readers in |RecordFree| that have already obtained the map,
+  g_sampled_addresses_set.store(new_set.release(), std::memory_order_release);
+  // We leak the older set because we still have to keep all the old maps alive
+  // as there might be reader threads that have already obtained the map,
   // but haven't yet managed to access it.
-  sampled_addresses_stack_.push_back(std::move(new_set));
 }
 
 // static
 LockFreeAddressHashSet& PoissonAllocationSampler::sampled_addresses_set() {
-  return *g_sampled_addresses_set.load(std::memory_order_relaxed);
+  return *g_sampled_addresses_set.load(std::memory_order_acquire);
 }
 
 // static
diff --git a/base/sampling_heap_profiler/poisson_allocation_sampler.h b/base/sampling_heap_profiler/poisson_allocation_sampler.h
index 7709e02..f0751e4 100644
--- a/base/sampling_heap_profiler/poisson_allocation_sampler.h
+++ b/base/sampling_heap_profiler/poisson_allocation_sampler.h
@@ -5,7 +5,6 @@
 #ifndef BASE_SAMPLING_HEAP_PROFILER_POISSON_ALLOCATION_SAMPLER_H_
 #define BASE_SAMPLING_HEAP_PROFILER_POISSON_ALLOCATION_SAMPLER_H_
 
-#include <memory>
 #include <vector>
 
 #include "base/base_export.h"
@@ -110,7 +109,6 @@
   void BalanceAddressesHashSet();
 
   Lock mutex_;
-  std::vector<std::unique_ptr<LockFreeAddressHashSet>> sampled_addresses_stack_;
   std::vector<SamplesObserver*> observers_;
 
   static PoissonAllocationSampler* instance_;