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