More JDWP functionality: breakpoints, single-stepping.

By-line still needs to be implemented. I've also implemented
VirtualMachine.Dispose so that it disconnects from the caller, as it's
supposed to. This was the cause of a lot of test hangs in otherwise
successful tests.

I've implemented ClassLoaderReference.VisibleClasses, though we don't have
enough information to only return the classes for which the given ClassLoader
was an initiating class loader. We pass the tests with this, but mainly because
the tests aren't very thorough. Sadly, VirtualMachine.CapabilitiesNew doesn't
let us say we don't support this. (Though we could still just return
ERR_NOT_IMPLEMENTED...)

Change-Id: I8d02c2b568a77a4c1725bc737a8ee844f4591e81
diff --git a/src/debugger.cc b/src/debugger.cc
index 6f381d3..cd1081d 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -22,6 +22,7 @@
 
 #include "class_linker.h"
 #include "class_loader.h"
+#include "dex_verifier.h" // For Instruction.
 #include "context.h"
 #include "object_utils.h"
 #include "ScopedLocalRef.h"
@@ -112,6 +113,31 @@
   }
 };
 
+struct Breakpoint {
+  Method* method;
+  uint32_t pc;
+  Breakpoint(Method* method, uint32_t pc) : method(method), pc(pc) {}
+};
+
+static std::ostream& operator<<(std::ostream& os, const Breakpoint& rhs) {
+  os << "Breakpoint[" << PrettyMethod(rhs.method) << " @" << rhs.pc << "]";
+  return os;
+}
+
+struct SingleStepControl {
+  // Are we single-stepping right now?
+  bool is_active;
+  Thread* thread;
+
+  JDWP::JdwpStepSize step_size;
+  JDWP::JdwpStepDepth step_depth;
+
+  const Method* method;
+  int line; // May be -1.
+  //const AddressSet* pAddressSet;    /* if non-null, address set for line */
+  int stack_depth;
+};
+
 // JDWP is allowed unless the Zygote forbids it.
 static bool gJdwpAllowed = true;
 
@@ -125,6 +151,7 @@
 static JDWP::JdwpState* gJdwpState = NULL;
 static bool gDebuggerConnected;  // debugger or DDMS is connected.
 static bool gDebuggerActive;     // debugger is making requests.
+static bool gDisposed;           // debugger called VirtualMachine.Dispose, so we should drop the connection.
 
 static bool gDdmThreadNotification = false;
 
@@ -143,6 +170,22 @@
 static size_t gAllocRecordHead = 0;
 static size_t gAllocRecordCount = 0;
 
+// Breakpoints and single-stepping.
+static Mutex gBreakpointsLock("breakpoints lock");
+static std::vector<Breakpoint> gBreakpoints;
+static SingleStepControl gSingleStepControl;
+
+static bool IsBreakpoint(Method* m, uint32_t dex_pc) {
+  MutexLock mu(gBreakpointsLock);
+  for (size_t i = 0; i < gBreakpoints.size(); ++i) {
+    if (gBreakpoints[i].method == m && gBreakpoints[i].pc == dex_pc) {
+      VLOG(jdwp) << "Hit breakpoint #" << i << ": " << gBreakpoints[i];
+      return true;
+    }
+  }
+  return false;
+}
+
 static JDWP::JdwpTag BasicTagFromDescriptor(const char* descriptor) {
   // JDWP deliberately uses the descriptor characters' ASCII values for its enum.
   // Note that by "basic" we mean that we don't get more specific than JT_OBJECT.
@@ -379,6 +422,15 @@
   CHECK(!gDebuggerConnected);
   VLOG(jdwp) << "JDWP has attached";
   gDebuggerConnected = true;
+  gDisposed = false;
+}
+
+void Dbg::Disposed() {
+  gDisposed = true;
+}
+
+bool Dbg::IsDisposed() {
+  return gDisposed;
 }
 
 void Dbg::GoActive() {
@@ -493,6 +545,12 @@
   return o->AsClass();
 }
 
+static Thread* DecodeThread(JDWP::ObjectId threadId) {
+  Object* thread_peer = gRegistry->Get<Object*>(threadId);
+  CHECK(thread_peer != NULL);
+  return Thread::FromManagedThread(thread_peer);
+}
+
 JDWP::JdwpError Dbg::GetSuperclass(JDWP::RefTypeId id, JDWP::RefTypeId& superclassId) {
   JDWP::JdwpError status;
   Class* c = DecodeClass(id, status);
@@ -557,10 +615,6 @@
   Runtime::Current()->GetClassLinker()->VisitClasses(ClassListCreator::Visit, &clc);
 }
 
-void Dbg::GetVisibleClassList(JDWP::ObjectId classLoaderId, uint32_t* pNumClasses, JDWP::RefTypeId** pClassRefBuf) {
-  UNIMPLEMENTED(FATAL);
-}
-
 bool Dbg::GetClassInfo(JDWP::RefTypeId classId, JDWP::JdwpTypeTag* pTypeTag, uint32_t* pStatus, std::string* pDescriptor) {
   Object* o = gRegistry->Get<Object*>(classId);
   if (o == NULL || !o->IsClass()) {
@@ -607,11 +661,6 @@
   *pRefTypeId = gRegistry->Add(o->GetClass());
 }
 
-uint8_t Dbg::GetClassObjectType(JDWP::RefTypeId refTypeId) {
-  UNIMPLEMENTED(FATAL);
-  return 0;
-}
-
 JDWP::JdwpError Dbg::GetSignature(JDWP::RefTypeId refTypeId, std::string& signature) {
   JDWP::JdwpError status;
   Class* c = DecodeClass(refTypeId, status);
@@ -797,7 +846,7 @@
   return gRegistry->Get<Class*>(instClassId)->InstanceOf(gRegistry->Get<Class*>(classId));
 }
 
-JDWP::FieldId ToFieldId(const Field* f) {
+static JDWP::FieldId ToFieldId(const Field* f) {
 #ifdef MOVING_GARBAGE_COLLECTOR
   UNIMPLEMENTED(FATAL);
 #else
@@ -805,7 +854,7 @@
 #endif
 }
 
-JDWP::MethodId ToMethodId(const Method* m) {
+static JDWP::MethodId ToMethodId(const Method* m) {
 #ifdef MOVING_GARBAGE_COLLECTOR
   UNIMPLEMENTED(FATAL);
 #else
@@ -813,7 +862,7 @@
 #endif
 }
 
-Field* FromFieldId(JDWP::FieldId fid) {
+static Field* FromFieldId(JDWP::FieldId fid) {
 #ifdef MOVING_GARBAGE_COLLECTOR
   UNIMPLEMENTED(FATAL);
 #else
@@ -821,7 +870,7 @@
 #endif
 }
 
-Method* FromMethodId(JDWP::MethodId mid) {
+static Method* FromMethodId(JDWP::MethodId mid) {
 #ifdef MOVING_GARBAGE_COLLECTOR
   UNIMPLEMENTED(FATAL);
 #else
@@ -829,7 +878,7 @@
 #endif
 }
 
-void SetLocation(JDWP::JdwpLocation& location, Method* m, uintptr_t native_pc) {
+static void SetLocation(JDWP::JdwpLocation& location, Method* m, uintptr_t native_pc) {
   if (m == NULL) {
     memset(&location, 0, sizeof(location));
   } else {
@@ -1127,12 +1176,6 @@
   return s->ToModifiedUtf8();
 }
 
-Thread* DecodeThread(JDWP::ObjectId threadId) {
-  Object* thread_peer = gRegistry->Get<Object*>(threadId);
-  CHECK(thread_peer != NULL);
-  return Thread::FromManagedThread(thread_peer);
-}
-
 bool Dbg::GetThreadName(JDWP::ObjectId threadId, std::string& name) {
   ScopedThreadListLock thread_list_lock;
   Thread* thread = DecodeThread(threadId);
@@ -1286,8 +1329,7 @@
   GetThreadGroupThreadsImpl(NULL, ppThreadIds, pThreadCount);
 }
 
-int Dbg::GetThreadFrameCount(JDWP::ObjectId threadId) {
-  ScopedThreadListLock thread_list_lock;
+static int GetStackDepth(Thread* thread) {
   struct CountStackDepthVisitor : public Thread::StackVisitor {
     CountStackDepthVisitor() : depth(0) {}
     virtual void VisitFrame(const Frame& f, uintptr_t) {
@@ -1299,10 +1341,15 @@
     size_t depth;
   };
   CountStackDepthVisitor visitor;
-  DecodeThread(threadId)->WalkStack(&visitor);
+  thread->WalkStack(&visitor);
   return visitor.depth;
 }
 
+int Dbg::GetThreadFrameCount(JDWP::ObjectId threadId) {
+  ScopedThreadListLock thread_list_lock;
+  return GetStackDepth(DecodeThread(threadId));
+}
+
 bool Dbg::GetThreadFrame(JDWP::ObjectId threadId, int desired_frame_number, JDWP::FrameId* pFrameId, JDWP::JdwpLocation* pLoc) {
   ScopedThreadListLock thread_list_lock;
   struct GetFrameVisitor : public Thread::StackVisitor {
@@ -1597,6 +1644,9 @@
     return;
   }
 
+  Frame f(sp);
+  f.Next(); // Skip callee save frame.
+  Method* m = f.GetMethod();
   int event_flags = 0;
 
   // Update xtra.currentPc on every instruction.  We need to do this if
@@ -1611,85 +1661,73 @@
     event_flags |= kMethodEntry;
   }
 
-  /*
   // See if we have a breakpoint here.
   // Depending on the "mods" associated with event(s) on this address,
   // we may or may not actually send a message to the debugger.
-  if (GET_OPCODE(*pc) == OP_BREAKPOINT) {
-    ALOGV("+++ breakpoint hit at %p", pc);
-    event_flags |= kBreakPoint;
+  if (IsBreakpoint(m, dex_pc)) {
+    event_flags |= kBreakpoint;
   }
-  */
 
-  /*
   // If the debugger is single-stepping one of our threads, check to
   // see if we're that thread and we've reached a step point.
-  const StepControl* pCtrl = &gDvm.stepControl;
-  if (pCtrl->active && pCtrl->thread == self) {
-    int frameDepth;
-    bool doStop = false;
-    const char* msg = NULL;
-
-    assert(!dvmIsNativeMethod(method));
-
-    if (pCtrl->depth == SD_INTO) {
+  if (gSingleStepControl.is_active && gSingleStepControl.thread == self) {
+    CHECK(!m->IsNative());
+    if (gSingleStepControl.step_depth == JDWP::SD_INTO) {
       // Step into method calls.  We break when the line number
       // or method pointer changes.  If we're in SS_MIN mode, we
       // always stop.
-      if (pCtrl->method != method) {
-        doStop = true;
-        msg = "new method";
-      } else if (pCtrl->size == SS_MIN) {
-        doStop = true;
-        msg = "new instruction";
-      } else if (!dvmAddressSetGet(pCtrl->pAddressSet, pc - method->insns)) {
-        doStop = true;
-        msg = "new line";
+      if (gSingleStepControl.method != m) {
+        event_flags |= kSingleStep;
+        VLOG(jdwp) << "SS new method";
+      } else if (gSingleStepControl.step_size == JDWP::SS_MIN) {
+        event_flags |= kSingleStep;
+        VLOG(jdwp) << "SS new instruction";
+//      } else if (!dvmAddressSetGet(gSingleStepControl.pAddressSet, pc - method->insns)) {
+//        event_flags |= kSingleStep;
+//        VLOG(jdwp) << "SS new line";
       }
-    } else if (pCtrl->depth == SD_OVER) {
+    } else if (gSingleStepControl.step_depth == JDWP::SD_OVER) {
       // Step over method calls.  We break when the line number is
       // different and the frame depth is <= the original frame
       // depth.  (We can't just compare on the method, because we
       // might get unrolled past it by an exception, and it's tricky
       // to identify recursion.)
-      frameDepth = dvmComputeVagueFrameDepth(self, fp);
-      if (frameDepth < pCtrl->frameDepth) {
+
+      // TODO: can we just use the value of 'sp'?
+      int stack_depth = GetStackDepth(self);
+
+      if (stack_depth < gSingleStepControl.stack_depth) {
         // popped up one or more frames, always trigger
-        doStop = true;
-        msg = "method pop";
-      } else if (frameDepth == pCtrl->frameDepth) {
+        event_flags |= kSingleStep;
+        VLOG(jdwp) << "SS method pop";
+      } else if (stack_depth == gSingleStepControl.stack_depth) {
         // same depth, see if we moved
-        if (pCtrl->size == SS_MIN) {
-          doStop = true;
-          msg = "new instruction";
-        } else if (!dvmAddressSetGet(pCtrl->pAddressSet, pc - method->insns)) {
-          doStop = true;
-          msg = "new line";
+        if (gSingleStepControl.step_size == JDWP::SS_MIN) {
+          event_flags |= kSingleStep;
+          VLOG(jdwp) << "SS new instruction";
+//        } else if (!dvmAddressSetGet(gSingleStepControl.pAddressSet, pc - method->insns)) {
+//          event_flags |= kSingleStep;
+//          VLOG(jdwp) << "SS new line";
         }
       }
     } else {
-      assert(pCtrl->depth == SD_OUT);
+      CHECK_EQ(gSingleStepControl.step_depth, JDWP::SD_OUT);
       // Return from the current method.  We break when the frame
       // depth pops up.
 
       // This differs from the "method exit" break in that it stops
       // with the PC at the next instruction in the returned-to
       // function, rather than the end of the returning function.
-      frameDepth = dvmComputeVagueFrameDepth(self, fp);
-      if (frameDepth < pCtrl->frameDepth) {
-        doStop = true;
-        msg = "method pop";
+
+      // TODO: can we just use the value of 'sp'?
+      int stack_depth = GetStackDepth(self);
+      if (stack_depth < gSingleStepControl.stack_depth) {
+        event_flags |= kSingleStep;
+        VLOG(jdwp) << "SS method pop";
       }
     }
-
-    if (doStop) {
-      ALOGV("#####S %s", msg);
-      event_flags |= kSingleStep;
-    }
   }
-  */
 
-  /*
   // Check to see if this is a "return" instruction.  JDWP says we should
   // send the event *after* the code has been executed, but it also says
   // the location we provide is the last instruction.  Since the "return"
@@ -1698,37 +1736,80 @@
   // we potentially need to combine it with other events.)
   // We're also not supposed to generate a method exit event if the method
   // terminates "with a thrown exception".
-  u2 opcode = GET_OPCODE(*pc);
-  if (opcode == OP_RETURN_VOID || opcode == OP_RETURN || opcode == OP_RETURN_WIDE ||opcode == OP_RETURN_OBJECT) {
-    event_flags |= kMethodExit;
+  if (dex_pc >= 0) {
+    const DexFile::CodeItem* code_item = MethodHelper(m).GetCodeItem();
+    CHECK(code_item != NULL);
+    CHECK_LT(dex_pc, static_cast<int32_t>(code_item->insns_size_in_code_units_));
+    if (Instruction::At(&code_item->insns_[dex_pc])->IsReturn()) {
+      event_flags |= kMethodExit;
+    }
   }
-  */
 
   // If there's something interesting going on, see if it matches one
   // of the debugger filters.
   if (event_flags != 0) {
-    Frame f(sp);
-    f.Next(); // Skip callee save frame.
-    Dbg::PostLocationEvent(f.GetMethod(), dex_pc, GetThis(f), event_flags);
+    Dbg::PostLocationEvent(m, dex_pc, GetThis(f), event_flags);
   }
 }
 
-bool Dbg::WatchLocation(const JDWP::JdwpLocation* pLoc) {
-  UNIMPLEMENTED(FATAL);
-  return false;
+void Dbg::WatchLocation(const JDWP::JdwpLocation* location) {
+  MutexLock mu(gBreakpointsLock);
+  Method* m = FromMethodId(location->methodId);
+  gBreakpoints.push_back(Breakpoint(m, location->idx));
+  VLOG(jdwp) << "Set breakpoint #" << (gBreakpoints.size() - 1) << ": " << gBreakpoints[gBreakpoints.size() - 1];
 }
 
-void Dbg::UnwatchLocation(const JDWP::JdwpLocation* pLoc) {
-  UNIMPLEMENTED(FATAL);
+void Dbg::UnwatchLocation(const JDWP::JdwpLocation* location) {
+  MutexLock mu(gBreakpointsLock);
+  Method* m = FromMethodId(location->methodId);
+  for (size_t i = 0; i < gBreakpoints.size(); ++i) {
+    if (gBreakpoints[i].method == m && gBreakpoints[i].pc == location->idx) {
+      VLOG(jdwp) << "Removed breakpoint #" << i << ": " << gBreakpoints[i];
+      gBreakpoints.erase(gBreakpoints.begin() + i);
+      return;
+    }
+  }
 }
 
-bool Dbg::ConfigureStep(JDWP::ObjectId threadId, JDWP::JdwpStepSize size, JDWP::JdwpStepDepth depth) {
-  UNIMPLEMENTED(FATAL);
-  return false;
+bool Dbg::ConfigureStep(JDWP::ObjectId threadId, JDWP::JdwpStepSize step_size, JDWP::JdwpStepDepth step_depth) {
+  Thread* thread = DecodeThread(threadId);
+
+  // TODO: there's no theoretical reason why we couldn't support single-stepping
+  // of multiple threads at once, but we never did so historically.
+  if (gSingleStepControl.thread != NULL && thread != gSingleStepControl.thread) {
+    LOG(WARNING) << "single-step already active for " << *gSingleStepControl.thread
+                 << "; switching to " << *thread;
+  }
+
+  struct SingleStepStackVisitor : public Thread::StackVisitor {
+    SingleStepStackVisitor() {
+      gSingleStepControl.method = NULL;
+      gSingleStepControl.stack_depth = 0;
+    }
+    virtual void VisitFrame(const Frame& f, uintptr_t) {
+      // TODO: we'll need to skip callee-save frames too.
+      if (f.HasMethod()) {
+        ++gSingleStepControl.stack_depth;
+        if (gSingleStepControl.method == NULL) {
+          gSingleStepControl.method = f.GetMethod();
+        }
+      }
+    }
+  };
+  SingleStepStackVisitor visitor;
+  thread->WalkStack(&visitor);
+
+  gSingleStepControl.thread = thread;
+  gSingleStepControl.step_size = step_size;
+  gSingleStepControl.step_depth = step_depth;
+  gSingleStepControl.is_active = true;
+
+  return true;
 }
 
 void Dbg::UnconfigureStep(JDWP::ObjectId threadId) {
-  UNIMPLEMENTED(FATAL);
+  gSingleStepControl.is_active = false;
+  gSingleStepControl.thread = NULL;
 }
 
 JDWP::JdwpError Dbg::InvokeMethod(JDWP::ObjectId threadId, JDWP::ObjectId objectId, JDWP::RefTypeId classId, JDWP::MethodId methodId, uint32_t numArgs, uint64_t* argArray, uint32_t options, JDWP::JdwpTag* pResultTag, uint64_t* pResultValue, JDWP::ObjectId* pExceptionId) {
diff --git a/src/debugger.h b/src/debugger.h
index dbb99ff..5089995 100644
--- a/src/debugger.h
+++ b/src/debugger.h
@@ -93,6 +93,7 @@
   static void Connected();
   static void GoActive();
   static void Disconnected();
+  static void Disposed();
 
   /*
    * Returns "true" if a debugger is connected.  Returns "false" if it's
@@ -102,6 +103,8 @@
 
   static bool IsDebuggingEnabled();
 
+  static bool IsDisposed();
+
   /*
    * Time, in milliseconds, since the last debugger activity.  Does not
    * include DDMS activity.  Returns -1 if there has been no activity.
@@ -135,11 +138,9 @@
   static bool GetAccessFlags(JDWP::RefTypeId id, uint32_t& access_flags);
   static bool IsInterface(JDWP::RefTypeId classId, bool& is_interface);
   static void GetClassList(std::vector<JDWP::RefTypeId>& classes);
-  static void GetVisibleClassList(JDWP::ObjectId classLoaderId, uint32_t* pNumClasses, JDWP::RefTypeId** pClassRefBuf);
   static bool GetClassInfo(JDWP::RefTypeId classId, JDWP::JdwpTypeTag* pTypeTag, uint32_t* pStatus, std::string* pDescriptor);
   static void FindLoadedClassBySignature(const char* descriptor, std::vector<JDWP::RefTypeId>& ids);
   static void GetObjectType(JDWP::ObjectId objectId, JDWP::JdwpTypeTag* pRefTypeTag, JDWP::RefTypeId* pRefTypeId);
-  static uint8_t GetClassObjectType(JDWP::RefTypeId refTypeId);
   static JDWP::JdwpError GetSignature(JDWP::RefTypeId refTypeId, std::string& signature);
   static bool GetSourceFile(JDWP::RefTypeId refTypeId, std::string& source_file);
   static uint8_t GetObjectTag(JDWP::ObjectId objectId);
@@ -209,7 +210,7 @@
    * Debugger notification
    */
   enum {
-    kBreakPoint     = 0x01,
+    kBreakpoint     = 0x01,
     kSingleStep     = 0x02,
     kMethodEntry    = 0x04,
     kMethodExit     = 0x08,
@@ -222,7 +223,7 @@
 
   static void UpdateDebugger(int32_t dex_pc, Thread* self, Method** sp);
 
-  static bool WatchLocation(const JDWP::JdwpLocation* pLoc);
+  static void WatchLocation(const JDWP::JdwpLocation* pLoc);
   static void UnwatchLocation(const JDWP::JdwpLocation* pLoc);
   static bool ConfigureStep(JDWP::ObjectId threadId, JDWP::JdwpStepSize size, JDWP::JdwpStepDepth depth);
   static void UnconfigureStep(JDWP::ObjectId threadId);
diff --git a/src/jdwp/jdwp_event.cc b/src/jdwp/jdwp_event.cc
index c0e6295..d68d011 100644
--- a/src/jdwp/jdwp_event.cc
+++ b/src/jdwp/jdwp_event.cc
@@ -760,7 +760,7 @@
 
   {
     MutexLock mu(event_lock_);
-    if ((eventFlags & Dbg::kBreakPoint) != 0) {
+    if ((eventFlags & Dbg::kBreakpoint) != 0) {
       FindMatchingEvents(EK_BREAKPOINT, &basket, matchList, &matchCount);
     }
     if ((eventFlags & Dbg::kSingleStep) != 0) {
@@ -771,11 +771,14 @@
     }
     if ((eventFlags & Dbg::kMethodExit) != 0) {
       FindMatchingEvents(EK_METHOD_EXIT, &basket, matchList, &matchCount);
+
+      // TODO: match EK_METHOD_EXIT_WITH_RETURN_VALUE too; we need to include the 'value', though.
+      //FindMatchingEvents(EK_METHOD_EXIT_WITH_RETURN_VALUE, &basket, matchList, &matchCount);
     }
     if (matchCount != 0) {
       VLOG(jdwp) << "EVENT: " << matchList[0]->eventKind << "(" << matchCount << " total) "
-                   << basket.className << "." << Dbg::GetMethodName(pLoc->classId, pLoc->methodId)
-                   << " thread=" << (void*) basket.threadId << " code=" << (void*) pLoc->idx << ")";
+                 << basket.className << "." << Dbg::GetMethodName(pLoc->classId, pLoc->methodId)
+                 << " thread=" << (void*) basket.threadId << " code=" << (void*) pLoc->idx << ")";
 
       suspendPolicy = scanSuspendPolicy(matchList, matchCount);
       VLOG(jdwp) << "  suspendPolicy=" << suspendPolicy;
diff --git a/src/jdwp/jdwp_handler.cc b/src/jdwp/jdwp_handler.cc
index 3dbf437..d49454d 100644
--- a/src/jdwp/jdwp_handler.cc
+++ b/src/jdwp/jdwp_handler.cc
@@ -275,13 +275,8 @@
   return ERR_NONE;
 }
 
-/*
- * The debugger is politely asking to disconnect.  We're good with that.
- *
- * We could resume threads and clean up pinned references, but we can do
- * that when the TCP connection drops.
- */
 static JdwpError handleVM_Dispose(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
+  Dbg::Disposed();
   return ERR_NONE;
 }
 
@@ -416,7 +411,7 @@
   return ERR_NONE;
 }
 
-static JdwpError handleVM_AllClasses(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply, bool generic) {
+static JdwpError handleVM_AllClasses(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply, bool descriptor_and_status, bool generic) {
   std::vector<JDWP::RefTypeId> classes;
   Dbg::GetClassList(classes);
 
@@ -433,22 +428,24 @@
 
     expandBufAdd1(pReply, refTypeTag);
     expandBufAddRefTypeId(pReply, classes[i]);
-    expandBufAddUtf8String(pReply, descriptor);
-    if (generic) {
-      expandBufAddUtf8String(pReply, genericSignature);
+    if (descriptor_and_status) {
+      expandBufAddUtf8String(pReply, descriptor);
+      if (generic) {
+        expandBufAddUtf8String(pReply, genericSignature);
+      }
+      expandBufAdd4BE(pReply, status);
     }
-    expandBufAdd4BE(pReply, status);
   }
 
   return ERR_NONE;
 }
 
 static JdwpError handleVM_AllClasses(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
-  return handleVM_AllClasses(state, buf, dataLen, pReply, false);
+  return handleVM_AllClasses(state, buf, dataLen, pReply, true, false);
 }
 
 static JdwpError handleVM_AllClassesWithGeneric(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
-  return handleVM_AllClasses(state, buf, dataLen, pReply, true);
+  return handleVM_AllClasses(state, buf, dataLen, pReply, true, true);
 }
 
 /*
@@ -1187,56 +1184,13 @@
   return Dbg::SetArrayElements(arrayId, firstIndex, values, buf);
 }
 
-/*
- * Return the set of classes visible to a class loader.  All classes which
- * have the class loader as a defining or initiating loader are returned.
- */
 static JdwpError handleCLR_VisibleClasses(JdwpState* state, const uint8_t* buf, int dataLen, ExpandBuf* pReply) {
   ObjectId classLoaderObject;
-  uint32_t numClasses = 0;
-  RefTypeId* classRefBuf = NULL;
-  int i;
-
   classLoaderObject = ReadObjectId(&buf);
-
-  Dbg::GetVisibleClassList(classLoaderObject, &numClasses, &classRefBuf);
-
-  expandBufAdd4BE(pReply, numClasses);
-  for (i = 0; i < (int) numClasses; i++) {
-    uint8_t refTypeTag = Dbg::GetClassObjectType(classRefBuf[i]);
-
-    expandBufAdd1(pReply, refTypeTag);
-    expandBufAddRefTypeId(pReply, classRefBuf[i]);
-  }
-
-  return ERR_NONE;
-}
-
-/*
- * Return a newly-allocated string in which all occurrences of '.' have
- * been changed to '/'.  If we find a '/' in the original string, NULL
- * is returned to avoid ambiguity.
- */
-char* dvmDotToSlash(const char* str) {
-  char* newStr = strdup(str);
-  char* cp = newStr;
-
-  if (newStr == NULL) {
-    return NULL;
-  }
-
-  while (*cp != '\0') {
-    if (*cp == '/') {
-      CHECK(false);
-      return NULL;
-    }
-    if (*cp == '.') {
-      *cp = '/';
-    }
-    cp++;
-  }
-
-  return newStr;
+  // TODO: we should only return classes which have the given class loader as a defining or
+  // initiating loader. The former would be easy; the latter is hard, because we don't have
+  // any such notion.
+  return handleVM_AllClasses(state, buf, dataLen, pReply, false, false);
 }
 
 /*
@@ -1305,17 +1259,20 @@
       break;
     case MK_CLASS_MATCH:    /* restrict events to matching classes */
       {
+        // pattern is "java.foo.*", we want "java/foo/*".
         std::string pattern(ReadNewUtf8String(&buf));
+        std::replace(pattern.begin(), pattern.end(), '.', '/');
         VLOG(jdwp) << StringPrintf("    ClassMatch: '%s'", pattern.c_str());
-        /* pattern is "java.foo.*", we want "java/foo/ *" */
-        pEvent->mods[idx].classMatch.classPattern = dvmDotToSlash(pattern.c_str());
+        pEvent->mods[idx].classMatch.classPattern = strdup(pattern.c_str());
       }
       break;
     case MK_CLASS_EXCLUDE:  /* restrict events to non-matching classes */
       {
+        // pattern is "java.foo.*", we want "java/foo/*".
         std::string pattern(ReadNewUtf8String(&buf));
+        std::replace(pattern.begin(), pattern.end(), '.', '/');
         VLOG(jdwp) << StringPrintf("    ClassExclude: '%s'", pattern.c_str());
-        pEvent->mods[idx].classExclude.classPattern = dvmDotToSlash(pattern.c_str());
+        pEvent->mods[idx].classExclude.classPattern = strdup(pattern.c_str());
       }
       break;
     case MK_LOCATION_ONLY:  /* restrict certain events based on loc */
diff --git a/src/jdwp/jdwp_main.cc b/src/jdwp/jdwp_main.cc
index 72aff5c..e408a76 100644
--- a/src/jdwp/jdwp_main.cc
+++ b/src/jdwp/jdwp_main.cc
@@ -325,7 +325,7 @@
 
     /* process requests until the debugger drops */
     bool first = true;
-    while (true) {
+    while (!Dbg::IsDisposed()) {
       // sanity check -- shouldn't happen?
       if (Thread::Current()->GetState() != Thread::kVmWait) {
         LOG(ERROR) << "JDWP thread no longer in VMWAIT (now " << Thread::Current()->GetState() << "); resetting";