Deadlock on class locks
Patrick Tullmann
tullmann at cs.utah.edu
Tue Dec 14 15:49:41 PST 1999
> The attached test case demonstrates a deadlock in classMethod.c.
The fix was easier than I thought. Its attached below.
> The root of the problem is that lots of code within Kaffe is using
> the "public" class lock for internal locking.
The problem isn't actually that severe. Most of the class locking
happens in processClass() where conflicts with "user" code aren't
likely (...possible(?)). The only other bit that locks the class lock
is resolveString().
The fix adds an internalLock field to each class (its a iLock ptr).
And uses that for all "internal" class locking. The patch also makes
all the internal class locks explicit by using a new macro
'lockClass()'.
A regression test case with 'Expected Output' is included.
(BTW, please let me know if there are any problems applying the diff).
-Pat
----- ----- ---- --- --- -- - - - - -
Pat Tullmann tullmann at cs.utah.edu
Don't hate yourself in the morning -- sleep until noon!
-------------- next part --------------
Index: kaffe/kaffevm/classMethod.c
===================================================================
RCS file: /cvs/kaffe/kaffe/kaffe/kaffevm/classMethod.c,v
retrieving revision 1.73
diff -u -r1.73 classMethod.c
--- kaffe/kaffevm/classMethod.c 1999/11/29 23:49:47 1.73
+++ kaffe/kaffevm/classMethod.c 1999/12/14 23:40:32
@@ -70,6 +70,18 @@
static bool resolveConstants(Hjava_lang_Class*, errorInfo *einfo);
static bool resolveInterfaces(Hjava_lang_Class *class, errorInfo *einfo);
+
+/*
+ * Functions for locking and waiting on *internal* class
+ * bits. This is not for static+synchronized methods.
+ */
+#define lockClass(C) lockStaticMutex(&((C)->internalLock))
+#define unlockClass(C) unlockStaticMutex(&((C)->internalLock))
+#define waitOnClass(C) waitStaticCond(&((C)->internalLock), 0)
+#define signalOnClass(C) signalStaticCond(&((C)->internalLock))
+#define broadcastOnClass(C) broadcastStaticCond(&((C)->internalLock))
+
+
#if !defined(ALIGNMENT_OF_SIZE)
#define ALIGNMENT_OF_SIZE(S) (S)
#endif
@@ -109,7 +121,7 @@
* we've got to work out.
*/
- lockMutex(&class->head);
+ lockClass(class);
DBG(RESERROR,
/* show calls to processClass when debugging resolution errors */
@@ -145,7 +157,7 @@
goto done;
} else {
while (class->state == CSTATE_DOING_PREPARE) {
- waitCond(&class->head, 0);
+ waitOnClass(class);
goto retry;
}
}
@@ -171,7 +183,7 @@
* upcall to a classloader, we must release the
* classLock here.
*/
- unlockMutex(&class->head);
+ unlockClass(class);
#if defined(HAVE_GCJ_SUPPORT)
if (CLASS_GCJ(class)) {
class->superclass
@@ -185,7 +197,7 @@
#if defined(HAVE_GCJ_SUPPORT)
}
#endif
- lockMutex(&class->head);
+ lockClass(class);
if (class->superclass == 0) {
success = false;
goto done;
@@ -262,7 +274,7 @@
SET_CLASS_STATE(CSTATE_PREPARED);
}
- assert(class == ObjectClass || class->superclass != 0);
+ assert((class == ObjectClass) || (class->superclass != NULL));
DO_CLASS_STATE(CSTATE_LINKED) {
@@ -335,7 +347,7 @@
goto done;
} else {
while (class->state == CSTATE_DOING_SUPER) {
- waitCond(&class->head, 0);
+ waitOnClass(class);
goto retry;
}
}
@@ -387,10 +399,10 @@
* might call out into the superclass's initializer
* here!
*/
- unlockMutex(&class->head);
+ unlockClass(class);
success = processClass(class->superclass,
CSTATE_COMPLETE, einfo);
- lockMutex(&class->head);
+ lockClass(class);
if (success == false) {
if (class->superclass->state == CSTATE_INIT_FAILED)
SET_CLASS_STATE(CSTATE_INIT_FAILED);
@@ -431,7 +443,7 @@
goto done;
} else {
while (class->state == CSTATE_DOING_INIT) {
- waitCond(&class->head, 0);
+ waitOnClass(class);
goto retry;
}
}
@@ -441,7 +453,7 @@
class->processingThread = THREAD_NATIVE();
/* give classLock up for the duration of this call */
- unlockMutex(&class->head);
+ unlockClass(class);
/* we use JNI to catch possible exceptions, except
* during initialization, when JNI doesn't work yet.
@@ -464,7 +476,7 @@
callMethodA(meth, METHOD_INDIRECTMETHOD(meth), 0, 0, 0, 1);
}
- lockMutex(&class->head);
+ lockClass(class);
if (exc != 0) {
/* this is special-cased in throwError */
@@ -510,8 +522,8 @@
}
/* wake up any waiting threads */
- broadcastCond(&class->head);
- unlockMutex(&class->head);
+ broadcastOnClass(class);
+ unlockClass(class);
DBG(RESERROR,
for (i = 0; i < depth; dprintf(" ", i++));
@@ -549,7 +561,7 @@
}
for (i = 0; i < class->interface_len; i++) {
uintp iface = (uintp)class->interfaces[i];
- unlockMutex(&class->head);
+ unlockClass(class);
#if defined(HAVE_GCJ_SUPPORT)
if (CLASS_GCJ(class)) {
@@ -562,7 +574,7 @@
#endif /* HAVE_GCJ_SUPPORT */
class->interfaces[i] = nclass;
- lockMutex(&class->head);
+ lockClass(class);
if (class->interfaces[i] == 0) {
success = false;
goto done;
@@ -1959,7 +1971,7 @@
pool = CLASS_CONSTANTS(clazz);
- lockMutex(&clazz->head);
+ lockClass(clazz);
switch (pool->tags[idx]) {
case CONSTANT_String:
utf8 = WORD2UTF(pool->data[idx]);
@@ -1980,7 +1992,7 @@
default:
assert(!!!"Neither String nor ResolvedString?");
}
- unlockMutex(&clazz->head);
+ unlockClass(clazz);
return (str);
}
@@ -2340,7 +2352,9 @@
}
centry->class = arr_class;
/*
- * See JDC Bug: 4208179
+ * See JDC Bug: 4208179. The story is that the Spec leaves the
+ * flags "unspecified" and the JDK actually sets them.... most
+ * of the time.
*/
arr_flags = ACC_ABSTRACT | ACC_FINAL;
if (c->accflags & ACC_PUBLIC) {
Index: kaffe/kaffevm/classMethod.h
===================================================================
RCS file: /cvs/kaffe/kaffe/kaffe/kaffevm/classMethod.h,v
retrieving revision 1.38
diff -u -r1.38 classMethod.h
--- kaffe/kaffevm/classMethod.h 1999/11/29 23:49:47 1.38
+++ kaffe/kaffevm/classMethod.h 1999/12/14 23:40:32
@@ -45,6 +45,10 @@
struct Hjava_lang_Class {
Hjava_lang_Object head; /* A class is an object too */
+ /* Lock protecting internal class state. Public lock is for
+ * synchronized methods. */
+ struct _iLock* internalLock;
+
/* Link to class entry */
struct _classEntry* centry;
-------------- next part --------------
/**
* Test case for deadlocking in the JIT3. The deadlock results
* from using the public class lock to protect internal class state.
* <p>
* Sequence of events:
*
* A call to blockOnJitter locks the class lock
*
* A call to blockInJitter starts the jit and then calls
* classMethod.c:resolveString() attempting to resolve the string for
* `blockInJitter'. However, the blockOnJitter thread already has
* the class lock so it blocks.
*
* Next the blockOnJitter thread tries to call the loop() method which
* blocks on the jitter lock
*
* And we have deadlock...
*
* @author Tim Stack
*/
public class Classdeadlock
{
public static void loop()
{
for( int lpc = 0; lpc < 100; lpc++ )
{
}
}
public static void blockInJitter()
{
System.out.println("(!) This is blockInJitter");
}
public static synchronized void blockOnJitter()
{
System.out.println("(2) Have class lock");
System.out.println("(3) Yielding for blockInJitter thread");
Thread.currentThread().setPriority(3);
System.out.println("(*) Returned from yield.");
System.out.println("(*) Get set to deadlock when calling loop()");
DeadLock.loop();
}
public static void main(String args[]) throws Exception
{
Thread th1, th2;
Thread.currentThread().setPriority(9);
th1 = new Thread() {
public void run()
{
System.out.println("(1) Start blockOnJitter");
DeadLock.blockOnJitter();
System.out.println("(*) End blockOnJitter");
}
};
th1.setPriority(6);
th2 = new Thread() {
public void run()
{
System.out.println("(4) Start blockInJitter");
// implicitly gets static class lock to look up 'blockInJitter' string
DeadLock.blockInJitter(); // block indefinetly.
System.out.println("(*) End blockInJitter");
}
};
th2.setPriority(5);
th1.start();
th2.start();
th1.join();
th2.join();
}
}
/* Expected Output:
(1) Start blockOnJitter
(2) Have class lock
(3) Yielding for blockInJitter thread
(4) Start blockInJitter
(!) This is blockInJitter
(*) End blockInJitter
(*) Returned from yield.
(*) Get set to deadlock when calling loop()
(*) End blockOnJitter
*/
More information about the kaffe
mailing list