[MNG-7312] Revert ThreadLocal approach from MNG-6843 and MNG-7251

Revert "[MNG-7251] Fix threadLocalArtifactsHolder leaking into cloned project"

This reverts commit 4e5b3d5.

Revert "[MNG-6843] Parallel build fails due to missing JAR artifacts in compilePath"

This reverts commit 76d5f0d.

===

This closes #595

Author: michael-o

maven-core/src/main/java/org/apache/maven/project/MavenProject.java

@@ -92,9 +92,13 @@
 public class MavenProject
     implements Cloneable
 {
+
     private static final Logger LOGGER = LoggerFactory.getLogger( MavenProject.class );
+
     public static final String EMPTY_PROJECT_GROUP_ID = "unknown";
+
     public static final String EMPTY_PROJECT_ARTIFACT_ID = "empty-project";
+
     public static final String EMPTY_PROJECT_VERSION = "0";
 
     private Model model;
@@ -107,6 +111,10 @@ public class MavenProject
 
     private Set<Artifact> resolvedArtifacts;
 
+    private ArtifactFilter artifactFilter;
+
+    private Set<Artifact> artifacts;
+
     private Artifact parentArtifact;
 
     private Set<Artifact> pluginArtifacts;
@@ -143,7 +151,8 @@ public class MavenProject
 
     private Artifact artifact;
 
-    private ThreadLocal<ArtifactsHolder> threadLocalArtifactsHolder = newThreadLocalArtifactsHolder();
+    // calculated.
+    private Map<String, Artifact> artifactMap;
 
     private Model originalModel;
 
@@ -176,21 +185,12 @@ public class MavenProject
     public MavenProject()
     {
         Model model = new Model();
+
         model.setGroupId( EMPTY_PROJECT_GROUP_ID );
         model.setArtifactId( EMPTY_PROJECT_ARTIFACT_ID );
         model.setVersion( EMPTY_PROJECT_VERSION );
-        setModel( model );
-    }
 
-    private static ThreadLocal<ArtifactsHolder> newThreadLocalArtifactsHolder()
-    {
-        return new ThreadLocal<ArtifactsHolder>()
-        {
-            protected ArtifactsHolder initialValue()
-            {
-                return new ArtifactsHolder();
-            }
-        };
+        setModel( model );
     }
 
     public MavenProject( Model model )
@@ -695,11 +695,10 @@ public void addLicense( License license )
 
     public void setArtifacts( Set<Artifact> artifacts )
     {
-        ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
-        artifactsHolder.artifacts = artifacts;
+        this.artifacts = artifacts;
 
         // flush the calculated artifactMap
-        artifactsHolder.artifactMap = null;
+        artifactMap = null;
     }
 
     /**
@@ -712,36 +711,34 @@ public void setArtifacts( Set<Artifact> artifacts )
      */
     public Set<Artifact> getArtifacts()
     {
-        ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
-        if ( artifactsHolder.artifacts == null )
+        if ( artifacts == null )
         {
-            if ( artifactsHolder.artifactFilter == null || resolvedArtifacts == null )
+            if ( artifactFilter == null || resolvedArtifacts == null )
             {
-                artifactsHolder.artifacts = new LinkedHashSet<>();
+                artifacts = new LinkedHashSet<>();
             }
             else
             {
-                artifactsHolder.artifacts = new LinkedHashSet<>( resolvedArtifacts.size() * 2 );
+                artifacts = new LinkedHashSet<>( resolvedArtifacts.size() * 2 );
                 for ( Artifact artifact : resolvedArtifacts )
                 {
-                    if ( artifactsHolder.artifactFilter.include( artifact ) )
+                    if ( artifactFilter.include( artifact ) )
                     {
-                        artifactsHolder.artifacts.add( artifact );
+                        artifacts.add( artifact );
                     }
                 }
             }
         }
-        return artifactsHolder.artifacts;
+        return artifacts;
     }
 
     public Map<String, Artifact> getArtifactMap()
     {
-        ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
-        if ( artifactsHolder.artifactMap == null )
+        if ( artifactMap == null )
         {
-            artifactsHolder.artifactMap = ArtifactUtils.artifactMapByVersionlessId( getArtifacts() );
+            artifactMap = ArtifactUtils.artifactMapByVersionlessId( getArtifacts() );
         }
-        return artifactsHolder.artifactMap;
+        return artifactMap;
     }
 
     public void setPluginArtifacts( Set<Artifact> pluginArtifacts )
@@ -1186,8 +1183,7 @@ public MavenProject clone()
         {
             throw new UnsupportedOperationException( e );
         }
-        // clone must have it's own TL, otherwise the artifacts are intermingled!
-        clone.threadLocalArtifactsHolder = newThreadLocalArtifactsHolder();
+
         clone.deepCopy( this );
 
         return clone;
@@ -1230,7 +1226,6 @@ private void deepCopy( MavenProject project )
         // copy fields
         file = project.file;
         basedir = project.basedir;
-        threadLocalArtifactsHolder.set( project.threadLocalArtifactsHolder.get().copy() );
 
         // don't need a deep copy, they don't get modified or added/removed to/from - but make them unmodifiable to be
         // sure!
@@ -1239,6 +1234,11 @@ private void deepCopy( MavenProject project )
             setDependencyArtifacts( Collections.unmodifiableSet( project.getDependencyArtifacts() ) );
         }
 
+        if ( project.getArtifacts() != null )
+        {
+            setArtifacts( Collections.unmodifiableSet( project.getArtifacts() ) );
+        }
+
         if ( project.getParentFile() != null )
         {
             parentFile = new File( project.getParentFile().getAbsolutePath() );
@@ -1433,9 +1433,8 @@ public DependencyFilter getExtensionDependencyFilter()
     public void setResolvedArtifacts( Set<Artifact> artifacts )
     {
         this.resolvedArtifacts = ( artifacts != null ) ? artifacts : Collections.<Artifact>emptySet();
-        ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
-        artifactsHolder.artifacts = null;
-        artifactsHolder.artifactMap = null;
+        this.artifacts = null;
+        this.artifactMap = null;
     }
 
     /**
@@ -1448,10 +1447,9 @@ public void setResolvedArtifacts( Set<Artifact> artifacts )
      */
     public void setArtifactFilter( ArtifactFilter artifactFilter )
     {
-        ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
-        artifactsHolder.artifactFilter = artifactFilter;
-        artifactsHolder.artifacts = null;
-        artifactsHolder.artifactMap = null;
+        this.artifactFilter = artifactFilter;
+        this.artifacts = null;
+        this.artifactMap = null;
     }
 
     /**
@@ -1481,7 +1479,13 @@ public void addLifecyclePhase( String lifecyclePhase )
 
     // ----------------------------------------------------------------------------------------------------------------
     //
-    // D E P R E C A T E D - Everything below will be removed for Maven 4.0.0
+    //
+    // D E P R E C A T E D
+    //
+    //
+    // ----------------------------------------------------------------------------------------------------------------
+    //
+    // Everything below will be removed for Maven 4.0.0
     //
     // ----------------------------------------------------------------------------------------------------------------
 
@@ -1502,6 +1506,7 @@ public String getModulePathAdjustment( MavenProject moduleProject )
         if ( moduleFile != null )
         {
             File moduleDir = moduleFile.getCanonicalFile().getParentFile();
+
             module = moduleDir.getName();
         }
 
@@ -1822,6 +1827,7 @@ public Reporting getReporting()
     public void setReportArtifacts( Set<Artifact> reportArtifacts )
     {
         this.reportArtifacts = reportArtifacts;
+
         reportArtifactMap = null;
     }
 
@@ -1838,13 +1844,15 @@ public Map<String, Artifact> getReportArtifactMap()
         {
             reportArtifactMap = ArtifactUtils.artifactMapByVersionlessId( getReportArtifacts() );
         }
+
         return reportArtifactMap;
     }
 
     @Deprecated
     public void setExtensionArtifacts( Set<Artifact> extensionArtifacts )
     {
         this.extensionArtifacts = extensionArtifacts;
+
         extensionArtifactMap = null;
     }
 
@@ -1861,6 +1869,7 @@ public Map<String, Artifact> getExtensionArtifactMap()
         {
             extensionArtifactMap = ArtifactUtils.artifactMapByVersionlessId( getExtensionArtifacts() );
         }
+
         return extensionArtifactMap;
     }
 
@@ -1878,6 +1887,7 @@ public List<ReportPlugin> getReportPlugins()
     public Xpp3Dom getReportConfiguration( String pluginGroupId, String pluginArtifactId, String reportSetId )
     {
         Xpp3Dom dom = null;
+
         // ----------------------------------------------------------------------
         // I would like to be able to lookup the Mojo object using a key but
         // we have a limitation in modello that will be remedied shortly. So
@@ -1981,20 +1991,4 @@ public void setProjectBuildingRequest( ProjectBuildingRequest projectBuildingReq
     {
         this.projectBuilderConfiguration = projectBuildingRequest;
     }
-
-    private static class ArtifactsHolder
-    {
-        private ArtifactFilter artifactFilter;
-        private Set<Artifact> artifacts;
-        private Map<String, Artifact> artifactMap;
-
-        ArtifactsHolder copy()
-        {
-           ArtifactsHolder copy = new ArtifactsHolder();
-           copy.artifactFilter = artifactFilter;
-           copy.artifacts = artifacts != null ? new LinkedHashSet<>( artifacts ) : null;
-           copy.artifactMap = artifactMap != null ? new LinkedHashMap<>( artifactMap ) : null;
-           return copy;
-        }
-    }
 }

maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java

@@ -21,18 +21,13 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.atomic.AtomicReference;
 
-import org.apache.maven.artifact.Artifact;
 import org.apache.maven.model.DependencyManagement;
 import org.apache.maven.model.Model;
 import org.apache.maven.model.Parent;
 import org.apache.maven.model.Profile;
-import org.mockito.Mockito;
 
 public class MavenProjectTest
     extends AbstractMavenProjectTestCase
@@ -193,44 +188,6 @@ public void testCloneWithBaseDir()
         assertEquals( "Base directory is preserved across clone", projectToClone.getBasedir(), clonedProject.getBasedir() );
     }
 
-    public void testCloneWithArtifacts()
-        throws InterruptedException
-    {
-        Artifact initialArtifact = Mockito.mock( Artifact.class, "initialArtifact" );
-        MavenProject originalProject = new MavenProject();
-        originalProject.setArtifacts( Collections.singleton( initialArtifact ) );
-        assertEquals( "Sanity check: originalProject returns artifact that has just been set",
-                      Collections.singleton( initialArtifact ), originalProject.getArtifacts() );
-
-        final MavenProject clonedProject = originalProject.clone();
-
-        assertEquals( "Cloned project returns the artifact that was set for the original project",
-                      Collections.singleton( initialArtifact ), clonedProject.getArtifacts() );
-
-        Artifact anotherArtifact = Mockito.mock( Artifact.class, "anotherArtifact" );
-        clonedProject.setArtifacts( Collections.singleton( anotherArtifact ) );
-        assertEquals( "Sanity check: clonedProject returns artifact that has just been set",
-                      Collections.singleton( anotherArtifact ), clonedProject.getArtifacts() );
-
-        assertEquals( "Original project returns the artifact that was set initially (not the one for clonedProject)",
-                      Collections.singleton( initialArtifact ), originalProject.getArtifacts() );
-
-        final AtomicReference<Set<Artifact>> artifactsFromThread = new AtomicReference<>();
-        Thread thread = new Thread( new Runnable()
-        {
-            @Override
-            public void run()
-            {
-                artifactsFromThread.set( clonedProject.getArtifacts() );
-            }
-        } );
-        thread.start();
-        thread.join();
-
-        assertEquals( "Another thread does not see the same artifacts",
-                      Collections.emptySet(), artifactsFromThread.get() );
-    }
-
     public void testUndefinedOutputDirectory()
         throws Exception
     {