Remove breakpoints from redefined classes

Test: Manual
Change-Id: If8d9a38635bda7a0d69925b735b6f10055192b34

4 files changed, 69 insertions, 2 deletions

diff --git a/runtime/jdwp/jdwp.h b/runtime/jdwp/jdwp.h
index e5d34e1a2d..86af6d44db 100644
--- a/runtime/jdwp/jdwp.h
+++ b/runtime/jdwp/jdwp.h
@@ -22,6 +22,7 @@
 #include "jdwp/jdwp_bits.h"
 #include "jdwp/jdwp_constants.h"
 #include "jdwp/jdwp_expand_buf.h"
+#include "obj_ptr.h"
 
 #include <pthread.h>
 #include <stddef.h>
@@ -286,6 +287,10 @@ struct JdwpState {
       REQUIRES(!event_list_lock_)
       REQUIRES_SHARED(Locks::mutator_lock_);
 
+  void UnregisterLocationEventsOnClass(ObjPtr<mirror::Class> klass)
+      REQUIRES(!event_list_lock_)
+      REQUIRES_SHARED(Locks::mutator_lock_);
+
   /*
    * Unregister all events.
    */
diff --git a/runtime/jdwp/jdwp_event.cc b/runtime/jdwp/jdwp_event.cc
index 172f52a974..96249f9b58 100644
--- a/runtime/jdwp/jdwp_event.cc
+++ b/runtime/jdwp/jdwp_event.cc
@@ -251,6 +251,43 @@ JdwpError JdwpState::RegisterEvent(JdwpEvent* pEvent) {
   return ERR_NONE;
 }
 
+void JdwpState::UnregisterLocationEventsOnClass(ObjPtr<mirror::Class> klass) {
+  VLOG(jdwp) << "Removing events within " << klass->PrettyClass();
+  StackHandleScope<1> hs(Thread::Current());
+  Handle<mirror::Class> h_klass(hs.NewHandle(klass));
+  std::vector<JdwpEvent*> to_remove;
+  MutexLock mu(Thread::Current(), event_list_lock_);
+  for (JdwpEvent* cur_event = event_list_; cur_event != nullptr; cur_event = cur_event->next) {
+    // Fill in the to_remove list
+    bool found_event = false;
+    for (int i = 0; i < cur_event->modCount && !found_event; i++) {
+      JdwpEventMod& mod = cur_event->mods[i];
+      switch (mod.modKind) {
+        case MK_LOCATION_ONLY: {
+          JdwpLocation& loc = mod.locationOnly.loc;
+          JdwpError error;
+          ObjPtr<mirror::Class> breakpoint_class(
+              Dbg::GetObjectRegistry()->Get<art::mirror::Class*>(loc.class_id, &error));
+          DCHECK_EQ(error, ERR_NONE);
+          if (breakpoint_class == h_klass.Get()) {
+            to_remove.push_back(cur_event);
+            found_event = true;
+          }
+          break;
+        }
+        default:
+          // TODO Investigate how we should handle non-locationOnly events.
+          break;
+      }
+    }
+  }
+
+  for (JdwpEvent* event : to_remove) {
+    UnregisterEvent(event);
+    EventFree(event);
+  }
+}
+
 /*
  * Remove an event from the list.  This will also remove the event from
  * any optimization tables, e.g. breakpoints.
diff --git a/runtime/openjdkjvmti/ti_redefine.cc b/runtime/openjdkjvmti/ti_redefine.cc
index 4b8108accf..baa3b4ad02 100644
--- a/runtime/openjdkjvmti/ti_redefine.cc
+++ b/runtime/openjdkjvmti/ti_redefine.cc
@@ -38,12 +38,17 @@
 #include "art_jvmti.h"
 #include "base/array_slice.h"
 #include "base/logging.h"
+#include "debugger.h"
 #include "dex_file.h"
 #include "dex_file_types.h"
 #include "events-inl.h"
 #include "gc/allocation_listener.h"
 #include "gc/heap.h"
 #include "instrumentation.h"
+#include "jdwp/jdwp.h"
+#include "jdwp/jdwp_constants.h"
+#include "jdwp/jdwp_event.h"
+#include "jdwp/object_registry.h"
 #include "jit/jit.h"
 #include "jit/jit_code_cache.h"
 #include "jni_env_ext-inl.h"
@@ -749,6 +754,23 @@ bool Redefiner::ClassRedefinition::FinishRemainingAllocations(
   return true;
 }
 
+void Redefiner::ClassRedefinition::UnregisterBreakpoints() {
+  DCHECK(art::Dbg::IsDebuggerActive());
+  art::JDWP::JdwpState* state = art::Dbg::GetJdwpState();
+  if (state != nullptr) {
+    state->UnregisterLocationEventsOnClass(GetMirrorClass());
+  }
+}
+
+void Redefiner::UnregisterAllBreakpoints() {
+  if (LIKELY(!art::Dbg::IsDebuggerActive())) {
+    return;
+  }
+  for (Redefiner::ClassRedefinition& redef : redefinitions_) {
+    redef.UnregisterBreakpoints();
+  }
+}
+
 bool Redefiner::CheckAllRedefinitionAreValid() {
   for (Redefiner::ClassRedefinition& redef : redefinitions_) {
     if (!redef.CheckRedefinitionIsValid()) {
@@ -815,6 +837,7 @@ jvmtiError Redefiner::Run() {
     // cleaned up by the GC eventually.
     return result_;
   }
+  // At this point we can no longer fail without corrupting the runtime state.
   int32_t counter = 0;
   for (Redefiner::ClassRedefinition& redef : redefinitions_) {
     if (holder.GetSourceClassLoader(counter) == nullptr) {
@@ -822,6 +845,7 @@ jvmtiError Redefiner::Run() {
     }
     counter++;
   }
+  UnregisterAllBreakpoints();
   // Disable GC and wait for it to be done if we are a moving GC.  This is fine since we are done
   // allocating so no deadlocks.
   art::gc::Heap* heap = runtime_->GetHeap();
@@ -854,9 +878,7 @@ jvmtiError Redefiner::Run() {
                       holder.GetOriginalDexFileBytes(counter));
     counter++;
   }
-  // TODO Verify the new Class.
   // TODO Shrink the obsolete method maps if possible?
-  // TODO find appropriate class loader.
   // TODO Put this into a scoped thing.
   runtime_->GetThreadList()->ResumeAll();
   // Get back shared mutator lock as expected for return.
diff --git a/runtime/openjdkjvmti/ti_redefine.h b/runtime/openjdkjvmti/ti_redefine.h
index 5bcaef8971..8acd03de80 100644
--- a/runtime/openjdkjvmti/ti_redefine.h
+++ b/runtime/openjdkjvmti/ti_redefine.h
@@ -201,6 +201,8 @@ class Redefiner {
 
     void ReleaseDexFile() REQUIRES_SHARED(art::Locks::mutator_lock_);
 
+    void UnregisterBreakpoints() REQUIRES_SHARED(art::Locks::mutator_lock_);
+
    private:
     Redefiner* driver_;
     jclass klass_;
@@ -242,6 +244,7 @@ class Redefiner {
   bool FinishAllRemainingAllocations(RedefinitionDataHolder& holder)
       REQUIRES_SHARED(art::Locks::mutator_lock_);
   void ReleaseAllDexFiles() REQUIRES_SHARED(art::Locks::mutator_lock_);
+  void UnregisterAllBreakpoints() REQUIRES_SHARED(art::Locks::mutator_lock_);
 
   void RecordFailure(jvmtiError result, const std::string& class_sig, const std::string& error_msg);
   void RecordFailure(jvmtiError result, const std::string& error_msg) {