commit bcd9dd7d1223eaa60f188d1a109d26e5e0143e10
author Mathieu Chartier <mathieuc@google.com> Mon Mar 07 10:25:04 2016 -0800
committer Mathieu Chartier <mathieuc@google.com> Mon Mar 07 13:14:45 2016 -0800
tree b9b121de21cbc0a97cf9138a6f66d3df4b1aee65
parent 810ef3768021b5fc7b47e663e037e49c46f8c916

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

compiler/image_writer.h

diff --git a/compiler/image_writer.h b/compiler/image_writer.h
index dba9dd7..f204b28 100644
--- a/compiler/image_writer.h
+++ b/compiler/image_writer.h
@@ -443,7 +443,7 @@
 
   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>

runtime/gc/collector/mark_compact.cc

diff --git a/runtime/gc/collector/mark_compact.cc b/runtime/gc/collector/mark_compact.cc
index 7727b2d..6beb606 100644
--- a/runtime/gc/collector/mark_compact.cc
+++ b/runtime/gc/collector/mark_compact.cc
@@ -131,7 +131,7 @@
 
 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));

runtime/gc/heap.cc

diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 3480483..faa3d3b 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -1545,7 +1545,6 @@
 }
 
 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();

runtime/gc/heap.h

diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h
index 889069d..e0a53a0 100644
--- a/runtime/gc/heap.h
+++ b/runtime/gc/heap.h
@@ -651,8 +651,8 @@
     }
   }
 
-  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;

runtime/gc/space/large_object_space.cc

diff --git a/runtime/gc/space/large_object_space.cc b/runtime/gc/space/large_object_space.cc
index e70fe21..010f677 100644
--- 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 @@
   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";
   }