Check for any forced deopts before restoring instrumentation stack

We need to check if there are any frames that are marked for
deoptimization before restoring the instrumentation stack. We
do this when updating the instrumentation level but miss it when
undeoptimizing.

Test: art/test.py
Change-Id: Ie7f0d3d48d4ae9f7d39b8d1541d853580d39ffe6
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 557b281..7da959a 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -905,6 +905,38 @@
   instrumentation_level_ = requested_level;
 }
 
+void Instrumentation::MaybeRestoreInstrumentationStack() {
+  // Restore stack only if there is no method currently deoptimized.
+  if (!IsDeoptimizedMethodsEmpty()) {
+    return;
+  }
+
+  Thread* self = Thread::Current();
+  MutexLock mu(self, *Locks::thread_list_lock_);
+  bool no_remaining_deopts = true;
+  // Check that there are no other forced deoptimizations. Do it here so we only need to lock
+  // thread_list_lock once.
+  // The compiler gets confused on the thread annotations, so use
+  // NO_THREAD_SAFETY_ANALYSIS. Note that we hold the mutator lock
+  // exclusively at this point.
+  Locks::mutator_lock_->AssertExclusiveHeld(self);
+  Runtime::Current()->GetThreadList()->ForEach([&](Thread* t) NO_THREAD_SAFETY_ANALYSIS {
+    no_remaining_deopts =
+        no_remaining_deopts && !t->IsForceInterpreter() &&
+        std::all_of(t->GetInstrumentationStack()->cbegin(),
+                    t->GetInstrumentationStack()->cend(),
+                    [&](const auto& frame) REQUIRES_SHARED(Locks::mutator_lock_) {
+                      return frame.second.force_deopt_id_ == current_force_deopt_id_;
+                    });
+  });
+  if (no_remaining_deopts) {
+    Runtime::Current()->GetThreadList()->ForEach(InstrumentationRestoreStack, this);
+    // Only do this after restoring, as walking the stack when restoring will see
+    // the instrumentation exit pc.
+    instrumentation_stubs_installed_ = false;
+  }
+}
+
 void Instrumentation::UpdateStubs() {
   // Look for the highest required instrumentation level.
   InstrumentationLevel requested_level = InstrumentationLevel::kInstrumentNothing;
@@ -932,33 +964,7 @@
   } else {
     InstallStubsClassVisitor visitor(this);
     runtime->GetClassLinker()->VisitClasses(&visitor);
-    // Restore stack only if there is no method currently deoptimized.
-    bool empty = IsDeoptimizedMethodsEmpty();
-    if (empty) {
-      MutexLock mu(self, *Locks::thread_list_lock_);
-      bool no_remaining_deopts = true;
-      // Check that there are no other forced deoptimizations. Do it here so we only need to lock
-      // thread_list_lock once.
-      // The compiler gets confused on the thread annotations, so use
-      // NO_THREAD_SAFETY_ANALYSIS. Note that we hold the mutator lock
-      // exclusively at this point.
-      Locks::mutator_lock_->AssertExclusiveHeld(self);
-      runtime->GetThreadList()->ForEach([&](Thread* t) NO_THREAD_SAFETY_ANALYSIS {
-        no_remaining_deopts =
-            no_remaining_deopts && !t->IsForceInterpreter() &&
-            std::all_of(t->GetInstrumentationStack()->cbegin(),
-                        t->GetInstrumentationStack()->cend(),
-                        [&](const auto& frame) REQUIRES_SHARED(Locks::mutator_lock_) {
-                          return frame.second.force_deopt_id_ == current_force_deopt_id_;
-                        });
-      });
-      if (no_remaining_deopts) {
-        Runtime::Current()->GetThreadList()->ForEach(InstrumentationRestoreStack, this);
-        // Only do this after restoring, as walking the stack when restoring will see
-        // the instrumentation exit pc.
-        instrumentation_stubs_installed_ = false;
-      }
-    }
+    MaybeRestoreInstrumentationStack();
   }
 }
 
@@ -1171,13 +1177,11 @@
   CHECK(method->IsInvokable());
 
   Thread* self = Thread::Current();
-  bool empty;
   {
     WriterMutexLock mu(self, *GetDeoptimizedMethodsLock());
     bool found_and_erased = RemoveDeoptimizedMethod(method);
     CHECK(found_and_erased) << "Method " << ArtMethod::PrettyMethod(method)
         << " is not deoptimized";
-    empty = IsDeoptimizedMethodsEmptyLocked();
   }
 
   // Restore code and possibly stack only if we did not deoptimize everything.
@@ -1193,10 +1197,8 @@
     }
 
     // If there is no deoptimized method left, we can restore the stack of each thread.
-    if (empty && !EntryExitStubsInstalled()) {
-      MutexLock mu(self, *Locks::thread_list_lock_);
-      Runtime::Current()->GetThreadList()->ForEach(InstrumentationRestoreStack, this);
-      instrumentation_stubs_installed_ = false;
+    if (!EntryExitStubsInstalled()) {
+      MaybeRestoreInstrumentationStack();
     }
   }
 }
diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
index 0d00ccc..34f08e7 100644
--- a/runtime/instrumentation.h
+++ b/runtime/instrumentation.h
@@ -578,6 +578,10 @@
                !Locks::thread_list_lock_,
                !Locks::classlinker_classes_lock_);
 
+  // If there are no pending deoptimizations restores the stack to the normal state by updating the
+  // return pcs to actual return addresses from the instrumentation stack and clears the
+  // instrumentation stack.
+  void MaybeRestoreInstrumentationStack() REQUIRES(Locks::mutator_lock_);
 
   // No thread safety analysis to get around SetQuickAllocEntryPointsInstrumented requiring
   // exclusive access to mutator lock which you can't get if the runtime isn't started.