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/22 19:58:29 UTC
[maven] branch maven-3.8.x updated: [MNG-7312] Revert ThreadLocal
approach from MNG-6843 and MNG-7251
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 5c36bf5 [MNG-7312] Revert ThreadLocal approach from MNG-6843 and MNG-7251
5c36bf5 is described below
commit 5c36bf5ef78a162cefea47ccdaf0d28e01c1426c
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.
===
This closes #595
---
.../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
{