[sling-org-apache-sling-api] 01/01: SLING-10371 - SlingAdaptable: ConcurrentModificationException on nested adaptTo() calls

[js...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jsedding pushed a commit to branch bug/SLING-10371-jsedding
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-api.git

commit 11566368c35ecdc6edf45d58717c1f42034bd410
Author: Julian Sedding <js...@apache.org>
AuthorDate: Wed May 12 15:08:05 2021 +0200

    SLING-10371 - SlingAdaptable: ConcurrentModificationException on nested adaptTo() calls
    
    - propose alternative fix
---
 .../apache/sling/api/adapter/SlingAdaptable.java   | 35 ++++++++++++----------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/src/main/java/org/apache/sling/api/adapter/SlingAdaptable.java b/src/main/java/org/apache/sling/api/adapter/SlingAdaptable.java
index 5adf0ce..08b337d 100644
--- a/src/main/java/org/apache/sling/api/adapter/SlingAdaptable.java
+++ b/src/main/java/org/apache/sling/api/adapter/SlingAdaptable.java
@@ -18,8 +18,10 @@
  */
 package org.apache.sling.api.adapter;
 
-import java.util.HashMap;
+import org.jetbrains.annotations.NotNull;
+
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * The <code>SlingAdaptable</code> class is an (abstract) default implementation
@@ -74,7 +76,7 @@ public abstract class SlingAdaptable implements Adaptable {
      * are intended to be short-lived to not hold on to objects and classes for
      * too long.
      */
-    private Map<Class<?>, Object> adaptersCache;
+    private volatile Map<Class<?>, Object> adaptersCache;
 
     /**
      * Calls into the registered {@link AdapterManager} to adapt this object to
@@ -94,22 +96,23 @@ public abstract class SlingAdaptable implements Adaptable {
      */
     @SuppressWarnings("unchecked")
     public <AdapterType> AdapterType adaptTo(Class<AdapterType> type) {
-        AdapterType result = null;
-        synchronized ( this ) {
-            if ( adaptersCache != null ) {
-                result = (AdapterType) adaptersCache.get(type);
-            }
-            if ( result == null ) {
-                final AdapterManager mgr = ADAPTER_MANAGER;
-                result = (mgr == null ? null : mgr.getAdapter(this, type));
-                if ( result != null ) {
-                    if ( adaptersCache == null ) {
-                        adaptersCache = new HashMap<Class<?>, Object>();
-                    }
-                    adaptersCache.put(type, result);
+        final AdapterManager mgr = ADAPTER_MANAGER;
+        if (mgr == null) {
+            return null;
+        }
+        return (AdapterType) getAdaptersCache().computeIfAbsent(type, t -> mgr.getAdapter(this, t));
+    }
+
+    @NotNull
+    private Map<Class<?>, Object> getAdaptersCache() {
+        // correct use of double checked locking using volatile field
+        if (adaptersCache == null) {
+            synchronized (this) {
+                if (adaptersCache == null) {
+                    adaptersCache = new ConcurrentHashMap<>();
                 }
             }
         }
-        return result;
+        return adaptersCache;
     }
 }