On Wed, 2005-09-14 at 08:51 +0200, Andreas Fredriksson wrote:
> I think the only fix is to rewrite the class definition code in Derby to
> avoid taking a lock on the helper objects, but I don't know the source
> code well enough to write a proper fix.
Maybe I should spend more time tweaking before sending email in the
future.. :-)
The attached patch fixes the problem for me. I'm sure there's more than
this to it, but please feel free to use this a reference. We'll use it
locally and see if anything else creeps up.
Regards,
Andreas
diff -r -u db-derby-10.1.1.0-src-orig/java/engine/org/apache/derby/impl/services/reflect/ReflectClassesJava2.java db-derby-10.1.1.0-src/java/engine/org/apache/derby/impl/services/reflect/ReflectClassesJava2.java
--- db-derby-10.1.1.0-src-orig/java/engine/org/apache/derby/impl/services/reflect/ReflectClassesJava2.java 2005-07-29 19:37:53.000000000 +0200
+++ db-derby-10.1.1.0-src/java/engine/org/apache/derby/impl/services/reflect/ReflectClassesJava2.java 2005-09-14 11:42:00.000000000 +0200
@@ -19,8 +19,13 @@
*/
package org.apache.derby.impl.services.reflect;
+
import org.apache.derby.iapi.util.ByteArray;
+import java.util.Collections;
+import java.util.Map;
+import java.util.HashMap;
+
/**
Relfect loader with Privileged block for Java 2 security.
*/
@@ -29,35 +34,44 @@
implements java.security.PrivilegedAction
{
- private java.util.HashMap preCompiled;
+ private final Map preCompiled = Collections.synchronizedMap(new HashMap());
private int action;
- protected synchronized LoadedGeneratedClass loadGeneratedClassFromData(String fullyQualifiedName, ByteArray classDump) {
+ protected LoadedGeneratedClass loadGeneratedClassFromData(String fullyQualifiedName, ByteArray classDump) {
if (classDump == null || classDump.getArray() == null) {
- if (preCompiled == null)
- preCompiled = new java.util.HashMap();
- else
- {
- ReflectGeneratedClass gc = (ReflectGeneratedClass) preCompiled.get(fullyQualifiedName);
- if (gc != null)
- return gc;
- }
+ ReflectGeneratedClass gc = (ReflectGeneratedClass)
+ preCompiled.get(fullyQualifiedName);
+ if (gc != null)
+ return gc;
- // not a generated class, just load the class directly.
+ // Not a generated class, just load the class directly.
+ //
+ // There's a slight chance of a race condition here--if two
+ // threads decide to generate the same class at once there will be
+ // a redundant instance of ReflectGeneratedClass created. This
+ // will only cause trouble if two callers decide to generate the
+ // same class with different contents which is a bug in itself.
+ //
+ // By not locking we allow ClassLoader objects to use the database
+ // in their findClass() methods to resolve bytecode; had we locked
+ // this object there would have been a deadlock when multiple
+ // threads issue queries and/or load classes.
try {
Class jvmClass = Class.forName(fullyQualifiedName);
- ReflectGeneratedClass gc = new ReflectGeneratedClass(this, jvmClass, null);
- preCompiled.put(fullyQualifiedName, gc);
- return gc;
+ ReflectGeneratedClass newgc = new ReflectGeneratedClass(this, jvmClass, null);
+ preCompiled.put(fullyQualifiedName, newgc);
+ return newgc;
} catch (ClassNotFoundException cnfe) {
throw new NoClassDefFoundError(cnfe.toString());
}
}
- action = 1;
+ synchronized (this) {
+ action = 1;
+ }
return ((ReflectLoaderJava2) java.security.AccessController.doPrivileged(this)).loadGeneratedClass(fullyQualifiedName, classDump);
}
@@ -73,10 +87,12 @@
}
}
- protected synchronized Class loadClassNotInDatabaseJar(String name) throws ClassNotFoundException {
+ protected Class loadClassNotInDatabaseJar(String name) throws ClassNotFoundException {
Class foundClass = null;
- action = 2;
+ synchronized (this) {
+ action = 2;
+ }
// We may have two problems with calling getContextClassLoader()
// when trying to find our own classes for aggregates.
// 1) If using the URLClassLoader a ClassNotFoundException may be
