OPENJPA-2767: Incomplete ValueMapDiscriminatorStrategy cache and MetaDataRepository race condition
Signed-off-by: Will Dazey <dazeydev.3@gmail.com>
diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/ValueMapDiscriminatorStrategy.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/ValueMapDiscriminatorStrategy.java
index cedfa85..5c44c27 100644
--- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/ValueMapDiscriminatorStrategy.java
+++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/ValueMapDiscriminatorStrategy.java
@@ -24,6 +24,8 @@
import java.util.List;
import java.util.Map;
import java.util.TreeSet;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.apache.openjpa.jdbc.kernel.JDBCStore;
import org.apache.openjpa.jdbc.meta.ClassMapping;
@@ -43,7 +45,6 @@
public class ValueMapDiscriminatorStrategy
extends InValueDiscriminatorStrategy {
-
private static final long serialVersionUID = 1L;
public static final String ALIAS = "value-map";
@@ -51,7 +52,10 @@
private static final Localizer _loc = Localizer.forPackage
(ValueMapDiscriminatorStrategy.class);
- private Map _vals = null;
+ private Map<String, Class<?>> _vals;
+ private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock(true);
+ private final Lock _readLock = rwl.readLock();
+ private final Lock _writeLock = rwl.writeLock();
@Override
public String getAlias() {
@@ -67,8 +71,8 @@
// if the user wants the type to be null, we need a jdbc-type
// on the column or an existing column to figure out the java type
DiscriminatorMappingInfo info = disc.getMappingInfo();
- List cols = info.getColumns();
- Column col = (cols.isEmpty()) ? null : (Column) cols.get(0);
+ List<Column> cols = info.getColumns();
+ Column col = (cols.isEmpty()) ? null : cols.get(0);
if (col != null) {
if (col.getJavaType() != JavaTypes.OBJECT)
return col.getJavaType();
@@ -88,29 +92,67 @@
@Override
protected Class getClass(Object val, JDBCStore store)
throws ClassNotFoundException {
- if (_vals == null) {
- ClassMapping cls = disc.getClassMapping();
- ClassMapping[] subs = cls.getJoinablePCSubclassMappings();
- Map map = new HashMap((int) ((subs.length + 1) * 1.33 + 1));
- mapDiscriminatorValue(cls, map);
- for (int i = 0; i < subs.length; i++)
- mapDiscriminatorValue(subs[i], map);
- _vals = map;
+
+ if(_vals == null) {
+ _writeLock.lock();
+ try {
+ if(_vals == null) {
+ _vals = constructCache(disc);
+ }
+ } finally {
+ _writeLock.unlock();
+ }
}
- String str = (val == null) ? null : val.toString();
- Class cls = (Class) _vals.get(str);
- if (cls != null)
- return cls;
- throw new ClassNotFoundException(_loc.get("unknown-discrim-value",
- new Object[]{ str, disc.getClassMapping().getDescribedType().
- getName(), new TreeSet(_vals.keySet()) }).getMessage());
+ String className = (val == null) ? null : val.toString();
+ _readLock.lock();
+ try {
+ Class<?> clz = _vals.get(className);
+ if (clz != null)
+ return clz;
+ } finally {
+ _readLock.unlock();
+ }
+
+ _writeLock.lock();
+ try {
+ Class<?> clz = _vals.get(className);
+ if (clz != null)
+ return clz;
+
+ //Rebuild the cache to check for updates
+ _vals = constructCache(disc);
+
+ //Try get again
+ clz = _vals.get(className);
+ if (clz != null)
+ return clz;
+ throw new ClassNotFoundException(_loc.get("unknown-discrim-value",
+ new Object[]{ className, disc.getClassMapping().getDescribedType().
+ getName(), new TreeSet<String>(_vals.keySet()) }).getMessage());
+ } finally {
+ _writeLock.unlock();
+ }
+ }
+
+ /**
+ * Build a class cache map from the discriminator
+ */
+ private static Map<String, Class<?>> constructCache(Discriminator disc) {
+ //Build the cache map
+ ClassMapping cls = disc.getClassMapping();
+ ClassMapping[] subs = cls.getJoinablePCSubclassMappings();
+ Map<String, Class<?>> map = new HashMap<String, Class<?>>((int) ((subs.length + 1) * 1.33 + 1));
+ mapDiscriminatorValue(cls, map);
+ for (int i = 0; i < subs.length; i++)
+ mapDiscriminatorValue(subs[i], map);
+ return map;
}
/**
* Map the stringified version of the discriminator value of the given type.
*/
- private static void mapDiscriminatorValue(ClassMapping cls, Map map) {
+ private static void mapDiscriminatorValue(ClassMapping cls, Map<String, Class<?>> map) {
// possible that some types will never be persisted and therefore
// can have no discriminator value
Object val = cls.getDiscriminator().getValue();
@@ -118,7 +160,7 @@
return;
String str = (val == Discriminator.NULL) ? null : val.toString();
- Class exist = (Class) map.get(str);
+ Class<?> exist = map.get(str);
if (exist != null)
throw new MetaDataException(_loc.get("dup-discrim-value",
str, exist, cls));
diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataRepository.java b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataRepository.java
index 7bc33b9..0525e22 100644
--- a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataRepository.java
+++ b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataRepository.java
@@ -126,7 +126,8 @@
private Map<Class<?>, Class<?>> _metamodel = Collections.synchronizedMap(new HashMap<Class<?>, Class<?>>());
// map of classes to lists of their subclasses
- private Map<Class<?>, List<Class<?>>> _subs = Collections.synchronizedMap(new HashMap<Class<?>, List<Class<?>>>());
+ private Map<Class<?>, Collection<Class<?>>> _subs =
+ Collections.synchronizedMap(new HashMap<Class<?>, Collection<Class<?>>>());
// xml mapping
protected final XMLMetaData[] EMPTY_XMLMETAS;
@@ -1625,19 +1626,20 @@
}
/**
- * Updates our datastructures with the latest registered classes.
+ * Updates our data structures with the latest registered classes.
+ *
+ * This method is synchronized to make sure that all data structures are fully updated
+ * before other threads attempt to call this method
*/
- Class<?>[] processRegisteredClasses(ClassLoader envLoader) {
- if (_registered.isEmpty())
+ synchronized Class<?>[] processRegisteredClasses(ClassLoader envLoader) {
+ if (_registered.isEmpty()) {
return EMPTY_CLASSES;
+ }
// copy into new collection to avoid concurrent mod errors on reentrant
// registrations
- Class<?>[] reg;
- synchronized (_registered) {
- reg = _registered.toArray(new Class[_registered.size()]);
- _registered.clear();
- }
+ Class<?>[] reg = _registered.toArray(new Class[_registered.size()]);
+ _registered.clear();
Collection<String> pcNames = getPersistentTypeNames(false, envLoader);
Collection<Class<?>> failed = null;
@@ -1652,8 +1654,8 @@
if (_filterRegisteredClasses) {
Log log = (_conf == null) ? null : _conf.getLog(OpenJPAConfiguration.LOG_RUNTIME);
ClassLoader loadCL = (envLoader != null) ?
- envLoader :
- AccessController.doPrivileged(J2DoPrivHelper.getContextClassLoaderAction());
+ envLoader :
+ AccessController.doPrivileged(J2DoPrivHelper.getContextClassLoaderAction());
try {
Class<?> classFromAppClassLoader = Class.forName(reg[i].getName(), true, loadCL);
@@ -1840,7 +1842,7 @@
/**
* Add the given value to the collection cached in the given map under the given key.
*/
- private void addToCollection(Map map, Class<?> key, Class<?> value, boolean inheritance) {
+ private void addToCollection(Map<Class<?>, Collection<Class<?>>> map, Class<?> key, Class<?> value, boolean inheritance) {
if (_locking) {
synchronized (map) {
addToCollectionInternal(map, key, value, inheritance);
@@ -1850,8 +1852,8 @@
}
}
- private void addToCollectionInternal(Map map, Class<?> key, Class<?> value, boolean inheritance) {
- Collection coll = (Collection) map.get(key);
+ private void addToCollectionInternal(Map<Class<?>, Collection<Class<?>>> map, Class<?> key, Class<?> value, boolean inheritance) {
+ Collection<Class<?>> coll = map.get(key);
if (coll == null) {
if (inheritance) {
InheritanceComparator comp = new InheritanceComparator();
@@ -1927,8 +1929,8 @@
@Override
public void endConfiguration() {
initializeMetaDataFactory();
- if (_implGen == null)
- _implGen = new InterfaceImplGenerator(this);
+ if (_implGen == null)
+ _implGen = new InterfaceImplGenerator(this);
if (_preload == true) {
_oids = new HashMap<>();
_impls = new HashMap<>();
