Merge pull request #45 from dazey3/2767_master

OPENJPA-2767: Incomplete ValueMapDiscriminatorStrategy cache and MetaDataRepository race condition
This commit is contained in:
Will Dazey 2019-05-16 13:41:57 -05:00 committed by GitHub
commit cd81d36024
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 81 additions and 37 deletions

View File

@ -24,6 +24,8 @@ import java.util.HashMap;
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 @@ import org.apache.openjpa.util.MetaDataException;
public class ValueMapDiscriminatorStrategy
extends InValueDiscriminatorStrategy {
private static final long serialVersionUID = 1L;
public static final String ALIAS = "value-map";
@ -51,7 +52,10 @@ public class ValueMapDiscriminatorStrategy
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 @@ public class ValueMapDiscriminatorStrategy
// 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 @@ public class ValueMapDiscriminatorStrategy
@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 @@ public class ValueMapDiscriminatorStrategy
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));

View File

@ -126,7 +126,8 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
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 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
}
/**
* 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 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
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 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
/**
* 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 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
}
}
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 @@ public class MetaDataRepository implements PCRegistry.RegisterClassListener, Con
@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<>();