Fix #354 Make artifact cache safe for concurrent access (#355)

* Fix #354 Make artifact cache safe for concurrent access * Fix remaining issues related to artifactCache * Mark AddThirdPartyMojo as threadSafe * Fix one more use of projectDependencies and checkstyle --------- Co-authored-by: Frantisek Hartman <frant.hartm@gmail.com>
mojohaus · May 15, 2023 · 560edbb · 560edbb
1 parent d9a92ae
commit 560edbb
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 12 deletions.
diff --git a/src/main/java/org/codehaus/mojo/license/AddThirdPartyMojo.java b/src/main/java/org/codehaus/mojo/license/AddThirdPartyMojo.java
@@ -67,7 +67,7 @@
  */
 // CHECKSTYLE_ON: LineLength
 @Mojo( name = "add-third-party", requiresDependencyResolution = ResolutionScope.TEST,
-       defaultPhase = LifecyclePhase.GENERATE_RESOURCES )
+       defaultPhase = LifecyclePhase.GENERATE_RESOURCES, threadSafe = true )
 public class AddThirdPartyMojo extends AbstractAddThirdPartyMojo implements MavenProjectDependenciesConfigurator
 {
     private static final Logger LOG = LoggerFactory.getLogger( AddThirdPartyMojo.class );

diff --git a/src/main/java/org/codehaus/mojo/license/api/DefaultThirdPartyHelper.java b/src/main/java/org/codehaus/mojo/license/api/DefaultThirdPartyHelper.java
@@ -36,6 +36,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -101,7 +102,10 @@ public class DefaultThirdPartyHelper
     /**
      * Cache of dependencies (as maven project) loaded.
      */
-    private static SortedMap<String, MavenProject> artifactCache;
+    private static volatile SortedMap<String, MavenProject> artifactCache;
+
+    // Mutex to guard lazy initialization of artifactCache
+    private static final Object ARTIFACT_CACHE_MUTEX = new Object();
 
     /**
      * Constructor of the helper.
@@ -138,7 +142,13 @@ public SortedMap<String, MavenProject> getArtifactCache()
     {
         if ( artifactCache == null )
         {
-            artifactCache = new TreeMap<>();
+            synchronized ( ARTIFACT_CACHE_MUTEX )
+            {
+                if ( artifactCache == null )
+                {
+                    artifactCache = Collections.synchronizedSortedMap( new TreeMap<String, MavenProject>() );
+                }
+            }
         }
         return artifactCache;
     }
@@ -185,9 +195,12 @@ public LicenseMap createLicenseMap( SortedMap<String, MavenProject> dependencies
 
         LicenseMap licenseMap = new LicenseMap();
 
-        for ( MavenProject project : dependencies.values() )
+        synchronized ( dependencies )
         {
-            thirdPartyTool.addLicense( licenseMap, project, project.getLicenses() );
+            for ( MavenProject project : dependencies.values() )
+            {
+                thirdPartyTool.addLicense( licenseMap, project, project.getLicenses() );
+            }
         }
         return licenseMap;
     }
@@ -234,7 +247,11 @@ public SortedProperties createUnsafeMapping( LicenseMap licenseMap, File missing
 
                 // try to load missing third party files from dependencies
 
-                Collection<MavenProject> projects = new ArrayList<>( projectDependencies.values() );
+                Collection<MavenProject> projects;
+                synchronized ( projectDependencies )
+                {
+                    projects = new ArrayList<>( projectDependencies.values() );
+                }
                 projects.remove( project );
                 projects.removeAll( unsafeDependencies );
 

diff --git a/src/main/java/org/codehaus/mojo/license/api/DefaultThirdPartyTool.java b/src/main/java/org/codehaus/mojo/license/api/DefaultThirdPartyTool.java
@@ -458,13 +458,16 @@ public SortedProperties loadUnsafeMapping( LicenseMap licenseMap,
     {
         Map<String, MavenProject> snapshots = new HashMap<>();
 
-        //find snapshot dependencies
-        for ( Map.Entry<String, MavenProject> entry : artifactCache.entrySet() )
+        synchronized ( artifactCache )
         {
-            MavenProject mavenProject = entry.getValue();
-            if ( mavenProject.getVersion().endsWith( Artifact.SNAPSHOT_VERSION ) )
+            //find snapshot dependencies
+            for ( Map.Entry<String, MavenProject> entry : artifactCache.entrySet() )
             {
-                snapshots.put( entry.getKey(), mavenProject );
+                MavenProject mavenProject = entry.getValue();
+                if ( mavenProject.getVersion().endsWith( Artifact.SNAPSHOT_VERSION ) )
+                {
+                    snapshots.put( entry.getKey(), mavenProject );
+                }
             }
         }
 

diff --git a/src/main/java/org/codehaus/mojo/license/api/DependenciesTool.java b/src/main/java/org/codehaus/mojo/license/api/DependenciesTool.java
@@ -117,7 +117,10 @@ public SortedMap<String, MavenProject> loadProjectDependencies( ResolvedProjectD
         SortedMap<String, MavenProject> localCache = new TreeMap<>();
         if ( cache != null )
         {
-            localCache.putAll( cache );
+            synchronized ( cache )
+            {
+                localCache.putAll( cache );
+            }
         }
         ProjectBuildingRequest projectBuildingRequest
                 = new DefaultProjectBuildingRequest( mavenSession.getProjectBuildingRequest() )