1 files changed, 18 insertions, 49 deletions

diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc
index 167140c..6790144 100644
--- a/runtime/gc/collector/mark_sweep.cc
+++ b/runtime/gc/collector/mark_sweep.cc
@@ -207,10 +207,6 @@ bool MarkSweep::HandleDirtyObjectsPhase() {
   }
 
   ProcessReferences(self);
-  {
-    ReaderMutexLock mu(self, *Locks::heap_bitmap_lock_);
-    SweepSystemWeaks();
-  }
 
   // Only need to do this if we have the card mark verification on, and only during concurrent GC.
   if (GetHeap()->verify_missing_card_marks_ || GetHeap()->verify_pre_gc_heap_||
@@ -228,6 +224,12 @@ bool MarkSweep::HandleDirtyObjectsPhase() {
   // Ensure that nobody inserted items in the live stack after we swapped the stacks.
   ReaderMutexLock mu(self, *Locks::heap_bitmap_lock_);
   CHECK_GE(live_stack_freeze_size_, GetHeap()->GetLiveStack()->Size());
+
+  // Disallow new system weaks to prevent a race which occurs when someone adds a new system
+  // weak before we sweep them. Since this new system weak may not be marked, the GC may
+  // incorrectly sweep it. This also fixes a race where interning may attempt to return a strong
+  // reference to a string that is about to be swept.
+  Runtime::Current()->DisallowNewSystemWeaks();
   return true;
 }
 
@@ -289,14 +291,16 @@ void MarkSweep::ReclaimPhase() {
 
   if (!IsConcurrent()) {
     ProcessReferences(self);
-    {
-      ReaderMutexLock mu(self, *Locks::heap_bitmap_lock_);
-      SweepSystemWeaks();
-    }
-    timings_.StartSplit("PreSweepingGcVerification");
-    heap_->PreSweepingGcVerification(this);
-    timings_.EndSplit();
-  } else {
+  }
+
+  {
+    WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
+    SweepSystemWeaks();
+  }
+
+  if (IsConcurrent()) {
+    Runtime::Current()->AllowNewSystemWeaks();
+
     base::TimingLogger::ScopedSplit split("UnMarkAllocStack", &timings_);
     WriterMutexLock mu(self, *Locks::heap_bitmap_lock_);
     accounting::ObjectStack* allocation_stack = GetHeap()->allocation_stack_.get();
@@ -1002,13 +1006,7 @@ void MarkSweep::ReMarkRoots() {
 }
 
 void MarkSweep::SweepJniWeakGlobals(IsMarkedTester is_marked, void* arg) {
-  JavaVMExt* vm = Runtime::Current()->GetJavaVM();
-  WriterMutexLock mu(Thread::Current(), vm->weak_globals_lock);
-  for (const Object** entry : vm->weak_globals) {
-    if (!is_marked(*entry, arg)) {
-      *entry = kClearedJniWeakGlobal;
-    }
-  }
+  Runtime::Current()->GetJavaVM()->SweepWeakGlobals(is_marked, arg);
 }
 
 struct ArrayMarkedCheck {
@@ -1029,31 +1027,8 @@ bool MarkSweep::IsMarkedArrayCallback(const Object* object, void* arg) {
   return false;
 }
 
-void MarkSweep::SweepSystemWeaksArray(accounting::ObjectStack* allocations) {
-  Runtime* runtime = Runtime::Current();
-  // The callbacks check
-  // !is_marked where is_marked is the callback but we want
-  // !IsMarked && IsLive
-  // So compute !(!IsMarked && IsLive) which is equal to (IsMarked || !IsLive).
-  // Or for swapped (IsLive || !IsMarked).
-
-  timings_.StartSplit("SweepSystemWeaksArray");
-  ArrayMarkedCheck visitor;
-  visitor.live_stack = allocations;
-  visitor.mark_sweep = this;
-  runtime->GetInternTable()->SweepInternTableWeaks(IsMarkedArrayCallback, &visitor);
-  runtime->GetMonitorList()->SweepMonitorList(IsMarkedArrayCallback, &visitor);
-  SweepJniWeakGlobals(IsMarkedArrayCallback, &visitor);
-  timings_.EndSplit();
-}
-
 void MarkSweep::SweepSystemWeaks() {
   Runtime* runtime = Runtime::Current();
-  // The callbacks check
-  // !is_marked where is_marked is the callback but we want
-  // !IsMarked && IsLive
-  // So compute !(!IsMarked && IsLive) which is equal to (IsMarked || !IsLive).
-  // Or for swapped (IsLive || !IsMarked).
   timings_.StartSplit("SweepSystemWeaks");
   runtime->GetInternTable()->SweepInternTableWeaks(IsMarkedCallback, this);
   runtime->GetMonitorList()->SweepMonitorList(IsMarkedCallback, this);
@@ -1087,12 +1062,7 @@ void MarkSweep::VerifySystemWeaks() {
   // Verify system weaks, uses a special IsMarked callback which always returns true.
   runtime->GetInternTable()->SweepInternTableWeaks(VerifyIsLiveCallback, this);
   runtime->GetMonitorList()->SweepMonitorList(VerifyIsLiveCallback, this);
-
-  JavaVMExt* vm = runtime->GetJavaVM();
-  ReaderMutexLock mu(Thread::Current(), vm->weak_globals_lock);
-  for (const Object** entry : vm->weak_globals) {
-    VerifyIsLive(*entry);
-  }
+  runtime->GetJavaVM()->SweepWeakGlobals(VerifyIsLiveCallback, this);
 }
 
 struct SweepCallbackContext {
@@ -1172,7 +1142,6 @@ void MarkSweep::ZygoteSweepCallback(size_t num_ptrs, Object** ptrs, void* arg) {
 
 void MarkSweep::SweepArray(accounting::ObjectStack* allocations, bool swap_bitmaps) {
   space::DlMallocSpace* space = heap_->GetAllocSpace();
-
   timings_.StartSplit("SweepArray");
   // Newly allocated objects MUST be in the alloc space and those are the only objects which we are
   // going to free.