Merge "Try to show lock owner in MONITOR thread dump."

6 files changed, 168 insertions, 21 deletions

diff --git a/vm/Sync.c b/vm/Sync.c
index e6de6c3b6..fae35dea1 100644
--- a/vm/Sync.c
+++ b/vm/Sync.c
@@ -258,6 +258,21 @@ static u4 lockOwner(Object* obj)
 }
 
 /*
+ * Get the thread that holds the lock on the specified object.  The
+ * object may be unlocked, thin-locked, or fat-locked.
+ *
+ * The caller must lock the thread list before calling here.
+ */
+Thread* dvmGetObjectLockHolder(Object* obj)
+{
+    u4 threadId = lockOwner(obj);
+
+    if (threadId == 0)
+        return NULL;
+    return dvmGetThreadByThreadId(threadId);
+}
+
+/*
  * Checks whether the given thread holds the given
  * objects's lock.
  */
diff --git a/vm/Sync.h b/vm/Sync.h
index b273538da..1a168b98d 100644
--- a/vm/Sync.h
+++ b/vm/Sync.h
@@ -141,6 +141,14 @@ void dvmFreeMonitorList(void);
 struct Object* dvmGetMonitorObject(Monitor* mon);
 
 /*
+ * Get the thread that holds the lock on the specified object.  The
+ * object may be unlocked, thin-locked, or fat-locked.
+ *
+ * The caller must lock the thread list before calling here.
+ */
+struct Thread* dvmGetObjectLockHolder(struct Object* obj);
+
+/*
  * Checks whether the object is held by the specified thread.
  */
 bool dvmHoldsLock(struct Thread* thread, struct Object* obj);
diff --git a/vm/Thread.c b/vm/Thread.c
index 93759213f..434369403 100644
--- a/vm/Thread.c
+++ b/vm/Thread.c
@@ -3155,22 +3155,33 @@ Thread* dvmGetThreadFromThreadObject(Object* vmThreadObj)
 
 /*
  * Given a pthread handle, return the associated Thread*.
- * Caller must NOT hold the thread list lock.
+ * Caller must hold the thread list lock.
  *
  * Returns NULL if the thread was not found.
  */
 Thread* dvmGetThreadByHandle(pthread_t handle)
 {
-    dvmLockThreadList(NULL);
-    Thread* thread = gDvm.threadList;
-    while (thread != NULL) {
+    Thread* thread;
+    for (thread = gDvm.threadList; thread != NULL; thread = thread->next) {
         if (thread->handle == handle)
             break;
-
-        thread = thread->next;
     }
-    dvmUnlockThreadList();
+    return thread;
+}
 
+/*
+ * Given a threadId, return the associated Thread*.
+ * Caller must hold the thread list lock.
+ *
+ * Returns NULL if the thread was not found.
+ */
+Thread* dvmGetThreadByThreadId(u4 threadId)
+{
+    Thread* thread;
+    for (thread = gDvm.threadList; thread != NULL; thread = thread->next) {
+        if (thread->threadId == threadId)
+            break;
+    }
     return thread;
 }
 
diff --git a/vm/Thread.h b/vm/Thread.h
index 2d6ce5f25..f397fba70 100644
--- a/vm/Thread.h
+++ b/vm/Thread.h
@@ -438,13 +438,21 @@ Thread* dvmGetThreadFromThreadObject(Object* vmThreadObj);
 
 /*
  * Given a pthread handle, return the associated Thread*.
- * Caller must NOT hold the thread list lock.
+ * Caller must hold the thread list lock.
  *
  * Returns NULL if the thread was not found.
  */
 Thread* dvmGetThreadByHandle(pthread_t handle);
 
 /*
+ * Given a thread ID, return the associated Thread*.
+ * Caller must hold the thread list lock.
+ *
+ * Returns NULL if the thread was not found.
+ */
+Thread* dvmGetThreadByThreadId(u4 threadId);
+
+/*
  * Sleep in a thread.  Returns when the sleep timer returns or the thread
  * is interrupted.
  */
diff --git a/vm/alloc/HeapWorker.c b/vm/alloc/HeapWorker.c
index e6e9209f2..7d2b687b9 100644
--- a/vm/alloc/HeapWorker.c
+++ b/vm/alloc/HeapWorker.c
@@ -154,7 +154,10 @@ void dvmAssertHeapWorkerThreadRunning()
              * foreground we'll just leave it there (it doesn't do anything
              * if the process isn't GCing).
              */
+            dvmLockThreadList(NULL);
             Thread* thread = dvmGetThreadByHandle(gDvm.heapWorkerHandle);
+            dvmUnlockThreadList();
+
             if (thread != NULL) {
                 int priChangeFlags, threadPrio;
                 SchedPolicy threadPolicy;
diff --git a/vm/interp/Stack.c b/vm/interp/Stack.c
index 8d8cc1bf2..1ebbcf051 100644
--- a/vm/interp/Stack.c
+++ b/vm/interp/Stack.c
@@ -1093,6 +1093,85 @@ void dvmCleanupStackOverflow(Thread* self, const Object* exception)
 
 
 /*
+ * Extract the object that is the target of a monitor-enter instruction
+ * in the top stack frame of "thread".
+ *
+ * The other thread might be alive, so this has to work carefully.
+ *
+ * We assume the thread list lock is currently held.
+ *
+ * Returns "true" if we successfully recover the object.  "*pOwner" will
+ * be NULL if we can't determine the owner for some reason (e.g. race
+ * condition on ownership transfer).
+ */
+static bool extractMonitorEnterObject(Thread* thread, Object** pLockObj,
+    Thread** pOwner)
+{
+    void* framePtr = thread->curFrame;
+
+    if (framePtr == NULL || dvmIsBreakFrame(framePtr))
+        return false;
+
+    const StackSaveArea* saveArea = SAVEAREA_FROM_FP(framePtr);
+    const Method* method = saveArea->method;
+    const u2* currentPc = saveArea->xtra.currentPc;
+
+    /* check Method* */
+    if (!dvmLinearAllocContains(method, sizeof(Method))) {
+        LOGD("ExtrMon: method %p not valid\n", method);
+        return false;
+    }
+
+    /* check currentPc */
+    u4 insnsSize = dvmGetMethodInsnsSize(method);
+    if (currentPc < method->insns ||
+        currentPc >= method->insns + insnsSize)
+    {
+        LOGD("ExtrMon: insns %p not valid (%p - %p)\n",
+            currentPc, method->insns, method->insns + insnsSize);
+        return false;
+    }
+
+    /* check the instruction */
+    if ((*currentPc & 0xff) != OP_MONITOR_ENTER) {
+        LOGD("ExtrMon: insn at %p is not monitor-enter (0x%02x)\n",
+            currentPc, *currentPc & 0xff);
+        return false;
+    }
+
+    /* get and check the register index */
+    unsigned int reg = *currentPc >> 8;
+    if (reg >= method->registersSize) {
+        LOGD("ExtrMon: invalid register %d (max %d)\n",
+            reg, method->registersSize);
+        return false;
+    }
+
+    /* get and check the object in that register */
+    u4* fp = (u4*) framePtr;
+    Object* obj = (Object*) fp[reg];
+    if (!dvmIsValidObject(obj)) {
+        LOGD("ExtrMon: invalid object %p at %p[%d]\n", obj, fp, reg);
+        return false;
+    }
+    *pLockObj = obj;
+
+    /*
+     * Try to determine the object's lock holder; it's okay if this fails.
+     *
+     * We're assuming the thread list lock is already held by this thread.
+     * If it's not, we may be living dangerously if we have to scan through
+     * the thread list to find a match.  (The VM will generally be in a
+     * suspended state when executing here, so this is a minor concern
+     * unless we're dumping while threads are running, in which case there's
+     * a good chance of stuff blowing up anyway.)
+     */
+    *pOwner = dvmGetObjectLockHolder(obj);
+
+    return true;
+}
+
+/*
  * Dump stack frames, starting from the specified frame and moving down.
  *
  * Each frame holds a pointer to the currently executing method, and the
@@ -1151,21 +1230,44 @@ static void dumpFrames(const DebugOutputTarget* target, void* framePtr,
             }
             free(className);
 
-            if (first &&
-                (thread->status == THREAD_WAIT ||
-                 thread->status == THREAD_TIMED_WAIT))
-            {
-                /* warning: wait status not stable, even in suspend */
-                Monitor* mon = thread->waitMonitor;
-                Object* obj = dvmGetMonitorObject(mon);
-                if (obj != NULL) {
-                    className = dvmDescriptorToDot(obj->clazz->descriptor);
-                    dvmPrintDebugMessage(target,
-                        "  - waiting on <%p> (a %s)\n", mon, className);
-                    free(className);
+            if (first) {
+                /*
+                 * Decorate WAIT and MONITOR threads with some detail on
+                 * the first frame.
+                 *
+                 * warning: wait status not stable, even in suspend
+                 */
+                if (thread->status == THREAD_WAIT ||
+                    thread->status == THREAD_TIMED_WAIT)
+                {
+                    Monitor* mon = thread->waitMonitor;
+                    Object* obj = dvmGetMonitorObject(mon);
+                    if (obj != NULL) {
+                        className = dvmDescriptorToDot(obj->clazz->descriptor);
+                        dvmPrintDebugMessage(target,
+                            "  - waiting on <%p> (a %s)\n", obj, className);
+                        free(className);
+                    }
+                } else if (thread->status == THREAD_MONITOR) {
+                    Object* obj;
+                    Thread* owner;
+                    if (extractMonitorEnterObject(thread, &obj, &owner)) {
+                        className = dvmDescriptorToDot(obj->clazz->descriptor);
+                        if (owner != NULL) {
+                            char* threadName = dvmGetThreadName(owner);
+                            dvmPrintDebugMessage(target,
+                                "  - waiting to lock <%p> (a %s) held by threadid=%d (%s)\n",
+                                obj, className, owner->threadId, threadName);
+                            free(threadName);
+                        } else {
+                            dvmPrintDebugMessage(target,
+                                "  - waiting to lock <%p> (a %s) held by ???\n",
+                                obj, className);
+                        }
+                        free(className);
+                    }
                 }
             }
-
         }
 
         /*