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

gnodet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git


The following commit(s) were added to refs/heads/master by this push:
     new ce15193394 Fix concurrent cache access (#1425)
ce15193394 is described below

commit ce151933949ab39eca78b8374531d39bc8c76b0f
Author: Guillaume Nodet <gno...@gmail.com>
AuthorDate: Thu Feb 29 00:31:49 2024 +0100

    Fix concurrent cache access (#1425)
---
 .../internal/LifecycleDependencyResolver.java      | 31 ++++++++++++----------
 .../maven/plugin/DefaultPluginDescriptorCache.java | 31 +++++++++-------------
 .../artifact/DefaultProjectArtifactsCache.java     |  4 ++-
 3 files changed, 33 insertions(+), 33 deletions(-)

diff --git 
a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java
 
b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java
index 525fb59612..6b1df8b775 100644
--- 
a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java
+++ 
b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleDependencyResolver.java
@@ -195,27 +195,30 @@ public class LifecycleDependencyResolver {
             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
index 3e5f8e8ccb..b47e258f08 100644
--- 
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 @@ import org.eclipse.aether.repository.WorkspaceRepository;
 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 class DefaultPluginDescriptorCache implements 
PluginDescriptorCache {
     @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
index c7842452c1..f5005c5fc6 100644
--- 
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 class DefaultProjectArtifactsCache implements 
ProjectArtifactsCache {
     }
 
     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 class DefaultProjectArtifactsCache implements 
ProjectArtifactsCache {
             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

Reply via email to