Deadlock on class locks
Patrick Tullmann
tullmann at cs.utah.edu
Wed Dec 15 11:03:30 PST 1999
Tim Wilkinson wrote:
> There is already a classEntry lock associated with each class - perhaps
> that could be used instead of adding a new lock?
Never thought of that... I looked through the code and, guess
what... the centry lock is already being used for exactly the same
type of syncronization. So, I changed the macros and fixed the
existing sites to use lockClass(). Here's a new patch (i.e., don't
apply the old one...).
As an alternative to this patch (which is 90% cleanup), one can just
change the locking in resolveString() to do lockMutex(clazz->centry)
instead of lockMutex(clazz).
-Pat
----- ----- ---- --- --- -- - - - - -
Pat Tullmann tullmann at cs.utah.edu
To understand recursion one must first understand recursion.
-------------- 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/15 18:55:28
@@ -70,6 +70,8 @@
static bool resolveConstants(Hjava_lang_Class*, errorInfo *einfo);
static bool resolveInterfaces(Hjava_lang_Class *class, errorInfo *einfo);
+
+
#if !defined(ALIGNMENT_OF_SIZE)
#define ALIGNMENT_OF_SIZE(S) (S)
#endif
@@ -109,7 +111,7 @@
* we've got to work out.
*/
- lockMutex(&class->head);
+ lockClass(class);
DBG(RESERROR,
/* show calls to processClass when debugging resolution errors */
@@ -145,7 +147,7 @@
goto done;
} else {
while (class->state == CSTATE_DOING_PREPARE) {
- waitCond(&class->head, 0);
+ waitOnClass(class);
goto retry;
}
}
@@ -171,7 +173,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 +187,7 @@
#if defined(HAVE_GCJ_SUPPORT)
}
#endif
- lockMutex(&class->head);
+ lockClass(class);
if (class->superclass == 0) {
success = false;
goto done;
@@ -262,7 +264,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 +337,7 @@
goto done;
} else {
while (class->state == CSTATE_DOING_SUPER) {
- waitCond(&class->head, 0);
+ waitOnClass(class);
goto retry;
}
}
@@ -387,10 +389,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 +433,7 @@
goto done;
} else {
while (class->state == CSTATE_DOING_INIT) {
- waitCond(&class->head, 0);
+ waitOnClass(class);
goto retry;
}
}
@@ -441,7 +443,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 +466,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 +512,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 +551,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 +564,7 @@
#endif /* HAVE_GCJ_SUPPORT */
class->interfaces[i] = nclass;
- lockMutex(&class->head);
+ lockClass(class);
if (class->interfaces[i] == 0) {
success = false;
goto done;
@@ -1135,13 +1137,13 @@
* else may update it while we're doing this. Once we've got the
* name we don't really care.
*/
- lockMutex(this->centry);
+ lockClass(this);
if (FIELD_RESOLVED(fld)) {
- unlockMutex(this->centry);
+ unlockClass(this);
return (FIELD_TYPE(fld));
}
name = ((Utf8Const*)fld->type)->data;
- unlockMutex(this->centry);
+ unlockClass(this);
clas = getClassFromSignature(name, this->loader, einfo);
@@ -1959,7 +1961,7 @@
pool = CLASS_CONSTANTS(clazz);
- lockMutex(&clazz->head);
+ lockClass(clazz);
switch (pool->tags[idx]) {
case CONSTANT_String:
utf8 = WORD2UTF(pool->data[idx]);
@@ -1980,7 +1982,7 @@
default:
assert(!!!"Neither String nor ResolvedString?");
}
- unlockMutex(&clazz->head);
+ unlockClass(clazz);
return (str);
}
@@ -2004,7 +2006,7 @@
constants* pool;
Utf8Const* utf8;
- lockMutex(class->centry);
+ lockClass(class);
/* Scan constant pool and convert any constant strings into true
* java strings.
@@ -2030,7 +2032,7 @@
}
done:
- unlockMutex(this->centry);
+ unlockClass(this);
#endif /* EAGER_LOADING */
return (success);
}
@@ -2340,7 +2342,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/15 18:55:28
@@ -117,6 +117,17 @@
typedef struct Hjava_lang_Class Hjava_lang_Class;
#endif
+/*
+ * Functions for locking and waiting on *internal* class
+ * bits. This is not for static+synchronized methods.
+ * Use the centry lock, as it is convenient and available.
+ */
+#define lockClass(C) lockMutex(((C)->centry))
+#define unlockClass(C) unlockMutex(((C)->centry))
+#define waitOnClass(C) waitCond(((C)->centry), 0)
+#define signalOnClass(C) signalCond(((C)->centry))
+#define broadcastOnClass(C) broadcastCond(((C)->centry))
+
#define METHOD_TRANSLATED(M) ((M)->accflags & ACC_TRANSLATED)
#define METHOD_JITTED(M) ((M)->accflags & ACC_JITTED)
@@ -150,6 +161,9 @@
/*
* Class hash entry.
+ *
+ * Note that the lock on this struct is used to
+ * synchronize internal Class object state, too.
*/
typedef struct _classEntry {
Utf8Const* name;
Index: kaffe/kaffevm/lookup.c
===================================================================
RCS file: /cvs/kaffe/kaffe/kaffe/kaffevm/lookup.c,v
retrieving revision 1.21
diff -u -r1.21 lookup.c
--- kaffe/kaffevm/lookup.c 1999/11/04 20:29:13 1.21
+++ kaffe/kaffevm/lookup.c 1999/12/15 18:55:28
@@ -159,10 +159,10 @@
* lock and get the tag & name again just in case. If we
* have been resolved then we just return the answer.
*/
- lockMutex(this->centry);
+ lockClass(this);
tag = pool->tags[idx];
name = WORD2UTF(pool->data[idx]);
- unlockMutex(this->centry);
+ unlockClass(this);
if (tag == CONSTANT_ResolvedClass) {
return (CLASS_CLASS(idx, pool));
@@ -190,10 +190,10 @@
/* Lock the class while we update the constant pool. Someone
* may have done this already but we don't care.
*/
- lockMutex(this->centry);
+ lockClass(this);
pool->data[idx] = (ConstSlot)class;
pool->tags[idx] = CONSTANT_ResolvedClass;
- unlockMutex(this->centry);
+ unlockClass(this);
return (class);
}
More information about the kaffe
mailing list