Use GC critical section for profile saver

Since visit classes uses DecodeWeakGlobal, it may block on decoding
weak globals. This can cause deadlocks since it will be blocked while
holding the classlinker classes lock.

Using a GC critical section ensures that the GC is not running while
we visit the classes for the profile saver.

I didn't put the logic in the VisitClasses call since some of the
callers already have no thread suspension checks that would fail from
this.

Bug: 38350590
Test: test-art-host ART_TEST_JIT=true

(cherry picked from commit 39100372d5131b3327e5af076a949da9eb6f7a33)

Change-Id: I1b8350638ce0a0486b2672100e582dc3a049b0fd

diff --git a/runtime/gc/collector_type.h b/runtime/gc/collector_type.h
index f0e1029..8979e74 100644 (file)
--- a/runtime/gc/collector_type.h
+++ b/runtime/gc/collector_type.h
@@ -61,6 +61,8 @@ enum CollectorType {
   kCollectorTypeAddRemoveSystemWeakHolder,
   // Fake collector type for GetObjectsAllocated
   kCollectorTypeGetObjectsAllocated,
+  // Fake collector type for ScopedGCCriticalSection
+  kCollectorTypeCriticalSection,
 };
 std::ostream& operator<<(std::ostream& os, const CollectorType& collector_type);
 

diff --git a/runtime/gc/gc_cause.cc b/runtime/gc/gc_cause.cc
index c35ec7c..6586116 100644 (file)
--- a/runtime/gc/gc_cause.cc
+++ b/runtime/gc/gc_cause.cc
@@ -42,6 +42,7 @@ const char* PrettyCause(GcCause cause) {
     case kGcCauseAddRemoveSystemWeakHolder: return "SystemWeakHolder";
     case kGcCauseHprof: return "Hprof";
     case kGcCauseGetObjectsAllocated: return "ObjectsAllocated";
+    case kGcCauseProfileSaver: return "ProfileSaver";
   }
   LOG(FATAL) << "Unreachable";
   UNREACHABLE();

diff --git a/runtime/gc/gc_cause.h b/runtime/gc/gc_cause.h
index 41c8943..e381392 100644 (file)
--- a/runtime/gc/gc_cause.h
+++ b/runtime/gc/gc_cause.h
@@ -59,6 +59,8 @@ enum GcCause {
   kGcCauseHprof,
   // Not a real GC cause, used to prevent GetObjectsAllocated running in the middle of GC.
   kGcCauseGetObjectsAllocated,
+  // GC cause for the profile saver.
+  kGcCauseProfileSaver,
 };
 
 const char* PrettyCause(GcCause cause);

diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc
index 1441987..c2eb7bd 100644 (file)
--- a/runtime/jit/profile_saver.cc
+++ b/runtime/jit/profile_saver.cc
@@ -28,10 +28,12 @@
 #include "base/systrace.h"
 #include "base/time_utils.h"
 #include "compiler_filter.h"
+#include "gc/collector_type.h"
+#include "gc/gc_cause.h"
+#include "gc/scoped_gc_critical_section.h"
 #include "oat_file_manager.h"
 #include "scoped_thread_state_change-inl.h"
 
-
 namespace art {
 
 ProfileSaver* ProfileSaver::instance_ = nullptr;
@@ -216,20 +218,27 @@ void ProfileSaver::FetchAndCacheResolvedClassesAndMethods() {
   // Resolve any new registered locations.
   ResolveTrackedLocations();
 
-  ClassLinker* const class_linker = Runtime::Current()->GetClassLinker();
-  std::set<DexCacheResolvedClasses> resolved_classes =
-      class_linker->GetResolvedClasses(/*ignore boot classes*/ true);
-
+  Thread* const self = Thread::Current();
   std::vector<MethodReference> methods;
+  std::set<DexCacheResolvedClasses> resolved_classes;
   {
-    ScopedTrace trace2("Get hot methods");
-    GetMethodsVisitor visitor(&methods, options_.GetStartupMethodSamples());
-    ScopedObjectAccess soa(Thread::Current());
-    class_linker->VisitClasses(&visitor);
-    VLOG(profiler) << "Methods with samples greater than "
-                   << options_.GetStartupMethodSamples() << " = " << methods.size();
+    ScopedObjectAccess soa(self);
+    gc::ScopedGCCriticalSection sgcs(self,
+                                     gc::kGcCauseProfileSaver,
+                                     gc::kCollectorTypeCriticalSection);
+
+    ClassLinker* const class_linker = Runtime::Current()->GetClassLinker();
+    resolved_classes = class_linker->GetResolvedClasses(/*ignore boot classes*/ true);
+
+    {
+      ScopedTrace trace2("Get hot methods");
+      GetMethodsVisitor visitor(&methods, options_.GetStartupMethodSamples());
+      class_linker->VisitClasses(&visitor);
+      VLOG(profiler) << "Methods with samples greater than "
+                     << options_.GetStartupMethodSamples() << " = " << methods.size();
+    }
   }
-  MutexLock mu(Thread::Current(), *Locks::profiler_lock_);
+  MutexLock mu(self, *Locks::profiler_lock_);
   uint64_t total_number_of_profile_entries_cached = 0;
 
   for (const auto& it : tracked_dex_base_locations_) {