Fix concurrent cache access (#1425)

apache · Feb 28, 2024 · ce15193 · ce15193
1 parent 5307bcb
commit ce15193
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 33 deletions.
diff --git a/...n-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java b/...n-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java
@@ -195,27 +195,30 @@ public Set<Artifact> resolveProjectArtifacts(
             boolean aggregating,
             Set<Artifact> projectArtifacts)
             throws LifecycleExecutionException {
-        Set<Artifact> resolvedArtifacts;
         ProjectArtifactsCache.Key cacheKey = projectArtifactsCache.createKey(
                 project, scopesToCollect, scopesToResolve, aggregating, session.getRepositorySession());
+
         ProjectArtifactsCache.CacheRecord recordArtifacts;
         recordArtifacts = projectArtifactsCache.get(cacheKey);
-
-        if (recordArtifacts != null) {
-            resolvedArtifacts = recordArtifacts.getArtifacts();
-        } else {
-            try {
-                resolvedArtifacts = getDependencies(
-                        project, scopesToCollect, scopesToResolve, session, aggregating, projectArtifacts);
-                recordArtifacts = projectArtifactsCache.put(cacheKey, resolvedArtifacts);
-            } catch (LifecycleExecutionException e) {
-                projectArtifactsCache.put(cacheKey, e);
-                projectArtifactsCache.register(project, cacheKey, recordArtifacts);
-                throw e;
+        if (recordArtifacts == null) {
+            synchronized (cacheKey) {
+                recordArtifacts = projectArtifactsCache.get(cacheKey);
+                if (recordArtifacts == null) {
+                    try {
+                        Set<Artifact> resolvedArtifacts = getDependencies(
+                                project, scopesToCollect, scopesToResolve, session, aggregating, projectArtifacts);
+                        recordArtifacts = projectArtifactsCache.put(cacheKey, resolvedArtifacts);
+                    } catch (LifecycleExecutionException e) {
+                        projectArtifactsCache.put(cacheKey, e);
+                        projectArtifactsCache.register(project, cacheKey, recordArtifacts);
+                        throw e;
+                    }
+                }
             }
         }
         projectArtifactsCache.register(project, cacheKey, recordArtifacts);
-        return resolvedArtifacts;
+
+        return recordArtifacts.getArtifacts();
     }
 
     private Set<Artifact> getDependencies(

diff --git a/maven-core/src/main/java/org/apache/maven/plugin/DefaultPluginDescriptorCache.java b/maven-core/src/main/java/org/apache/maven/plugin/DefaultPluginDescriptorCache.java
@@ -49,13 +49,14 @@
 public class DefaultPluginDescriptorCache implements PluginDescriptorCache {
 
     private Map<Key, PluginDescriptor> descriptors = new ConcurrentHashMap<>(128);
+    private Map<Key, Key> keys = new ConcurrentHashMap<>();
 
     public void flush() {
         descriptors.clear();
     }
 
     public Key createKey(Plugin plugin, List<RemoteRepository> repositories, RepositorySystemSession session) {
-        return new CacheKey(plugin, repositories, session);
+        return keys.computeIfAbsent(new CacheKey(plugin, repositories, session), k -> k);
     }
 
     public PluginDescriptor get(Key cacheKey) {
@@ -65,26 +66,20 @@ public PluginDescriptor get(Key cacheKey) {
     @Override
     public PluginDescriptor get(Key key, PluginDescriptorSupplier supplier)
             throws PluginDescriptorParsingException, PluginResolutionException, InvalidPluginDescriptorException {
+
         try {
-            return clone(descriptors.computeIfAbsent(key, k -> {
-                try {
-                    return clone(supplier.load());
-                } catch (PluginDescriptorParsingException
-                        | PluginResolutionException
-                        | InvalidPluginDescriptorException e) {
-                    throw new RuntimeException(e);
+            PluginDescriptor desc = descriptors.get(key);
+            if (desc == null) {
+                synchronized (key) {
+                    desc = descriptors.get(key);
+                    if (desc == null) {
+                        desc = supplier.load();
+                        descriptors.putIfAbsent(key, clone(desc));
+                    }
                 }
-            }));
-        } catch (RuntimeException e) {
-            if (e.getCause() instanceof PluginDescriptorParsingException) {
-                throw (PluginDescriptorParsingException) e.getCause();
-            }
-            if (e.getCause() instanceof PluginResolutionException) {
-                throw (PluginResolutionException) e.getCause();
-            }
-            if (e.getCause() instanceof InvalidPluginDescriptorException) {
-                throw (InvalidPluginDescriptorException) e.getCause();
             }
+            return clone(desc);
+        } catch (PluginDescriptorParsingException | PluginResolutionException | InvalidPluginDescriptorException e) {
             throw e;
         }
     }

diff --git a/maven-core/src/main/java/org/apache/maven/project/artifact/DefaultProjectArtifactsCache.java b/maven-core/src/main/java/org/apache/maven/project/artifact/DefaultProjectArtifactsCache.java
@@ -159,6 +159,7 @@ public boolean equals(Object o) {
     }
 
     protected final Map<Key, CacheRecord> cache = new ConcurrentHashMap<>();
+    protected final Map<Key, Key> keys = new ConcurrentHashMap<>();
 
     @Override
     public Key createKey(
@@ -167,13 +168,14 @@ public Key createKey(
             Collection<String> scopesToResolve,
             boolean aggregating,
             RepositorySystemSession session) {
-        return new CacheKey(
+        Key key = new CacheKey(
                 project,
                 project.getRemoteProjectRepositories(),
                 scopesToCollect,
                 scopesToResolve,
                 aggregating,
                 session);
+        return keys.computeIfAbsent(key, k -> k);
     }
 
     @Override