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

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


The following commit(s) were added to refs/heads/maven-3.8.x by this push:
     new 4e5b3d5  [MNG-7251] Fix threadLocalArtifactsHolder leaking into cloned 
project
4e5b3d5 is described below

commit 4e5b3d55545e5f03f05ac7b0cd1b56689df36201
Author: Falko Modler <[email protected]>
AuthorDate: Sun Aug 22 22:52:30 2021 +0200

    [MNG-7251] Fix threadLocalArtifactsHolder leaking into cloned project
    
    This closes #527
---
 .../org/apache/maven/project/MavenProject.java     | 49 +++++++++++-----------
 .../org/apache/maven/project/MavenProjectTest.java | 43 +++++++++++++++++++
 2 files changed, 68 insertions(+), 24 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 157b7a0..94d6788 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
@@ -143,13 +143,7 @@ public class MavenProject
 
     private Artifact artifact;
 
-    private final ThreadLocal<ArtifactsHolder> threadLocalArtifactsHolder = 
new ThreadLocal()
-    {
-        protected ArtifactsHolder initialValue()
-        {
-            return new ArtifactsHolder();
-        }
-    };
+    private ThreadLocal<ArtifactsHolder> threadLocalArtifactsHolder = 
newThreadLocalArtifactsHolder();
 
     private Model originalModel;
 
@@ -188,6 +182,17 @@ public class MavenProject
         setModel( model );
     }
 
+    private static ThreadLocal<ArtifactsHolder> newThreadLocalArtifactsHolder()
+    {
+        return new ThreadLocal<ArtifactsHolder>()
+        {
+            protected ArtifactsHolder initialValue()
+            {
+                return new ArtifactsHolder();
+            }
+        };
+    }
+
     public MavenProject( Model model )
     {
         setModel( model );
@@ -1181,7 +1186,8 @@ 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;
@@ -1224,6 +1230,7 @@ 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!
@@ -1232,11 +1239,6 @@ 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() );
@@ -1479,13 +1481,7 @@ 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
     //
     // 
----------------------------------------------------------------------------------------------------------------
 
@@ -1506,7 +1502,6 @@ public class MavenProject
         if ( moduleFile != null )
         {
             File moduleDir = moduleFile.getCanonicalFile().getParentFile();
-
             module = moduleDir.getName();
         }
 
@@ -1827,7 +1822,6 @@ public class MavenProject
     public void setReportArtifacts( Set<Artifact> reportArtifacts )
     {
         this.reportArtifacts = reportArtifacts;
-
         reportArtifactMap = null;
     }
 
@@ -1851,7 +1845,6 @@ public class MavenProject
     public void setExtensionArtifacts( Set<Artifact> extensionArtifacts )
     {
         this.extensionArtifacts = extensionArtifacts;
-
         extensionArtifactMap = null;
     }
 
@@ -1885,7 +1878,6 @@ 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
@@ -1995,5 +1987,14 @@ public class MavenProject
         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 6b4258b..2344d8f 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,13 +21,18 @@ 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
@@ -188,6 +193,44 @@ 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
     {

Reply via email to