Try to show lock owner in MONITOR thread dump.
A thread in the MONITOR state is blocked waiting on a monitor. This
raises two interesting questions: (1) what lock is it waiting on, and
(2) who holds that lock? The answer to (1) can be determined easily by
looking at the source code, but (2) is a bit harder.
This change extracts the target object from the instruction stream and
prints some information about it, e.g.:
- waiting to lock <0x40028c68> (a java.lang.Object) held by threadid=1 (main)
Change-Id: Iad18fc6f2df4142368bdf1063b8cc71de2d66156
Also: fiddled with "must [not] be locked" on a recently-added function.
diff --git a/vm/Sync.c b/vm/Sync.c
index 20d6dab..fba40e2 100644
--- a/vm/Sync.c
+++ b/vm/Sync.c
@@ -258,6 +258,21 @@
}
/*
+ * 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 b273538..1a168b9 100644
--- a/vm/Sync.h
+++ b/vm/Sync.h
@@ -141,6 +141,14 @@
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 9375921..4343694 100644
--- a/vm/Thread.c
+++ b/vm/Thread.c
@@ -3155,22 +3155,33 @@
/*
* 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 44004bf..e2af253 100644
--- a/vm/Thread.h
+++ b/vm/Thread.h
@@ -441,13 +441,21 @@
/*
* 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 a4b523e..fe37b2a 100644
--- a/vm/alloc/HeapWorker.c
+++ b/vm/alloc/HeapWorker.c
@@ -150,7 +150,10 @@
* 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 8d8cc1b..1ebbcf0 100644
--- a/vm/interp/Stack.c
+++ b/vm/interp/Stack.c
@@ -1093,6 +1093,85 @@
/*
+ * 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 @@
}
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);
+ }
}
}
-
}
/*
