Pull 138592-p9 from master to donut.

Switch from VMWAIT to RUNNING when executing code on behalf of the
debugger.  Reset the thread-suspend lock timeout if we have to suspend
while trying to acquire it.

Also, clear any pending exception before calling interpreted code.

Reworked some log messages to make them easier to decipher.

(For internal bug 1952616.)

diff --git a/vm/Debugger.c b/vm/Debugger.c
index c667893..3affa97 100644 (file)
--- a/vm/Debugger.c
+++ b/vm/Debugger.c
@@ -2697,10 +2697,10 @@ JdwpError dvmDbgInvokeMethod(ObjectId threadId, ObjectId objectId,
     dvmUnlockThreadList();
 
     /*
-     * We change our thread status (which should be THREAD_RUNNING) so the
-     * VM can suspend for a GC if the invoke request causes us to run out
-     * of memory.  It's also a good idea to change it before locking the
-     * invokeReq mutex, although that should never be held for long.
+     * We change our (JDWP thread) status, which should be THREAD_RUNNING,
+     * so the VM can suspend for a GC if the invoke request causes us to
+     * run out of memory.  It's also a good idea to change it before locking
+     * the invokeReq mutex, although that should never be held for long.
      */
     Thread* self = dvmThreadSelf();
     int oldStatus = dvmChangeStatus(self, THREAD_VMWAIT);
@@ -2774,18 +2774,25 @@ static u1 resultTagFromSignature(const Method* method)
 
 /*
  * Execute the method described by "*pReq".
+ *
+ * We're currently in VMWAIT, because we're stopped on a breakpoint.  We
+ * want to switch to RUNNING while we execute.
  */
 void dvmDbgExecuteMethod(DebugInvokeReq* pReq)
 {
     Thread* self = dvmThreadSelf();
     const Method* meth;
     Object* oldExcept;
+    int oldStatus;
 
     /*
      * We can be called while an exception is pending in the VM.  We need
      * to preserve that across the method invocation.
      */
     oldExcept = dvmGetException(self);
+    dvmClearException(self);
+
+    oldStatus = dvmChangeStatus(self, THREAD_RUNNING);
 
     /*
      * Translate the method through the vtable, unless we're calling a
@@ -2832,6 +2839,7 @@ void dvmDbgExecuteMethod(DebugInvokeReq* pReq)
 
     if (oldExcept != NULL)
         dvmSetException(self, oldExcept);
+    dvmChangeStatus(self, oldStatus);
 }
 
 // for dvmAddressSetForLine

diff --git a/vm/Thread.c b/vm/Thread.c
index 0adde38..7116dfd 100644 (file)
--- a/vm/Thread.c
+++ b/vm/Thread.c
@@ -481,6 +481,20 @@ void dvmUnlockThreadList(void)
     assert(cc == 0);
 }
 
+/*
+ * Convert SuspendCause to a string.
+ */
+static const char* getSuspendCauseStr(SuspendCause why)
+{
+    switch (why) {
+    case SUSPEND_NOT:               return "NOT?";
+    case SUSPEND_FOR_GC:            return "gc";
+    case SUSPEND_FOR_DEBUG:         return "debug";
+    case SUSPEND_FOR_DEBUG_EVENT:   return "debug-event";
+    case SUSPEND_FOR_STACK_DUMP:    return "stack-dump";
+    default:                        return "UNKNOWN";
+    }
+}
 
 /*
  * Grab the "thread suspend" lock.  This is required to prevent the
@@ -492,7 +506,6 @@ void dvmUnlockThreadList(void)
  */
 static void lockThreadSuspend(const char* who, SuspendCause why)
 {
-    const int kMaxRetries = 10;
     const int kSpinSleepTime = 3*1000*1000;        /* 3s */
     u8 startWhen = 0;       // init req'd to placate gcc
     int sleepIter = 0;
@@ -503,23 +516,30 @@ static void lockThreadSuspend(const char* who, SuspendCause why)
         if (cc != 0) {
             if (!dvmCheckSuspendPending(NULL)) {
                 /*
-                 * Could be unusual JNI-attach thing, could be we hit
-                 * the window as the suspend or resume was started.  Could
-                 * also be the debugger telling us to resume at roughly
+                 * Could be we hit the window as the suspend or resume
+                 * was started (i.e. the lock has been grabbed but the
+                 * other thread hasn't yet set our "please suspend" flag).
+                 *
+                 * Could be an unusual JNI thread-attach thing.
+                 *
+                 * Could be the debugger telling us to resume at roughly
                  * the same time we're posting an event.
                  */
-                LOGI("threadid=%d ODD: thread-suspend lock held (%s:%d)"
-                     " but suspend not pending\n",
-                    dvmThreadSelf()->threadId, who, why);
+                LOGI("threadid=%d ODD: want thread-suspend lock (%s:%s),"
+                     " it's held, no suspend pending\n",
+                    dvmThreadSelf()->threadId, who, getSuspendCauseStr(why));
+            } else {
+                /* we suspended; reset timeout */
+                sleepIter = 0;
             }
 
             /* give the lock-holder a chance to do some work */
             if (sleepIter == 0)
                 startWhen = dvmGetRelativeTimeUsec();
             if (!dvmIterativeSleep(sleepIter++, kSpinSleepTime, startWhen)) {
-                LOGE("threadid=%d: couldn't get thread-suspend lock (%s:%d),"
+                LOGE("threadid=%d: couldn't get thread-suspend lock (%s:%s),"
                      " bailing\n",
-                    dvmThreadSelf()->threadId, who, why);
+                    dvmThreadSelf()->threadId, who, getSuspendCauseStr(why));
                 dvmDumpAllThreads(false);
                 dvmAbort();
             }
@@ -2202,8 +2222,15 @@ void dvmSuspendSelf(bool jdwpActivity)
                 &gDvm.threadSuspendCountLock);
         assert(cc == 0);
         if (self->suspendCount != 0) {
-            LOGD("threadid=%d: still suspended after undo (s=%d d=%d)\n",
-                self->threadId, self->suspendCount, self->dbgSuspendCount);
+            /*
+             * The condition was signaled but we're still suspended.  This
+             * can happen if the debugger lets go while a SIGQUIT thread
+             * dump event is pending (assuming SignalCatcher was resumed for
+             * just long enough to try to grab the thread-suspend lock).
+             */
+            LOGD("threadid=%d: still suspended after undo (sc=%d dc=%d s=%c)\n",
+                self->threadId, self->suspendCount, self->dbgSuspendCount,
+                self->isSuspended ? 'Y' : 'N');
         }
     }
     assert(self->suspendCount == 0 && self->dbgSuspendCount == 0);
@@ -2962,9 +2989,9 @@ void dvmDumpThreadEx(const DebugOutputTarget* target, Thread* thread,
         threadName, isDaemon ? " daemon" : "",
         priority, thread->threadId, kStatusNames[thread->status]);
     dvmPrintDebugMessage(target,
-        "  | group=\"%s\" sCount=%d dsCount=%d s=%d obj=%p\n",
+        "  | group=\"%s\" sCount=%d dsCount=%d s=%c obj=%p\n",
         groupName, thread->suspendCount, thread->dbgSuspendCount,
-        thread->isSuspended, thread->threadObj);
+        thread->isSuspended ? 'Y' : 'N', thread->threadObj);
     dvmPrintDebugMessage(target,
         "  | sysTid=%d nice=%d sched=%d/%d handle=%d\n",
         thread->systemTid, getpriority(PRIO_PROCESS, thread->systemTid),

diff --git a/vm/interp/Stack.c b/vm/interp/Stack.c
index 103d2b4..d5c5fe9 100644 (file)
--- a/vm/interp/Stack.c
+++ b/vm/interp/Stack.c
@@ -351,7 +351,8 @@ static ClassObject* callPrep(Thread* self, const Method* method, Object* obj,
 
 #ifndef NDEBUG
     if (self->status != THREAD_RUNNING) {
-        LOGW("Status=%d on call to %s.%s -\n", self->status,
+        LOGW("threadid=%d: status=%d on call to %s.%s -\n",
+            self->threadId, self->status,
             method->clazz->descriptor, method->name);
     }
 #endif