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/09/14 15:28:55 UTC
[maven] branch master updated: [MNG-7251] Fix
threadLocalArtifactsHolder leaking into cloned project
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push:
new a6e462b [MNG-7251] Fix threadLocalArtifactsHolder leaking into cloned project
a6e462b is described below
commit a6e462b53a4b6c87ef43f6cfe7fbde0b0939e3d6
Author: Falko Modler <fa...@users.noreply.github.com>
AuthorDate: Sun Sep 12 22:59:48 2021 +0200
[MNG-7251] Fix threadLocalArtifactsHolder leaking into cloned project
This closes #535
---
.../org/apache/maven/project/MavenProject.java | 33 +++++++----------
.../org/apache/maven/project/MavenProjectTest.java | 43 ++++++++++++++++++++++
2 files changed, 57 insertions(+), 19 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 0fce71a..83b86c3 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
@@ -147,8 +147,7 @@ public class MavenProject
private Artifact artifact;
- private final ThreadLocal<ArtifactsHolder> threadLocalArtifactsHolder =
- ThreadLocal.withInitial( ArtifactsHolder::new );
+ private ThreadLocal<ArtifactsHolder> threadLocalArtifactsHolder = ThreadLocal.withInitial( ArtifactsHolder::new );
private Model originalModel;
@@ -1177,7 +1176,8 @@ public class MavenProject
{
throw new UnsupportedOperationException( e );
}
-
+ // clone must have its own TL, otherwise the artifacts are intermingled!
+ clone.threadLocalArtifactsHolder = ThreadLocal.withInitial( ArtifactsHolder::new );
clone.deepCopy( this );
return clone;
@@ -1220,6 +1220,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!
@@ -1228,11 +1229,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() );
@@ -1475,13 +1471,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
//
// ----------------------------------------------------------------------------------------------------------------
@@ -1502,7 +1492,6 @@ public class MavenProject
if ( moduleFile != null )
{
File moduleDir = moduleFile.getCanonicalFile().getParentFile();
-
module = moduleDir.getName();
}
@@ -1823,7 +1812,6 @@ public class MavenProject
public void setReportArtifacts( Set<Artifact> reportArtifacts )
{
this.reportArtifacts = reportArtifacts;
-
reportArtifactMap = null;
}
@@ -1847,7 +1835,6 @@ public class MavenProject
public void setExtensionArtifacts( Set<Artifact> extensionArtifacts )
{
this.extensionArtifacts = extensionArtifacts;
-
extensionArtifactMap = null;
}
@@ -1881,7 +1868,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
@@ -1991,5 +1977,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 d2cba20..aff0dde 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,14 +21,19 @@ 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.junit.jupiter.api.Test;
+import org.mockito.Mockito;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -204,6 +209,44 @@ public class MavenProjectTest
}
@Test
+ public void testCloneWithArtifacts()
+ throws InterruptedException
+ {
+ Artifact initialArtifact = Mockito.mock( Artifact.class, "initialArtifact" );
+ MavenProject originalProject = new MavenProject();
+ originalProject.setArtifacts( Collections.singleton( initialArtifact ) );
+ assertEquals( Collections.singleton( initialArtifact ), originalProject.getArtifacts(),
+ "Sanity check: originalProject returns artifact that has just been set" );
+
+ final MavenProject clonedProject = originalProject.clone();
+
+ assertEquals( Collections.singleton( initialArtifact ), clonedProject.getArtifacts(),
+ "Cloned project returns the artifact that was set for the original project" );
+
+ Artifact anotherArtifact = Mockito.mock( Artifact.class, "anotherArtifact" );
+ clonedProject.setArtifacts( Collections.singleton( anotherArtifact ) );
+ assertEquals( Collections.singleton( anotherArtifact ), clonedProject.getArtifacts(),
+ "Sanity check: clonedProject returns artifact that has just been set" );
+
+ assertEquals( Collections.singleton( initialArtifact ), originalProject.getArtifacts(),
+ "Original project returns the artifact that was set initially (not the one for clonedProject)" );
+
+ 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( Collections.emptySet(), artifactsFromThread.get(),
+ "Another thread does not see the same artifacts" );
+ }
+
public void testUndefinedOutputDirectory()
throws Exception
{