You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by mi...@apache.org on 2021/10/16 13:23:15 UTC

[maven] branch MNG-7312-maven-3.8.x created (now c8707ce)

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

michaelo pushed a change to branch MNG-7312-maven-3.8.x
in repository https://gitbox.apache.org/repos/asf/maven.git.


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

This branch includes the following new commits:

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

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[maven] 01/01: [MNG-7312] Revert ThreadLocal approach from MNG-6843 and MNG-7251

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

michaelo pushed a commit to branch MNG-7312-maven-3.8.x
in repository https://gitbox.apache.org/repos/asf/maven.git

commit c8707cefc15ae617492b2dbbadb668fff89db1ee
Author: Michael Osipov <mi...@apache.org>
AuthorDate: Sat Oct 16 15:21:28 2021 +0200

    [MNG-7312] Revert ThreadLocal approach from MNG-6843 and MNG-7251
    
    Revert "[MNG-7251] Fix threadLocalArtifactsHolder leaking into cloned project"
    
    This reverts commit 4e5b3d55545e5f03f05ac7b0cd1b56689df36201.
    
    Revert "[MNG-6843] Parallel build fails due to missing JAR artifacts in compilePath"
    
    This reverts commit 76d5f0d942f52650d3bdf775b6af42d23d69066b.
---
 .../org/apache/maven/project/MavenProject.java     | 102 ++++++++++-----------
 .../org/apache/maven/project/MavenProjectTest.java |  43 ---------
 2 files changed, 48 insertions(+), 97 deletions(-)

diff --git a/maven-core/src/main/java/org/apache/maven/project/MavenProject.java b/maven-core/src/main/java/org/apache/maven/project/MavenProject.java
index 94d6788..db0f4a9 100644
--- a/maven-core/src/main/java/org/apache/maven/project/MavenProject.java
+++ b/maven-core/src/main/java/org/apache/maven/project/MavenProject.java
@@ -92,9 +92,13 @@ import org.slf4j.LoggerFactory;
 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 class MavenProject
 
     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 class MavenProject
      */
     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 class MavenProject
         {
             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 @@ public class MavenProject
         // 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 @@ public class MavenProject
             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 class MavenProject
     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 class MavenProject
      */
     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 class MavenProject
 
     // ----------------------------------------------------------------------------------------------------------------
     //
-    // 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 class MavenProject
         if ( moduleFile != null )
         {
             File moduleDir = moduleFile.getCanonicalFile().getParentFile();
+
             module = moduleDir.getName();
         }
 
@@ -1822,6 +1827,7 @@ public class MavenProject
     public void setReportArtifacts( Set<Artifact> reportArtifacts )
     {
         this.reportArtifacts = reportArtifacts;
+
         reportArtifactMap = null;
     }
 
@@ -1838,6 +1844,7 @@ public class MavenProject
         {
             reportArtifactMap = ArtifactUtils.artifactMapByVersionlessId( getReportArtifacts() );
         }
+
         return reportArtifactMap;
     }
 
@@ -1845,6 +1852,7 @@ public class MavenProject
     public void setExtensionArtifacts( Set<Artifact> extensionArtifacts )
     {
         this.extensionArtifacts = extensionArtifacts;
+
         extensionArtifactMap = null;
     }
 
@@ -1861,6 +1869,7 @@ public class MavenProject
         {
             extensionArtifactMap = ArtifactUtils.artifactMapByVersionlessId( getExtensionArtifacts() );
         }
+
         return extensionArtifactMap;
     }
 
@@ -1878,6 +1887,7 @@ public class MavenProject
     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 class MavenProject
     {
         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;
-        }
-    }
 }
diff --git a/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java b/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java
index 2344d8f..6b4258b 100644
--- a/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java
+++ b/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java
@@ -21,18 +21,13 @@ package org.apache.maven.project;
 
 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 class MavenProjectTest
         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
     {