Bug 576660 - AdapterManager should use more modern concurrency
primitives
- replace classSearchOrderLookup with a ConcurrentHashMap
Change-Id: Iedd7a11277d8bc43fa897b1fe47a700501c21c02
Signed-off-by: Christoph Laeubrich <laeubi@laeubi-soft.de>
Reviewed-on: https://git.eclipse.org/r/c/equinox/rt.equinox.bundles/+/186604
Tested-by: Equinox Bot <equinox-bot@eclipse.org>
Reviewed-by: Andrey Loskutov <loskutov@gmx.de>
diff --git a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/internal/runtime/AdapterManager.java b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/internal/runtime/AdapterManager.java
index 315decc..8ba2081 100644
--- a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/internal/runtime/AdapterManager.java
+++ b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/internal/runtime/AdapterManager.java
@@ -71,10 +71,10 @@
/**
* Cache of class lookup order (Class -> Class[]). This avoids having to compute often, and
* provides clients with quick lookup for instanceOf checks based on type name.
- * Thread safety note: The map is synchronized using a synchronized
- * map wrapper class. The arrays within the map are immutable.
+ * Thread safety note: always use the compute methods to update the map
+ * and make sure the values (Class array) are never modified but replaced if necessary.
*/
- private Map<Class<?>, Class<?>[]> classSearchOrderLookup;
+ private final ConcurrentMap<Class<?>, Class<?>[]> classSearchOrderLookup;
/**
* Map of factories, keyed by <code>String</code>, fully qualified class name of
@@ -95,6 +95,7 @@
* Private constructor to block instance creation.
*/
private AdapterManager() {
+ classSearchOrderLookup = new ConcurrentHashMap<>();
adapterLookup = new ConcurrentHashMap<>();
lazyFactoryProviders = new ConcurrentLinkedQueue<>();
factories = new ConcurrentHashMap<>();
@@ -221,21 +222,21 @@
* The search order is defined in this class' comment.
*/
@Override
- @SuppressWarnings("unchecked")
public <T> Class<? super T>[] computeClassOrder(Class<T> adaptable) {
- //cache reference to lookup to protect against concurrent flush
- Map<Class<?>, Class<?>[]> lookup = classSearchOrderLookup;
- if (lookup == null) {
- classSearchOrderLookup = lookup = Collections.synchronizedMap(new HashMap<Class<?>, Class<?>[]>());
- }
- return (Class<? super T>[]) lookup.computeIfAbsent(adaptable, this::doComputeClassOrder);
+ Class<? super T>[] classOrder = getClassOrder(adaptable);
+ return Arrays.copyOf(classOrder, classOrder.length);
+ }
+
+ @SuppressWarnings("unchecked")
+ private <T> Class<? super T>[] getClassOrder(Class<T> adaptable) {
+ return (Class<? super T>[]) classSearchOrderLookup.computeIfAbsent(adaptable, AdapterManager::doComputeClassOrder);
}
/**
* Computes the super-type search order starting with <code>adaptable</code>.
* The search order is defined in this class' comment.
*/
- private Class<?>[] doComputeClassOrder(Class<?> adaptable) {
+ private static Class<?>[] doComputeClassOrder(Class<?> adaptable) {
List<Class<?>> classes = new ArrayList<>();
Class<?> clazz = adaptable;
Set<Class<?>> seen = new HashSet<>(4);
@@ -252,7 +253,7 @@
return classes.toArray(new Class[classes.size()]);
}
- private void computeInterfaceOrder(Class<?>[] interfaces, Collection<Class<?>> classes, Set<Class<?>> seen) {
+ private static void computeInterfaceOrder(Class<?>[] interfaces, Collection<Class<?>> classes, Set<Class<?>> seen) {
List<Class<?>> newInterfaces = new ArrayList<>(interfaces.length);
for (Class<?> interfac : interfaces) {
if (seen.add(interfac)) {
@@ -276,7 +277,7 @@
public synchronized void flushLookup() {
adapterLookup.clear();
classLookup = null;
- classSearchOrderLookup = null;
+ classSearchOrderLookup.clear();
}
@Override