Add ScopedAssertNoThreadSuspension

Added a new class, ScopedAssertNoThreadSuspension.
Deleted some unnecessary ScopedAssertNoThreadSuspension since
VisitObjects already has a ScopedAssertNoThreadSuspension.

Change-Id: I29ec0006120c39a27184d30e2d1d0c179e203776

7 files changed, 50 insertions, 60 deletions

diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index d743f907a3..990c1c87cf 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -795,14 +795,11 @@ void CompilerDriver::UpdateImageClasses(TimingLogger* timings) {
   if (IsImage()) {
     TimingLogger::ScopedTiming t("UpdateImageClasses", timings);
     // Update image_classes_ with classes for objects created by <clinit> methods.
-    Thread* self = Thread::Current();
-    const char* old_cause = self->StartAssertNoThreadSuspension("ImageWriter");
     gc::Heap* heap = Runtime::Current()->GetHeap();
     // TODO: Image spaces only?
     ScopedObjectAccess soa(Thread::Current());
-    WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
+    WriterMutexLock mu(soa.Self(), *Locks::heap_bitmap_lock_);
     heap->VisitObjects(FindClinitImageClassesCallback, this);
-    self->EndAssertNoThreadSuspension(old_cause);
   }
 }
 
@@ -1872,7 +1869,8 @@ static void InitializeClass(const ParallelCompilationManager* manager, size_t cl
             // TODO we detach transaction from runtime to indicate we quit the transactional
             // mode which prevents the GC from visiting objects modified during the transaction.
             // Ensure GC is not run so don't access freed objects when aborting transaction.
-            const char* old_casue = soa.Self()->StartAssertNoThreadSuspension("Transaction end");
+
+            ScopedAssertNoThreadSuspension ants(soa.Self(), "Transaction end");
             runtime->ExitTransactionMode();
 
             if (!success) {
@@ -1885,7 +1883,6 @@ static void InitializeClass(const ParallelCompilationManager* manager, size_t cl
               transaction.Abort();
               CHECK_EQ(old_status, klass->GetStatus()) << "Previous class status not restored";
             }
-            soa.Self()->EndAssertNoThreadSuspension(old_casue);
           }
         }
         soa.Self()->AssertNoPendingException();
diff --git a/compiler/elf_patcher.cc b/compiler/elf_patcher.cc
index 92eb4d8955..0646b75f37 100644
--- a/compiler/elf_patcher.cc
+++ b/compiler/elf_patcher.cc
@@ -188,9 +188,8 @@ bool ElfPatcher::PatchElf() {
                      compiler_driver_->GetMethodsToPatch().size() +
                      compiler_driver_->GetClassesToPatch().size());
   }
-  Thread* self = Thread::Current();
   ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
-  const char* old_cause = self->StartAssertNoThreadSuspension("ElfPatcher");
+  ScopedAssertNoThreadSuspension ants(Thread::Current(), "ElfPatcher");
 
   typedef std::vector<const CompilerDriver::CallPatchInformation*> CallPatches;
   const CallPatches& code_to_patch = compiler_driver_->GetCodeToPatch();
@@ -259,8 +258,6 @@ bool ElfPatcher::PatchElf() {
     SetPatchLocation(patch, PointerToLowMemUInt32(get_image_address_(cb_data_, target)));
   }
 
-  self->EndAssertNoThreadSuspension(old_cause);
-
   if (write_patches_) {
     return WriteOutPatchData();
   }
diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc
index 9c9cdf2700..c08d3bdf8b 100644
--- a/compiler/image_writer.cc
+++ b/compiler/image_writer.cc
@@ -543,11 +543,9 @@ void ImageWriter::CalculateNewObjectOffsets(size_t oat_loaded_size, size_t oat_d
   {
     WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
     // TODO: Image spaces only?
-    const char* old = self->StartAssertNoThreadSuspension("ImageWriter");
     DCHECK_LT(image_end_, image_->Size());
     // Clear any pre-existing monitors which may have been in the monitor words.
     heap->VisitObjects(WalkFieldsCallback, this);
-    self->EndAssertNoThreadSuspension(old);
   }
 
   const byte* oat_file_begin = image_begin_ + RoundUp(image_end_, kPageSize);
@@ -577,20 +575,18 @@ void ImageWriter::CalculateNewObjectOffsets(size_t oat_loaded_size, size_t oat_d
 
 void ImageWriter::CopyAndFixupObjects()
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
-  Thread* self = Thread::Current();
-  const char* old_cause = self->StartAssertNoThreadSuspension("ImageWriter");
+  ScopedAssertNoThreadSuspension ants(Thread::Current(), "ImageWriter");
   gc::Heap* heap = Runtime::Current()->GetHeap();
   // TODO: heap validation can't handle this fix up pass
   heap->DisableObjectValidation();
   // TODO: Image spaces only?
-  WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
+  WriterMutexLock mu(ants.Self(), *Locks::heap_bitmap_lock_);
   heap->VisitObjects(CopyAndFixupObjectsCallback, this);
   // Fix up the object previously had hash codes.
   for (const std::pair<mirror::Object*, uint32_t>& hash_pair : saved_hashes_) {
     hash_pair.first->SetLockWord(LockWord::FromHashCode(hash_pair.second), false);
   }
   saved_hashes_.clear();
-  self->EndAssertNoThreadSuspension(old_cause);
 }
 
 void ImageWriter::CopyAndFixupObjectsCallback(Object* obj, void* arg) {
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index cb0fe0a7dc..f927720ce4 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -2950,7 +2950,7 @@ mirror::ArtMethod* ClassLinker::LoadMethod(Thread* self, const DexFile& dex_file
   }
   DCHECK(dst->IsArtMethod()) << PrettyDescriptor(dst->GetClass());
 
-  const char* old_cause = self->StartAssertNoThreadSuspension("LoadMethod");
+  ScopedAssertNoThreadSuspension ants(self, "LoadMethod");
   dst->SetDexMethodIndex(dex_method_idx);
   dst->SetDeclaringClass(klass.Get());
   dst->SetCodeItemOffset(it.GetMethodCodeItemOffset());
@@ -2997,7 +2997,6 @@ mirror::ArtMethod* ClassLinker::LoadMethod(Thread* self, const DexFile& dex_file
   }
   dst->SetAccessFlags(access_flags);
 
-  self->EndAssertNoThreadSuspension(old_cause);
   return dst;
 }
 
@@ -3474,8 +3473,7 @@ void ClassLinker::MoveImageClassesToClassTable() {
   if (!dex_cache_image_class_lookup_required_) {
     return;  // All dex cache classes are already in the class table.
   }
-  const char* old_no_suspend_cause =
-      self->StartAssertNoThreadSuspension("Moving image classes to class table");
+  ScopedAssertNoThreadSuspension ants(self, "Moving image classes to class table");
   mirror::ObjectArray<mirror::DexCache>* dex_caches = GetImageDexCaches();
   std::string temp;
   for (int32_t i = 0; i < dex_caches->GetLength(); i++) {
@@ -3501,13 +3499,10 @@ void ClassLinker::MoveImageClassesToClassTable() {
     }
   }
   dex_cache_image_class_lookup_required_ = false;
-  self->EndAssertNoThreadSuspension(old_no_suspend_cause);
 }
 
 mirror::Class* ClassLinker::LookupClassFromImage(const char* descriptor) {
-  Thread* self = Thread::Current();
-  const char* old_no_suspend_cause =
-      self->StartAssertNoThreadSuspension("Image class lookup");
+  ScopedAssertNoThreadSuspension ants(Thread::Current(), "Image class lookup");
   mirror::ObjectArray<mirror::DexCache>* dex_caches = GetImageDexCaches();
   for (int32_t i = 0; i < dex_caches->GetLength(); ++i) {
     mirror::DexCache* dex_cache = dex_caches->Get(i);
@@ -3521,13 +3516,11 @@ mirror::Class* ClassLinker::LookupClassFromImage(const char* descriptor) {
         uint16_t type_idx = dex_file->GetIndexForTypeId(*type_id);
         mirror::Class* klass = dex_cache->GetResolvedType(type_idx);
         if (klass != nullptr) {
-          self->EndAssertNoThreadSuspension(old_no_suspend_cause);
           return klass;
         }
       }
     }
   }
-  self->EndAssertNoThreadSuspension(old_no_suspend_cause);
   return nullptr;
 }
 
@@ -5077,7 +5070,7 @@ bool ClassLinker::LinkFields(Thread* self, Handle<mirror::Class> klass, bool is_
   // we want a relatively stable order so that adding new fields
   // minimizes disruption of C++ version such as Class and Method.
   std::deque<mirror::ArtField*> grouped_and_sorted_fields;
-  const char* old_no_suspend_cause  = self->StartAssertNoThreadSuspension(
+  const char* old_no_suspend_cause = self->StartAssertNoThreadSuspension(
       "Naked ArtField references in deque");
   for (size_t i = 0; i < num_fields; i++) {
     mirror::ArtField* f = fields->Get(i);
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index aced95465b..e57133d016 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -1991,7 +1991,7 @@ JDWP::JdwpError Dbg::GetThreadGroup(JDWP::ObjectId thread_id, JDWP::ExpandBuf* p
   if (error != JDWP::ERR_NONE) {
     return JDWP::ERR_INVALID_OBJECT;
   }
-  const char* old_cause = soa.Self()->StartAssertNoThreadSuspension("Debugger: GetThreadGroup");
+  ScopedAssertNoThreadSuspension ants(soa.Self(), "Debugger: GetThreadGroup");
   // Okay, so it's an object, but is it actually a thread?
   {
     MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
@@ -2012,7 +2012,6 @@ JDWP::JdwpError Dbg::GetThreadGroup(JDWP::ObjectId thread_id, JDWP::ExpandBuf* p
     JDWP::ObjectId thread_group_id = gRegistry->Add(group);
     expandBufAddObjectId(pReply, thread_group_id);
   }
-  soa.Self()->EndAssertNoThreadSuspension(old_cause);
   return error;
 }
 
@@ -2046,12 +2045,11 @@ JDWP::JdwpError Dbg::GetThreadGroupName(JDWP::ObjectId thread_group_id, JDWP::Ex
   if (error != JDWP::ERR_NONE) {
     return error;
   }
-  const char* old_cause = soa.Self()->StartAssertNoThreadSuspension("Debugger: GetThreadGroupName");
+  ScopedAssertNoThreadSuspension ants(soa.Self(), "Debugger: GetThreadGroupName");
   mirror::Class* c = soa.Decode<mirror::Class*>(WellKnownClasses::java_lang_ThreadGroup);
   mirror::ArtField* f = c->FindInstanceField("name", "Ljava/lang/String;");
   CHECK(f != nullptr);
   mirror::String* s = reinterpret_cast<mirror::String*>(f->GetObject(thread_group));
-  soa.Self()->EndAssertNoThreadSuspension(old_cause);
 
   std::string thread_group_name(s->ToModifiedUtf8());
   expandBufAddUtf8String(pReply, thread_group_name);
@@ -2065,14 +2063,15 @@ JDWP::JdwpError Dbg::GetThreadGroupParent(JDWP::ObjectId thread_group_id, JDWP::
   if (error != JDWP::ERR_NONE) {
     return error;
   }
-  const char* old_cause = soa.Self()->StartAssertNoThreadSuspension("Debugger: GetThreadGroupParent");
-  mirror::Class* c = soa.Decode<mirror::Class*>(WellKnownClasses::java_lang_ThreadGroup);
-  CHECK(c != nullptr);
-  mirror::ArtField* f = c->FindInstanceField("parent", "Ljava/lang/ThreadGroup;");
-  CHECK(f != nullptr);
-  mirror::Object* parent = f->GetObject(thread_group);
-  soa.Self()->EndAssertNoThreadSuspension(old_cause);
-
+  mirror::Object* parent;
+  {
+    ScopedAssertNoThreadSuspension ants(soa.Self(), "Debugger: GetThreadGroupParent");
+    mirror::Class* c = soa.Decode<mirror::Class*>(WellKnownClasses::java_lang_ThreadGroup);
+    CHECK(c != nullptr);
+    mirror::ArtField* f = c->FindInstanceField("parent", "Ljava/lang/ThreadGroup;");
+    CHECK(f != nullptr);
+    parent = f->GetObject(thread_group);
+  }
   JDWP::ObjectId parent_group_id = gRegistry->Add(parent);
   expandBufAddObjectId(pReply, parent_group_id);
   return JDWP::ERR_NONE;
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index b744a62d3a..864bb7251f 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -685,9 +685,8 @@ void Heap::CreateThreadPool() {
 }
 
 void Heap::VisitObjects(ObjectCallback callback, void* arg) {
-  Thread* self = Thread::Current();
   // GCs can move objects, so don't allow this.
-  const char* old_cause = self->StartAssertNoThreadSuspension("Visiting objects");
+  ScopedAssertNoThreadSuspension ants(Thread::Current(), "Visiting objects");
   if (bump_pointer_space_ != nullptr) {
     // Visit objects in bump pointer space.
     bump_pointer_space_->Walk(callback, arg);
@@ -704,7 +703,6 @@ void Heap::VisitObjects(ObjectCallback callback, void* arg) {
     }
   }
   GetLiveBitmap()->Walk(callback, arg);
-  self->EndAssertNoThreadSuspension(old_cause);
 }
 
 void Heap::MarkAllocStackAsLive(accounting::ObjectStack* stack) {
@@ -1429,12 +1427,10 @@ class InstanceCounter {
 void Heap::CountInstances(const std::vector<mirror::Class*>& classes, bool use_is_assignable_from,
                           uint64_t* counts) {
   // Can't do any GC in this function since this may move classes.
-  Thread* self = Thread::Current();
-  auto* old_cause = self->StartAssertNoThreadSuspension("CountInstances");
+  ScopedAssertNoThreadSuspension ants(Thread::Current(), "CountInstances");
   InstanceCounter counter(classes, use_is_assignable_from, counts);
-  WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
+  ReaderMutexLock mu(ants.Self(), *Locks::heap_bitmap_lock_);
   VisitObjects(InstanceCounter::Callback, &counter);
-  self->EndAssertNoThreadSuspension(old_cause);
 }
 
 class InstanceCollector {
@@ -1447,8 +1443,7 @@ class InstanceCollector {
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_, Locks::heap_bitmap_lock_) {
     DCHECK(arg != nullptr);
     InstanceCollector* instance_collector = reinterpret_cast<InstanceCollector*>(arg);
-    mirror::Class* instance_class = obj->GetClass();
-    if (instance_class == instance_collector->class_) {
+    if (obj->GetClass() == instance_collector->class_) {
       if (instance_collector->max_count_ == 0 ||
           instance_collector->instances_.size() < instance_collector->max_count_) {
         instance_collector->instances_.push_back(obj);
@@ -1457,8 +1452,8 @@ class InstanceCollector {
   }
 
  private:
-  mirror::Class* class_;
-  uint32_t max_count_;
+  const mirror::Class* const class_;
+  const uint32_t max_count_;
   std::vector<mirror::Object*>& instances_;
   DISALLOW_COPY_AND_ASSIGN(InstanceCollector);
 };
@@ -1466,12 +1461,10 @@ class InstanceCollector {
 void Heap::GetInstances(mirror::Class* c, int32_t max_count,
                         std::vector<mirror::Object*>& instances) {
   // Can't do any GC in this function since this may move classes.
-  Thread* self = Thread::Current();
-  auto* old_cause = self->StartAssertNoThreadSuspension("GetInstances");
+  ScopedAssertNoThreadSuspension ants(Thread::Current(), "GetInstances");
   InstanceCollector collector(c, max_count, instances);
-  WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
+  ReaderMutexLock mu(ants.Self(), *Locks::heap_bitmap_lock_);
   VisitObjects(&InstanceCollector::Callback, &collector);
-  self->EndAssertNoThreadSuspension(old_cause);
 }
 
 class ReferringObjectsFinder {
@@ -1504,8 +1497,8 @@ class ReferringObjectsFinder {
   }
 
  private:
-  mirror::Object* object_;
-  uint32_t max_count_;
+  const mirror::Object* const object_;
+  const uint32_t max_count_;
   std::vector<mirror::Object*>& referring_objects_;
   DISALLOW_COPY_AND_ASSIGN(ReferringObjectsFinder);
 };
@@ -1513,12 +1506,10 @@ class ReferringObjectsFinder {
 void Heap::GetReferringObjects(mirror::Object* o, int32_t max_count,
                                std::vector<mirror::Object*>& referring_objects) {
   // Can't do any GC in this function since this may move the object o.
-  Thread* self = Thread::Current();
-  auto* old_cause = self->StartAssertNoThreadSuspension("GetReferringObjects");
+  ScopedAssertNoThreadSuspension ants(Thread::Current(), "GetReferringObjects");
   ReferringObjectsFinder finder(o, max_count, referring_objects);
-  WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
+  ReaderMutexLock mu(ants.Self(), *Locks::heap_bitmap_lock_);
   VisitObjects(&ReferringObjectsFinder::Callback, &finder);
-  self->EndAssertNoThreadSuspension(old_cause);
 }
 
 void Heap::CollectGarbage(bool clear_soft_references) {
diff --git a/runtime/thread.h b/runtime/thread.h
index d96b50ba13..164eb86f67 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -1180,6 +1180,23 @@ class Thread {
   DISALLOW_COPY_AND_ASSIGN(Thread);
 };
 
+class ScopedAssertNoThreadSuspension {
+ public:
+  ScopedAssertNoThreadSuspension(Thread* self, const char* cause)
+      : self_(self), old_cause_(self->StartAssertNoThreadSuspension(cause)) {
+  }
+  ~ScopedAssertNoThreadSuspension() {
+    self_->EndAssertNoThreadSuspension(old_cause_);
+  }
+  Thread* Self() {
+    return self_;
+  }
+
+ private:
+  Thread* const self_;
+  const char* old_cause_;
+};
+
 std::ostream& operator<<(std::ostream& os, const Thread& thread);
 std::ostream& operator<<(std::ostream& os, const ThreadState& state);