Visit invalid roots of only suspended threads
Since this always happens with suspended threads or self, you can
just visit these threads and do not require a suspend all. This
will not miss any roots if the caller was marking a thread root.
Fixes issues like transitioning to suspended and back blocking on a
thread suspension request from another thread. This could cause
deadlocks previously.
Bug: 29062271
Change-Id: I2fef149387aacf0cdc9a773d4f172c42fa53e4dc
diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc
index 894ceba..ac5931f 100644
--- a/runtime/gc/collector/mark_sweep.cc
+++ b/runtime/gc/collector/mark_sweep.cc
@@ -444,27 +444,8 @@
}
PrintFileToLog("/proc/self/maps", LogSeverity::INTERNAL_FATAL);
MemMap::DumpMaps(LOG(INTERNAL_FATAL), true);
- {
- LOG(INTERNAL_FATAL) << "Attempting see if it's a bad root";
- Thread* self = Thread::Current();
- if (Locks::mutator_lock_->IsExclusiveHeld(self)) {
- mark_sweep_->VerifyRoots();
- } else {
- const bool heap_bitmap_exclusive_locked =
- Locks::heap_bitmap_lock_->IsExclusiveHeld(self);
- if (heap_bitmap_exclusive_locked) {
- Locks::heap_bitmap_lock_->ExclusiveUnlock(self);
- }
- {
- ScopedThreadSuspension(self, kSuspended);
- ScopedSuspendAll ssa(__FUNCTION__);
- mark_sweep_->VerifyRoots();
- }
- if (heap_bitmap_exclusive_locked) {
- Locks::heap_bitmap_lock_->ExclusiveLock(self);
- }
- }
- }
+ LOG(INTERNAL_FATAL) << "Attempting see if it's a bad thread root\n";
+ mark_sweep_->VerifySuspendedThreadRoots();
LOG(FATAL) << "Can't mark invalid object";
}
}
@@ -591,15 +572,15 @@
if (heap->GetLiveBitmap()->GetContinuousSpaceBitmap(root) == nullptr) {
space::LargeObjectSpace* large_object_space = heap->GetLargeObjectsSpace();
if (large_object_space != nullptr && !large_object_space->Contains(root)) {
- LOG(INTERNAL_FATAL) << "Found invalid root: " << root << " " << info;
+ LOG(INTERNAL_FATAL) << "Found invalid root: " << root << " " << info << "\n";
}
}
}
};
-void MarkSweep::VerifyRoots() {
+void MarkSweep::VerifySuspendedThreadRoots() {
VerifyRootVisitor visitor;
- Runtime::Current()->GetThreadList()->VisitRoots(&visitor);
+ Runtime::Current()->GetThreadList()->VisitRootsForSuspendedThreads(&visitor);
}
void MarkSweep::MarkRoots(Thread* self) {
diff --git a/runtime/gc/collector/mark_sweep.h b/runtime/gc/collector/mark_sweep.h
index c19107a..7168f96 100644
--- a/runtime/gc/collector/mark_sweep.h
+++ b/runtime/gc/collector/mark_sweep.h
@@ -250,8 +250,8 @@
// Verify the roots of the heap and print out information related to any invalid roots.
// Called in MarkObject, so may we may not hold the mutator lock.
- void VerifyRoots()
- NO_THREAD_SAFETY_ANALYSIS;
+ void VerifySuspendedThreadRoots()
+ SHARED_REQUIRES(Locks::mutator_lock_);
// Expand mark stack to 2x its current size.
void ExpandMarkStack()