From 511683764f2580d877725e20514ca4976c91bbfb Mon Sep 17 00:00:00 2001
From: Mathieu Chartier <mathieuc@google.com>
Date: Wed, 12 Aug 2015 16:40:32 -0700
Subject: [PATCH] Wait for GC to finish in ThreadList::~ThreadList

Added a DisableGC to the heap instead of using the runtime shutdown
boolean. The runtime shutting down boolean is set to true before the
non daemon threads have exited. These threads may still be doing
allocations and we don't want to throw OOME if unnecessary.

Bug: 18577101
Change-Id: Iceb7048e6bd799aa2716099459c54f8dc0fb8feb
---
 runtime/gc/heap.cc     | 18 ++++++++++++------
 runtime/gc/heap.h      |  6 ++++++
 runtime/thread_list.cc |  7 +++++++
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index e019d1127..1033d713a 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -229,7 +229,8 @@ Heap::Heap(size_t initial_size,
       alloc_tracking_enabled_(false),
       backtrace_lock_(nullptr),
       seen_backtrace_count_(0u),
-      unique_backtrace_count_(0u) {
+      unique_backtrace_count_(0u),
+      gc_disabled_for_shutdown_(false) {
   if (VLOG_IS_ON(heap) || VLOG_IS_ON(startup)) {
     LOG(INFO) << "Heap() entering";
   }
@@ -2401,11 +2402,6 @@ collector::GcType Heap::CollectGarbageInternal(collector::GcType gc_type,
                                                bool clear_soft_references) {
   Thread* self = Thread::Current();
   Runtime* runtime = Runtime::Current();
-  // Don't allow the GC to start if the runtime is shutting down. This can occur if a Daemon thread
-  // is still allocating.
-  if (runtime->IsShuttingDown(self)) {
-    return collector::kGcTypeNone;
-  }
   // If the heap can't run the GC, silently fail and return that no GC was run.
   switch (gc_type) {
     case collector::kGcTypePartial: {
@@ -2439,6 +2435,9 @@ collector::GcType Heap::CollectGarbageInternal(collector::GcType gc_type,
       LOG(WARNING) << "Skipping GC due to disable moving GC count " << disable_moving_gc_count_;
       return collector::kGcTypeNone;
     }
+    if (gc_disabled_for_shutdown_) {
+      return collector::kGcTypeNone;
+    }
     collector_type_running_ = collector_type_;
   }
   if (gc_cause == kGcCauseForAlloc && runtime->HasStatsEnabled()) {
@@ -3852,5 +3851,12 @@ void Heap::CheckGcStressMode(Thread* self, mirror::Object** obj) {
   }
 }
 
+void Heap::DisableGCForShutdown() {
+  Thread* const self = Thread::Current();
+  CHECK(Runtime::Current()->IsShuttingDown(self));
+  MutexLock mu(self, *gc_complete_lock_);
+  gc_disabled_for_shutdown_ = true;
+}
+
 }  // namespace gc
 }  // namespace art
diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h
index 4703685f1..283a3cb21 100644
--- a/runtime/gc/heap.h
+++ b/runtime/gc/heap.h
@@ -745,6 +745,8 @@ class Heap {
       SHARED_REQUIRES(Locks::mutator_lock_)
       REQUIRES(!Locks::alloc_tracker_lock_);
 
+  void DisableGCForShutdown() REQUIRES(!*gc_complete_lock_);
+
  private:
   class ConcurrentGCTask;
   class CollectorTransitionTask;
@@ -1293,6 +1295,10 @@ class Heap {
   // Stack trace hashes that we already saw,
   std::unordered_set<uint64_t> seen_backtraces_ GUARDED_BY(backtrace_lock_);
 
+  // We disable GC when we are shutting down the runtime in case there are daemon threads still
+  // allocating.
+  bool gc_disabled_for_shutdown_ GUARDED_BY(gc_complete_lock_);
+
   friend class CollectorTransitionTask;
   friend class collector::GarbageCollector;
   friend class collector::MarkCompact;
diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc
index 62d1e84b7..d449f4270 100644
--- a/runtime/thread_list.cc
+++ b/runtime/thread_list.cc
@@ -78,6 +78,13 @@ ThreadList::~ThreadList() {
     Runtime::Current()->DetachCurrentThread();
   }
   WaitForOtherNonDaemonThreadsToExit();
+  // Disable GC and wait for GC to complete in case there are still daemon threads doing
+  // allocations.
+  gc::Heap* const heap = Runtime::Current()->GetHeap();
+  heap->DisableGCForShutdown();
+  // In case a GC is in progress, wait for it to finish.
+  heap->WaitForGcToComplete(gc::kGcCauseBackground, Thread::Current());
+
   // TODO: there's an unaddressed race here where a thread may attach during shutdown, see
   //       Thread::Init.
   SuspendAllDaemonThreads();
-- 
2.11.0