Don't assume debugger wants all exceptions.

The JDWP implementation in the VM keeps a list of the objects that the
debugger knows about, and prevents the GC from collecting them (which
isn't strictly necessary, but it's a whole lot easier than doing it
right).  Because of the way it's implemented, it actually ended up
keeping track of all thrown exceptions, even if the debugger wasn't
interested in hearing about them.

With this change we now do a "late" registration of the exception
object, preventing exception-happy code from filling memory when the
debugger is attached.

4 files changed, 36 insertions, 8 deletions

diff --git a/docs/debugger.html b/docs/debugger.html
index 033eeb62d..919739d71 100644
--- a/docs/debugger.html
+++ b/docs/debugger.html
@@ -197,12 +197,6 @@ debugger is connected.  For example, if the debugger sees a running
 thread, the associated Thread object will not be collected, even after
 the thread terminates.
 </p><p>
-The situation is exacerbated by a flaw in the exception processing code,
-which results in nearly all exceptions being added to the "do not discard"
-list, even if the debugger never sees them.  Having a debugger attached
-to a program that throws lots of exceptions can result in out-of-memory
-errors.  This will be fixed in a future release.
-</p><p>
 The only way to "unlock" the references is to detach and reattach the
 debugger.
 </p><p>
diff --git a/vm/Debugger.c b/vm/Debugger.c
index 4891d93ce..67a83a343 100644
--- a/vm/Debugger.c
+++ b/vm/Debugger.c
@@ -321,6 +321,20 @@ static Object* objectIdToObject(ObjectId id)
 }
 
 /*
+ * Register an object ID that might not have been registered previously.
+ *
+ * Normally this wouldn't happen -- the conversion to an ObjectId would
+ * have added the object to the registry -- but in some cases (e.g.
+ * throwing exceptions) we really want to do the registration late.
+ */
+void dvmDbgRegisterObjectId(ObjectId id)
+{
+    Object* obj = (Object*)(u4) id;
+    LOGV("+++ registering %p (%s)\n", obj, obj->clazz->descriptor);
+    registerObject(obj, kObjectId, true);
+}
+
+/*
  * Convert to/from a MethodId.
  *
  * These IDs are only guaranteed unique within a class, so they could be
@@ -2496,7 +2510,7 @@ void dvmDbgPostLocationEvent(const Method* method, int pcOffset,
 
     /*
      * Note we use "NoReg" so we don't keep track of references that are
-     * never actually sent to the debugger.  The "thisPtr" is used to
+     * never actually sent to the debugger.  The "thisPtr" is only used to
      * compare against registered events.
      */
 
@@ -2543,7 +2557,17 @@ void dvmDbgPostException(void* throwFp, int throwRelPc, void* catchFp,
     /* need this for InstanceOnly filters */
     Object* thisObj = getThisObject(throwFp);
 
-    dvmJdwpPostException(gDvm.jdwpState, &throwLoc, objectToObjectId(exception),
+    /*
+     * Hand the event to the JDWP exception handler.  Note we're using the
+     * "NoReg" objectID on the exception, which is not strictly correct --
+     * the exception object WILL be passed up to the debugger if the
+     * debugger is interested in the event.  We do this because the current
+     * implementation of the debugger object registry never throws anything
+     * away, and some people were experiencing a fatal build up of exception
+     * objects when dealing with certain libraries.
+     */
+    dvmJdwpPostException(gDvm.jdwpState, &throwLoc,
+        objectToObjectIdNoReg(exception),
         classObjectToRefTypeId(exception->clazz), &catchLoc,
         objectToObjectId(thisObj));
 }
diff --git a/vm/Debugger.h b/vm/Debugger.h
index eb9c43926..1eea04ce4 100644
--- a/vm/Debugger.h
+++ b/vm/Debugger.h
@@ -291,6 +291,9 @@ void dvmDbgExecuteMethod(DebugInvokeReq* pReq);
 /* Make an AddressSet for a line, for single stepping */
 const AddressSet *dvmAddressSetForLine(const struct Method* method, int line);
 
+/* perform "late registration" of an object ID */
+void dvmDbgRegisterObjectId(ObjectId id);
+
 /*
  * DDM support.
  */
diff --git a/vm/jdwp/JdwpEvent.c b/vm/jdwp/JdwpEvent.c
index a3ff05ae7..3233463a4 100644
--- a/vm/jdwp/JdwpEvent.c
+++ b/vm/jdwp/JdwpEvent.c
@@ -1026,6 +1026,10 @@ bool dvmJdwpPostVMDeath(JdwpState* state)
  * Valid mods:
  *  Count, ThreadOnly, ClassOnly, ClassMatch, ClassExclude, LocationOnly,
  *    ExceptionOnly, InstanceOnly
+ *
+ * The "exceptionId" has not been added to the GC-visible object registry,
+ * because there's a pretty good chance that we're not going to send it
+ * up the debugger.
  */
 bool dvmJdwpPostException(JdwpState* state, const JdwpLocation* pThrowLoc,
     ObjectId exceptionId, RefTypeId exceptionClassId,
@@ -1100,6 +1104,9 @@ bool dvmJdwpPostException(JdwpState* state, const JdwpLocation* pThrowLoc,
             expandBufAdd8BE(pReq, exceptionId);
             dvmJdwpAddLocation(pReq, pCatchLoc);
         }
+
+        /* don't let the GC discard it */
+        dvmDbgRegisterObjectId(exceptionId);
     }
 
     cleanupMatchList(state, matchList, matchCount);