This is an automated email from the ASF dual-hosted git repository.
dazeydev pushed a commit to branch 2.2.x
in repository https://gitbox.apache.org/repos/asf/openjpa.git
The following commit(s) were added to refs/heads/2.2.x by this push:
new 29b82b2 OPENJPA-2767: Incomplete ValueMapDiscriminatorStrategy cache
and MetaDataRepository race condition
new a41cd39 Merge pull request #43 from dazey3/2767_2.2.x
29b82b2 is described below
commit 29b82b23efa1db5024706851063c1988fb130398
Author: Will Dazey <[email protected]>
AuthorDate: Tue May 14 14:32:06 2019 -0500
OPENJPA-2767: Incomplete ValueMapDiscriminatorStrategy cache and
MetaDataRepository race condition
Signed-off-by: Will Dazey <[email protected]>
---
.../meta/strats/ValueMapDiscriminatorStrategy.java | 83 ++++++++++++++++------
.../apache/openjpa/meta/MetaDataRepository.java | 42 +++++------
2 files changed, 85 insertions(+), 40 deletions(-)
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 86f8320..c14db3f 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.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;
@@ -48,7 +50,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();
public String getAlias() {
return ALIAS;
@@ -62,8 +67,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();
@@ -81,29 +86,67 @@ public class ValueMapDiscriminatorStrategy
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 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();
}
+ }
- 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());
+ /**
+ * 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();
@@ -111,7 +154,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));
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 b631e3d..37c8a6b 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
@@ -124,7 +124,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;
@@ -1622,19 +1623,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;
@@ -1643,18 +1645,18 @@ public class MetaDataRepository implements
PCRegistry.RegisterClassListener, Con
if (pcNames != null && !pcNames.isEmpty() &&
!pcNames.contains(reg[i].getName())) {
continue;
}
-
+
// If the compatibility option "filterPCRegistryClasses" is
enabled, then verify that the type is
// accessible to the envLoader/Thread Context ClassLoader
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);
-
+
if (!reg[i].equals(classFromAppClassLoader)) {
// This is a class that belongs to a ClassLoader not
associated with the Application,
// so it should be processed.
@@ -1837,7 +1839,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);
@@ -1847,8 +1849,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();
@@ -1921,8 +1923,8 @@ public class MetaDataRepository implements
PCRegistry.RegisterClassListener, Con
public void endConfiguration() {
initializeMetaDataFactory();
- if (_implGen == null)
- _implGen = new InterfaceImplGenerator(this);
+ if (_implGen == null)
+ _implGen = new InterfaceImplGenerator(this);
if (_preload == true) {
_oids = new HashMap<Class<?>, Class<?>>();
_impls = new HashMap<Class<?>, Collection<Class<?>>>();
@@ -1930,7 +1932,7 @@ public class MetaDataRepository implements
PCRegistry.RegisterClassListener, Con
_aliases = new HashMap<String, List<Class<?>>>();
_pawares = new HashMap<Class<?>, NonPersistentMetaData>();
_nonMapped = new HashMap<Class<?>, NonPersistentMetaData>();
- _subs = new HashMap<Class<?>, List<Class<?>>>();
+ _subs = new HashMap<Class<?>, Collection<Class<?>>>();
// Wait till we're done loading MetaData to flip _lock boolean.
}
}
