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
{