Don't use ScopedObjectAccess in Heap::DumpSpaces

ScopedObjectAccess does not work well if the mutator lock is
excusively held while in a suspend thread state. This caused
deadlocks and DCHECK failures.

Bug: 27493316

(cherry picked from commit 03d21bc5bed887243ff6ce3531179185ffd3532c)

Change-Id: I5d67f74fc7082761e45dc1d7778b0ea7fceaaf8f

diff --git a/compiler/image_writer.h b/compiler/image_writer.h
index dba9dd7..f204b28 100644 (file)
--- a/compiler/image_writer.h
+++ b/compiler/image_writer.h
@@ -443,7 +443,7 @@ class ImageWriter FINAL {
 
   static Bin BinTypeForNativeRelocationType(NativeObjectRelocationType type);
 
-  uintptr_t NativeOffsetInImage(void* obj);
+  uintptr_t NativeOffsetInImage(void* obj) SHARED_REQUIRES(Locks::mutator_lock_);
 
   // Location of where the object will be when the image is loaded at runtime.
   template <typename T>

diff --git a/runtime/gc/collector/mark_compact.cc b/runtime/gc/collector/mark_compact.cc
index 7727b2d..6beb606 100644 (file)
--- a/runtime/gc/collector/mark_compact.cc
+++ b/runtime/gc/collector/mark_compact.cc
@@ -131,7 +131,7 @@ void MarkCompact::ProcessReferences(Thread* self) {
 
 class BitmapSetSlowPathVisitor {
  public:
-  void operator()(const mirror::Object* obj) const {
+  void operator()(const mirror::Object* obj) const SHARED_REQUIRES(Locks::mutator_lock_) {
     // Marking a large object, make sure its aligned as a sanity check.
     if (!IsAligned<kPageSize>(obj)) {
       Runtime::Current()->GetHeap()->DumpSpaces(LOG(ERROR));

diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 3480483..faa3d3b 100644 (file)
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -1545,7 +1545,6 @@ std::string Heap::DumpSpaces() const {
 }
 
 void Heap::DumpSpaces(std::ostream& stream) const {
-  ScopedObjectAccess soa(Thread::Current());
   for (const auto& space : continuous_spaces_) {
     accounting::ContinuousSpaceBitmap* live_bitmap = space->GetLiveBitmap();
     accounting::ContinuousSpaceBitmap* mark_bitmap = space->GetMarkBitmap();

diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h
index 889069d..e0a53a0 100644 (file)
--- a/runtime/gc/heap.h
+++ b/runtime/gc/heap.h
@@ -651,8 +651,8 @@ class Heap {
     }
   }
 
-  std::string DumpSpaces() const WARN_UNUSED;
-  void DumpSpaces(std::ostream& stream) const;
+  void DumpSpaces(std::ostream& stream) const SHARED_REQUIRES(Locks::mutator_lock_);
+  std::string DumpSpaces() const SHARED_REQUIRES(Locks::mutator_lock_);
 
   // Dump object should only be used by the signal handler.
   void DumpObject(std::ostream& stream, mirror::Object* obj) NO_THREAD_SAFETY_ANALYSIS;

diff --git a/runtime/gc/space/large_object_space.cc b/runtime/gc/space/large_object_space.cc
index e70fe21..010f677 100644 (file)
--- a/runtime/gc/space/large_object_space.cc
+++ b/runtime/gc/space/large_object_space.cc
@@ -27,6 +27,7 @@
 #include "base/stl_util.h"
 #include "image.h"
 #include "os.h"
+#include "scoped_thread_state_change.h"
 #include "space-inl.h"
 #include "thread-inl.h"
 
@@ -190,6 +191,7 @@ size_t LargeObjectMapSpace::Free(Thread* self, mirror::Object* ptr) {
   MutexLock mu(self, lock_);
   auto it = large_objects_.find(ptr);
   if (UNLIKELY(it == large_objects_.end())) {
+    ScopedObjectAccess soa(self);
     Runtime::Current()->GetHeap()->DumpSpaces(LOG(INTERNAL_FATAL));
     LOG(FATAL) << "Attempted to free large object " << ptr << " which was not live";
   }